[PATCH] D109375: [WebAssembly] Error out on indirect uses of setjmp

[Heejin Ahn via Phabricator via llvm-commits]

aheejin created this revision.
aheejin added reviewers: dschuff, tlively.
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.

Both Wasm & Emscripten SjLj handling has a restriction that `setjmp`
cannot be called indirectly. I thought we have been erroring out on
indirect uses of `setjmp`, but some recent CL disrupted the logic and
we are not erroring out anymore.

We currently

1. Collect functions that contain `setjmp` calls in `SetjmpUsers`. This only counts direct calls: https://github.com/llvm/llvm-project/blob/8f77dc459e31aad6daab89a124fa92067916274c/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L869-L878
2. Run `runSjLjOnFunction` only on those `SetjmpUsers`. Within `runSjLjOnFunction`, if we see an indirect use of `setjmp`, we error out: https://github.com/llvm/llvm-project/blob/8f77dc459e31aad6daab89a124fa92067916274c/llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp#L1218-L1221

So if there are only indirect setjmp calls within the module,
`SetjmpUsers` will be empty, and `runSjLjOnFunction` is not even entered
once. And the indirect `setjmp` call will error out at link time. So in
this CL we check for the indirect uses of `setjmp` upfront before we
enter `runSjLjOnFunction`.

Also this currently errors out on `invoke @setjmp`, which can only occur
when using Wasm EH + Wasm SjLj within a function. We recently added Wasm
SjLj support but we don't support using Wasm EH + Wasm SjLj in the same
function yet. We plan to add this support very soon, so I don't think
it's worth creating another test file just for this. (This is an error
test so it needs its own file)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D109375

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


Index: llvm/test/CodeGen/WebAssembly/lower-em-sjlj-indirect-setjmp.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/WebAssembly/lower-em-sjlj-indirect-setjmp.ll
@@ -0,0 +1,27 @@
+; RUN: not --crash llc < %s -enable-emscripten-sjlj 2>&1 | FileCheck %s
+; RUN: not --crash llc < %s -wasm-enable-sjlj -mattr=+exception-handling -exception-model=wasm 2>&1 | FileCheck %s
+
+target datalayout = "e-m:e-p:32:32-i64:64-n32:64-S128"
+target triple = "wasm32-unknown-unknown"
+
+%struct.__jmp_buf_tag = type { [6 x i32], i32, [32 x i32] }
+
+; CHECK: LLVM ERROR: Does not support indirect uses of setjmp
+ at setjmp_fp = global i32 (%struct.__jmp_buf_tag*)* @setjmp, align 4
+
+define void @indirect_setjmp_call() {
+entry:
+  %buf = alloca [1 x %struct.__jmp_buf_tag], align 16
+  %0 = load i32 (%struct.__jmp_buf_tag*)*, i32 (%struct.__jmp_buf_tag*)** @setjmp_fp, align 4
+  %arraydecay = getelementptr inbounds [1 x %struct.__jmp_buf_tag], [1 x %struct.__jmp_buf_tag]* %buf, i32 0, i32 0
+  %call = call i32 %0(%struct.__jmp_buf_tag* %arraydecay)
+  call void @foo()
+  ret void
+}
+
+declare void @foo()
+; Function Attrs: returns_twice
+declare i32 @setjmp(%struct.__jmp_buf_tag*) #0
+
+attributes #0 = { returns_twice }
+
Index: llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
===================================================================
--- llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
+++ llvm/lib/Target/WebAssembly/WebAssemblyLowerEmscriptenEHSjLj.cpp
@@ -867,13 +867,19 @@
   if ((EnableEmSjLj || EnableWasmSjLj) && SetjmpF) {
     // Precompute setjmp users
     for (User *U : SetjmpF->users()) {
-      if (auto *UI = dyn_cast<Instruction>(U)) {
-        auto *UserF = UI->getFunction();
+      if (auto *CB = dyn_cast<CallBase>(U)) {
+        auto *UserF = CB->getFunction();
         // If a function that calls setjmp does not contain any other calls that
         // can longjmp, we don't need to do any transformation on that function,
         // so can ignore it
         if (containsLongjmpableCalls(UserF))
           SetjmpUsers.insert(UserF);
+      } else {
+        std::string S;
+        raw_string_ostream SS(S);
+        SS << *U;
+        report_fatal_error(Twine("Does not support indirect uses of setjmp: ") +
+                           SS.str());
       }
     }
   }
@@ -1217,9 +1223,10 @@
   Function *SetjmpF = M.getFunction("setjmp");
   for (User *U : SetjmpF->users()) {
     auto *CI = dyn_cast<CallInst>(U);
+    // FIXME 'invoke' to setjmp can happen when we use Wasm EH + Wasm SjLj, but
+    // we don't support two being used together yet.
     if (!CI)
-      report_fatal_error("Does not support indirect calls to setjmp");
-
+      report_fatal_error("Wasm EH + Wasm SjLj is not fully supported yet");
     BasicBlock *BB = CI->getParent();
     if (BB->getParent() != &F) // in other function
       continue;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D109375.371115.patch
Type: text/x-patch
Size: 2951 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210907/ca77d990/attachment.bin>