[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 01/12] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c..81bb3f3c5a5e3 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496..c696cc38167cd 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 02/12] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd..2051fa9467817 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 03/12] Now only emit metadata when using a ASan, and tag it
with an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec..d8fdacf30e12e 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 01/12] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c..81bb3f3c5a5e3 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496..c696cc38167cd 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 02/12] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd..2051fa9467817 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 03/12] Now only emit metadata when using a ASan, and tag it
with an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec..d8fdacf30e12e 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
rorth wrote: Both Solaris bots are back green again. Thanks. https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
mstorsjo wrote: > You're right, I don't think there is a reason to check the stack location in > this test, just how it was serialized. I'll push a change and hopefully > this'll be fixed. Thanks for the notes! Thanks for the fix in 4da5e9dd320e9d48be0fa05ba1a8faf50fb53834, that does seem to have fixed the issue on i686 windows at least! https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
gbMattN wrote: You're right, I don't think there is a reason to check the stack location in this test, just how it was serialized. I'll push a change and hopefully this'll be fixed. Thanks for the notes! https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
mstorsjo wrote: > Maybe the test needs to be relaxed a bit because of stack layout differences > in other OS targets? Although I'm not sure why they're different. See > https://lab.llvm.org/buildbot/#/builders/186/builds/7896: > > ``` > TEST 'AddressSanitizer-arm-android :: > TestCases/shadowed-stack-serialization.cpp' FAILED > Exit Code: 1 > Command Output (stderr): > -- > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/android_commands/android_compile.py > > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/bin/clang > --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer > -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only > --target=armv7-linux-androideabi24 > --sysroot=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot > > --gcc-toolchain=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 > > -B/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 > -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -fuse-ld=lld > -shared-libasan -O0 > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp > -o > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/asan/ARMAndroidConfig/TestCases/Output/shadowed-stack-serialization.cpp.tmp > # RUN: at line 1 > + > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/android_commands/android_compile.py > > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/bin/clang > --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer > -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only > --target=armv7-linux-androideabi24 > --sysroot=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot > > --gcc-toolchain=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 > > -B/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 > -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -fuse-ld=lld > -shared-libasan -O0 > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp > -o > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/asan/ARMAndroidConfig/TestCases/Output/shadowed-stack-serialization.cpp.tmp > not > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/asan/ARMAndroidConfig/TestCases/Output/shadowed-stack-serialization.cpp.tmp > 2>&1 | FileCheck > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp > # RUN: at line 2 > + not > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/asan/ARMAndroidConfig/TestCases/Output/shadowed-stack-serialization.cpp.tmp > + FileCheck > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp:12:11: > error: CHECK: expected string not found in input > // CHECK: [32, 36) 'x' > ^ > :1:1: note: scanning from here > = > ^ > :11:2: note: possible intended match here > [16, 20) 'x' (line 7) <== Memory access at offset 16 is inside this variable > ^ > Input file: > Check file: > /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp > ``` I'm also hitting this failure; in my case it's cropping up on i686 windows: https://github.com/mstorsjo/llvm-mingw/actions/runs/14255796193/job/39971583775 https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
rorth wrote: The patch also breaks both the [Solaris/sparcv9](https://lab.llvm.org/buildbot/#/builders/13/builds/6362) and [Solaris/amd64](https://lab.llvm.org/staging/#/builders/120/builds/7512) bots. https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
aaronpuchert wrote: Maybe the test needs to be relaxed a bit because of stack layout differences in other OS targets? Although I'm not sure why they're different. See https://lab.llvm.org/buildbot/#/builders/186/builds/7896: ``` TEST 'AddressSanitizer-arm-android :: TestCases/shadowed-stack-serialization.cpp' FAILED Exit Code: 1 Command Output (stderr): -- /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/android_commands/android_compile.py /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only --target=armv7-linux-androideabi24 --sysroot=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot --gcc-toolchain=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 -B/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -fuse-ld=lld -shared-libasan -O0 /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp -o /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/asan/ARMAndroidConfig/TestCases/Output/shadowed-stack-serialization.cpp.tmp # RUN: at line 1 + /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/sanitizer_common/android_commands/android_compile.py /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm_build64/bin/clang --driver-mode=g++ -fsanitize=address -mno-omit-leaf-frame-pointer -fno-omit-frame-pointer -fno-optimize-sibling-calls -gline-tables-only --target=armv7-linux-androideabi24 --sysroot=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64/sysroot --gcc-toolchain=/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 -B/var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/android_ndk/toolchains/llvm/prebuilt/linux-x86_64 -Wthread-safety -Wthread-safety-reference -Wthread-safety-beta -fuse-ld=lld -shared-libasan -O0 /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp -o /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/asan/ARMAndroidConfig/TestCases/Output/shadowed-stack-serialization.cpp.tmp not /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/asan/ARMAndroidConfig/TestCases/Output/shadowed-stack-serialization.cpp.tmp 2>&1 | FileCheck /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp # RUN: at line 2 + not /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/compiler_rt_build_android_arm/test/asan/ARMAndroidConfig/TestCases/Output/shadowed-stack-serialization.cpp.tmp + FileCheck /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp:12:11: error: CHECK: expected string not found in input // CHECK: [32, 36) 'x' ^ :1:1: note: scanning from here = ^ :11:2: note: possible intended match here [16, 20) 'x' (line 7) <== Memory access at offset 16 is inside this variable ^ Input file: Check file: /var/lib/buildbot/sanitizer-buildbot6/sanitizer-x86_64-linux-android/build/llvm-project/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp ``` https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN closed https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From b0845c970847aca0f50cc72fec6fb2334b4f10d3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Thu, 3 Apr 2025 11:50:31 +0100
Subject: [PATCH] [ASan] Add metadata to renamed instructions so ASan doesn't
use the incorrect name
---
clang/lib/CodeGen/CGExpr.cpp | 3 +++
.../shadowed-stack-serialization.cpp | 12 +
.../TestCases/use-after-scope-inlined.cpp | 2 +-
.../Instrumentation/AddressSanitizer.cpp | 26 ++-
4 files changed, 41 insertions(+), 2 deletions(-)
create mode 100644
compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 9676e61cf322d..91c5e2d58f17c 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -136,6 +136,9 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (SanOpts.Mask & SanitizerKind::Address) {
+Alloca->addAnnotationMetadata({"alloca_name_altered", Name.str()});
+ }
if (Allocas) {
Allocas->Add(Alloca);
}
diff --git a/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
b/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
new file mode 100644
index 0..f2706c671c261
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
@@ -0,0 +1,12 @@
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+int main() {
+ int x;
+ {
+int x;
+delete &x;
+ }
+}
+
+// CHECK: [32, 36) 'x'
diff --git a/compiler-rt/test/asan/TestCases/use-after-scope-inlined.cpp
b/compiler-rt/test/asan/TestCases/use-after-scope-inlined.cpp
index 1014ff919b9ef..fbb67499ab3d7 100644
--- a/compiler-rt/test/asan/TestCases/use-after-scope-inlined.cpp
+++ b/compiler-rt/test/asan/TestCases/use-after-scope-inlined.cpp
@@ -23,5 +23,5 @@ int main(int argc, char *argv[]) {
// CHECK: Address 0x{{.*}} is located in stack of thread T0 at offset
[[OFFSET:[^ ]*]] in frame
// CHECK: {{.*}} in main
// CHECK: This frame has
- // CHECK: {{\[}}[[OFFSET]], {{.*}}) 'x.i' (line [[@LINE-15]])
+ // CHECK: {{\[}}[[OFFSET]], {{.*}}) 'x' (line [[@LINE-15]])
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index bbe7040121649..51a186e9596a5 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3392,6 +3392,29 @@ static void findStoresToUninstrumentedArgAllocas(
}
}
+static StringRef getAllocaName(AllocaInst *AI) {
+ // Alloca could have been renamed for uniqueness. Its true name will have
been
+ // recorded as an annotation.
+ if (AI->hasMetadata(LLVMContext::MD_annotation)) {
+MDTuple *AllocaAnnotations =
+cast(AI->getMetadata(LLVMContext::MD_annotation));
+for (auto &Annotation : AllocaAnnotations->operands()) {
+ if (!isa(Annotation))
+continue;
+ auto AnnotationTuple = cast(Annotation);
+ for (int Index = 0; Index < AnnotationTuple->getNumOperands(); Index++) {
+// All annotations are strings
+auto MetadataString =
+cast(AnnotationTuple->getOperand(Index));
+if (MetadataString->getString() == "alloca_name_altered")
+ return cast(AnnotationTuple->getOperand(Index + 1))
+ ->getString();
+ }
+}
+ }
+ return AI->getName();
+}
+
void FunctionStackPoisoner::processStaticAllocas() {
if (AllocaVec.empty()) {
assert(StaticAllocaPoisonCallVec.empty());
@@ -3432,7 +3455,8 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+StringRef Name = getAllocaName(AI);
+ASanStackVariableDescription D = {Name.data(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From fb5ad588f299bdba727dbf8288b983359ac29480 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Thu, 3 Apr 2025 11:50:31 +0100
Subject: [PATCH] [ASan] Add metadata to renamed instructions so ASan doesn't
use the incorrect name
---
clang/lib/CodeGen/CGExpr.cpp | 3 ++
.../shadowed-stack-serialization.cpp | 12
.../TestCases/use-after-scope-inlined.cpp | 2 +-
.../Instrumentation/AddressSanitizer.cpp | 28 +--
4 files changed, 42 insertions(+), 3 deletions(-)
create mode 100644
compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 9676e61cf322d..91c5e2d58f17c 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -136,6 +136,9 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (SanOpts.Mask & SanitizerKind::Address) {
+Alloca->addAnnotationMetadata({"alloca_name_altered", Name.str()});
+ }
if (Allocas) {
Allocas->Add(Alloca);
}
diff --git a/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
b/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
new file mode 100644
index 0..f2706c671c261
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
@@ -0,0 +1,12 @@
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+int main() {
+ int x;
+ {
+int x;
+delete &x;
+ }
+}
+
+// CHECK: [32, 36) 'x'
diff --git a/compiler-rt/test/asan/TestCases/use-after-scope-inlined.cpp
b/compiler-rt/test/asan/TestCases/use-after-scope-inlined.cpp
index 1014ff919b9ef..fbb67499ab3d7 100644
--- a/compiler-rt/test/asan/TestCases/use-after-scope-inlined.cpp
+++ b/compiler-rt/test/asan/TestCases/use-after-scope-inlined.cpp
@@ -23,5 +23,5 @@ int main(int argc, char *argv[]) {
// CHECK: Address 0x{{.*}} is located in stack of thread T0 at offset
[[OFFSET:[^ ]*]] in frame
// CHECK: {{.*}} in main
// CHECK: This frame has
- // CHECK: {{\[}}[[OFFSET]], {{.*}}) 'x.i' (line [[@LINE-15]])
+ // CHECK: {{\[}}[[OFFSET]], {{.*}}) 'x' (line [[@LINE-15]])
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index bbe7040121649..3bee26a15c0c9 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3392,6 +3392,29 @@ static void findStoresToUninstrumentedArgAllocas(
}
}
+static StringRef getAllocaName(AllocaInst *AI) {
+ // Alloca could have been renamed for uniqueness. Its true name will have
been
+ // recorded as an annotation.
+ if (AI->hasMetadata(LLVMContext::MD_annotation)) {
+MDTuple *AllocaAnnotations =
+cast(AI->getMetadata(LLVMContext::MD_annotation));
+for (auto &Annotation : AllocaAnnotations->operands()) {
+ if (!isa(Annotation))
+continue;
+ auto AnnotationTuple = cast(Annotation);
+ for (int Index = 0; Index < AnnotationTuple->getNumOperands(); Index++) {
+// All annotations are strings
+auto MetadataString =
+cast(AnnotationTuple->getOperand(Index));
+if (MetadataString->getString() == "alloca_name_altered")
+ return cast(AnnotationTuple->getOperand(Index + 1))
+ ->getString();
+ }
+}
+ }
+ return AI->getName();
+}
+
void FunctionStackPoisoner::processStaticAllocas() {
if (AllocaVec.empty()) {
assert(StaticAllocaPoisonCallVec.empty());
@@ -3426,13 +3449,14 @@ void FunctionStackPoisoner::processStaticAllocas() {
ArgInitInst->moveBefore(InsBefore->getIterator());
// If we have a call to llvm.localescape, keep it in the entry block.
- if (LocalEscapeCall)
+ if (LocalEscapeCall)
LocalEscapeCall->moveBefore(InsBefore->getIterator());
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+StringRef Name = getAllocaName(AI);
+ASanStackVariableDescription D = {Name.data(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/vitalybuka approved this pull request. https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/vitalybuka updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 01/11] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c..81bb3f3c5a5e3 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496..c696cc38167cd 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 02/11] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd..2051fa9467817 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 03/11] Now only emit metadata when using a ASan, and tag it
with an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec..d8fdacf30e12e 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -137,6 +137,10 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + if (Alloca->getName() != Name.str() && gbMattN wrote: Ah, I misunderstood. Check has been removed! https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 01/11] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c5..81bb3f3c5a5e35 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..c696cc38167cd4 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 02/11] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd4..2051fa94678175 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 03/11] Now only emit metadata when using a ASan, and tag it
with an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -137,6 +137,10 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + if (Alloca->getName() != Name.str() && vitalybuka wrote: `Alloca->getName() != Name.str()` is still here? https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 01/10] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c5..81bb3f3c5a5e35 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..c696cc38167cd4 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 02/10] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd4..2051fa94678175 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 03/10] Now only emit metadata when using a ASan, and tag it
with an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3425,12 +3446,14 @@ void FunctionStackPoisoner::processStaticAllocas() {
ArgInitInst->moveBefore(InsBefore);
// If we have a call to llvm.localescape, keep it in the entry block.
- if (LocalEscapeCall) LocalEscapeCall->moveBefore(InsBefore);
+ if (LocalEscapeCall)
+LocalEscapeCall->moveBefore(InsBefore);
gbMattN wrote:
Good catch, I think I was overzealous on the clang-format
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3391,6 +3391,27 @@ static void findStoresToUninstrumentedArgAllocas(
}
}
+StringRef getAllocaName(AllocaInst *AI) {
+ // Alloca could have been renamed for uniqueness. Its true name will have
been
+ // recorded as an annotation.
+ if (AI->hasMetadata(LLVMContext::MD_annotation)) {
+MDTuple *Annotation =
+(MDTuple *)AI->getMetadata(LLVMContext::MD_annotation);
fhahn wrote:
don't use old-style casts, this should probably be `cast<>`
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3391,6 +3391,27 @@ static void findStoresToUninstrumentedArgAllocas(
}
}
+StringRef getAllocaName(AllocaInst *AI) {
fhahn wrote:
```suggestion
static StringRef getAllocaName(AllocaInst *AI) {
```
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3425,12 +3446,14 @@ void FunctionStackPoisoner::processStaticAllocas() {
ArgInitInst->moveBefore(InsBefore);
// If we have a call to llvm.localescape, keep it in the entry block.
- if (LocalEscapeCall) LocalEscapeCall->moveBefore(InsBefore);
+ if (LocalEscapeCall)
+LocalEscapeCall->moveBefore(InsBefore);
fhahn wrote:
unrelated change?
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -134,6 +134,12 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + llvm::errs() << "Alloca name '" << Alloca->getName() << "'\n"; + llvm::errs() << "NAME str '" << Name.str() << "'\n"; fhahn wrote: left over dbg ? https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3391,6 +3391,27 @@ static void findStoresToUninstrumentedArgAllocas(
}
}
+StringRef getAllocaName(AllocaInst *AI) {
+ // Alloca could have been renamed for uniqueness. Its true name will have
been
+ // recorded as an annotation.
+ if (AI->hasMetadata(LLVMContext::MD_annotation)) {
+MDTuple *Annotation =
+(MDTuple *)AI->getMetadata(LLVMContext::MD_annotation);
+for (int i = 0; i < Annotation->getNumOperands(); i++) {
+ if (auto Tuple = dyn_cast(Annotation->getOperand(i))) {
+for (int i = 0; i < Tuple->getNumOperands(); i++) {
+ if (auto stringMetadata = dyn_cast(Tuple->getOperand(i))) {
+if (stringMetadata->getString() == "alloca_name_altered") {
+ return ((MDString *)Tuple->getOperand(i + 1).get())->getString();
fhahn wrote:
old style cast
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3391,6 +3391,27 @@ static void findStoresToUninstrumentedArgAllocas(
}
}
+StringRef getAllocaName(AllocaInst *AI) {
+ // Alloca could have been renamed for uniqueness. Its true name will have
been
+ // recorded as an annotation.
+ if (AI->hasMetadata(LLVMContext::MD_annotation)) {
+MDTuple *Annotation =
+(MDTuple *)AI->getMetadata(LLVMContext::MD_annotation);
+for (int i = 0; i < Annotation->getNumOperands(); i++) {
+ if (auto Tuple = dyn_cast(Annotation->getOperand(i))) {
+for (int i = 0; i < Tuple->getNumOperands(); i++) {
fhahn wrote:
Local variables usually start with upper case in LLVM (here and below)
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3391,6 +3391,27 @@ static void findStoresToUninstrumentedArgAllocas(
}
}
+StringRef getAllocaName(AllocaInst *AI) {
+ // Alloca could have been renamed for uniqueness. Its true name will have
been
+ // recorded as an annotation.
+ if (AI->hasMetadata(LLVMContext::MD_annotation)) {
+MDTuple *Annotation =
+(MDTuple *)AI->getMetadata(LLVMContext::MD_annotation);
+for (int i = 0; i < Annotation->getNumOperands(); i++) {
+ if (auto Tuple = dyn_cast(Annotation->getOperand(i))) {
+for (int i = 0; i < Tuple->getNumOperands(); i++) {
+ if (auto stringMetadata = dyn_cast(Tuple->getOperand(i))) {
+if (stringMetadata->getString() == "alloca_name_altered") {
+ return ((MDString *)Tuple->getOperand(i + 1).get())->getString();
+}
fhahn wrote:
There should be an easy way to do this :)
Might be good to use some early continues to reduce the nesting here.
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3391,6 +3391,27 @@ static void findStoresToUninstrumentedArgAllocas(
}
}
+StringRef getAllocaName(AllocaInst *AI) {
+ // Alloca could have been renamed for uniqueness. Its true name will have
been
+ // recorded as an annotation.
+ if (AI->hasMetadata(LLVMContext::MD_annotation)) {
+MDTuple *Annotation =
+(MDTuple *)AI->getMetadata(LLVMContext::MD_annotation);
+for (int i = 0; i < Annotation->getNumOperands(); i++) {
fhahn wrote:
Can we directly iterate over the operands?
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 1/9] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c5..81bb3f3c5a5e35 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..c696cc38167cd4 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 2/9] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd4..2051fa94678175 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 3/9] Now only emit metadata when using a ASan, and tag it with
an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -137,6 +137,10 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + if (Alloca->getName() != Name.str() && gbMattN wrote: I think they want the `Alloca->getName() != Name.str()` check removed, so that when running ASan, we emit this name metadata on every `AllocaInst` I thought it would be nice to reduce the amount of additional metadata emitted, but I don't have particularly strong feelings on the matter https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 1/9] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c5..81bb3f3c5a5e35 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..c696cc38167cd4 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 2/9] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd4..2051fa94678175 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 3/9] Now only emit metadata when using a ASan, and tag it with
an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -137,6 +137,10 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + if (Alloca->getName() != Name.str() && vitalybuka wrote: My understanding was you are asking to produce medatada for all compilation modes, even without asan? https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -137,6 +137,10 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + if (Alloca->getName() != Name.str() && efriedma-quic wrote: Yes, something like that. I don't think llvm-compile-time-tracker tracks compile time with asan enabled, so I can't see how this change could have any effect either way. https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3391,6 +3391,29 @@ static void findStoresToUninstrumentedArgAllocas(
}
}
+StringRef getAllocaName(AllocaInst *AI) {
+ StringRef Name = AI->getName();
+
+ // Alloca could have been renamed for uniqueness. Its true name will have
been
+ // recorded as an annotation.
+ if (AI->hasMetadata(LLVMContext::MD_annotation)) {
+MDTuple *Annotation =
+(MDTuple *)AI->getMetadata(LLVMContext::MD_annotation);
+for (int i = 0; i < Annotation->getNumOperands(); i++) {
+ if (auto Tuple = dyn_cast(Annotation->getOperand(i))) {
+for (int i = 0; i < Tuple->getNumOperands(); i++) {
+ if (auto stringMetadata = dyn_cast(Tuple->getOperand(i))) {
+if (stringMetadata->getString() == "alloca_name_altered") {
+ Name = ((MDString *)Tuple->getOperand(i + 1).get())->getString();
vitalybuka wrote:
break or better return here
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -137,6 +137,10 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + if (Alloca->getName() != Name.str() && vitalybuka wrote: @efriedma-quic To clarify we do: 1. set metadata always 2. remove DiscardValueNames hack (in a different future patch please, as this may cause regressions if we loose metdata more often than name) LGTM if https://llvm-compile-time-tracker.com/ fail to notice a difference https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/efriedma-quic edited https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -137,6 +137,10 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + if (Alloca->getName() != Name.str() && efriedma-quic wrote: Oh, we have some code that specifically messes with the default when asan is enabled. https://github.com/llvm/llvm-project/blob/3caa68a021c2f795de3df7f6ad7953556e825fab/clang/lib/Frontend/CompilerInvocation.cpp#L4973 If we have to support metadata anyway, I'd prefer to use the metadata unconditionally, instead of messing with LLVM IR instruction names. https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 1/8] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c5..81bb3f3c5a5e35 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..c696cc38167cd4 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 2/8] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd4..2051fa94678175 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 3/8] Now only emit metadata when using a ASan, and tag it with
an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -137,6 +137,10 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + if (Alloca->getName() != Name.str() && gbMattN wrote: > I don't see the point of the `Alloca->getName() != Name.str()` check; it will > almost always fail. (In release builds of clang, we suppress instruction > names, so `Alloca->getName()` is just the empty string.) I'm still getting names from Allocas when I build in release mode, is there some other flag that does this? https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN edited https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
gbMattN wrote: > I don't see (from description) why it's needed. A reproducer of the issue can be seen [failing on godbolt here](https://godbolt.org/z/KjPP4jrod) This is based off [this bugzilla ticket](https://bugs.llvm.org/show_bug.cgi?id=47982) I forgot to put this at the top when I undrafted the pull request, apologies for the confusion. https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3430,13 +3430,25 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
- ASan.getAllocaSizeInBytes(*AI),
- 0,
- AI->getAlign().value(),
- AI,
- 0,
- 0};
+StringRef Name = AI->getName();
+if (AI->hasMetadata(LLVMContext::MD_annotation)) {
+ MDTuple *Annotation = (MDTuple
*)AI->getMetadata(LLVMContext::MD_annotation);
vitalybuka wrote:
it would be nice to have it into some utility function
getAllocaOriginalName(AI) or something.
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -137,6 +137,10 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + if (Alloca->getName() != Name.str() && vitalybuka wrote: It works with Release compiler. https://godbolt.org/z/93ojPGM64 ``` [32, 36) 'x' (line 7) <== Memory access at offset 32 is inside this variable ``` https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
vitalybuka wrote: I don't see (from description) why it's needed. https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -137,6 +137,10 @@ llvm::AllocaInst *CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, Alloca = new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(), ArraySize, Name, AllocaInsertPt->getIterator()); + if (Alloca->getName() != Name.str() && efriedma-quic wrote: I don't see the point of the `Alloca->getName() != Name.str()` check; it will almost always fail. (In release builds of clang, we suppress instruction names, so `Alloca->getName()` is just the empty string.) https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 1/7] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c5..81bb3f3c5a5e35 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..c696cc38167cd4 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 2/7] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd4..2051fa94678175 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 3/7] Now only emit metadata when using a ASan, and tag it with
an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -53,3 +53,4 @@ LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38) LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39) LLVM_FIXED_MD_KIND(MD_mmra, "mmra", 40) LLVM_FIXED_MD_KIND(MD_noalias_addrspace, "noalias.addrspace", 41) +LLVM_FIXED_MD_KIND(MD_unaltered_name, "unaltered.name", 42) gbMattN wrote: Aha! I hadn't noticed that metadata type. I've switched to using it https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3430,13 +3430,15 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
- ASan.getAllocaSizeInBytes(*AI),
- 0,
- AI->getAlign().value(),
- AI,
- 0,
- 0};
+const char *Name = AI->getName().data();
gbMattN wrote:
It is!
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 1/6] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c5..81bb3f3c5a5e35 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..c696cc38167cd4 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 2/6] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd4..2051fa94678175 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 3/6] Now only emit metadata when using a ASan, and tag it with
an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
gbMattN wrote: @vitalybuka manually pinging you for a last review since you were active on the [bugzilla ticket](https://bugs.llvm.org/show_bug.cgi?id=47982) https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3430,13 +3430,15 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
- ASan.getAllocaSizeInBytes(*AI),
- 0,
- AI->getAlign().value(),
- AI,
- 0,
- 0};
+const char *Name = AI->getName().data();
+if (AI->hasMetadata(LLVMContext::MD_unaltered_name)) {
+ MDTuple *tuple =
fhahn wrote:
```suggestion
MDTuple *Tuple =
```
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3430,13 +3430,15 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
- ASan.getAllocaSizeInBytes(*AI),
- 0,
- AI->getAlign().value(),
- AI,
- 0,
- 0};
+const char *Name = AI->getName().data();
fhahn wrote:
Is is possible to use StringRef here?
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -53,3 +53,4 @@ LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38) LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39) LLVM_FIXED_MD_KIND(MD_mmra, "mmra", 40) LLVM_FIXED_MD_KIND(MD_noalias_addrspace, "noalias.addrspace", 41) +LLVM_FIXED_MD_KIND(MD_unaltered_name, "unaltered.name", 42) fhahn wrote: Not sure if it is worth adding a new metadata kind to work around not using debug info? If we need new metadata, it should be documented and also needs to be checked by the verifier, but perhaps it would be possible to use https://llvm.org/docs/LangRef.html#annotation-metadata instead? https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3430,13 +3430,15 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
- ASan.getAllocaSizeInBytes(*AI),
- 0,
- AI->getAlign().value(),
- AI,
- 0,
- 0};
+const char *Name = AI->getName().data();
+if (AI->hasMetadata(LLVMContext::MD_unaltered_name)) {
+ MDTuple *tuple =
+ dyn_cast(AI->getMetadata(LLVMContext::MD_unaltered_name));
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
fhahn wrote:
Should those be casts? If the dyn_cast can return nullptr, you need to check
the return value
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 1/5] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c5..81bb3f3c5a5e35 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..c696cc38167cd4 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 2/5] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd4..2051fa94678175 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 3/5] Now only emit metadata when using a ASan, and tag it with
an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 1/5] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c5..81bb3f3c5a5e35 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..c696cc38167cd4 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 2/5] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd4..2051fa94678175 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 3/5] Now only emit metadata when using a ASan, and tag it with
an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3430,13 +3430,15 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
- ASan.getAllocaSizeInBytes(*AI),
- 0,
- AI->getAlign().value(),
- AI,
- 0,
- 0};
+const char *Name = AI->getName().data();
+if (AI->hasMetadata(LLVMContext::MD_unaltered_name)) {
+ MDTuple *tuple =
gbMattN wrote:
We only add this metadata in one place, and expect it to have a MDString in it.
If dyn_cast fails and we try doing this to a `nullptr`, LLVM will exit with a
stack trace and request for a bug report, which I think is what we'd want to
happen
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
@@ -3430,13 +3430,15 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
- ASan.getAllocaSizeInBytes(*AI),
- 0,
- AI->getAlign().value(),
- AI,
- 0,
- 0};
+const char *Name = AI->getName().data();
+if (AI->hasMetadata(LLVMContext::MD_unaltered_name)) {
+ MDTuple *tuple =
goussepi wrote:
Do we need to check the result of those dyn_cast are non null ? Otherwise LGTM!
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
llvmbot wrote:
@llvm/pr-subscribers-compiler-rt-sanitizer
Author: None (gbMattN)
Changes
…ncorrect name
---
Full diff: https://github.com/llvm/llvm-project/pull/119387.diff
4 Files Affected:
- (modified) clang/lib/CodeGen/CGExpr.cpp (+8)
- (added) compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
(+13)
- (modified) llvm/include/llvm/IR/FixedMetadataKinds.def (+1)
- (modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+9-7)
``diff
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+Alloca->setMetadata(llvm::LLVMContext::MD_unaltered_name, tuple);
+ }
if (Allocas) {
Allocas->Add(Alloca);
}
diff --git a/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
b/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
new file mode 100644
index 00..f4d9ad5f6ea5f7
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
@@ -0,0 +1,13 @@
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+int main()
+{
+ int x;
+ {
+ int x;
+ delete &x;
+ }
+}
+
+// CHECK: [32, 36) 'x'
diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def
b/llvm/include/llvm/IR/FixedMetadataKinds.def
index df572e8791e13b..41fa34bf09ff65 100644
--- a/llvm/include/llvm/IR/FixedMetadataKinds.def
+++ b/llvm/include/llvm/IR/FixedMetadataKinds.def
@@ -53,3 +53,4 @@ LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38)
LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39)
LLVM_FIXED_MD_KIND(MD_mmra, "mmra", 40)
LLVM_FIXED_MD_KIND(MD_noalias_addrspace, "noalias.addrspace", 41)
+LLVM_FIXED_MD_KIND(MD_unaltered_name, "unaltered.name", 42)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..87f79bdaa16429 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,13 +3430,15 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
- ASan.getAllocaSizeInBytes(*AI),
- 0,
- AI->getAlign().value(),
- AI,
- 0,
- 0};
+const char *Name = AI->getName().data();
+if (AI->hasMetadata(LLVMContext::MD_unaltered_name)) {
+ MDTuple *tuple =
+ dyn_cast(AI->getMetadata(LLVMContext::MD_unaltered_name));
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
+}
+ASanStackVariableDescription D = {
+Name, ASan.getAllocaSizeInBytes(*AI), 0, AI->getAlign().value(), AI, 0,
+0};
SVD.push_back(D);
}
``
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
llvmbot wrote:
@llvm/pr-subscribers-clang
Author: None (gbMattN)
Changes
…ncorrect name
---
Full diff: https://github.com/llvm/llvm-project/pull/119387.diff
4 Files Affected:
- (modified) clang/lib/CodeGen/CGExpr.cpp (+8)
- (added) compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
(+13)
- (modified) llvm/include/llvm/IR/FixedMetadataKinds.def (+1)
- (modified) llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp (+9-7)
``diff
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+Alloca->setMetadata(llvm::LLVMContext::MD_unaltered_name, tuple);
+ }
if (Allocas) {
Allocas->Add(Alloca);
}
diff --git a/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
b/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
new file mode 100644
index 00..f4d9ad5f6ea5f7
--- /dev/null
+++ b/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp
@@ -0,0 +1,13 @@
+// RUN: %clangxx_asan -O0 %s -o %t
+// RUN: not %run %t 2>&1 | FileCheck %s
+
+int main()
+{
+ int x;
+ {
+ int x;
+ delete &x;
+ }
+}
+
+// CHECK: [32, 36) 'x'
diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def
b/llvm/include/llvm/IR/FixedMetadataKinds.def
index df572e8791e13b..41fa34bf09ff65 100644
--- a/llvm/include/llvm/IR/FixedMetadataKinds.def
+++ b/llvm/include/llvm/IR/FixedMetadataKinds.def
@@ -53,3 +53,4 @@ LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38)
LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39)
LLVM_FIXED_MD_KIND(MD_mmra, "mmra", 40)
LLVM_FIXED_MD_KIND(MD_noalias_addrspace, "noalias.addrspace", 41)
+LLVM_FIXED_MD_KIND(MD_unaltered_name, "unaltered.name", 42)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..87f79bdaa16429 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,13 +3430,15 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
- ASan.getAllocaSizeInBytes(*AI),
- 0,
- AI->getAlign().value(),
- AI,
- 0,
- 0};
+const char *Name = AI->getName().data();
+if (AI->hasMetadata(LLVMContext::MD_unaltered_name)) {
+ MDTuple *tuple =
+ dyn_cast(AI->getMetadata(LLVMContext::MD_unaltered_name));
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
+}
+ASanStackVariableDescription D = {
+Name, ASan.getAllocaSizeInBytes(*AI), 0, AI->getAlign().value(), AI, 0,
+0};
SVD.push_back(D);
}
``
https://github.com/llvm/llvm-project/pull/119387
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN ready_for_review https://github.com/llvm/llvm-project/pull/119387 ___ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [llvm] [ASan] Add metadata to renamed instructions so ASan doesn't use the i… (PR #119387)
https://github.com/gbMattN updated
https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 10 Dec 2024 15:01:37 +
Subject: [PATCH 1/4] [ASan] Add metadata to renamed instructions so ASan
doesn't use the incorrect name
---
llvm/lib/IR/ValueSymbolTable.cpp | 8
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 7 ++-
2 files changed, 14 insertions(+), 1 deletion(-)
diff --git a/llvm/lib/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp
index a020acf22a96c5..81bb3f3c5a5e35 100644
--- a/llvm/lib/IR/ValueSymbolTable.cpp
+++ b/llvm/lib/IR/ValueSymbolTable.cpp
@@ -123,6 +123,14 @@ ValueName *ValueSymbolTable::createValueName(StringRef
Name, Value *V) {
}
// Otherwise, there is a naming conflict. Rename this value.
+ // If we are renaming an instruction, ASan needs to know for it to serialize
+ // properly
+ if (auto *I = dyn_cast(V)) {
+MDString *trueNameMetadata = MDString::get(V->getContext(), Name);
+llvm::MDTuple *tuple =
+llvm::MDTuple::get(V->getContext(), trueNameMetadata);
+I->setMetadata("OriginalName", tuple);
+ }
SmallString<256> UniqueName(Name.begin(), Name.end());
return makeUniqueName(V, UniqueName);
}
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index cb84588318496c..c696cc38167cd4 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,7 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-ASanStackVariableDescription D = {AI->getName().data(),
+std::string Name = AI->getName().data();
+if (AI->hasMetadata("OriginalName")) {
+ MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
+ Name = dyn_cast(tuple->getOperand(0))->getString();
+}
+ASanStackVariableDescription D = {Name.c_str(),
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From 25efafa3d67afb6a9107fdd502f5f6e4f40c311c Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Wed, 11 Dec 2024 11:44:01 +
Subject: [PATCH 2/4] [bugfix] Fixed string pointer being used out of scope
---
llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
index c696cc38167cd4..2051fa94678175 100644
--- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp
@@ -3430,12 +3430,12 @@ void FunctionStackPoisoner::processStaticAllocas() {
SmallVector SVD;
SVD.reserve(AllocaVec.size());
for (AllocaInst *AI : AllocaVec) {
-std::string Name = AI->getName().data();
+const char* Name = AI->getName().data();
if (AI->hasMetadata("OriginalName")) {
MDTuple *tuple = dyn_cast(AI->getMetadata("OriginalName"));
- Name = dyn_cast(tuple->getOperand(0))->getString();
+ Name = dyn_cast(tuple->getOperand(0))->getString().data();
}
-ASanStackVariableDescription D = {Name.c_str(),
+ASanStackVariableDescription D = {Name,
ASan.getAllocaSizeInBytes(*AI),
0,
AI->getAlign().value(),
>From c15ec2fe9bbe4855272a519b7f6dccca57a294eb Mon Sep 17 00:00:00 2001
From: gbMattN
Date: Tue, 17 Dec 2024 16:47:11 +
Subject: [PATCH 3/4] Now only emit metadata when using a ASan, and tag it with
an enum rather than a string
---
clang/lib/CodeGen/CGExpr.cpp| 8
llvm/include/llvm/IR/FixedMetadataKinds.def | 1 +
llvm/lib/IR/ValueSymbolTable.cpp| 8
.../Instrumentation/AddressSanitizer.cpp| 17 +++--
4 files changed, 16 insertions(+), 18 deletions(-)
diff --git a/clang/lib/CodeGen/CGExpr.cpp b/clang/lib/CodeGen/CGExpr.cpp
index 5fccc9cbb37ec1..d8fdacf30e12e3 100644
--- a/clang/lib/CodeGen/CGExpr.cpp
+++ b/clang/lib/CodeGen/CGExpr.cpp
@@ -137,6 +137,14 @@ llvm::AllocaInst
*CodeGenFunction::CreateTempAlloca(llvm::Type *Ty,
Alloca =
new llvm::AllocaInst(Ty, CGM.getDataLayout().getAllocaAddrSpace(),
ArraySize, Name, AllocaInsertPt->getIterator());
+ if (Alloca->getName() != Name.str() &&
+ SanOpts.Mask & SanitizerKind::Address) {
+
+llvm::LLVMContext &ctx = Alloca->getContext();
+llvm::MDString *trueNameMetadata = llvm::MDString::get(ctx, Name.str());
+llvm::MDTuple *tuple = llvm::MDTuple::get(ctx, trueNameMetadata);
+
