[WebAssembly] Free setjmpTable before exiting calls in EmSjLj

[aheejin]

Summary

This is an improvement over D107852. We don't need to enumerate specific
function names; we can just check for noreturn attribute. This also
requires us to make sure __resumeExeption and emscripten_longjmp
have noreturn attribute too; one of them is a JS function and the
other calls a JS function so Clang does not have a way to deduce they
don't return.

This is effectively NFC, because I'm not sure if there is an additional
case this case covers; if we add a custom function call that has
noreturn attribute, it will be processed within the SjLj handling and
turned into __invoke call. So this really applies to some special
functions like emscripten_longjmp.

[aheejin]

llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

437–444

This removes noreturn attribute from emscripten_longjmp, which makes this CL incorrect. Also, I'm not sure why this code was there in the first place. We don't call those noreturn functions within the LLVM IR anymore; we call invokes instead, which call the noreturn function via indirection, so it doesn't violate the noreturn property.

I traced the origin and found fastcomp's Emscripten EH handling code has the same snippet. I guess then I conservatively pasted that out of concerns something might break without that. But all emscripten tests pass without this fine.

llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp

 // Postamble:
 // %__THREW__.val = __THREW__; __THREW__ = 0;
 // Returns %__THREW__.val, which indicates whether an exception is thrown (or
 // whether longjmp occurred), for future use.
 Value *WebAssemblyLowerEmscriptenEHSjLj::wrapInvoke(CallBase *CI) {
   Module *M = CI->getModule();
   LLVMContext &C = M->getContext();
 
-  // If we are calling a function that is noreturn, we must remove that
-  // attribute. The code we insert here does expect it to return, after we
-  // catch the exception.
-  if (CI->doesNotReturn()) {
-    if (auto *F = CI->getCalledFunction())
-      F->removeFnAttr(Attribute::NoReturn);
-    CI->removeFnAttr(Attribute::NoReturn);
-  }
-
   IRBuilder<> IRB(C);
   IRB.SetInsertPoint(CI);
 
   // Pre-invoke
   // __THREW__ = 0;
   IRB.CreateStore(getAddrSizeInt(M, 0), ThrewGV);
 
   // Invoke function wrapper in JavaScript
bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
   bool Changed = false;
 
   // Function registration for exception handling
   if (EnableEmEH) {
     // Register __resumeException function
     FunctionType *ResumeFTy =
         FunctionType::get(IRB.getVoidTy(), IRB.getInt8PtrTy(), false);
     ResumeF = getEmscriptenFunction(ResumeFTy, "__resumeException", &M);
+    ResumeF->addFnAttr(Attribute::NoReturn);
 
     // Register llvm_eh_typeid_for function
     FunctionType *EHTypeIDTy =
         FunctionType::get(IRB.getInt32Ty(), IRB.getInt8PtrTy(), false);
     EHTypeIDF = getEmscriptenFunction(EHTypeIDTy, "llvm_eh_typeid_for", &M);
   }
 
   if (EnableEmSjLj && SetjmpF) {
bool WebAssemblyLowerEmscriptenEHSjLj::runOnModule(Module &M) {
 
   // Function registration and data pre-gathering for setjmp/longjmp handling
   if (DoSjLj) {
     assert(EnableEmSjLj || EnableWasmSjLj);
     // Register emscripten_longjmp function
     FunctionType *FTy = FunctionType::get(
         IRB.getVoidTy(), {getAddrIntType(&M), IRB.getInt32Ty()}, false);
     EmLongjmpF = getEmscriptenFunction(FTy, "emscripten_longjmp", &M);
+    EmLongjmpF->addFnAttr(Attribute::NoReturn);
 
     if (SetjmpF) {
       // Register saveSetjmp function
       FunctionType *SetjmpFTy = SetjmpF->getFunctionType();
       FTy = FunctionType::get(Type::getInt32PtrTy(C),
                               {SetjmpFTy->getParamType(0), IRB.getInt32Ty(),
                                Type::getInt32PtrTy(C), IRB.getInt32Ty()},
                               false);
bool WebAssemblyLowerEmscriptenEHSjLj::runSjLjOnFunction(Function &F) {
 
   // Free setjmpTable buffer before each return instruction + function-exiting
   // call
   SmallVector<Instruction *, 16> ExitingInsts;
   for (BasicBlock &BB : F) {
     Instruction *TI = BB.getTerminator();
     if (isa<ReturnInst>(TI))
       ExitingInsts.push_back(TI);
+    // Any 'call' instruction with 'noreturn' attribute exits the function at
+    // this point. If this throws but unwinds to another EH pad within this
+    // function instead of exiting, this would have been an 'invoke', which
+    // happens if we use Wasm EH or Wasm SjLJ.
     for (auto &I : BB) {
-      if (auto *CB = dyn_cast<CallBase>(&I)) {
-        StringRef CalleeName = CB->getCalledOperand()->getName();
-        if (CalleeName == "__resumeException" ||
-            CalleeName == "emscripten_longjmp" || CalleeName == "__cxa_throw")
+      if (auto *CI = dyn_cast<CallInst>(&I)) {
+        bool IsNoReturn = CI->hasFnAttr(Attribute::NoReturn);
+        if (auto *CalleeF = dyn_cast<Function>(CI->getCalledOperand()))
+          IsNoReturn |= CalleeF->hasFnAttribute(Attribute::NoReturn);
+        if (IsNoReturn)
           ExitingInsts.push_back(&I);
       }
     }
   }
   for (auto *I : ExitingInsts) {
     DebugLoc DL = getOrCreateDebugLoc(I, F.getSubprogram());
     auto *Free = CallInst::CreateFree(SetjmpTable, I);
     Free->setDebugLoc(DL);

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

 attributes #1 = { noreturn }
 attributes #2 = { nounwind }
 attributes #3 = { allocsize(0) }
 ; CHECK-DAG: attributes #{{[0-9]+}} = { nounwind "wasm-import-module"="env" "wasm-import-name"="getTempRet0" }
 ; CHECK-DAG: attributes #{{[0-9]+}} = { nounwind "wasm-import-module"="env" "wasm-import-name"="setTempRet0" }
 ; CHECK-DAG: attributes #{{[0-9]+}} = { "wasm-import-module"="env" "wasm-import-name"="__invoke_void" }
 ; CHECK-DAG: attributes #{{[0-9]+}} = { "wasm-import-module"="env" "wasm-import-name"="saveSetjmp" }
 ; CHECK-DAG: attributes #{{[0-9]+}} = { "wasm-import-module"="env" "wasm-import-name"="testSetjmp" }
-; CHECK-DAG: attributes #{{[0-9]+}} = { "wasm-import-module"="env" "wasm-import-name"="emscripten_longjmp" }
+; CHECK-DAG: attributes #{{[0-9]+}} = { noreturn "wasm-import-module"="env" "wasm-import-name"="emscripten_longjmp" }
 ; CHECK-DAG: attributes #{{[0-9]+}} = { "wasm-import-module"="env" "wasm-import-name"="__invoke_i8*_i32_%struct.__jmp_buf_tag*" }
 ; CHECK-DAG: attributes #[[ALLOCSIZE_ATTR]] = { allocsize(1) }
 
 !llvm.dbg.cu = !{!2}
 !llvm.module.flags = !{!0}
 
 !0 = !{i32 2, !"Debug Info Version", i32 3}
 !1 = !DIFile(filename: "lower-em-sjlj.c", directory: "test")
 !2 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1)
 !3 = distinct !DISubprogram(name: "setjmp_debug_info", unit:!2, file: !1, line: 1)
 !4 = !DILocation(line:2, scope: !3)
 !5 = !DILocation(line:3, scope: !3)
 !6 = !DILocation(line:4, scope: !3)
 !7 = !DILocation(line:5, scope: !3)
 !8 = !DILocation(line:6, scope: !3)