[SEH] Ignore EH pad check for internal intrinsics

[phoebewang]

Intrinsics like @llvm.seh.scope.begin and @llvm.seh.scope.end which do not throw do not need funclets in catchpads or cleanuppads.

Fixes #69428

Co-authored-by: Robert Cox robert.cox@intel.com

[llvmbot]

@llvm/pr-subscribers-llvm-ir

Author: Phoebe Wang (phoebewang)

Changes

Intrinsics like @llvm.seh.scope.begin and @llvm.seh.scope.end which do not throw do not need funclets in catchpads or cleanuppads.

Fixes #69428

Co-authored-by: Robert Cox <robert.cox@intel.com>

---

Full diff: https://github.com/llvm/llvm-project/pull/79694.diff

2 Files Affected:

• (modified) llvm/lib/IR/Verifier.cpp (+5)
• (added) llvm/test/Verifier/pr69428.ll (+80)

diff --git a/llvm/lib/IR/Verifier.cpp b/llvm/lib/IR/Verifier.cpp
index 91cf91fbc788bd9..0f1e9fa40c9b037 100644
--- a/llvm/lib/IR/Verifier.cpp
+++ b/llvm/lib/IR/Verifier.cpp
@@ -4280,6 +4280,11 @@ void Verifier::visitEHPadPredecessors(Instruction &I) {
     if (auto *II = dyn_cast<InvokeInst>(TI)) {
       Check(II->getUnwindDest() == BB && II->getNormalDest() != BB,
             "EH pad must be jumped to via an unwind edge", ToPad, II);
+      auto *CalledFn =
+          dyn_cast<Function>(II->getCalledOperand()->stripPointerCasts());
+      if (CalledFn && CalledFn->isIntrinsic() && II->doesNotThrow() &&
+          !IntrinsicInst::mayLowerToFunctionCall(CalledFn->getIntrinsicID()))
+        continue;
       if (auto Bundle = II->getOperandBundle(LLVMContext::OB_funclet))
         FromPad = Bundle->Inputs[0];
       else
diff --git a/llvm/test/Verifier/pr69428.ll b/llvm/test/Verifier/pr69428.ll
new file mode 100644
index 000000000000000..22d732076e3af7a
--- /dev/null
+++ b/llvm/test/Verifier/pr69428.ll
@@ -0,0 +1,80 @@
+; RUN: llvm-as -disable-output %s
+
+%struct._List_node_emplace_op2 = type { i8 }
+
+$"??1?$_List_node_emplace_op2@H@@QEAA@XZ" = comdat any
+
+@"?_List@@3HA" = dso_local local_unnamed_addr global i32 0, align 4
+
+; Function Attrs: mustprogress noreturn
+define dso_local void @"?ExecutionEngineaddExecutableDependency@@YAXXZ"() local_unnamed_addr #0 personality ptr @__CxxFrameHandler3 {
+entry:
+  %agg.tmp.ensured.i = alloca %struct._List_node_emplace_op2, align 1
+  call void @llvm.lifetime.start.p0(i64 1, ptr nonnull %agg.tmp.ensured.i)
+  %0 = load i32, ptr @"?_List@@3HA", align 4
+  %call.i = call noundef ptr @"??0?$_List_node_emplace_op2@H@@QEAA@H@Z"(ptr noundef nonnull align 1 dereferenceable(1) %agg.tmp.ensured.i, i32 noundef %0)
+  invoke void @llvm.seh.scope.begin()
+          to label %invoke.cont.i unwind label %ehcleanup.i
+
+invoke.cont.i:                                    ; preds = %entry
+  invoke void @llvm.seh.scope.end()
+          to label %invoke.cont2.i unwind label %ehcleanup.i
+
+invoke.cont2.i:                                   ; preds = %invoke.cont.i
+  call void @"??1?$_List_node_emplace_op2@H@@QEAA@XZ"(ptr noundef nonnull align 1 dereferenceable(1) %agg.tmp.ensured.i) #6
+  unreachable
+
+ehcleanup.i:                                      ; preds = %invoke.cont.i, %entry
+  %1 = cleanuppad within none []
+  invoke void @llvm.seh.scope.begin()
+          to label %invoke.cont.i.i unwind label %ehcleanup.i.i
+
+invoke.cont.i.i:                                  ; preds = %ehcleanup.i
+  invoke void @llvm.seh.scope.end()
+          to label %"??1?$_List_node_emplace_op2@H@@QEAA@XZ.exit.i" unwind label %ehcleanup.i.i
+
+ehcleanup.i.i:                                    ; preds = %invoke.cont.i.i, %ehcleanup.i
+  %2 = cleanuppad within %1 []
+  call void @"??1_Alloc_construct_ptr@@QEAA@XZ"(ptr noundef nonnull align 1 dereferenceable(1) %agg.tmp.ensured.i) #6 [ "funclet"(token %2) ]
+  cleanupret from %2 unwind to caller
+
+"??1?$_List_node_emplace_op2@H@@QEAA@XZ.exit.i":  ; preds = %invoke.cont.i.i
+  call void @"??1_Alloc_construct_ptr@@QEAA@XZ"(ptr noundef nonnull align 1 dereferenceable(1) %agg.tmp.ensured.i) #6 [ "funclet"(token %1) ]
+  cleanupret from %1 unwind to caller
+}
+
+declare dso_local noundef ptr @"??0?$_List_node_emplace_op2@H@@QEAA@H@Z"(ptr noundef nonnull returned align 1 dereferenceable(1), i32 noundef) unnamed_addr #1
+
+declare dso_local i32 @__CxxFrameHandler3(...)
+
+; Function Attrs: nofree nosync nounwind memory(none)
+declare dso_local void @llvm.seh.scope.begin() #2
+
+; Function Attrs: nofree nosync nounwind memory(none)
+declare dso_local void @llvm.seh.scope.end() #2
+
+; Function Attrs: mustprogress nounwind
+define linkonce_odr dso_local void @"??1?$_List_node_emplace_op2@H@@QEAA@XZ"(ptr noundef nonnull align 1 dereferenceable(1) %this) unnamed_addr #3 comdat align 2 personality ptr @__CxxFrameHandler3 {
+entry:
+  invoke void @llvm.seh.scope.begin()
+          to label %invoke.cont unwind label %ehcleanup
+
+invoke.cont:                                      ; preds = %entry
+  invoke void @llvm.seh.scope.end()
+          to label %invoke.cont2 unwind label %ehcleanup
+
+invoke.cont2:                                     ; preds = %invoke.cont
+  tail call void @"??1_Alloc_construct_ptr@@QEAA@XZ"(ptr noundef nonnull align 1 dereferenceable(1) %this) #6
+  ret void
+
+ehcleanup:                                        ; preds = %invoke.cont, %entry
+  %0 = cleanuppad within none []
+  call void @"??1_Alloc_construct_ptr@@QEAA@XZ"(ptr noundef nonnull align 1 dereferenceable(1) %this) #6 [ "funclet"(token %0) ]
+  cleanupret from %0 unwind to caller
+}
+
+; Function Attrs: nounwind
+declare dso_local void @"??1_Alloc_construct_ptr@@QEAA@XZ"(ptr noundef nonnull align 1 dereferenceable(1)) unnamed_addr #4
+
+; Function Attrs: nocallback nofree nosync nounwind willreturn memory(argmem: readwrite)
+declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture) #5

[phoebewang]

Ping?

[rnk]

Is there some reason we can't apply the funclet bundle to these invokes in clang?

I really worry that this could allow tail merging transforms that the funclet bundles are designed to prevent, something like this:

if (cond()) {
  __try {
    maythrow(1);
  } __except(1) {
    __try {
      __builtin_trap();
    } __finally { dtor1(); }
  }
} else {
  __try {
    maythrow(2);
  } __except(1) {
    __try {
      __builtin_trap();
    } __finally { dtor2(); }
  }
}

I guess cases like this are less common for SEH than C++ EH, especially since __except bodies are not modelled as separate funclets, and we fully outline __finally blocks in the frontend.

So, having reasoned through that, I think this is pretty low risk, but I want to push back just a little bit and ask if this can be implemented in the frontend.

Sorry for the delay, as usual, life and work are extremely busy.

[phoebewang]

Understood! I'm back from a short vacation too :)

I really worry that this could allow tail merging transforms that the funclet bundles are designed to prevent, something like this:

Not sure if I understood the question. It looks to me they are unrelated. The patch is to fix #69428, where a cleanup pad has more than one edges. This is intended to these SEH intrinsics because they are used to mark a scope of instructions that may trigger cleanup for a CPP object.

I don't see any risks to normal invokes since we limited it to internal intrinsics only.

[rnk]

My question is, if we mark calls to @llvm.seh.try with funclet bundles in the frontend, would that also fix the issue? If it does, that seems like a better solution. If not, we can do this instead.

[phoebewang]

My question is, if we mark calls to @llvm.seh.try with funclet bundles in the frontend, would that also fix the issue? If it does, that seems like a better solution. If not, we can do this instead.

Sorry, I'm not familiar with frontend code. And thanks for the explanation! I got the point now. It looks to me we tried to bundle them but failed: https://github.com/llvm/llvm-project/blob/main/clang/lib/CodeGen/CGCleanup.cpp#L1346-L1349

The EmitSehCppScopeEnd is called in several places. I'm guessing one of them at least not bundled successfully for some reason, I had to admit I cannot root casue it myself :(

[phoebewang]

FYI. MSVC cannot compile the reproducer in #69428, while I changed it a bit to make MSVC happy, the code passed on Clang too: https://godbolt.org/z/Ghv7ebhzo

Not sure if something related to C++ new feature.

[emmenlau]

Very kindly pinging this issue again, because it blocks building OpenCV with any clang-cl newer than 16.x

[Nick-Kooij]

Likewise, I have an internal project who's build is similarly blocked by this issue, and are also stuck on LLVM 16.0.6. Anything version after either hangs or crashes the compiler, matching the symptoms described in #69428.

I would respectfully ask this issue be prioritized.

[rnk]

Let's go with this change then.