FIX Prevent segfaults in CLI runner#4836
Conversation
After many hours of debugging, I minimized the problem down to this: emscripten-core/emscripten#22052 Thanks to @henryiii for reporting. See thread here for my comments while I was debugging: scikit-hep/boost-histogram#938
|
Actually, we could just backport emscripten-core/emscripten#22053 |
ryanking13
left a comment
There was a problem hiding this comment.
Thanks for analysing and fixing the issue @hoodmane!
| // Warning: this sounds like it might not do anything important, but it | ||
| // fills in the GOT. There can be segfaults if we leave it out. | ||
| // See https://github.com/emscripten-core/emscripten/issues/22052 | ||
| Module.reportUndefinedSymbols(); |
There was a problem hiding this comment.
Huh, If I remember correctly, that function throws if it encounters a symbol that doesn't actually exist, but I may be misremembering something.
There was a problem hiding this comment.
It fills in the Global Object Table:
var reportUndefinedSymbols = () => {
for (var [symName, entry] of Object.entries(GOT)) {
if (entry.value == 0) {
var value = resolveGlobalSymbol(symName, true).sym;
if (!value && !entry.required) {
// Ignore undefined symbols that are imported as weak.
continue;
}
if (typeof value == "function") {
/** @suppress {checkTypes} */
entry.value = addFunction(value, value.sig);
} else if (typeof value == "number") {
entry.value = value;
} else {
throw new Error(
`bad export type for '${symName}': ${typeof value}`,
);
}
}
}
};
ryanking13
left a comment
There was a problem hiding this comment.
Thanks, let's see if your upstream PR will get merged.
|
I think upstream PR is going to take a while and probably change, but I wouldn't be against using the patch in that PR here instead of this change. |
Sure, I have no objection with it. |
|
Except that it seems to break pyxel and pygame?? I'll have to investigate further. |
|
The problem has to do with dynamic linking of JavaScript symbols. |
This reverts commit 3de4958e02df86d534fc8b95009303cedb120416.
for more information, see https://pre-commit.ci
After many hours of debugging, I minimized the problem down to this: emscripten-core/emscripten#22052 Thanks to @henryiii for reporting. See thread here for my comments while I was debugging: scikit-hep/boost-histogram#938 Resolves pyodide#2964.
After many hours of debugging, I minimized the problem down to this: emscripten-core/emscripten#22052
Thanks to @henryiii for reporting. See thread here for my comments while I was debugging:
scikit-hep/boost-histogram#938
Resolves #2964.