Skip to content

Conversation

@AbhayKanhere
Copy link
Contributor

when exception handling with setjmp/longjmp (exception-mode=eh_sjlj is enabled,
eh_sjlj_callsite intrinsic is inserted in same basic block as the throwing/exception instruction. This fix ensures stack protector insertion code does not split the block and move these apart into different basic blocks.

@llvmbot
Copy link
Member

llvmbot commented Jul 7, 2025

@llvm/pr-subscribers-backend-arm

Author: Abhay Kanhere (AbhayKanhere)

Changes

when exception handling with setjmp/longjmp (exception-mode=eh_sjlj is enabled,
eh_sjlj_callsite intrinsic is inserted in same basic block as the throwing/exception instruction. This fix ensures stack protector insertion code does not split the block and move these apart into different basic blocks.


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

2 Files Affected:

  • (modified) llvm/lib/CodeGen/StackProtector.cpp (+9-1)
  • (added) llvm/test/CodeGen/ARM/stack-protector-eh-sjlj.ll (+166)
diff --git a/llvm/lib/CodeGen/StackProtector.cpp b/llvm/lib/CodeGen/StackProtector.cpp
index 5f866eea7d4e7..a2bc69ef5ff4d 100644
--- a/llvm/lib/CodeGen/StackProtector.cpp
+++ b/llvm/lib/CodeGen/StackProtector.cpp
@@ -588,7 +588,14 @@ bool InsertStackProtectors(const TargetMachine *TM, Function *F,
       continue;
     Instruction *CheckLoc = dyn_cast<ReturnInst>(BB.getTerminator());
     if (!CheckLoc && !DisableCheckNoReturn)
-      for (auto &Inst : BB)
+      for (auto &Inst : BB) {
+        if(auto *IB = dyn_cast<IntrinsicInst>(&Inst) )
+           if(IB->getIntrinsicID()==Intrinsic::eh_sjlj_callsite) {
+              // eh_sjlj_callsite has to be in same BB as the
+              // bb terminator. Dont insert within this range.
+              CheckLoc = IB;
+              break;
+           }
         if (auto *CB = dyn_cast<CallBase>(&Inst))
           // Do stack check before noreturn calls that aren't nounwind (e.g:
           // __cxa_throw).
@@ -596,6 +603,7 @@ bool InsertStackProtectors(const TargetMachine *TM, Function *F,
             CheckLoc = CB;
             break;
           }
+      }
 
     if (!CheckLoc)
       continue;
diff --git a/llvm/test/CodeGen/ARM/stack-protector-eh-sjlj.ll b/llvm/test/CodeGen/ARM/stack-protector-eh-sjlj.ll
new file mode 100644
index 0000000000000..860d4d468505d
--- /dev/null
+++ b/llvm/test/CodeGen/ARM/stack-protector-eh-sjlj.ll
@@ -0,0 +1,166 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
+; RUN: llc  -O0 -mtriple=thumbv7s-apple-darwin < %s  | FileCheck %s
+target datalayout = "e-m:o-p:32:32-Fi8-f64:32:64-v64:32:64-v128:32:128-a:0:32-n32-S32"
+
+; Function Attrs: mustprogress noinline optnone ssp
+define ptr @foo() #0 personality ptr @__gxx_personality_sj0 {
+; CHECK-LABEL: foo:
+; CHECK:       Lfunc_begin0:
+; CHECK-NEXT:  @ %bb.0:
+; CHECK-NEXT:    push {r4, r5, r6, r7, lr}
+; CHECK-NEXT:    add r7, sp, #12
+; CHECK-NEXT:    push.w {r8, r10, r11}
+; CHECK-NEXT:    sub.w r4, sp, #64
+; CHECK-NEXT:    bfc r4, #0, #4
+; CHECK-NEXT:    mov sp, r4
+; CHECK-NEXT:    vst1.64 {d8, d9, d10, d11}, [r4:128]!
+; CHECK-NEXT:    vst1.64 {d12, d13, d14, d15}, [r4:128]
+; CHECK-NEXT:    sub sp, #96
+; CHECK-NEXT:    movw r0, :lower16:(L___stack_chk_guard$non_lazy_ptr-(LPC0_2+4))
+; CHECK-NEXT:    movt r0, :upper16:(L___stack_chk_guard$non_lazy_ptr-(LPC0_2+4))
+; CHECK-NEXT:  LPC0_2:
+; CHECK-NEXT:    add r0, pc
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    movw r0, :lower16:(L___stack_chk_guard$non_lazy_ptr-(LPC0_3+4))
+; CHECK-NEXT:    movt r0, :upper16:(L___stack_chk_guard$non_lazy_ptr-(LPC0_3+4))
+; CHECK-NEXT:  LPC0_3:
+; CHECK-NEXT:    add r0, pc
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    str r0, [sp, #92]
+; CHECK-NEXT:    movw r0, :lower16:(L___gxx_personality_sj0$non_lazy_ptr-(LPC0_4+4))
+; CHECK-NEXT:    movt r0, :upper16:(L___gxx_personality_sj0$non_lazy_ptr-(LPC0_4+4))
+; CHECK-NEXT:  LPC0_4:
+; CHECK-NEXT:    add r0, pc
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    str r0, [sp, #36]
+; CHECK-NEXT:    ldr r0, LCPI0_0
+; CHECK-NEXT:  LPC0_0:
+; CHECK-NEXT:    add r0, pc
+; CHECK-NEXT:    str r0, [sp, #40]
+; CHECK-NEXT:    str r7, [sp, #44]
+; CHECK-NEXT:    mov r0, sp
+; CHECK-NEXT:    str r0, [sp, #52]
+; CHECK-NEXT:    ldr r0, LCPI0_1
+; CHECK-NEXT:    orr r0, r0, #1
+; CHECK-NEXT:  LPC0_1:
+; CHECK-NEXT:    add r0, pc
+; CHECK-NEXT:    str r0, [sp, #48]
+; CHECK-NEXT:    add r0, sp, #12
+; CHECK-NEXT:    bl __Unwind_SjLj_Register
+; CHECK-NEXT:    movs r0, #1
+; CHECK-NEXT:    str r0, [sp, #16]
+; CHECK-NEXT:    movw r0, :lower16:(L___stack_chk_guard$non_lazy_ptr-(LPC0_5+4))
+; CHECK-NEXT:    movt r0, :upper16:(L___stack_chk_guard$non_lazy_ptr-(LPC0_5+4))
+; CHECK-NEXT:  LPC0_5:
+; CHECK-NEXT:    add r0, pc
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    ldr r1, [sp, #92]
+; CHECK-NEXT:    cmp r0, r1
+; CHECK-NEXT:    bne LBB0_7
+; CHECK-NEXT:  @ %bb.1: @ %SP_return
+; CHECK-NEXT:  Ltmp0:
+; CHECK-NEXT:    movs r2, #0
+; CHECK-NEXT:    mov r0, r2
+; CHECK-NEXT:    mov r1, r2
+; CHECK-NEXT:    blx r2
+; CHECK-NEXT:  Ltmp1:
+; CHECK-NEXT:    b LBB0_2
+; CHECK-NEXT:  LBB0_2:
+; CHECK-NEXT:    movs r0, #2
+; CHECK-NEXT:    str r0, [sp, #16]
+; CHECK-NEXT:    movw r0, :lower16:(L___stack_chk_guard$non_lazy_ptr-(LPC0_6+4))
+; CHECK-NEXT:    movt r0, :upper16:(L___stack_chk_guard$non_lazy_ptr-(LPC0_6+4))
+; CHECK-NEXT:  LPC0_6:
+; CHECK-NEXT:    add r0, pc
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    ldr r1, [sp, #92]
+; CHECK-NEXT:    cmp r0, r1
+; CHECK-NEXT:    bne LBB0_7
+; CHECK-NEXT:  @ %bb.3: @ %SP_return2
+; CHECK-NEXT:  Ltmp2:
+; CHECK-NEXT:    movs r3, #0
+; CHECK-NEXT:    mov r0, r3
+; CHECK-NEXT:    mov r1, r3
+; CHECK-NEXT:    mov r2, r3
+; CHECK-NEXT:    blx r3
+; CHECK-NEXT:  Ltmp3:
+; CHECK-NEXT:    b LBB0_6
+; CHECK-NEXT:  LBB0_4:
+; CHECK-NEXT:  Ltmp4:
+; CHECK-NEXT:    ldr r0, [sp, #20]
+; CHECK-NEXT:    ldr r0, [sp, #24]
+; CHECK-NEXT:    add r0, sp, #12
+; CHECK-NEXT:    bl __Unwind_SjLj_Unregister
+; CHECK-NEXT:    movw r0, :lower16:(L___stack_chk_guard$non_lazy_ptr-(LPC0_7+4))
+; CHECK-NEXT:    movt r0, :upper16:(L___stack_chk_guard$non_lazy_ptr-(LPC0_7+4))
+; CHECK-NEXT:  LPC0_7:
+; CHECK-NEXT:    add r0, pc
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    ldr r0, [r0]
+; CHECK-NEXT:    ldr r1, [sp, #92]
+; CHECK-NEXT:    cmp r0, r1
+; CHECK-NEXT:    bne LBB0_7
+; CHECK-NEXT:  @ %bb.5: @ %SP_return3
+; CHECK-NEXT:    movs r0, #0
+; CHECK-NEXT:    add r4, sp, #96
+; CHECK-NEXT:    vld1.64 {d8, d9, d10, d11}, [r4:128]!
+; CHECK-NEXT:    vld1.64 {d12, d13, d14, d15}, [r4:128]
+; CHECK-NEXT:    sub.w r4, r7, #24
+; CHECK-NEXT:    mov sp, r4
+; CHECK-NEXT:    pop.w {r8, r10, r11}
+; CHECK-NEXT:    pop {r4, r5, r6, r7, pc}
+; CHECK-NEXT:  LBB0_6:
+; CHECK-NEXT:    trap
+; CHECK-NEXT:  LBB0_7: @ %CallStackCheckFailBlk
+; CHECK-NEXT:    bl ___stack_chk_fail
+; CHECK-NEXT:  LBB0_8:
+; CHECK-NEXT:    ldr r0, [sp, #16]
+; CHECK-NEXT:    str r0, [sp, #8] @ 4-byte Spill
+; CHECK-NEXT:    cmp r0, #2
+; CHECK-NEXT:    bhi LBB0_12
+; CHECK-NEXT:  @ %bb.9:
+; CHECK-NEXT:    ldr r1, [sp, #8] @ 4-byte Reload
+; CHECK-NEXT:  LCPI0_2:
+; CHECK-NEXT:    tbb [pc, r1]
+; CHECK-NEXT:  @ %bb.10:
+; CHECK-NEXT:  LJTI0_0:
+; CHECK-NEXT:    .data_region jt8
+; CHECK-NEXT:    .byte (LBB0_11-(LCPI0_2+4))/2
+; CHECK-NEXT:    .byte (LBB0_11-(LCPI0_2+4))/2
+; CHECK-NEXT:    .end_data_region
+; CHECK-NEXT:    .p2align 1
+; CHECK-NEXT:  LBB0_11:
+; CHECK-NEXT:    b LBB0_4
+; CHECK-NEXT:  LBB0_12:
+; CHECK-NEXT:    trap
+; CHECK-NEXT:    .p2align 2
+; CHECK-NEXT:  @ %bb.13:
+  %1 = alloca [14 x i8], align 16
+  %2 = invoke i32 null(ptr noundef null, ptr noundef null) #1
+          to label %3 unwind label %4
+
+3:                                                ; preds = %0
+  invoke void null(ptr null, ptr null, ptr null) #2
+          to label %6 unwind label %4
+
+4:                                                ; preds = %3, %0
+  %5 = landingpad { ptr, i32 }
+          cleanup
+  ret ptr null
+
+6:                                                ; preds = %3
+  unreachable
+}
+
+declare i32 @__gxx_personality_sj0(...)
+
+; uselistorder directives
+uselistorder ptr null, { 2, 3, 4, 5, 0, 6, 7, 1, 8, 9 }
+
+attributes #0 = { mustprogress ssp "frame-pointer"="all" "no-builtin-calloc" "no-builtin-stpcpy" "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+attributes #1 = { "no-builtin-calloc" "no-builtin-stpcpy" }
+attributes #2 = { noreturn }

@github-actions
Copy link

github-actions bot commented Jul 7, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@AbhayKanhere
Copy link
Contributor Author

@nickdesaulniers @jroelofs

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The clang-format bot is complaining about the spacing of things here. You should apply the diff it gave you in this comment: #147411 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Un-resolving. There is still some spacing weirdness. You should copy the diff, and apply it with pbpaste | patch -p1 from the top-level of the llvm repo.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok. I took the diff and applied patch -p0 -- format did not complain this time. let me try what you suggest. thanks!

@AbhayKanhere AbhayKanhere force-pushed the dev/abhay/ehsjlj-stackprotect-bug branch from 4348e3d to 0df6e25 Compare July 9, 2025 00:20
@AbhayKanhere AbhayKanhere force-pushed the dev/abhay/ehsjlj-stackprotect-bug branch 4 times, most recently from 84ec35d to 0ce39a2 Compare July 16, 2025 21:04
@AbhayKanhere AbhayKanhere requested a review from jroelofs July 16, 2025 21:26
   Some armv7s targets use setjmp longjmp exception handling. In this,
form a llvm.eh.sjlj.callsite() intrinsic is inserted in each BB
with exception exit. This information is then recorded in a callsite
table, and used when emitExceptionTables() is called later.

  When stackprotectors are enabled on this target, StackProtector
pass ends up splitting the basic block with this intrinsic, thereby
removing information needed by emitExceptionTable call later.

  This fix ensures the eh.sjlj.callsite remains in same basic block
as block terminator instruction.
@AbhayKanhere AbhayKanhere force-pushed the dev/abhay/ehsjlj-stackprotect-bug branch from 0ce39a2 to ec9293e Compare July 17, 2025 16:40
@jroelofs jroelofs merged commit 33c9445 into llvm:main Jul 24, 2025
9 checks passed
mahesh-attarde pushed a commit to mahesh-attarde/llvm-project that referenced this pull request Jul 28, 2025
when exception handling with setjmp/longjmp (exception-mode=eh_sjlj is
enabled,
eh_sjlj_callsite intrinsic is inserted in same basic block as the
throwing/exception instruction. This fix ensures stack protector
insertion code does not split the block and move these apart into
different basic blocks.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants