[PATCH] D108955: [WebAssembly] Free setjmpTable before exiting calls in EmSjLj

[Heejin Ahn via Phabricator via llvm-commits]

aheejin created this revision.
aheejin added a reviewer: dschuff.
Herald added subscribers: wingo, ecnelises, sunfish, hiraditya, jgravelle-google, sbc100.
aheejin requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This is an improvement over D107852 <https://reviews.llvm.org/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`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108955

Files:
  llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
  llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll


Index: llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll
===================================================================
--- llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll
+++ llvm/test/CodeGen/WebAssembly/lower-em-sjlj.ll
@@ -311,7 +311,7 @@
 ; 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) }
 
Index: llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
===================================================================
--- llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
@@ -434,15 +434,6 @@
   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);
 
@@ -760,6 +751,7 @@
     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 =
@@ -790,6 +782,7 @@
     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
@@ -1152,11 +1145,16 @@
     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);
       }
     }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D108955.369573.patch
Type: text/x-patch
Size: 3408 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210830/adad05cf/attachment.bin>