[ASan][AMDGPU] Fix Assertion Failure. #78710
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be 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 If you have received no comments on your PR for a week, you can request a review 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. |
|
@llvm/pr-subscribers-llvm-transforms @llvm/pr-subscribers-backend-amdgpu Author: None (ampandey-1995) ChangesAssertion 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/78710.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index caab98c732eeeb..4a9e8119203438 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -1255,7 +1255,8 @@ 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.CreateAddrSpaceCast(MI->getOperand(0), PtrTy),
+ IRB.CreateAddrSpaceCast(MI->getOperand(1), PtrTy),
IRB.CreateIntCast(MI->getOperand(2), IntptrTy, false)});
} else if (isa<MemSetInst>(MI)) {
IRB.CreateCall(
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..f2f30592f500e8
--- /dev/null
+++ b/llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_instrument_mem_intrinsics.ll
@@ -0,0 +1,67 @@
+;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.__src__0 = private unnamed_addr constant [12 x i8] c"oldstring_0\00", align 16
+@__const.__dest__0 = private unnamed_addr constant [12 x i8] c"newstring_0\00", align 16
+@__const.__src__1 = private unnamed_addr addrspace(4) constant [12 x i8] c"oldstring_1\00", align 16
+@__const.__dest__1 = private unnamed_addr addrspace(4) constant [12 x i8] c"newstring_1\00", align 16
+
+declare void @llvm.memcpy.p0.p0.i64(ptr addrspace(4) noalias nocapture writeonly, ptr noalias nocapture readonly, i64, i1 immarg)
+declare void @llvm.memcpy.p0.p1.i64(ptr noalias nocapture writeonly, ptr addrspace(4) noalias nocapture readonly, i64, i1 immarg)
+declare void @llvm.memcpy.p0.p2.i64(ptr addrspace(4) noalias nocapture writeonly, ptr addrspace(4) noalias nocapture readonly, i64, i1 immarg)
+
+declare void @llvm.memcpy.p0.p0.i32(ptr addrspace(4) noalias nocapture writeonly, ptr noalias nocapture readonly, i32, i1 immarg)
+declare void @llvm.memcpy.p0.p1.i32(ptr noalias nocapture writeonly, ptr addrspace(4) noalias nocapture readonly, i32, i1 immarg)
+declare void @llvm.memcpy.p0.p2.i32(ptr addrspace(4) noalias nocapture writeonly, ptr addrspace(4) noalias nocapture readonly, i32, i1 immarg)
+
+declare void @llvm.memmove.p0.p0.i64(ptr addrspace(4) nocapture writeonly, ptr nocapture readonly, i64, i1 immarg)
+declare void @llvm.memmove.p0.p1.i64(ptr nocapture writeonly, ptr addrspace(4) nocapture readonly, i64, i1 immarg)
+declare void @llvm.memmove.p0.p2.i64(ptr addrspace(4) nocapture writeonly, ptr addrspace(4) nocapture readonly, i64, i1 immarg)
+
+declare void @llvm.memmove.p0.p0.i32(ptr addrspace(4) nocapture writeonly, ptr nocapture readonly, i32, i1 immarg)
+declare void @llvm.memmove.p0.p1.i32(ptr nocapture writeonly, ptr addrspace(4) nocapture readonly, i32, i1 immarg)
+declare void @llvm.memmove.p0.p2.i32(ptr addrspace(4) nocapture writeonly, ptr addrspace(4) nocapture readonly, i32, i1 immarg)
+
+define weak hidden void @test_mem_intrinsic_memcpy() sanitize_address {
+; CHECK: define weak hidden void @test_mem_intrinsic_memcpy() #0 {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: %0 = call ptr @__asan_memcpy(ptr addrspacecast (ptr addrspace(4) @__const.__dest__1 to ptr), ptr @__const.__src__0, i64 12)
+; CHECK-NEXT: %1 = call ptr @__asan_memcpy(ptr @__const.__dest__0, ptr addrspacecast (ptr addrspace(4) @__const.__src__1 to ptr), i64 12)
+; CHECK-NEXT: %2 = call ptr @__asan_memcpy(ptr addrspacecast (ptr addrspace(4) @__const.__dest__1 to ptr), ptr addrspacecast (ptr addrspace(4) @__const.__src__1 to ptr), i64 12)
+; CHECK-NEXT: %3 = call ptr @__asan_memcpy(ptr addrspacecast (ptr addrspace(4) @__const.__dest__1 to ptr), ptr @__const.__src__0, i64 12)
+; CHECK-NEXT: %4 = call ptr @__asan_memcpy(ptr @__const.__dest__0, ptr addrspacecast (ptr addrspace(4) @__const.__src__1 to ptr), i64 12)
+; CHECK-NEXT: %5 = call ptr @__asan_memcpy(ptr addrspacecast (ptr addrspace(4) @__const.__dest__1 to ptr), ptr addrspacecast (ptr addrspace(4) @__const.__src__1 to ptr), i64 12)
+; CHECK-NEXT: ret void
+entry:
+ call void @llvm.memcpy.p0.p0.i64(ptr addrspace(4) align 16 @__const.__dest__1, ptr align 16 @__const.__src__0, i64 12, i1 false)
+ call void @llvm.memcpy.p0.p1.i64(ptr align 16 @__const.__dest__0, ptr addrspace(4) align 16 @__const.__src__1, i64 12, i1 false)
+ call void @llvm.memcpy.p0.p2.i64(ptr addrspace(4) align 16 @__const.__dest__1, ptr addrspace(4) align 16 @__const.__src__1, i64 12, i1 false)
+ call void @llvm.memcpy.p0.p0.i32(ptr addrspace(4) align 16 @__const.__dest__1, ptr align 16 @__const.__src__0, i32 12, i1 false)
+ call void @llvm.memcpy.p0.p1.i32(ptr align 16 @__const.__dest__0, ptr addrspace(4) align 16 @__const.__src__1, i32 12, i1 false)
+ call void @llvm.memcpy.p0.p2.i32(ptr addrspace(4) align 16 @__const.__dest__1, ptr addrspace(4) align 16 @__const.__src__1, i32 12, i1 false)
+ ret void
+}
+
+define dso_local void @test_mem_intrinsic_memmove() sanitize_address {
+; CHECK: define dso_local void @test_mem_intrinsic_memmove() #0 {
+; CHECK-NEXT: entry:
+; CHECK-NEXT: %0 = call ptr @__asan_memmove(ptr addrspacecast (ptr addrspace(4) @__const.__dest__1 to ptr), ptr @__const.__src__0, i64 12)
+; CHECK-NEXT: %1 = call ptr @__asan_memmove(ptr @__const.__dest__0, ptr addrspacecast (ptr addrspace(4) @__const.__src__1 to ptr), i64 12)
+; CHECK-NEXT: %2 = call ptr @__asan_memmove(ptr addrspacecast (ptr addrspace(4) @__const.__dest__1 to ptr), ptr addrspacecast (ptr addrspace(4) @__const.__src__1 to ptr), i64 12)
+; CHECK-NEXT: %3 = call ptr @__asan_memmove(ptr addrspacecast (ptr addrspace(4) @__const.__dest__1 to ptr), ptr @__const.__src__0, i64 12)
+; CHECK-NEXT: %4 = call ptr @__asan_memmove(ptr @__const.__dest__0, ptr addrspacecast (ptr addrspace(4) @__const.__src__1 to ptr), i64 12)
+; CHECK-NEXT: %5 = call ptr @__asan_memmove(ptr addrspacecast (ptr addrspace(4) @__const.__dest__1 to ptr), ptr addrspacecast (ptr addrspace(4) @__const.__src__1 to ptr), i64 12)
+; CHECK-NEXT: ret void
+entry:
+ call void @llvm.memmove.p0.p0.i64(ptr addrspace(4) align 16 @__const.__dest__1, ptr align 16 @__const.__src__0, i64 12, i1 false)
+ call void @llvm.memmove.p0.p1.i64(ptr align 16 @__const.__dest__0, ptr addrspace(4) align 16 @__const.__src__1, i64 12, i1 false)
+ call void @llvm.memmove.p0.p2.i64(ptr addrspace(4) align 16 @__const.__dest__1, ptr addrspace(4) align 16 @__const.__src__1, i64 12, i1 false)
+ call void @llvm.memmove.p0.p0.i32(ptr addrspace(4) align 16 @__const.__dest__1, ptr align 16 @__const.__src__0, i32 12, i1 false)
+ call void @llvm.memmove.p0.p1.i32(ptr align 16 @__const.__dest__0, ptr addrspace(4) align 16 @__const.__src__1, i32 12, i1 false)
+ call void @llvm.memmove.p0.p2.i32(ptr addrspace(4) align 16 @__const.__dest__1, ptr addrspace(4) align 16 @__const.__src__1, i32 12, i1 false)
+ ret void
+}
+
+attributes #0 = { sanitize_address }
|
llvm/test/Instrumentation/AddressSanitizer/AMDGPU/asan_instrument_mem_intrinsics.ll
Outdated
Show resolved
Hide resolved
| ; CHECK-NEXT: %5 = call ptr @__asan_memcpy(ptr addrspacecast (ptr addrspace(4) @__const.__dest__1 to ptr), ptr addrspacecast (ptr addrspace(4) @__const.__src__1 to ptr), i64 12) | ||
| ; CHECK-NEXT: ret void | ||
| entry: | ||
| call void @llvm.memcpy.p0.p0.i64(ptr addrspace(4) align 16 @__const.__dest__1, ptr align 16 @__const.__src__0, i64 12, i1 false) |
There was a problem hiding this comment.
It would be more helpful to test with a non-constant pointer value, such that a true instruction needs to be emitted and not a ConstantExp. The ConstantExpr avoids testing the instruction placement
| @@ -1255,7 +1255,8 @@ 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.CreateAddrSpaceCast(MI->getOperand(0), PtrTy), | |||
| IRB.CreateAddrSpaceCast(MI->getOperand(1), PtrTy), | |||
There was a problem hiding this comment.
do we need it in memset?
There was a problem hiding this comment.
I don't think we need currently this for memset.
There was a problem hiding this comment.
It will be needed for any memory intrinsic
There was a problem hiding this comment.
It will be needed for any memory intrinsic
Logically we should never see an attempt to set memory with a pointer to constant memory, but I think we need to handle memset otherwise.
There was a problem hiding this comment.
I guess never-const applies to the first arg of memcpy, but the patch adds CreateAddrSpaceCast?
So maybe use CreateAddrSpaceCast for all intrinsics for consistency
There was a problem hiding this comment.
It will be needed for any memory intrinsic
Logically we should never see an attempt to set memory with a pointer to constant memory, but I think we need to handle memset otherwise.
The "constant" part here isn't what matters, it's the non-0 address space
There was a problem hiding this comment.
Hi all,
The latest PR is #79795 to fix this issue.
Thanks again for all of your inputs.
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.