[SjLj] Insert UnregisterFn before musttail call

[lxfind]

Summary

When inserting UnregisterFn, if there is a musttail call, we must insert before the call so that we don't break the musttail call contract.

[wenlei]

The change looks good. But...

• I wonder how many more times we'll have to deal with this.. It indeed seems hard to enforce without IR change :(
• Are we hitting SJLJ path? I thought there's no usage of that for our codebase.

[lxfind]

In D104807#2836836, @wenlei wrote:

The change looks good. But...

• I wonder how many more times we'll have to deal with this.. It indeed seems hard to enforce without IR change :(
• Are we hitting SJLJ path? I thought there's no usage of that for our codebase.

This was hit in armv7 for iOS.
Yeah I couldn't think of a way to enforce this.

[wenlei]

lgtm, thanks.

[modimo]

What would an IR change look like to enforce this?

[lxfind]

In D104807#2836908, @modimo wrote:

What would an IR change look like to enforce this?

Modify the IR insertion API and adjust insertion point whenever a musttail call is detected?

llvm/lib/CodeGen/SjLjEHPrepare.cpp

for (Instruction &I : BB) {
       Instruction *StackAddr = CallInst::Create(StackAddrFn, "sp");
       StackAddr->insertAfter(&I);
       new StoreInst(StackAddr, StackPtr, true, StackAddr->getNextNode());
     }
   }
 
   // Finally, for any returns from this function, if this function contains an
   // invoke, add a call to unregister the function context.
-  for (ReturnInst *Return : Returns)
-    CallInst::Create(UnregisterFn, FuncCtx, "", Return);
+  for (ReturnInst *Return : Returns) {
+    Instruction *InsertPoint = Return;
+    if (CallInst *CI = Return->getParent()->getTerminatingMustTailCall())
+      InsertPoint = CI;
+    CallInst::Create(UnregisterFn, FuncCtx, "", InsertPoint);
+  }
 
   return true;
 }
 
 bool SjLjEHPrepare::runOnFunction(Function &F) {
   Module &M = *F.getParent();
   RegisterFn = M.getOrInsertFunction(
       "_Unwind_SjLj_Register", Type::getVoidTy(M.getContext()),

llvm/test/CodeGen/X86/sjlj-eh-musttail.ll

+; FIXME: Fix machine verifier issues and remove -verify-machineinstrs=0. PR39439.
+; RUN: llc -mtriple i386-windows-gnu -exception-model sjlj -filetype asm -o - %s -verify-machineinstrs=0 | FileCheck %s
+
+declare void @_Z20function_that_throwsv()
+declare i32 @__gxx_personality_sj0(...)
+declare i8* @__cxa_begin_catch(i8*)
+declare void @__cxa_end_catch()
+declare void @_callee();
+
+define void @_Z8functionv() personality i8* bitcast (i32 (...)* @__gxx_personality_sj0 to i8*) {
+entry:
+  invoke void @_Z20function_that_throwsv()
+          to label %try.cont unwind label %lpad
+
+lpad:
+  %0 = landingpad { i8*, i32 }
+          catch i8* null
+  %1 = extractvalue { i8*, i32 } %0, 0
+  %2 = tail call i8* @__cxa_begin_catch(i8* %1)
+  tail call void @__cxa_end_catch()
+  br label %try.cont
+
+try.cont:
+  musttail call void @_callee();
+  ret void
+}
+
+; CHECK-LABEL: __Z8functionv:
+; CHECK:         calll   __Unwind_SjLj_Unregister
+; CHECK-NOT:     {{.*}}:
+; CHECK:         jmp     __callee