Skip to content

Rename Emscripten EHSjLj functions in wasm backend #3191

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Oct 11, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion src/passes/GenerateDynCalls.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ struct GenerateDynCalls : public WalkerPass<PostWalker<GenerateDynCalls>> {

void visitFunction(Function* func) {
// Generate dynCalls for invokes
if (func->imported() && func->base.startsWith("invoke_")) {
if (func->imported() && func->module == ENV &&
func->base.startsWith("invoke_")) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like maybe unneeded/unrelated?

If you do want to make this change you you should change the logic in src/wasm/wasm-emscripten.cpp which seems do duplicate this when generating invokeFuncs mabye?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was suggested in #3191 (comment), and yeah, I think the condition in wasm/wasm-emscripten.cpp should be the same. Should I duplicate the condition there? @kripken

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added the same condition in wasm-emscripten.cpp.

Signature sig = func->sig;
// The first parameter is a pointer to the original function that's called
// by the invoke, so skip it
Expand Down
18 changes: 0 additions & 18 deletions src/passes/PostEmscripten.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -78,31 +78,13 @@ struct OptimizeCalls : public WalkerPass<PostWalker<OptimizeCalls>> {

struct PostEmscripten : public Pass {
void run(PassRunner* runner, Module* module) override {
// Optimize imports
optimizeImports(runner, module);

// Optimize calls
OptimizeCalls().run(runner, module);

// Optimize exceptions
optimizeExceptions(runner, module);
}

void optimizeImports(PassRunner* runner, Module* module) {
// Calling emscripten_longjmp_jmpbuf is the same as emscripten_longjmp.
Name EMSCRIPTEN_LONGJMP("emscripten_longjmp");
Name EMSCRIPTEN_LONGJMP_JMPBUF("emscripten_longjmp_jmpbuf");
ImportInfo info(*module);
auto* emscripten_longjmp =
info.getImportedFunction(ENV, EMSCRIPTEN_LONGJMP);
auto* emscripten_longjmp_jmpbuf =
info.getImportedFunction(ENV, EMSCRIPTEN_LONGJMP_JMPBUF);
if (emscripten_longjmp && emscripten_longjmp_jmpbuf) {
// Both exist, so it is worth renaming so that we have only one.
emscripten_longjmp_jmpbuf->base = EMSCRIPTEN_LONGJMP;
}
}

// Optimize exceptions (and setjmp) by removing unnecessary invoke* calls.
// An invoke is a call to JS with a function pointer; JS does a try-catch
// and calls the pointer, catching and reporting any error. If we know no
Expand Down
2 changes: 0 additions & 2 deletions src/tools/wasm-emscripten-finalize.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -259,8 +259,6 @@ int main(int argc, const char* argv[]) {
generator.onlyI64DynCalls = onlyI64DynCalls;
generator.noDynCalls = noDynCalls;

generator.fixInvokeFunctionNames();

std::vector<Name> initializerFunctions;

// The wasm backend emits "__indirect_function_table" as the import name for
Expand Down
130 changes: 1 addition & 129 deletions src/wasm/wasm-emscripten.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -487,134 +487,6 @@ EmJsWalker fixEmJsFuncsAndReturnWalker(Module& wasm) {
return walker;
}

// Fixes function name hacks caused by LLVM exception & setjmp/longjmp
// handling pass for wasm.
// This does two things:
// 1. Change emscripten_longjmp_jmpbuf to emscripten_longjmp.
// In setjmp/longjmp handling pass in wasm backend, what we want to do is
// to change all function calls to longjmp to calls to emscripten_longjmp.
// Because we replace all calls to longjmp to emscripten_longjmp, the
// signature of that function should be the same as longjmp:
// emscripten_longjmp(jmp_buf, int)
// But after calling a function that might longjmp, while we test whether
// a longjmp occurred, we have to load an int address value and call
// emscripten_longjmp again with that address as the first argument. (Refer
// to lib/Target/WebAssembly/WebAssemblyEmscriptenEHSjLj.cpp in LLVM for
// details.)
// In this case we need the signature of emscripten_longjmp to be (int,
// int). So we need two different kinds of emscripten_longjmp signatures in
// LLVM IR. Both signatures will be lowered to (int, int) eventually, but
// in LLVM IR, types are not lowered yet.
// So we declare two functions in LLVM:
// emscripten_longjmp_jmpbuf(jmp_buf, int)
// emscripten_longjmp(int, int)
// And we change the name of emscripten_longjmp_jmpbuf to
// emscripten_longjmp here.
// 2. Converts invoke wrapper names.
// Refer to the comments in fixEmExceptionInvoke below.
struct FixInvokeFunctionNamesWalker
: public PostWalker<FixInvokeFunctionNamesWalker> {
Module& wasm;
std::vector<Name> toRemove;
std::map<Name, Name> importRenames;
std::map<Name, Name> functionRenames;
std::set<Signature> invokeSigs;
ImportInfo imports;

FixInvokeFunctionNamesWalker(Module& _wasm) : wasm(_wasm), imports(wasm) {}

// Converts invoke wrapper names generated by LLVM backend to real invoke
// wrapper names that are expected by JavaScript glue code.
// This is required to support wasm exception handling (asm.js style).
//
// LLVM backend lowers
// invoke @func(arg1, arg2) to label %invoke.cont unwind label %lpad
// into
// ... (some code)
// call @invoke_SIG(func, arg1, arg2)
// ... (some code)
// SIG is a mangled string generated based on the LLVM IR-level function
// signature. In LLVM IR, types are not lowered yet, so this mangling scheme
// simply takes LLVM's string representtion of parameter types and concatenate
// them with '_'. For example, the name of an invoke wrapper for function
// void foo(struct mystruct*, int) will be
// "__invoke_void_%struct.mystruct*_int".
// This function converts the names of invoke wrappers based on their lowered
// argument types and a return type. In the example above, the resulting new
// wrapper name becomes "invoke_vii".
Name fixEmExceptionInvoke(const Name& name, Signature sig) {
std::string nameStr = name.c_str();
if (nameStr.front() == '"' && nameStr.back() == '"') {
nameStr = nameStr.substr(1, nameStr.size() - 2);
}
if (nameStr.find("__invoke_") != 0) {
return name;
}

std::vector<Type> newParams(sig.params.begin() + 1, sig.params.end());
Signature sigWoOrigFunc = Signature(Type(newParams), sig.results);
invokeSigs.insert(sigWoOrigFunc);
return Name("invoke_" +
getSig(sigWoOrigFunc.results, sigWoOrigFunc.params));
}

void visitFunction(Function* curr) {
if (!curr->imported()) {
return;
}

Name newname = fixEmExceptionInvoke(curr->base, curr->sig);
if (newname == curr->base) {
return;
}

BYN_TRACE("renaming import: " << curr->module << "." << curr->base << " ("
<< curr->name << ") -> " << newname << "\n");

if (auto* f = imports.getImportedFunction(curr->module, newname)) {
BYN_TRACE("remove redundant import: " << curr->base << "\n");
toRemove.push_back(curr->name);
// Make sure the existing import has the correct internal name.
if (f->name != newname) {
functionRenames[f->name] = newname;
}
} else {
BYN_TRACE("rename import: " << curr->base << "\n");
curr->base = newname;
}

functionRenames[curr->name] = newname;

// Ensure that an imported functions of this name exists.
importRenames[curr->base] = newname;
}

void visitModule(Module* curr) {
for (auto name : toRemove) {
wasm.removeFunction(name);
}

// Rename all uses of the old function to the new import name
ModuleUtils::renameFunctions(wasm, functionRenames);

// For imports that for renamed, update any associated GOT.func imports.
for (auto& pair : importRenames) {
BYN_TRACE("looking for: GOT.func." << pair.first << "\n");
if (auto g = imports.getImportedGlobal("GOT.func", pair.first)) {
BYN_TRACE("renaming corresponding GOT entry: " << g->base << " -> "
<< pair.second << "\n");
g->base = pair.second;
}
}
}
};

void EmscriptenGlueGenerator::fixInvokeFunctionNames() {
BYN_TRACE("fixInvokeFunctionNames\n");
FixInvokeFunctionNamesWalker walker(wasm);
walker.walkModule(&wasm);
}

void printSignatures(std::ostream& o, const std::set<Signature>& c) {
o << "[";
bool first = true;
Expand Down Expand Up @@ -746,7 +618,7 @@ std::string EmscriptenGlueGenerator::generateEmscriptenMetadata(
meta << " \"invokeFuncs\": [";
commaFirst = true;
ModuleUtils::iterImportedFunctions(wasm, [&](Function* import) {
if (import->base.startsWith("invoke_")) {
if (import->module == ENV && import->base.startsWith("invoke_")) {
if (invokeFuncs.insert(import->base.str).second) {
meta << nextElement() << '"' << import->base.str << '"';
}
Expand Down
13 changes: 0 additions & 13 deletions test/passes/post-emscripten.txt
Original file line number Diff line number Diff line change
Expand Up @@ -176,16 +176,3 @@
)
)
)
(module
(type $i32_i32_=>_none (func (param i32 i32)))
(import "env" "emscripten_longjmp" (func $a (param i32 i32)))
(import "env" "emscripten_longjmp" (func $b (param i32 i32)))
)
(module
(type $i32_i32_=>_none (func (param i32 i32)))
(import "env" "emscripten_longjmp_jmpbuf" (func $b (param i32 i32)))
)
(module
(type $i32_i32_=>_none (func (param i32 i32)))
(import "env" "emscripten_longjmp" (func $a (param i32 i32)))
)
11 changes: 0 additions & 11 deletions test/passes/post-emscripten.wast
Original file line number Diff line number Diff line change
Expand Up @@ -148,14 +148,3 @@
)
)
)
;; longjmp renaming
(module
(import "env" "emscripten_longjmp" (func $a (param i32 i32)))
(import "env" "emscripten_longjmp_jmpbuf" (func $b (param i32 i32)))
)
(module
(import "env" "emscripten_longjmp_jmpbuf" (func $b (param i32 i32)))
)
(module
(import "env" "emscripten_longjmp" (func $a (param i32 i32)))
)