Skip to content

Commit 0969e2c

Browse files
nikicnikic
authored
[CaptureTracking] Fix handling for non-returning read-only calls (#158979)
We currently infer `captures(none)` for calls that are read-only, nounwind and willreturn. As pointed out in #129090, this is not correct even with this set of pre-conditions, because the function could conditionally cause UB depending on the address. As such, change this logic to instead report `captures(address)`. This also allows dropping the nounwind and willreturn checks, as these can also only capture the address.
1 parent b29c7de commit 0969e2c

File tree

8 files changed

+38
-38
lines changed

8 files changed

+38
-38
lines changed

‎llvm/lib/Analysis/CaptureTracking.cpp‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -275,13 +275,6 @@ UseCaptureInfo llvm::DetermineUseCaptureKind(const Use &U, const Value *Base) {
275275
case Instruction::Call:
276276
case Instruction::Invoke: {
277277
auto *Call = cast<CallBase>(I);
278-
// Not captured if the callee is readonly, doesn't return a copy through
279-
// its return value and doesn't unwind or diverge (a readonly function can
280-
// leak bits by throwing an exception or not depending on the input value).
281-
if (Call->onlyReadsMemory() && Call->doesNotThrow() && Call->willReturn() &&
282-
Call->getType()->isVoidTy())
283-
return CaptureComponents::None;
284-
285278
// The pointer is not captured if returned pointer is not captured.
286279
// NOTE: CaptureTracking users should not assume that only functions
287280
// marked with nocapture do not capture. This means that places like
@@ -305,10 +298,17 @@ UseCaptureInfo llvm::DetermineUseCaptureKind(const Use &U, const Value *Base) {
305298
if (Call->isCallee(&U))
306299
return CaptureComponents::None;
307300

308-
// Not captured if only passed via 'nocapture' arguments.
309301
assert(Call->isDataOperand(&U) && "Non-callee must be data operand");
310302
CaptureInfo CI = Call->getCaptureInfo(Call->getDataOperandNo(&U));
311-
return UseCaptureInfo(CI.getOtherComponents(), CI.getRetComponents());
303+
304+
// If the call is readonly and doesn't return a value, only the address
305+
// may be captured.
306+
CaptureComponents Mask = CaptureComponents::All;
307+
if (Call->onlyReadsMemory() && Call->getType()->isVoidTy())
308+
Mask = CaptureComponents::Address;
309+
310+
return UseCaptureInfo(CI.getOtherComponents() & Mask,
311+
CI.getRetComponents());
312312
}
313313
case Instruction::Load:
314314
// Volatile loads make the address observable.

‎llvm/test/Other/cgscc-devirt-iteration.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ define void @test2_a(ptr %ignore) {
5959
; AFTER1: Function Attrs: nofree memory(read)
6060
; AFTER2: Function Attrs: nofree nosync memory(none)
6161
; BEFORE: define void @test2_a(ptr %ignore)
62-
; AFTER: define void @test2_a(ptr readnone %ignore)
62+
; AFTER: define void @test2_a(ptr readnone captures(address) %ignore)
6363
entry:
6464
%f1ptr = alloca ptr
6565
store ptr @readnone_with_arg, ptr %f1ptr

‎llvm/test/Transforms/Attributor/nocapture-1.ll‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -350,7 +350,7 @@ define void @callsite_readonly_nounwind_not_willreturn(ptr %f, ptr %p) {
350350

351351
define void @callsite_readonly_nounwind_willreturn(ptr %f, ptr %p) {
352352
; CHECK-LABEL: define {{[^@]+}}@callsite_readonly_nounwind_willreturn
353-
; CHECK-SAME: (ptr nofree noundef nonnull captures(none) [[F:%.*]], ptr captures(none) [[P:%.*]]) {
353+
; CHECK-SAME: (ptr nofree noundef nonnull captures(none) [[F:%.*]], ptr [[P:%.*]]) {
354354
; CHECK-NEXT: call void [[F]](ptr captures(none) [[P]])
355355
; CHECK-NEXT: ret void
356356
;

‎llvm/test/Transforms/FunctionAttrs/nocapture.ll‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ declare void @throw_if_bit_set(ptr, i8) readonly
161161
define i1 @c6(ptr %q, i8 %bit) personality ptr @__gxx_personality_v0 {
162162
; FNATTRS: Function Attrs: nofree memory(read)
163163
; FNATTRS-LABEL: define noundef i1 @c6
164-
; FNATTRS-SAME: (ptr readonly [[Q:%.*]], i8 [[BIT:%.*]]) #[[ATTR5:[0-9]+]] personality ptr @__gxx_personality_v0 {
164+
; FNATTRS-SAME: (ptr readonly captures(address) [[Q:%.*]], i8 [[BIT:%.*]]) #[[ATTR5:[0-9]+]] personality ptr @__gxx_personality_v0 {
165165
; FNATTRS-NEXT: invoke void @throw_if_bit_set(ptr [[Q]], i8 [[BIT]])
166166
; FNATTRS-NEXT: to label [[RET0:%.*]] unwind label [[RET1:%.*]]
167167
; FNATTRS: ret0:
@@ -364,7 +364,7 @@ declare void @external_not_willreturn(ptr) readonly nounwind
364364
define void @readonly_nounwind_not_willreturn(ptr %p) {
365365
; FNATTRS: Function Attrs: nofree nounwind memory(read)
366366
; FNATTRS-LABEL: define void @readonly_nounwind_not_willreturn
367-
; FNATTRS-SAME: (ptr readonly [[P:%.*]]) #[[ATTR9:[0-9]+]] {
367+
; FNATTRS-SAME: (ptr readonly captures(address) [[P:%.*]]) #[[ATTR9:[0-9]+]] {
368368
; FNATTRS-NEXT: call void @external_not_willreturn(ptr [[P]])
369369
; FNATTRS-NEXT: ret void
370370
;
@@ -382,7 +382,7 @@ declare void @external_willreturn(ptr) readonly nounwind willreturn
382382
define void @readonly_nounwind_willreturn(ptr %p) {
383383
; FNATTRS: Function Attrs: mustprogress nofree nounwind willreturn memory(read)
384384
; FNATTRS-LABEL: define void @readonly_nounwind_willreturn
385-
; FNATTRS-SAME: (ptr readonly captures(none) [[P:%.*]]) #[[ATTR11:[0-9]+]] {
385+
; FNATTRS-SAME: (ptr readonly captures(address) [[P:%.*]]) #[[ATTR11:[0-9]+]] {
386386
; FNATTRS-NEXT: call void @external_willreturn(ptr [[P]])
387387
; FNATTRS-NEXT: ret void
388388
;
@@ -398,7 +398,7 @@ define void @readonly_nounwind_willreturn(ptr %p) {
398398

399399
define void @callsite_readonly_nounwind_not_willreturn(ptr %f, ptr %p) {
400400
; FNATTRS-LABEL: define void @callsite_readonly_nounwind_not_willreturn
401-
; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr [[P:%.*]]) {
401+
; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr captures(address) [[P:%.*]]) {
402402
; FNATTRS-NEXT: call void [[F]](ptr [[P]]) #[[ATTR8:[0-9]+]]
403403
; FNATTRS-NEXT: call void [[F]](ptr captures(none) [[P]])
404404
; FNATTRS-NEXT: ret void
@@ -416,13 +416,13 @@ define void @callsite_readonly_nounwind_not_willreturn(ptr %f, ptr %p) {
416416

417417
define void @callsite_readonly_nounwind_willreturn(ptr %f, ptr %p) {
418418
; FNATTRS-LABEL: define void @callsite_readonly_nounwind_willreturn
419-
; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr captures(none) [[P:%.*]]) {
419+
; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr captures(address) [[P:%.*]]) {
420420
; FNATTRS-NEXT: call void [[F]](ptr [[P]]) #[[ATTR10:[0-9]+]]
421421
; FNATTRS-NEXT: call void [[F]](ptr captures(none) [[P]])
422422
; FNATTRS-NEXT: ret void
423423
;
424424
; ATTRIBUTOR-LABEL: define void @callsite_readonly_nounwind_willreturn
425-
; ATTRIBUTOR-SAME: (ptr nofree nonnull captures(none) [[F:%.*]], ptr captures(none) [[P:%.*]]) {
425+
; ATTRIBUTOR-SAME: (ptr nofree nonnull captures(none) [[F:%.*]], ptr [[P:%.*]]) {
426426
; ATTRIBUTOR-NEXT: call void [[F]](ptr [[P]]) #[[ATTR8:[0-9]+]]
427427
; ATTRIBUTOR-NEXT: call void [[F]](ptr captures(none) [[P]])
428428
; ATTRIBUTOR-NEXT: ret void
@@ -1032,7 +1032,7 @@ define void @recurse_fptr(ptr %f, ptr %p) {
10321032
define void @readnone_indirec(ptr %f, ptr %p) {
10331033
; FNATTRS: Function Attrs: nofree nosync memory(none)
10341034
; FNATTRS-LABEL: define void @readnone_indirec
1035-
; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr readnone [[P:%.*]]) #[[ATTR19:[0-9]+]] {
1035+
; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr readnone captures(address) [[P:%.*]]) #[[ATTR19:[0-9]+]] {
10361036
; FNATTRS-NEXT: call void [[F]](ptr [[P]]) #[[ATTR23:[0-9]+]]
10371037
; FNATTRS-NEXT: ret void
10381038
;

‎llvm/test/Transforms/FunctionAttrs/nonnull.ll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,7 +1032,7 @@ define ptr @g1() {
10321032
declare void @use_i32_ptr(ptr) readnone nounwind willreturn
10331033
define internal void @called_by_weak(ptr %a) {
10341034
; FNATTRS-LABEL: define internal void @called_by_weak(
1035-
; FNATTRS-SAME: ptr readnone captures(none) [[A:%.*]]) #[[ATTR10:[0-9]+]] {
1035+
; FNATTRS-SAME: ptr readnone captures(address) [[A:%.*]]) #[[ATTR10:[0-9]+]] {
10361036
; FNATTRS-NEXT: call void @use_i32_ptr(ptr [[A]])
10371037
; FNATTRS-NEXT: ret void
10381038
;
@@ -1064,7 +1064,7 @@ define weak_odr void @weak_caller(ptr nonnull %a) {
10641064
; Expect nonnull
10651065
define internal void @control(ptr dereferenceable(4) %a) {
10661066
; FNATTRS-LABEL: define internal void @control(
1067-
; FNATTRS-SAME: ptr readnone captures(none) dereferenceable(4) [[A:%.*]]) #[[ATTR10]] {
1067+
; FNATTRS-SAME: ptr readnone captures(address) dereferenceable(4) [[A:%.*]]) #[[ATTR10]] {
10681068
; FNATTRS-NEXT: call void @use_i32_ptr(ptr [[A]])
10691069
; FNATTRS-NEXT: ret void
10701070
;

‎llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,14 @@ define void @va_func(ptr readonly %b, ...) readonly nounwind willreturn {
1717
}
1818

1919
define i32 @caller(ptr %x) {
20-
; CHECK-LABEL: define noundef i32 @caller(ptr readonly captures(none) %x)
20+
; CHECK-LABEL: define noundef i32 @caller(ptr readonly captures(address) %x)
2121
entry:
2222
call void(ptr,...) @va_func(ptr null, i32 0, i32 0, i32 0, ptr %x)
2323
ret i32 42
2424
}
2525

2626
define void @va_func2(ptr readonly %b, ...) {
27-
; CHECK-LABEL: define void @va_func2(ptr readonly captures(none) %b, ...)
27+
; CHECK-LABEL: define void @va_func2(ptr readonly captures(address) %b, ...)
2828
entry:
2929
%valist = alloca i8
3030
call void @llvm.va_start(ptr %valist)
@@ -34,7 +34,7 @@ define void @va_func2(ptr readonly %b, ...) {
3434
}
3535

3636
define i32 @caller2(ptr %x, ptr %y) {
37-
; CHECK-LABEL: define noundef i32 @caller2(ptr readonly captures(none) %x, ptr %y)
37+
; CHECK-LABEL: define noundef i32 @caller2(ptr readonly captures(address) %x, ptr %y)
3838
entry:
3939
call void(ptr,...) @va_func2(ptr %x, i32 0, i32 0, i32 0, ptr %y)
4040
ret i32 42

‎llvm/test/Transforms/FunctionAttrs/readattrs.ll‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ define void @fptr_test1b(ptr %p, ptr %f) {
483483
define void @fptr_test1c(ptr %p, ptr %f) {
484484
; FNATTRS: Function Attrs: nofree memory(read)
485485
; FNATTRS-LABEL: define {{[^@]+}}@fptr_test1c
486-
; FNATTRS-SAME: (ptr readnone [[P:%.*]], ptr readonly captures(none) [[F:%.*]]) #[[ATTR3]] {
486+
; FNATTRS-SAME: (ptr readnone captures(address) [[P:%.*]], ptr readonly captures(none) [[F:%.*]]) #[[ATTR3]] {
487487
; FNATTRS-NEXT: call void [[F]](ptr readnone [[P]]) #[[ATTR2:[0-9]+]]
488488
; FNATTRS-NEXT: ret void
489489
;
@@ -547,7 +547,7 @@ define void @fptr_test2b(ptr %p, ptr %f) {
547547
define void @fptr_test2c(ptr %p, ptr %f) {
548548
; FNATTRS: Function Attrs: nofree memory(read)
549549
; FNATTRS-LABEL: define {{[^@]+}}@fptr_test2c
550-
; FNATTRS-SAME: (ptr readonly [[P:%.*]], ptr readonly captures(none) [[F:%.*]]) #[[ATTR3]] {
550+
; FNATTRS-SAME: (ptr readonly captures(address) [[P:%.*]], ptr readonly captures(none) [[F:%.*]]) #[[ATTR3]] {
551551
; FNATTRS-NEXT: call void [[F]](ptr readonly [[P]]) #[[ATTR2]]
552552
; FNATTRS-NEXT: ret void
553553
;

‎llvm/test/Transforms/LICM/promote-capture.ll‎

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
; RUN: opt -S -passes='loop-mssa(licm)' < %s | FileCheck %s
33

44
declare i1 @cond(i32 %v) readnone
5-
declare void @capture(ptr %p) readnone
5+
declare ptr @capture(ptr %p) readnone
66

77
define void @test_captured_after_loop(i32 %len) {
88
; CHECK-LABEL: @test_captured_after_loop(
@@ -27,7 +27,7 @@ define void @test_captured_after_loop(i32 %len) {
2727
; CHECK: exit:
2828
; CHECK-NEXT: [[C_INC1_LCSSA:%.*]] = phi i32 [ [[C_INC1]], [[LATCH]] ]
2929
; CHECK-NEXT: store i32 [[C_INC1_LCSSA]], ptr [[COUNT]], align 4
30-
; CHECK-NEXT: call void @capture(ptr [[COUNT]])
30+
; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr [[COUNT]])
3131
; CHECK-NEXT: ret void
3232
;
3333
entry:
@@ -52,7 +52,7 @@ latch:
5252
br i1 %cmp, label %exit, label %loop
5353

5454
exit:
55-
call void @capture(ptr %count)
55+
call ptr @capture(ptr %count)
5656
ret void
5757
}
5858

@@ -71,7 +71,7 @@ define void @test_captured_in_loop(i32 %len) {
7171
; CHECK: if:
7272
; CHECK-NEXT: [[C_INC:%.*]] = add i32 [[C_INC2]], 1
7373
; CHECK-NEXT: store i32 [[C_INC]], ptr [[COUNT]], align 4
74-
; CHECK-NEXT: call void @capture(ptr [[COUNT]])
74+
; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr [[COUNT]])
7575
; CHECK-NEXT: br label [[LATCH]]
7676
; CHECK: latch:
7777
; CHECK-NEXT: [[C_INC1]] = phi i32 [ [[C_INC]], [[IF]] ], [ [[C_INC2]], [[LOOP]] ]
@@ -95,7 +95,7 @@ if:
9595
%c = load i32, ptr %count
9696
%c.inc = add i32 %c, 1
9797
store i32 %c.inc, ptr %count
98-
call void @capture(ptr %count)
98+
call ptr @capture(ptr %count)
9999
br label %latch
100100

101101
latch:
@@ -112,7 +112,7 @@ define void @test_captured_before_loop(i32 %len) {
112112
; CHECK-NEXT: entry:
113113
; CHECK-NEXT: [[COUNT:%.*]] = alloca i32, align 4
114114
; CHECK-NEXT: store i32 0, ptr [[COUNT]], align 4
115-
; CHECK-NEXT: call void @capture(ptr [[COUNT]])
115+
; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr [[COUNT]])
116116
; CHECK-NEXT: [[COUNT_PROMOTED:%.*]] = load i32, ptr [[COUNT]], align 4
117117
; CHECK-NEXT: br label [[LOOP:%.*]]
118118
; CHECK: loop:
@@ -135,7 +135,7 @@ define void @test_captured_before_loop(i32 %len) {
135135
entry:
136136
%count = alloca i32
137137
store i32 0, ptr %count
138-
call void @capture(ptr %count)
138+
call ptr @capture(ptr %count)
139139
br label %loop
140140

141141
loop:
@@ -163,7 +163,7 @@ define void @test_captured_before_loop_address_only(i32 %len) {
163163
; CHECK-NEXT: entry:
164164
; CHECK-NEXT: [[COUNT:%.*]] = alloca i32, align 4
165165
; CHECK-NEXT: store i32 0, ptr [[COUNT]], align 4
166-
; CHECK-NEXT: call void @capture(ptr captures(address) [[COUNT]])
166+
; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr captures(address) [[COUNT]])
167167
; CHECK-NEXT: [[COUNT_PROMOTED:%.*]] = load i32, ptr [[COUNT]], align 4
168168
; CHECK-NEXT: br label [[LOOP:%.*]]
169169
; CHECK: loop:
@@ -187,7 +187,7 @@ define void @test_captured_before_loop_address_only(i32 %len) {
187187
entry:
188188
%count = alloca i32
189189
store i32 0, ptr %count
190-
call void @capture(ptr captures(address) %count)
190+
call ptr @capture(ptr captures(address) %count)
191191
br label %loop
192192

193193
loop:
@@ -216,7 +216,7 @@ define void @test_captured_before_loop_byval(ptr byval(i32) align 4 %count, i32
216216
; CHECK-LABEL: @test_captured_before_loop_byval(
217217
; CHECK-NEXT: entry:
218218
; CHECK-NEXT: store i32 0, ptr [[COUNT:%.*]], align 4
219-
; CHECK-NEXT: call void @capture(ptr [[COUNT]])
219+
; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr [[COUNT]])
220220
; CHECK-NEXT: [[COUNT_PROMOTED:%.*]] = load i32, ptr [[COUNT]], align 4
221221
; CHECK-NEXT: br label [[LOOP:%.*]]
222222
; CHECK: loop:
@@ -238,7 +238,7 @@ define void @test_captured_before_loop_byval(ptr byval(i32) align 4 %count, i32
238238
;
239239
entry:
240240
store i32 0, ptr %count
241-
call void @capture(ptr %count)
241+
call ptr @capture(ptr %count)
242242
br label %loop
243243

244244
loop:
@@ -283,7 +283,7 @@ define void @test_captured_after_loop_byval(ptr byval(i32) align 4 %count, i32 %
283283
; CHECK: exit:
284284
; CHECK-NEXT: [[C_INC1_LCSSA:%.*]] = phi i32 [ [[C_INC1]], [[LATCH]] ]
285285
; CHECK-NEXT: store i32 [[C_INC1_LCSSA]], ptr [[COUNT]], align 4
286-
; CHECK-NEXT: call void @capture(ptr [[COUNT]])
286+
; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr [[COUNT]])
287287
; CHECK-NEXT: ret void
288288
;
289289
entry:
@@ -307,6 +307,6 @@ latch:
307307
br i1 %cmp, label %exit, label %loop
308308

309309
exit:
310-
call void @capture(ptr %count)
310+
call ptr @capture(ptr %count)
311311
ret void
312312
}

0 commit comments

Comments
 (0)