Skip to content

[ASan] Prevent assert from scalable vectors in FunctionStackPoisoner. #155357

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Aug 27, 2025
Merged

[ASan] Prevent assert from scalable vectors in FunctionStackPoisoner. #155357

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1219,7 +1219,9 @@ struct FunctionStackPoisoner : public InstVisitor<FunctionStackPoisoner> {

std::optional<TypeSize> Size = AI->getAllocationSize(AI->getDataLayout());
// Check that size is known and can be stored in IntptrTy.
if (!Size || !ConstantInt::isValueValidForType(IntptrTy, *Size))
// TODO: Add support for scalable vectors if possible.
if (!Size || Size->isScalable() ||
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems this is better handled in isInterestingAlloca instead, which has its documentation:
Check if we want (and can) handle this alloca.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm. I believe that scalable vectors are interesting allocas, we just don't handle them here at the moment. There are some tests in llvm/test/Instrumentation/AddressSanitizer/vector-load-store.ll that fail if we don't handle any scalable vectors, as isInterestingAlloca is used in several places.

I could add a TODO maybe?

Copy link
Collaborator

Choose a reason for hiding this comment

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

For the other places where isInterestingAlloca is called, does the address sanitizer support those cases with scalable vectors? If not, it's probably safer to disable this entirely and add a TODO to make the address sanitizer to support scalable vectors in general.

Copy link
Contributor

@thurstond thurstond Aug 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

The regression is specifically for lifetime intrinsics (e.g., as seen in the new test case) - the crash started happening after "[IR] Remove size argument from lifetime intrinsics" (#150248), because now ASan doesn't know the size of the alloca for:

  %ctx_pg = alloca <vscale x 16 x i1>, align 2
  call void @llvm.lifetime.start.p0(ptr %ctx_pg)

(In the past, it could have used the size argument instead of trying to compute it.)

i.e., I think this localized fix is fine, because it is the only place where lifetime intrinsics are handled in ASan.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My concern is that other places in the AddressSanitizer where isInterestingAlloca returns true also don't support scalable vectors, not just the case that was described in that new test case, i.e. for a case we haven't hit yet. If you can confirm that all other cases where the AddressSanitizer encounters a scalable alloca can be handled correctly, I'm happy with the fix.

!ConstantInt::isValueValidForType(IntptrTy, *Size))
return;

bool DoPoison = (ID == Intrinsic::lifetime_end);
Expand Down
27 changes: 27 additions & 0 deletions llvm/test/Instrumentation/AddressSanitizer/asan-scalable-vector.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
; NOTE: Assertions have been autogenerated by utils/update_test_checks.py UTC_ARGS: --version 5
; RUN: opt < %s -passes='asan<use-after-scope>' -S | FileCheck %s

define void @test() #1 {
; CHECK-LABEL: define void @test(
; CHECK-SAME: ) #[[ATTR0:[0-9]+]] {
; CHECK-NEXT: [[ENTRY:.*:]]
; CHECK-NEXT: [[CTX_PG:%.*]] = alloca <vscale x 16 x i1>, align 2
; CHECK-NEXT: call void @llvm.lifetime.start.p0(ptr [[CTX_PG]])
; CHECK-NEXT: [[TMP0:%.*]] = load i8, ptr inttoptr (i64 17592186044416 to ptr), align 1
; CHECK-NEXT: [[TMP1:%.*]] = icmp ne i8 [[TMP0]], 0
; CHECK-NEXT: br i1 [[TMP1]], label %[[BB2:.*]], label %[[BB3:.*]]
; CHECK: [[BB2]]:
; CHECK-NEXT: call void @__asan_report_store8(i64 0) #[[ATTR4:[0-9]+]]
; CHECK-NEXT: unreachable
; CHECK: [[BB3]]:
; CHECK-NEXT: store ptr [[CTX_PG]], ptr null, align 8
; CHECK-NEXT: ret void
;
entry:
%ctx_pg = alloca <vscale x 16 x i1>, align 2
call void @llvm.lifetime.start.p0(ptr %ctx_pg)
store ptr %ctx_pg, ptr null, align 8
ret void
}

attributes #1 = { sanitize_address }
Loading