[ASan][AMDGPU] Fix Assertion Failure. #78242

ampandey-1995 · 2024-01-16T09:26:54Z

Assertion failure `(i >= FTy->getNumParams() || FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with a bad signature!"'. The 'llvm.memcpy' intercepted by ASan instrumentation pass is implemented by it's own __asan_memcpy implementation. The second argument of llvm.memcpy accepts ptr to addrspace(4), __asan_memcpy also has to follow ptr to addrspace(4) convention.

github-actions · 2024-01-16T09:27:12Z

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

llvmbot · 2024-01-16T09:27:39Z

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-llvm-transforms

Author: None (ampandey-1995)

Changes

Assertion failure `(i >= FTy->getNumParams() || FTy->getParamType(i) == Args[i]->getType()) && "Calling a function with a bad signature!"'. The 'llvm.memcpy' intercepted by ASan instrumentation pass is implemented by it's own __asan_memcpy implementation. The second argument of llvm.memcpy accepts ptr to addrspace(4), __asan_memcpy also has to follow ptr to addrspace(4) convention.

Full diff: https://github.com/llvm/llvm-project/pull/78242.diff

2 Files Affected:

(modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+5-3)
(added) llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_instrument_mem_intrinsics.ll (+48)

diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index fb5838bb7941ad..32e7f1b6fc3374 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1254,9 +1254,11 @@ Value *AddressSanitizer::memToShadow(Value *Shadow, IRBuilder<> &IRB) {
 void AddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI) {
   InstrumentationIRBuilder IRB(MI);
   if (isa<MemTransferInst>(MI)) {
-    IRB.CreateCall(isa<MemMoveInst>(MI) ? AsanMemmove : AsanMemcpy,
-                   {MI->getOperand(0), MI->getOperand(1),
-                    IRB.CreateIntCast(MI->getOperand(2), IntptrTy, false)});
+    IRB.CreateCall(
+        isa<MemMoveInst>(MI) ? AsanMemmove : AsanMemcpy,
+        {MI->getOperand(0),
+         IRB.CreatePointerBitCastOrAddrSpaceCast(MI->getOperand(1), PtrTy),
+         IRB.CreateIntCast(MI->getOperand(2), IntptrTy, false)});
   } else if (isa<MemSetInst>(MI)) {
     IRB.CreateCall(
         AsanMemset,
diff --git a/llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_instrument_mem_intrinsics.ll b/llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_instrument_mem_intrinsics.ll
new file mode 100644
index 00000000000000..26258ef384795d
--- /dev/null
+++ b/llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_instrument_mem_intrinsics.ll
@@ -0,0 +1,48 @@
+;RUN: opt < %s -passes=asan -S | FileCheck %s
+
+target datalayout = "e-p:64:64-p1:64:64-p2:32:32-p3:32:32-p4:64:64-p5:32:32-p6:32:32-p7:160:256:256:32-p8:128:128-p9:192:256:256:32-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-v2048:2048-n32:64-S32-A5-G1-ni:7:8:9"
+target triple = "amdgcn-amd-amdhsa"
+
+@__const.__assert_fail.fmt = private unnamed_addr addrspace(4) constant [47 x i8] c"%s:%u: %s: Device-side assertion `%s' failed.\0A\00", align 16
+
+; Function Attrs: nocallback nofree nounwind willreturn memory(argmem: readwrite)
+declare void @llvm.memcpy.p0.p4.i64(ptr noalias nocapture writeonly, ptr addrspace(4) noalias nocapture readonly, i64, i1 immarg) #1
+
+; Function Attrs: convergent mustprogress noinline nounwind optnone
+define weak hidden void @test_mem_intrinsic() sanitize_address #0 {
+; CHECK: define weak hidden void @test_mem_intrinsic() #1 {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: [[FMT:%.*]] = alloca [47 x i8], align 16, addrspace(5)
+; CHECK-NEXT: [[FADDRC:%.*]] = addrspacecast ptr addrspace(5) [[FMT]] to ptr
+; CHECK-NEXT: [[ITMP:%.*]] = call ptr @__asan_memcpy(ptr [[FADDRC]], ptr addrspacecast (ptr addrspace(4) @__const.__assert_fail.fmt to ptr), i64 47)
+; CHECK-NEXT: ret
+entry:
+%fmt = alloca [47 x i8], align 16, addrspace(5)
+%fmt.ascast = addrspacecast ptr addrspace(5) %fmt to ptr
+call void @llvm.memcpy.p0.p4.i64(ptr align 16 %fmt.ascast, ptr addrspace(4) align 16 @__const.__assert_fail.fmt, i64 47, i1 false)
+ret void
+}
+
+
+
+attributes #0 = { convergent mustprogress noinline nounwind optnone "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="gfx906" "target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64,+xnack" }
+attributes #1 = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
+attributes #2 = { convergent nounwind "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="gfx906" "target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64,+xnack" }
+attributes #3 = { cold noreturn nounwind memory(inaccessiblemem: write) }
+attributes #4 = { convergent mustprogress noinline norecurse nounwind optnone "amdgpu-flat-work-group-size"="1,1024" "frame-pointer"="all" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="gfx906" "target-features"="+16-bit-insts,+ci-insts,+dl-insts,+dot1-insts,+dot10-insts,+dot2-insts,+dot7-insts,+dpp,+gfx8-insts,+gfx9-insts,+s-memrealtime,+s-memtime-inst,+wavefrontsize64,+xnack" "uniform-work-group-size"="true" }
+attributes #5 = { convergent nounwind }
+
+!llvm.module.flags = !{!0, !1, !2, !3, !4}
+!llvm.ident = !{!5}
+
+!0 = !{i32 1, !"amdgpu_code_object_version", i32 400}
+!1 = !{i32 1, !"amdgpu_printf_kind", !"hostcall"}
+!2 = !{i32 1, !"wchar_size", i32 4}
+!3 = !{i32 8, !"PIC Level", i32 2}
+!4 = !{i32 7, !"frame-pointer", i32 2}
+!5 = !{!"clang version 18.0.0git (git@github.com:llvm/llvm-project.git a919fe73e2c4c26fbb2a6f0459ff6564a5c83943)"}
+!6 = distinct !{!6, !7}
+!7 = !{!"llvm.loop.mustprogress"}
+!8 = distinct !{!8, !7}
+!9 = distinct !{!9, !7}
+!10 = distinct !{!10, !7}

arsenm · 2024-01-16T10:19:57Z

llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp

+    IRB.CreateCall(
+        isa<MemMoveInst>(MI) ? AsanMemmove : AsanMemcpy,
+        {MI->getOperand(0),
+         IRB.CreatePointerBitCastOrAddrSpaceCast(MI->getOperand(1), PtrTy),


No reason to create pointer bit casts anymore. Just CreateAddrSpaceCast?

Hi @arsenm Can you please review #78410.

I have made changes according to your comments.

arsenm · 2024-01-17T05:25:04Z

llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_instrument_mem_intrinsics.ll

+!llvm.module.flags = !{!0, !1, !2, !3, !4}
+!llvm.ident = !{!5}
+
+!0 = !{i32 1, !"amdgpu_code_object_version", i32 400}


Don't need most of this metadata or most of the attributes

Made changes in PR #78410

ampandey-1995 · 2024-01-17T08:26:27Z

Closing this PR in favour of #78410

llvmbot added backend:AMDGPU compiler-rt:sanitizer llvm:transforms labels Jan 16, 2024

ampandey-1995 added 2 commits January 16, 2024 15:14

Merge branch 'llvm:main' into ampandey-LLVM/assertion-fix

2a635b1

Merge branch 'llvm:main' into ampandey-LLVM/assertion-fix

9f83b18

arsenm reviewed Jan 16, 2024

View reviewed changes

Merge branch 'llvm:main' into ampandey-LLVM/assertion-fix

f5787f0

arsenm reviewed Jan 17, 2024

View reviewed changes

ampandey-1995 closed this Jan 17, 2024

ampandey-1995 deleted the ampandey-LLVM/assertion-fix branch January 17, 2024 08:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ASan][AMDGPU] Fix Assertion Failure. #78242

[ASan][AMDGPU] Fix Assertion Failure. #78242

ampandey-1995 commented Jan 16, 2024

github-actions bot commented Jan 16, 2024

llvmbot commented Jan 16, 2024 •

edited

Loading

arsenm Jan 16, 2024

ampandey-1995 Jan 17, 2024

arsenm Jan 17, 2024

ampandey-1995 Jan 17, 2024

ampandey-1995 commented Jan 17, 2024

[ASan][AMDGPU] Fix Assertion Failure. #78242

[ASan][AMDGPU] Fix Assertion Failure. #78242

Conversation

ampandey-1995 commented Jan 16, 2024

github-actions bot commented Jan 16, 2024

llvmbot commented Jan 16, 2024 • edited Loading

arsenm Jan 16, 2024

Choose a reason for hiding this comment

ampandey-1995 Jan 17, 2024

Choose a reason for hiding this comment

arsenm Jan 17, 2024

Choose a reason for hiding this comment

ampandey-1995 Jan 17, 2024

Choose a reason for hiding this comment

ampandey-1995 commented Jan 17, 2024

llvmbot commented Jan 16, 2024 •

edited

Loading