Skip to content

Commit 7aa2b04

Browse files
[X86] Use unsigned comparison for stack clash probing loop (#192355)
The stack clash probing loop generated in `EmitLoweredProbedAlloca` used a signed comparison (`X86::COND_GE`) to determine when the allocation target had been reached. In 32-bit mode, memory addresses above `0x80000000` have the sign bit set. If the stack pointer lands in this region, treating the addresses as signed integers causes the comparison logic to fail. This leads to incorrect loop execution, resulting in an infinite loop and a crash (segmentation fault) when setting up custom stacks for pthreads mapped above `0x80000000` in a 32b process. This patch changes the condition code to `X86::COND_AE` (Above or Equal), which generates an unsigned comparison. This ensures that addresses are treated correctly as unsigned quantities on all targets. On 64-bit systems, this change has no practical effect on valid user-space addresses because they do not use the sign bit (being restricted to the lower half of the address space). However, using unsigned comparison is the correct behavior for pointer arithmetic and bounds checks. Reported-by: Wonsik Kim <wonsik@google.com>
1 parent 1317890 commit 7aa2b04

3 files changed

Lines changed: 7 additions & 7 deletions

File tree

llvm/lib/Target/X86/X86ISelLowering.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37069,7 +37069,7 @@ X86TargetLowering::EmitLoweredProbedAlloca(MachineInstr &MI,
3706937069

3707037070
BuildMI(testMBB, MIMD, TII->get(X86::JCC_1))
3707137071
.addMBB(tailMBB)
37072-
.addImm(X86::COND_GE);
37072+
.addImm(X86::COND_AE);
3707337073
testMBB->addSuccessor(blockMBB);
3707437074
testMBB->addSuccessor(tailMBB);
3707537075

llvm/test/CodeGen/X86/stack-clash-dynamic-alloca.ll

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,12 @@ define i32 @foo(i32 %n) local_unnamed_addr #0 {
1616
; CHECK-X86-64-NEXT: andq $-16, %rcx
1717
; CHECK-X86-64-NEXT: subq %rcx, %rax
1818
; CHECK-X86-64-NEXT: cmpq %rsp, %rax
19-
; CHECK-X86-64-NEXT: jge .LBB0_3
19+
; CHECK-X86-64-NEXT: jae .LBB0_3
2020
; CHECK-X86-64-NEXT: .LBB0_2: # =>This Inner Loop Header: Depth=1
2121
; CHECK-X86-64-NEXT: xorq $0, (%rsp)
2222
; CHECK-X86-64-NEXT: subq $4096, %rsp # imm = 0x1000
2323
; CHECK-X86-64-NEXT: cmpq %rsp, %rax
24-
; CHECK-X86-64-NEXT: jl .LBB0_2
24+
; CHECK-X86-64-NEXT: jb .LBB0_2
2525
; CHECK-X86-64-NEXT: .LBB0_3:
2626
; CHECK-X86-64-NEXT: movq %rax, %rsp
2727
; CHECK-X86-64-NEXT: movl $1, 4792(%rax)
@@ -45,12 +45,12 @@ define i32 @foo(i32 %n) local_unnamed_addr #0 {
4545
; CHECK-X86-32-NEXT: andl $-16, %ecx
4646
; CHECK-X86-32-NEXT: subl %ecx, %eax
4747
; CHECK-X86-32-NEXT: cmpl %esp, %eax
48-
; CHECK-X86-32-NEXT: jge .LBB0_3
48+
; CHECK-X86-32-NEXT: jae .LBB0_3
4949
; CHECK-X86-32-NEXT: .LBB0_2: # =>This Inner Loop Header: Depth=1
5050
; CHECK-X86-32-NEXT: xorl $0, (%esp)
5151
; CHECK-X86-32-NEXT: subl $4096, %esp # imm = 0x1000
5252
; CHECK-X86-32-NEXT: cmpl %esp, %eax
53-
; CHECK-X86-32-NEXT: jl .LBB0_2
53+
; CHECK-X86-32-NEXT: jb .LBB0_2
5454
; CHECK-X86-32-NEXT: .LBB0_3:
5555
; CHECK-X86-32-NEXT: movl %eax, %esp
5656
; CHECK-X86-32-NEXT: movl $1, 4792(%eax)

llvm/test/CodeGen/X86/stack-clash-small-alloc-medium-align.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,12 +103,12 @@ define i32 @foo4(i64 %i) local_unnamed_addr #0 {
103103
; CHECK-NEXT: andq $-16, %rcx
104104
; CHECK-NEXT: subq %rcx, %rax
105105
; CHECK-NEXT: cmpq %rsp, %rax
106-
; CHECK-NEXT: jge .LBB3_3
106+
; CHECK-NEXT: jae .LBB3_3
107107
; CHECK-NEXT: .LBB3_2: # =>This Inner Loop Header: Depth=1
108108
; CHECK-NEXT: xorq $0, (%rsp)
109109
; CHECK-NEXT: subq $4096, %rsp # imm = 0x1000
110110
; CHECK-NEXT: cmpq %rsp, %rax
111-
; CHECK-NEXT: jl .LBB3_2
111+
; CHECK-NEXT: jb .LBB3_2
112112
; CHECK-NEXT: .LBB3_3:
113113
; CHECK-NEXT: andq $-64, %rax
114114
; CHECK-NEXT: movq %rax, %rsp

0 commit comments

Comments
 (0)