https://github.com/gbMattN updated https://github.com/llvm/llvm-project/pull/119387
>From 8781ff2355750ae61d140620b1f6862537de07e3 Mon Sep 17 00:00:00 2001 From: gbMattN <matthew.n...@sony.com> Date: Tue, 10 Dec 2024 15:01:37 +0000 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<Instruction>(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<ASanStackVariableDescription, 16> 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<MDTuple>(AI->getMetadata("OriginalName")); + Name = dyn_cast<MDString>(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 <matthew.n...@sony.com> Date: Wed, 11 Dec 2024 11:44:01 +0000 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<ASanStackVariableDescription, 16> 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<MDTuple>(AI->getMetadata("OriginalName")); - Name = dyn_cast<MDString>(tuple->getOperand(0))->getString(); + Name = dyn_cast<MDString>(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 <matthew.n...@sony.com> Date: Tue, 17 Dec 2024 16:47:11 +0000 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); + Alloca->setMetadata(llvm::LLVMContext::MD_unaltered_name, tuple); + } if (Allocas) { Allocas->Add(Alloca); } 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/IR/ValueSymbolTable.cpp b/llvm/lib/IR/ValueSymbolTable.cpp index 81bb3f3c5a5e35..a020acf22a96c5 100644 --- a/llvm/lib/IR/ValueSymbolTable.cpp +++ b/llvm/lib/IR/ValueSymbolTable.cpp @@ -123,14 +123,6 @@ 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<Instruction>(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 2051fa94678175..87f79bdaa16429 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -3430,18 +3430,15 @@ void FunctionStackPoisoner::processStaticAllocas() { SmallVector<ASanStackVariableDescription, 16> SVD; SVD.reserve(AllocaVec.size()); for (AllocaInst *AI : AllocaVec) { - const char* Name = AI->getName().data(); - if (AI->hasMetadata("OriginalName")) { - MDTuple *tuple = dyn_cast<MDTuple>(AI->getMetadata("OriginalName")); + const char *Name = AI->getName().data(); + if (AI->hasMetadata(LLVMContext::MD_unaltered_name)) { + MDTuple *tuple = + dyn_cast<MDTuple>(AI->getMetadata(LLVMContext::MD_unaltered_name)); Name = dyn_cast<MDString>(tuple->getOperand(0))->getString().data(); } - ASanStackVariableDescription D = {Name, - ASan.getAllocaSizeInBytes(*AI), - 0, - AI->getAlign().value(), - AI, - 0, - 0}; + ASanStackVariableDescription D = { + Name, ASan.getAllocaSizeInBytes(*AI), 0, AI->getAlign().value(), AI, 0, + 0}; SVD.push_back(D); } >From 38f210b3b25ff05c8c61a96a564fa8a83c7a03ea Mon Sep 17 00:00:00 2001 From: gbMattN <matthew.n...@sony.com> Date: Wed, 18 Dec 2024 11:17:44 +0000 Subject: [PATCH 4/5] Added a test --- .../asan/TestCases/shadowed-stack-serialization.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp 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 00000000000000..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' >From 3a5c89ac2b6aa98d1a090a54605191263e6a0c71 Mon Sep 17 00:00:00 2001 From: gbMattN <matthew.n...@sony.com> Date: Tue, 7 Jan 2025 13:34:51 +0000 Subject: [PATCH 5/5] fixup --- .../asan/TestCases/shadowed-stack-serialization.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp b/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp index f4d9ad5f6ea5f7..f2706c671c2612 100644 --- a/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp +++ b/compiler-rt/test/asan/TestCases/shadowed-stack-serialization.cpp @@ -1,13 +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; - } +int main() { + int x; + { + int x; + delete &x; + } } // CHECK: [32, 36) 'x' _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits