[WebAssembly] Compare functions by names in Emscripten Sjlj

[aheejin]

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)

[[ https://github.com/llvm/llvm-project/blob/2a2c25ba480016f8ad8fa407af7627d84547637d/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L512 | Module::getFunction("malloc") in canLongjmp ]] returns nullptr
because Module::getFunction [[ https://github.com/llvm/llvm-project/blob/2a2c25ba480016f8ad8fa407af7627d84547637d/llvm/lib/IR/Module.cpp#L175 | dyncasts pointer into Function ]], but the
alias is a GlobalValue but not a Function. This makes canLongjmp
[[ https://github.com/llvm/llvm-project/blob/2a2c25ba480016f8ad8fa407af7627d84547637d/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L514 | 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.

[sbc100]

llvm/test/CodeGen/WebAssembly/lower-em-sjlj-malloc.ll

1 ↗

(On Diff #218540)

Seems like this test is more about "aliases" that it is about malloc in particular. Perhaps it should be called lower-em-sjlj-alias.ll?

[aheejin]

• Renamed to lower-em-sjlj-alias.ll

llvm/trunk/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

static cl::list<std::string>
     EHWhitelist("emscripten-cxx-exceptions-whitelist",
                 cl::desc("The list of function names in which Emscripten-style "
                          "exception handling is enabled (see emscripten "
                          "EMSCRIPTEN_CATCHING_WHITELIST options)"),
                 cl::CommaSeparated);
 
 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
 
   GlobalVariable *ThrewGV = nullptr;
   GlobalVariable *ThrewValueGV = nullptr;
   Function *GetTempRet0Func = nullptr;
   Function *SetTempRet0Func = nullptr;
   Function *ResumeF = nullptr;
public:
   bool runOnModule(Module &M) override;
 
   void getAnalysisUsage(AnalysisUsage &AU) const override {
     AU.addRequired<DominatorTreeWrapperPass>();
   }
 };
 } // 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",
                 false, false)
 
 ModulePass *llvm::createWebAssemblyLowerEmscriptenEHSjLj(bool EnableEH,
                                                          bool EnableSjLj) {
   return new WebAssemblyLowerEmscriptenEHSjLj(EnableEH, EnableSjLj);
static bool canThrow(const Value *V) {
   return true;
 }
 
 // Get a global variable with the given name.  If it doesn't exist declare it,
 // which will generate an import and asssumes that it will exist at link time.
 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);
 
   return GV;
 }
 
 // Simple function name mangler.
 // This function simply takes LLVM's string representation of parameter types

 Function *
 WebAssemblyLowerEmscriptenEHSjLj::getFindMatchingCatch(Module &M,
                                                        unsigned NumClauses) {
   if (FindMatchingCatches.count(NumClauses))
     return FindMatchingCatches[NumClauses];
   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;
 }
 
 // Generate invoke wrapper seqence with preamble and postamble
 // Preamble:
 // __THREW__ = 0;
 // Postamble:
Function *WebAssemblyLowerEmscriptenEHSjLj::getInvokeWrapper(CallOrInvoke *CI) {
 
   // Put the pointer to the callee as first argument
   ArgTys.push_back(PointerType::getUnqual(CalleeFTy));
   // Add argument types
   ArgTys.append(CalleeFTy->param_begin(), CalleeFTy->param_end());
 
   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;
 }
 
 bool WebAssemblyLowerEmscriptenEHSjLj::canLongjmp(Module &M,
                                                   const Value *Callee) const {
   if (auto *CalleeF = dyn_cast<Function>(Callee))
     if (CalleeF->isIntrinsic())
       return false;
 
   // Attempting to transform inline assembly will result in something like:
   //     call void @__invoke_void(void ()* asm ...)
   // which is invalid because inline assembly blocks do not have addresses
   // 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
   return true;
 }
 
 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.
 // The code this generates is equivalent to the following JavaScript code:
 // if (%__THREW__.val != 0 & threwValue != 0) {
 //   %label = _testSetjmp(mem[%__THREW__.val], setjmpTable, setjmpTableSize);
 //   if (%label == 0)
 //     emscripten_longjmp(%__THREW__.val, threwValue);
bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
   bool Changed = false;
 
   // Exception handling
   if (EnableEH) {
     // Register __resumeException function
     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())
         continue;
       Changed |= runEHOnFunction(F);
     }
   }
 
   // Setjmp/longjmp handling
   if (DoSjLj) {
     Changed = true; // We have setjmp or longjmp somewhere
 
     if (LongjmpF) {
       // Replace all uses of longjmp with emscripten_longjmp_jmpbuf, which is
       // defined in JS code
       EmLongjmpJmpbufF = Function::Create(LongjmpF->getFunctionType(),
                                           GlobalValue::ExternalLinkage,
-                                          EmLongjmpJmpbufFName, &M);
+                                          "emscripten_longjmp_jmpbuf", &M);
 
       LongjmpF->replaceAllUsesWith(EmLongjmpJmpbufF);
     }
 
     if (SetjmpF) {
       // Register saveSetjmp function
       FunctionType *SetjmpFTy = SetjmpF->getFunctionType();
       SmallVector<Type *, 4> Params = {SetjmpFTy->getParamType(0),
                                        IRB.getInt32Ty(), Type::getInt32PtrTy(C),
                                        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
       SmallPtrSet<Function *, 8> SetjmpUsers;
       for (User *U : SetjmpF->users()) {
         auto *UI = cast<Instruction>(U);
         SetjmpUsers.insert(UI->getFunction());
       }
for (Instruction &I : *BB) {
         report_fatal_error("Cannot use EM_ASM* alongside setjmp/longjmp in " +
                                F.getName() +
                                ". Please consider using EM_JS, or move the "
                                "EM_ASM into another function.",
                            false);
 
       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__;
         // in postamble after the invoke wrapper call
         LoadInst *ThrewLI = nullptr;
         StoreInst *ThrewResetSI = nullptr;
         for (auto I = std::next(BasicBlock::iterator(CI)), IE = BB->end();
              I != IE; ++I) {

llvm/trunk/test/CodeGen/WebAssembly/lower-em-sjlj-alias.ll

+; 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 }