[CaptureTracking] Fix handling for non-returning read-only calls #158979
+38
−38
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We currently infer `captures(none)` for calls that are read-only, nounwind and willreturn. As pointed out in llvm#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.
llvmbot
added
llvm:analysis
Member
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-llvm-analysis Author: Nikita Popov (nikic) ChangesWe currently infer As such, change this logic to instead report Full diff: https://github.com/llvm/llvm-project/pull/158979.diff 8 Files Affected:
diff --git a/llvm/lib/Analysis/CaptureTracking.cpp b/llvm/lib/Analysis/CaptureTracking.cpp
index 71ca5131ee128..a0fe7f9037e47 100644
--- a/llvm/lib/Analysis/CaptureTracking.cpp
+++ b/llvm/lib/Analysis/CaptureTracking.cpp
@@ -275,13 +275,6 @@ UseCaptureInfo llvm::DetermineUseCaptureKind(const Use &U, const Value *Base) {
case Instruction::Call:
case Instruction::Invoke: {
auto *Call = cast<CallBase>(I);
- // Not captured if the callee is readonly, doesn't return a copy through
- // its return value and doesn't unwind or diverge (a readonly function can
- // leak bits by throwing an exception or not depending on the input value).
- if (Call->onlyReadsMemory() && Call->doesNotThrow() && Call->willReturn() &&
- Call->getType()->isVoidTy())
- return CaptureComponents::None;
-
// The pointer is not captured if returned pointer is not captured.
// NOTE: CaptureTracking users should not assume that only functions
// marked with nocapture do not capture. This means that places like
@@ -305,10 +298,17 @@ UseCaptureInfo llvm::DetermineUseCaptureKind(const Use &U, const Value *Base) {
if (Call->isCallee(&U))
return CaptureComponents::None;
- // Not captured if only passed via 'nocapture' arguments.
assert(Call->isDataOperand(&U) && "Non-callee must be data operand");
CaptureInfo CI = Call->getCaptureInfo(Call->getDataOperandNo(&U));
- return UseCaptureInfo(CI.getOtherComponents(), CI.getRetComponents());
+
+ // If the call is readonly and doesn't return a value, only the address
+ // may be captured.
+ CaptureComponents Mask = CaptureComponents::All;
+ if (Call->onlyReadsMemory() && Call->getType()->isVoidTy())
+ Mask = CaptureComponents::Address;
+
+ return UseCaptureInfo(CI.getOtherComponents() & Mask,
+ CI.getRetComponents());
}
case Instruction::Load:
// Volatile loads make the address observable.
diff --git a/llvm/test/Other/cgscc-devirt-iteration.ll b/llvm/test/Other/cgscc-devirt-iteration.ll
index 2ee016a134baa..990d474f8f571 100644
--- a/llvm/test/Other/cgscc-devirt-iteration.ll
+++ b/llvm/test/Other/cgscc-devirt-iteration.ll
@@ -59,7 +59,7 @@ define void @test2_a(ptr %ignore) {
; AFTER1: Function Attrs: nofree memory(read)
; AFTER2: Function Attrs: nofree nosync memory(none)
; BEFORE: define void @test2_a(ptr %ignore)
-; AFTER: define void @test2_a(ptr readnone %ignore)
+; AFTER: define void @test2_a(ptr readnone captures(address) %ignore)
entry:
%f1ptr = alloca ptr
store ptr @readnone_with_arg, ptr %f1ptr
diff --git a/llvm/test/Transforms/Attributor/nocapture-1.ll b/llvm/test/Transforms/Attributor/nocapture-1.ll
index 1f1c442fbc5f7..034b5ef397f0a 100644
--- a/llvm/test/Transforms/Attributor/nocapture-1.ll
+++ b/llvm/test/Transforms/Attributor/nocapture-1.ll
@@ -350,7 +350,7 @@ define void @callsite_readonly_nounwind_not_willreturn(ptr %f, ptr %p) {
define void @callsite_readonly_nounwind_willreturn(ptr %f, ptr %p) {
; CHECK-LABEL: define {{[^@]+}}@callsite_readonly_nounwind_willreturn
-; CHECK-SAME: (ptr nofree noundef nonnull captures(none) [[F:%.*]], ptr captures(none) [[P:%.*]]) {
+; CHECK-SAME: (ptr nofree noundef nonnull captures(none) [[F:%.*]], ptr [[P:%.*]]) {
; CHECK-NEXT: call void [[F]](ptr captures(none) [[P]])
; CHECK-NEXT: ret void
;
diff --git a/llvm/test/Transforms/FunctionAttrs/nocapture.ll b/llvm/test/Transforms/FunctionAttrs/nocapture.ll
index 26b5dc2dc7760..9f9bc118fa525 100644
--- a/llvm/test/Transforms/FunctionAttrs/nocapture.ll
+++ b/llvm/test/Transforms/FunctionAttrs/nocapture.ll
@@ -161,7 +161,7 @@ declare void @throw_if_bit_set(ptr, i8) readonly
define i1 @c6(ptr %q, i8 %bit) personality ptr @__gxx_personality_v0 {
; FNATTRS: Function Attrs: nofree memory(read)
; FNATTRS-LABEL: define noundef i1 @c6
-; FNATTRS-SAME: (ptr readonly [[Q:%.*]], i8 [[BIT:%.*]]) #[[ATTR5:[0-9]+]] personality ptr @__gxx_personality_v0 {
+; FNATTRS-SAME: (ptr readonly captures(address) [[Q:%.*]], i8 [[BIT:%.*]]) #[[ATTR5:[0-9]+]] personality ptr @__gxx_personality_v0 {
; FNATTRS-NEXT: invoke void @throw_if_bit_set(ptr [[Q]], i8 [[BIT]])
; FNATTRS-NEXT: to label [[RET0:%.*]] unwind label [[RET1:%.*]]
; FNATTRS: ret0:
@@ -364,7 +364,7 @@ declare void @external_not_willreturn(ptr) readonly nounwind
define void @readonly_nounwind_not_willreturn(ptr %p) {
; FNATTRS: Function Attrs: nofree nounwind memory(read)
; FNATTRS-LABEL: define void @readonly_nounwind_not_willreturn
-; FNATTRS-SAME: (ptr readonly [[P:%.*]]) #[[ATTR9:[0-9]+]] {
+; FNATTRS-SAME: (ptr readonly captures(address) [[P:%.*]]) #[[ATTR9:[0-9]+]] {
; FNATTRS-NEXT: call void @external_not_willreturn(ptr [[P]])
; FNATTRS-NEXT: ret void
;
@@ -382,7 +382,7 @@ declare void @external_willreturn(ptr) readonly nounwind willreturn
define void @readonly_nounwind_willreturn(ptr %p) {
; FNATTRS: Function Attrs: mustprogress nofree nounwind willreturn memory(read)
; FNATTRS-LABEL: define void @readonly_nounwind_willreturn
-; FNATTRS-SAME: (ptr readonly captures(none) [[P:%.*]]) #[[ATTR11:[0-9]+]] {
+; FNATTRS-SAME: (ptr readonly captures(address) [[P:%.*]]) #[[ATTR11:[0-9]+]] {
; FNATTRS-NEXT: call void @external_willreturn(ptr [[P]])
; FNATTRS-NEXT: ret void
;
@@ -398,7 +398,7 @@ define void @readonly_nounwind_willreturn(ptr %p) {
define void @callsite_readonly_nounwind_not_willreturn(ptr %f, ptr %p) {
; FNATTRS-LABEL: define void @callsite_readonly_nounwind_not_willreturn
-; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr [[P:%.*]]) {
+; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr captures(address) [[P:%.*]]) {
; FNATTRS-NEXT: call void [[F]](ptr [[P]]) #[[ATTR8:[0-9]+]]
; FNATTRS-NEXT: call void [[F]](ptr captures(none) [[P]])
; FNATTRS-NEXT: ret void
@@ -416,13 +416,13 @@ define void @callsite_readonly_nounwind_not_willreturn(ptr %f, ptr %p) {
define void @callsite_readonly_nounwind_willreturn(ptr %f, ptr %p) {
; FNATTRS-LABEL: define void @callsite_readonly_nounwind_willreturn
-; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr captures(none) [[P:%.*]]) {
+; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr captures(address) [[P:%.*]]) {
; FNATTRS-NEXT: call void [[F]](ptr [[P]]) #[[ATTR10:[0-9]+]]
; FNATTRS-NEXT: call void [[F]](ptr captures(none) [[P]])
; FNATTRS-NEXT: ret void
;
; ATTRIBUTOR-LABEL: define void @callsite_readonly_nounwind_willreturn
-; ATTRIBUTOR-SAME: (ptr nofree nonnull captures(none) [[F:%.*]], ptr captures(none) [[P:%.*]]) {
+; ATTRIBUTOR-SAME: (ptr nofree nonnull captures(none) [[F:%.*]], ptr [[P:%.*]]) {
; ATTRIBUTOR-NEXT: call void [[F]](ptr [[P]]) #[[ATTR8:[0-9]+]]
; ATTRIBUTOR-NEXT: call void [[F]](ptr captures(none) [[P]])
; ATTRIBUTOR-NEXT: ret void
@@ -1032,7 +1032,7 @@ define void @recurse_fptr(ptr %f, ptr %p) {
define void @readnone_indirec(ptr %f, ptr %p) {
; FNATTRS: Function Attrs: nofree nosync memory(none)
; FNATTRS-LABEL: define void @readnone_indirec
-; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr readnone [[P:%.*]]) #[[ATTR19:[0-9]+]] {
+; FNATTRS-SAME: (ptr readonly captures(none) [[F:%.*]], ptr readnone captures(address) [[P:%.*]]) #[[ATTR19:[0-9]+]] {
; FNATTRS-NEXT: call void [[F]](ptr [[P]]) #[[ATTR23:[0-9]+]]
; FNATTRS-NEXT: ret void
;
diff --git a/llvm/test/Transforms/FunctionAttrs/nonnull.ll b/llvm/test/Transforms/FunctionAttrs/nonnull.ll
index 8df242fb023af..9d5ae1606f2e3 100644
--- a/llvm/test/Transforms/FunctionAttrs/nonnull.ll
+++ b/llvm/test/Transforms/FunctionAttrs/nonnull.ll
@@ -1032,7 +1032,7 @@ define ptr @g1() {
declare void @use_i32_ptr(ptr) readnone nounwind willreturn
define internal void @called_by_weak(ptr %a) {
; FNATTRS-LABEL: define internal void @called_by_weak(
-; FNATTRS-SAME: ptr readnone captures(none) [[A:%.*]]) #[[ATTR10:[0-9]+]] {
+; FNATTRS-SAME: ptr readnone captures(address) [[A:%.*]]) #[[ATTR10:[0-9]+]] {
; FNATTRS-NEXT: call void @use_i32_ptr(ptr [[A]])
; FNATTRS-NEXT: ret void
;
@@ -1064,7 +1064,7 @@ define weak_odr void @weak_caller(ptr nonnull %a) {
; Expect nonnull
define internal void @control(ptr dereferenceable(4) %a) {
; FNATTRS-LABEL: define internal void @control(
-; FNATTRS-SAME: ptr readnone captures(none) dereferenceable(4) [[A:%.*]]) #[[ATTR10]] {
+; FNATTRS-SAME: ptr readnone captures(address) dereferenceable(4) [[A:%.*]]) #[[ATTR10]] {
; FNATTRS-NEXT: call void @use_i32_ptr(ptr [[A]])
; FNATTRS-NEXT: ret void
;
diff --git a/llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll b/llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
index e2e3603e9cb43..6acb0af13772a 100644
--- a/llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
+++ b/llvm/test/Transforms/FunctionAttrs/out-of-bounds-iterator-bug.ll
@@ -17,14 +17,14 @@ define void @va_func(ptr readonly %b, ...) readonly nounwind willreturn {
}
define i32 @caller(ptr %x) {
-; CHECK-LABEL: define noundef i32 @caller(ptr readonly captures(none) %x)
+; CHECK-LABEL: define noundef i32 @caller(ptr readonly captures(address) %x)
entry:
call void(ptr,...) @va_func(ptr null, i32 0, i32 0, i32 0, ptr %x)
ret i32 42
}
define void @va_func2(ptr readonly %b, ...) {
-; CHECK-LABEL: define void @va_func2(ptr readonly captures(none) %b, ...)
+; CHECK-LABEL: define void @va_func2(ptr readonly captures(address) %b, ...)
entry:
%valist = alloca i8
call void @llvm.va_start(ptr %valist)
@@ -34,7 +34,7 @@ define void @va_func2(ptr readonly %b, ...) {
}
define i32 @caller2(ptr %x, ptr %y) {
-; CHECK-LABEL: define noundef i32 @caller2(ptr readonly captures(none) %x, ptr %y)
+; CHECK-LABEL: define noundef i32 @caller2(ptr readonly captures(address) %x, ptr %y)
entry:
call void(ptr,...) @va_func2(ptr %x, i32 0, i32 0, i32 0, ptr %y)
ret i32 42
diff --git a/llvm/test/Transforms/FunctionAttrs/readattrs.ll b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
index 5fc88d623c0ec..d0aec184f49c3 100644
--- a/llvm/test/Transforms/FunctionAttrs/readattrs.ll
+++ b/llvm/test/Transforms/FunctionAttrs/readattrs.ll
@@ -483,7 +483,7 @@ define void @fptr_test1b(ptr %p, ptr %f) {
define void @fptr_test1c(ptr %p, ptr %f) {
; FNATTRS: Function Attrs: nofree memory(read)
; FNATTRS-LABEL: define {{[^@]+}}@fptr_test1c
-; FNATTRS-SAME: (ptr readnone [[P:%.*]], ptr readonly captures(none) [[F:%.*]]) #[[ATTR3]] {
+; FNATTRS-SAME: (ptr readnone captures(address) [[P:%.*]], ptr readonly captures(none) [[F:%.*]]) #[[ATTR3]] {
; FNATTRS-NEXT: call void [[F]](ptr readnone [[P]]) #[[ATTR2:[0-9]+]]
; FNATTRS-NEXT: ret void
;
@@ -547,7 +547,7 @@ define void @fptr_test2b(ptr %p, ptr %f) {
define void @fptr_test2c(ptr %p, ptr %f) {
; FNATTRS: Function Attrs: nofree memory(read)
; FNATTRS-LABEL: define {{[^@]+}}@fptr_test2c
-; FNATTRS-SAME: (ptr readonly [[P:%.*]], ptr readonly captures(none) [[F:%.*]]) #[[ATTR3]] {
+; FNATTRS-SAME: (ptr readonly captures(address) [[P:%.*]], ptr readonly captures(none) [[F:%.*]]) #[[ATTR3]] {
; FNATTRS-NEXT: call void [[F]](ptr readonly [[P]]) #[[ATTR2]]
; FNATTRS-NEXT: ret void
;
diff --git a/llvm/test/Transforms/LICM/promote-capture.ll b/llvm/test/Transforms/LICM/promote-capture.ll
index eac670e6f3edd..19c5700740e7c 100644
--- a/llvm/test/Transforms/LICM/promote-capture.ll
+++ b/llvm/test/Transforms/LICM/promote-capture.ll
@@ -2,7 +2,7 @@
; RUN: opt -S -passes='loop-mssa(licm)' < %s | FileCheck %s
declare i1 @cond(i32 %v) readnone
-declare void @capture(ptr %p) readnone
+declare ptr @capture(ptr %p) readnone
define void @test_captured_after_loop(i32 %len) {
; CHECK-LABEL: @test_captured_after_loop(
@@ -27,7 +27,7 @@ define void @test_captured_after_loop(i32 %len) {
; CHECK: exit:
; CHECK-NEXT: [[C_INC1_LCSSA:%.*]] = phi i32 [ [[C_INC1]], [[LATCH]] ]
; CHECK-NEXT: store i32 [[C_INC1_LCSSA]], ptr [[COUNT]], align 4
-; CHECK-NEXT: call void @capture(ptr [[COUNT]])
+; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr [[COUNT]])
; CHECK-NEXT: ret void
;
entry:
@@ -52,7 +52,7 @@ latch:
br i1 %cmp, label %exit, label %loop
exit:
- call void @capture(ptr %count)
+ call ptr @capture(ptr %count)
ret void
}
@@ -71,7 +71,7 @@ define void @test_captured_in_loop(i32 %len) {
; CHECK: if:
; CHECK-NEXT: [[C_INC:%.*]] = add i32 [[C_INC2]], 1
; CHECK-NEXT: store i32 [[C_INC]], ptr [[COUNT]], align 4
-; CHECK-NEXT: call void @capture(ptr [[COUNT]])
+; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr [[COUNT]])
; CHECK-NEXT: br label [[LATCH]]
; CHECK: latch:
; CHECK-NEXT: [[C_INC1]] = phi i32 [ [[C_INC]], [[IF]] ], [ [[C_INC2]], [[LOOP]] ]
@@ -95,7 +95,7 @@ if:
%c = load i32, ptr %count
%c.inc = add i32 %c, 1
store i32 %c.inc, ptr %count
- call void @capture(ptr %count)
+ call ptr @capture(ptr %count)
br label %latch
latch:
@@ -112,7 +112,7 @@ define void @test_captured_before_loop(i32 %len) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[COUNT:%.*]] = alloca i32, align 4
; CHECK-NEXT: store i32 0, ptr [[COUNT]], align 4
-; CHECK-NEXT: call void @capture(ptr [[COUNT]])
+; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr [[COUNT]])
; CHECK-NEXT: [[COUNT_PROMOTED:%.*]] = load i32, ptr [[COUNT]], align 4
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
@@ -135,7 +135,7 @@ define void @test_captured_before_loop(i32 %len) {
entry:
%count = alloca i32
store i32 0, ptr %count
- call void @capture(ptr %count)
+ call ptr @capture(ptr %count)
br label %loop
loop:
@@ -163,7 +163,7 @@ define void @test_captured_before_loop_address_only(i32 %len) {
; CHECK-NEXT: entry:
; CHECK-NEXT: [[COUNT:%.*]] = alloca i32, align 4
; CHECK-NEXT: store i32 0, ptr [[COUNT]], align 4
-; CHECK-NEXT: call void @capture(ptr captures(address) [[COUNT]])
+; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr captures(address) [[COUNT]])
; CHECK-NEXT: [[COUNT_PROMOTED:%.*]] = load i32, ptr [[COUNT]], align 4
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
@@ -187,7 +187,7 @@ define void @test_captured_before_loop_address_only(i32 %len) {
entry:
%count = alloca i32
store i32 0, ptr %count
- call void @capture(ptr captures(address) %count)
+ call ptr @capture(ptr captures(address) %count)
br label %loop
loop:
@@ -216,7 +216,7 @@ define void @test_captured_before_loop_byval(ptr byval(i32) align 4 %count, i32
; CHECK-LABEL: @test_captured_before_loop_byval(
; CHECK-NEXT: entry:
; CHECK-NEXT: store i32 0, ptr [[COUNT:%.*]], align 4
-; CHECK-NEXT: call void @capture(ptr [[COUNT]])
+; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr [[COUNT]])
; CHECK-NEXT: [[COUNT_PROMOTED:%.*]] = load i32, ptr [[COUNT]], align 4
; CHECK-NEXT: br label [[LOOP:%.*]]
; CHECK: loop:
@@ -238,7 +238,7 @@ define void @test_captured_before_loop_byval(ptr byval(i32) align 4 %count, i32
;
entry:
store i32 0, ptr %count
- call void @capture(ptr %count)
+ call ptr @capture(ptr %count)
br label %loop
loop:
@@ -283,7 +283,7 @@ define void @test_captured_after_loop_byval(ptr byval(i32) align 4 %count, i32 %
; CHECK: exit:
; CHECK-NEXT: [[C_INC1_LCSSA:%.*]] = phi i32 [ [[C_INC1]], [[LATCH]] ]
; CHECK-NEXT: store i32 [[C_INC1_LCSSA]], ptr [[COUNT]], align 4
-; CHECK-NEXT: call void @capture(ptr [[COUNT]])
+; CHECK-NEXT: [[TMP0:%.*]] = call ptr @capture(ptr [[COUNT]])
; CHECK-NEXT: ret void
;
entry:
@@ -307,6 +307,6 @@ latch:
br i1 %cmp, label %exit, label %loop
exit:
- call void @capture(ptr %count)
+ call ptr @capture(ptr %count)
ret void
}
|
dtcxzyw
approved these changes
Sep 16, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.