[WebAssembly] Compare functions by names in Emscripten Sjlj

Summary:
This removes all string constants for function names and compares
functions by string directly when needed. Many of these constants are
used only once or twice so the benefit of defining them separately is
not very clear, and this actually fixes a bug.

When we already have a `malloc` declaration which is an alias to
something else within the module,
```
@malloc = weak hidden alias i8* (i32), i8* (i32)* @dlmalloc
```
(this happens compiling with emscripten with `-s WASM_OBJECT_FILES=0`
because all bc files are merged before being fed into `wasm-ld` which
runs the backend optimizations as LTO)

`Module::getFunction("malloc")` in `canLongjmp` returns `nullptr`
because `Module::getFunction` dyncasts pointer into `Function`, but the
alias is a `GlobalValue` but not a `Function`. This makes `canLongjmp`
return false for `malloc` in this case, and we end up adding a lot of
longjmp handling code around malloc. This is not only a code size
increase but actually a bug because `malloc` is used in the entry block
when preparing for setjmp tables for emscripten sjlj handling, and this
makes initial setjmp preparation, which has to happen in the entry
block, move to another split block, and this interferes with SSA update
later.

This also adds two more functions, `getTempRet0` and `setTempRet0`, in
the list of not longjmp-able functions.

Fixes https://github.com/emscripten-core/emscripten/issues/8935.

Reviewers: sbc100

Subscribers: mehdi_amini, jgravelle-google, hiraditya, sunfish, dexonsmith, dschuff, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D67129

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370828 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp b/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
index d70e197cbcbe3c4dd945b6d65f7e71802fe7b694..1cf397dd060bb67cf53644782cbae92e7e9f9202 100644 (file)
--- a/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
+++ b/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
@@ -227,15 +227,6 @@ static cl::list<std::string>
 
 namespace {
 class WebAssemblyLowerEmscriptenEHSjLj final : public ModulePass {
-  static const char *ResumeFName;
-  static const char *EHTypeIDFName;
-  static const char *EmLongjmpFName;
-  static const char *EmLongjmpJmpbufFName;
-  static const char *SaveSetjmpFName;
-  static const char *TestSetjmpFName;
-  static const char *FindMatchingCatchPrefix;
-  static const char *InvokePrefix;
-
   bool EnableEH;   // Enable exception handling
   bool EnableSjLj; // Enable setjmp/longjmp handling
 
@@ -293,19 +284,6 @@ public:
 };
 } // End anonymous namespace
 
-const char *WebAssemblyLowerEmscriptenEHSjLj::ResumeFName = "__resumeException";
-const char *WebAssemblyLowerEmscriptenEHSjLj::EHTypeIDFName =
-    "llvm_eh_typeid_for";
-const char *WebAssemblyLowerEmscriptenEHSjLj::EmLongjmpFName =
-    "emscripten_longjmp";
-const char *WebAssemblyLowerEmscriptenEHSjLj::EmLongjmpJmpbufFName =
-    "emscripten_longjmp_jmpbuf";
-const char *WebAssemblyLowerEmscriptenEHSjLj::SaveSetjmpFName = "saveSetjmp";
-const char *WebAssemblyLowerEmscriptenEHSjLj::TestSetjmpFName = "testSetjmp";
-const char *WebAssemblyLowerEmscriptenEHSjLj::FindMatchingCatchPrefix =
-    "__cxa_find_matching_catch_";
-const char *WebAssemblyLowerEmscriptenEHSjLj::InvokePrefix = "__invoke_";
-
 char WebAssemblyLowerEmscriptenEHSjLj::ID = 0;
 INITIALIZE_PASS(WebAssemblyLowerEmscriptenEHSjLj, DEBUG_TYPE,
                 "WebAssembly Lower Emscripten Exceptions / Setjmp / Longjmp",
@@ -336,7 +314,8 @@ static bool canThrow(const Value *V) {
 static GlobalVariable *getGlobalVariableI32(Module &M, IRBuilder<> &IRB,
                                             const char *Name) {
 
-  auto* GV = dyn_cast<GlobalVariable>(M.getOrInsertGlobal(Name, IRB.getInt32Ty()));
+  auto *GV =
+      dyn_cast<GlobalVariable>(M.getOrInsertGlobal(Name, IRB.getInt32Ty()));
   if (!GV)
     report_fatal_error(Twine("unable to create global: ") + Name);
 
@@ -377,9 +356,9 @@ WebAssemblyLowerEmscriptenEHSjLj::getFindMatchingCatch(Module &M,
   PointerType *Int8PtrTy = Type::getInt8PtrTy(M.getContext());
   SmallVector<Type *, 16> Args(NumClauses, Int8PtrTy);
   FunctionType *FTy = FunctionType::get(Int8PtrTy, Args, false);
-  Function *F =
-      Function::Create(FTy, GlobalValue::ExternalLinkage,
-                       FindMatchingCatchPrefix + Twine(NumClauses + 2), &M);
+  Function *F = Function::Create(
+      FTy, GlobalValue::ExternalLinkage,
+      "__cxa_find_matching_catch_" + Twine(NumClauses + 2), &M);
   FindMatchingCatches[NumClauses] = F;
   return F;
 }
@@ -487,8 +466,8 @@ Function *WebAssemblyLowerEmscriptenEHSjLj::getInvokeWrapper(CallOrInvoke *CI) {
 
   FunctionType *FTy = FunctionType::get(CalleeFTy->getReturnType(), ArgTys,
                                         CalleeFTy->isVarArg());
-  Function *F = Function::Create(FTy, GlobalValue::ExternalLinkage,
-                                 InvokePrefix + Sig, M);
+  Function *F =
+      Function::Create(FTy, GlobalValue::ExternalLinkage, "__invoke_" + Sig, M);
   InvokeWrappers[Sig] = F;
   return F;
 }
@@ -505,33 +484,27 @@ bool WebAssemblyLowerEmscriptenEHSjLj::canLongjmp(Module &M,
   // and can't be passed by pointer. The result is a crash with illegal IR.
   if (isa<InlineAsm>(Callee))
     return false;
+  StringRef CalleeName = Callee->getName();
 
   // The reason we include malloc/free here is to exclude the malloc/free
   // calls generated in setjmp prep / cleanup routines.
-  Function *SetjmpF = M.getFunction("setjmp");
-  Function *MallocF = M.getFunction("malloc");
-  Function *FreeF = M.getFunction("free");
-  if (Callee == SetjmpF || Callee == MallocF || Callee == FreeF)
+  if (CalleeName == "setjmp" || CalleeName == "malloc" || CalleeName == "free")
     return false;
 
   // There are functions in JS glue code
-  if (Callee == ResumeF || Callee == EHTypeIDF || Callee == SaveSetjmpF ||
-      Callee == TestSetjmpF)
+  if (CalleeName == "__resumeException" || CalleeName == "llvm_eh_typeid_for" ||
+      CalleeName == "saveSetjmp" || CalleeName == "testSetjmp" ||
+      CalleeName == "getTempRet0" || CalleeName == "setTempRet0")
     return false;
 
   // __cxa_find_matching_catch_N functions cannot longjmp
-  if (Callee->getName().startswith(FindMatchingCatchPrefix))
+  if (Callee->getName().startswith("__cxa_find_matching_catch_"))
     return false;
 
   // Exception-catching related functions
-  Function *BeginCatchF = M.getFunction("__cxa_begin_catch");
-  Function *EndCatchF = M.getFunction("__cxa_end_catch");
-  Function *AllocExceptionF = M.getFunction("__cxa_allocate_exception");
-  Function *ThrowF = M.getFunction("__cxa_throw");
-  Function *TerminateF = M.getFunction("__clang_call_terminate");
-  if (Callee == BeginCatchF || Callee == EndCatchF ||
-      Callee == AllocExceptionF || Callee == ThrowF || Callee == TerminateF ||
-      Callee == GetTempRet0Func || Callee == SetTempRet0Func)
+  if (CalleeName == "__cxa_begin_catch" || CalleeName == "__cxa_end_catch" ||
+      CalleeName == "__cxa_allocate_exception" || CalleeName == "__cxa_throw" ||
+      CalleeName == "__clang_call_terminate")
     return false;
 
   // Otherwise we don't know
@@ -540,19 +513,13 @@ bool WebAssemblyLowerEmscriptenEHSjLj::canLongjmp(Module &M,
 
 bool WebAssemblyLowerEmscriptenEHSjLj::isEmAsmCall(Module &M,
                                                    const Value *Callee) const {
+  StringRef CalleeName = Callee->getName();
   // This is an exhaustive list from Emscripten's <emscripten/em_asm.h>.
-  Function *EmAsmConstIntF = M.getFunction("emscripten_asm_const_int");
-  Function *EmAsmConstDoubleF = M.getFunction("emscripten_asm_const_double");
-  Function *EmAsmConstIntSyncMainF =
-      M.getFunction("emscripten_asm_const_int_sync_on_main_thread");
-  Function *EmAsmConstDoubleSyncMainF =
-      M.getFunction("emscripten_asm_const_double_sync_on_main_thread");
-  Function *EmAsmConstAsyncMainF =
-      M.getFunction("emscripten_asm_const_async_on_main_thread");
-
-  return Callee == EmAsmConstIntF || Callee == EmAsmConstDoubleF ||
-         Callee == EmAsmConstIntSyncMainF ||
-         Callee == EmAsmConstDoubleSyncMainF || Callee == EmAsmConstAsyncMainF;
+  return CalleeName == "emscripten_asm_const_int" ||
+         CalleeName == "emscripten_asm_const_double" ||
+         CalleeName == "emscripten_asm_const_int_sync_on_main_thread" ||
+         CalleeName == "emscripten_asm_const_double_sync_on_main_thread" ||
+         CalleeName == "emscripten_asm_const_async_on_main_thread";
 }
 
 // Generate testSetjmp function call seqence with preamble and postamble.
@@ -688,13 +655,13 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
     FunctionType *ResumeFTy =
         FunctionType::get(IRB.getVoidTy(), IRB.getInt8PtrTy(), false);
     ResumeF = Function::Create(ResumeFTy, GlobalValue::ExternalLinkage,
-                               ResumeFName, &M);
+                               "__resumeException", &M);
 
     // Register llvm_eh_typeid_for function
     FunctionType *EHTypeIDTy =
         FunctionType::get(IRB.getInt32Ty(), IRB.getInt8PtrTy(), false);
     EHTypeIDF = Function::Create(EHTypeIDTy, GlobalValue::ExternalLinkage,
-                                 EHTypeIDFName, &M);
+                                 "llvm_eh_typeid_for", &M);
 
     for (Function &F : M) {
       if (F.isDeclaration())
@@ -712,7 +679,7 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
       // defined in JS code
       EmLongjmpJmpbufF = Function::Create(LongjmpF->getFunctionType(),
                                           GlobalValue::ExternalLinkage,
-                                          EmLongjmpJmpbufFName, &M);
+                                          "emscripten_longjmp_jmpbuf", &M);
 
       LongjmpF->replaceAllUsesWith(EmLongjmpJmpbufF);
     }
@@ -725,19 +692,19 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
                                        IRB.getInt32Ty()};
       FunctionType *FTy =
           FunctionType::get(Type::getInt32PtrTy(C), Params, false);
-      SaveSetjmpF = Function::Create(FTy, GlobalValue::ExternalLinkage,
-                                     SaveSetjmpFName, &M);
+      SaveSetjmpF =
+          Function::Create(FTy, GlobalValue::ExternalLinkage, "saveSetjmp", &M);
 
       // Register testSetjmp function
       Params = {IRB.getInt32Ty(), Type::getInt32PtrTy(C), IRB.getInt32Ty()};
       FTy = FunctionType::get(IRB.getInt32Ty(), Params, false);
-      TestSetjmpF = Function::Create(FTy, GlobalValue::ExternalLinkage,
-                                     TestSetjmpFName, &M);
+      TestSetjmpF =
+          Function::Create(FTy, GlobalValue::ExternalLinkage, "testSetjmp", &M);
 
       FTy = FunctionType::get(IRB.getVoidTy(),
                               {IRB.getInt32Ty(), IRB.getInt32Ty()}, false);
       EmLongjmpF = Function::Create(FTy, GlobalValue::ExternalLinkage,
-                                    EmLongjmpFName, &M);
+                                    "emscripten_longjmp", &M);
 
       // Only traverse functions that uses setjmp in order not to insert
       // unnecessary prep / cleanup code in every function
@@ -1007,7 +974,7 @@ bool WebAssemblyLowerEmscriptenEHSjLj::runSjLjOnFunction(Function &F) {
 
       Value *Threw = nullptr;
       BasicBlock *Tail;
-      if (Callee->getName().startswith(InvokePrefix)) {
+      if (Callee->getName().startswith("__invoke_")) {
         // If invoke wrapper has already been generated for this call in
         // previous EH phase, search for the load instruction
         // %__THREW__.val = __THREW__;

diff --git a/test/CodeGen/WebAssembly/lower-em-sjlj-alias.ll b/test/CodeGen/WebAssembly/lower-em-sjlj-alias.ll
new file mode 100644 (file)
index 0000000..6cd8e12
--- /dev/null
+++ b/test/CodeGen/WebAssembly/lower-em-sjlj-alias.ll
@@ -0,0 +1,43 @@
+; RUN: opt < %s -wasm-lower-em-ehsjlj -S | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-emscripten"
+
+; Tests if an alias to a function (here malloc) is correctly handled as a
+; function that cannot longjmp.
+
+%struct.__jmp_buf_tag = type { [6 x i32], i32, [32 x i32] }
+@malloc = weak alias i8* (i32), i8* (i32)* @dlmalloc
+
+; CHECK-LABEL: @malloc_test
+define void @malloc_test() {
+entry:
+; CHECK-LABEL: entry
+  ; All setjmp table preparations have to happen within the entry block. These
+  ; check lines list only some of the instructions for that.
+  ; CHECK: call i8* @malloc
+  ; CHECK: call i32* @saveSetjmp
+  ; CHECK: call i32 @getTempRet0
+  %retval = alloca i32, align 4
+  %jmp = alloca [1 x %struct.__jmp_buf_tag], align 16
+  store i32 0, i32* %retval, align 4
+  %arraydecay = getelementptr inbounds [1 x %struct.__jmp_buf_tag], [1 x %struct.__jmp_buf_tag]* %jmp, i32 0, i32 0
+  %call = call i32 @setjmp(%struct.__jmp_buf_tag* %arraydecay) #0
+  ret void
+
+; CHECK-LABEL: entry.split
+  ; CHECK: call void @free
+  ; CHECK: ret void
+}
+
+; This is a dummy dlmalloc implemenation only to make compiler pass, because an
+; alias (malloc) has to point an actual definition.
+define i8* @dlmalloc(i32) {
+  %p = inttoptr i32 0 to i8*
+  ret i8* %p
+}
+
+; Function Attrs: returns_twice
+declare i32 @setjmp(%struct.__jmp_buf_tag*) #0
+
+attributes #0 = { returns_twice }