Skip to content

Commit b2aba39

Browse files
committed
[StackProtector] Handle atomicrmw xchg in HasAddressTaken heuristic
Atomicrmw xchg can directly take a pointer operand, so we should treat it similarly to store or cmpxchg. In practice, I believe that all targets that support stack protectors will convert this to an integer atomicrmw xchg in AtomicExpand, so there is no issue in practice. We still should handle it correctly if that doesn't happen.
1 parent 7defbf9 commit b2aba39

File tree

2 files changed

+18
-4
lines changed

2 files changed

+18
-4
lines changed

llvm/lib/CodeGen/StackProtector.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,10 @@ static bool HasAddressTaken(const Instruction *AI, TypeSize AllocSize,
275275
if (AI == cast<AtomicCmpXchgInst>(I)->getNewValOperand())
276276
return true;
277277
break;
278+
case Instruction::AtomicRMW:
279+
if (AI == cast<AtomicRMWInst>(I)->getValOperand())
280+
return true;
281+
break;
278282
case Instruction::PtrToInt:
279283
if (AI == cast<PtrToIntInst>(I)->getOperand(0))
280284
return true;
@@ -327,13 +331,9 @@ static bool HasAddressTaken(const Instruction *AI, TypeSize AllocSize,
327331
break;
328332
}
329333
case Instruction::Load:
330-
case Instruction::AtomicRMW:
331334
case Instruction::Ret:
332335
// These instructions take an address operand, but have load-like or
333336
// other innocuous behavior that should not trigger a stack protector.
334-
// atomicrmw conceptually has both load and store semantics, but the
335-
// value being stored must be integer; so if a pointer is being stored,
336-
// we'll catch it in the PtrToInt case above.
337337
break;
338338
default:
339339
// Conservatively return true for any instruction that takes an address

llvm/test/CodeGen/X86/stack-protector-atomicrmw-xchg.ll

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,25 @@
44
define void @atomicrmw_xchg(ptr %p) sspstrong {
55
; CHECK-LABEL: define void @atomicrmw_xchg(
66
; CHECK-SAME: ptr [[P:%.*]]) #[[ATTR0:[0-9]+]] {
7+
; CHECK-NEXT: [[STACKGUARDSLOT:%.*]] = alloca ptr, align 8
8+
; CHECK-NEXT: [[STACKGUARD:%.*]] = load volatile ptr, ptr addrspace(257) inttoptr (i32 40 to ptr addrspace(257)), align 8
9+
; CHECK-NEXT: call void @llvm.stackprotector(ptr [[STACKGUARD]], ptr [[STACKGUARDSLOT]])
710
; CHECK-NEXT: [[A:%.*]] = alloca i64, align 8
811
; CHECK-NEXT: [[TMP1:%.*]] = atomicrmw xchg ptr [[P]], ptr [[A]] acquire, align 8
12+
; CHECK-NEXT: [[STACKGUARD1:%.*]] = load volatile ptr, ptr addrspace(257) inttoptr (i32 40 to ptr addrspace(257)), align 8
13+
; CHECK-NEXT: [[TMP2:%.*]] = load volatile ptr, ptr [[STACKGUARDSLOT]], align 8
14+
; CHECK-NEXT: [[TMP3:%.*]] = icmp eq ptr [[STACKGUARD1]], [[TMP2]]
15+
; CHECK-NEXT: br i1 [[TMP3]], label %[[SP_RETURN:.*]], label %[[CALLSTACKCHECKFAILBLK:.*]], !prof [[PROF0:![0-9]+]]
16+
; CHECK: [[SP_RETURN]]:
917
; CHECK-NEXT: ret void
18+
; CHECK: [[CALLSTACKCHECKFAILBLK]]:
19+
; CHECK-NEXT: call void @__stack_chk_fail()
20+
; CHECK-NEXT: unreachable
1021
;
1122
%a = alloca i64
1223
atomicrmw xchg ptr %p, ptr %a acquire
1324
ret void
1425
}
26+
;.
27+
; CHECK: [[PROF0]] = !{!"branch_weights", i32 2147481600, i32 2048}
28+
;.

0 commit comments

Comments
 (0)