From ece2a2ba47d043f5225fd408c66749d8bd67c80c Mon Sep 17 00:00:00 2001 From: David Justo Date: Thu, 18 Sep 2025 10:17:12 -0700 Subject: [PATCH 1/4] do not asan-instrument catch parameters on windows --- .../Instrumentation/AddressSanitizer.cpp | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index 42c3d4a4f4c46..986d3c2861af0 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -1397,6 +1397,16 @@ void AddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI, MI->eraseFromParent(); } +// Check if an alloca is a catch block parameter +static bool isCatchParameter(const AllocaInst &AI) { + for (const Use &U : AI.uses()) { + if (isa(U.getUser())) { + return true; + } + } + return false; +} + /// Check if we want (and can) handle this alloca. bool AddressSanitizer::isInterestingAlloca(const AllocaInst &AI) { auto [It, Inserted] = ProcessedAllocas.try_emplace(&AI); @@ -1417,7 +1427,11 @@ bool AddressSanitizer::isInterestingAlloca(const AllocaInst &AI) { // swifterror allocas are register promoted by ISel !AI.isSwiftError() && // safe allocas are not interesting - !(SSGI && SSGI->isSafe(AI))); + !(SSGI && SSGI->isSafe(AI)) && + // Mitigation for https://github.com/google/sanitizers/issues/749 + // We don't instrument Windows catch-block parameters to avoid + // interfering with exception handling assumptions. + !(TargetTriple.isOSWindows() && isCatchParameter(AI))); It->second = IsInteresting; return IsInteresting; From bff409da82c600d5b07915b81f7ec51f99e7c276 Mon Sep 17 00:00:00 2001 From: David Justo Date: Thu, 18 Sep 2025 10:25:18 -0700 Subject: [PATCH 2/4] add basic unit test: catches exception 'inline' and in another frame --- .../Windows/basic_exception_handling.cpp | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 compiler-rt/test/asan/TestCases/Windows/basic_exception_handling.cpp diff --git a/compiler-rt/test/asan/TestCases/Windows/basic_exception_handling.cpp b/compiler-rt/test/asan/TestCases/Windows/basic_exception_handling.cpp new file mode 100644 index 0000000000000..94ca4b9bf2df0 --- /dev/null +++ b/compiler-rt/test/asan/TestCases/Windows/basic_exception_handling.cpp @@ -0,0 +1,36 @@ +// RUN: %clangxx_asan %s -o %t +// RUN: %run %t | FileCheck %s + +// This test tests that declaring a parameter in a catch-block does not produce a false positive +// ASan error on Windows. + +// This code is based on the repro in https://github.com/google/sanitizers/issues/749 +#include +#include + +void throwInFunction(){ + throw std::exception("test2"); +} + +int main() +{ + // case 1: direct throw + try { + throw std::exception("test1"); + } catch (const std::exception& ex){ + puts(ex.what()); + // CHECK: test1 + } + + // case 2: throw in function + try { + throwInFunction(); + } catch (const std::exception& ex){ + puts(ex.what()); + // CHECK: test2 + } + + printf("Success!\n"); + // CHECK: Success! + return 0; +} \ No newline at end of file From 043848d2fecc459b5d250298c20cb115d2f1c4ef Mon Sep 17 00:00:00 2001 From: David Justo Date: Thu, 18 Sep 2025 11:08:07 -0700 Subject: [PATCH 3/4] apply clang-format on `compiler-rt/test/asan/TestCases/Windows/basic_exception_handling.cpp` --- .../Windows/basic_exception_handling.cpp | 41 +++++++++---------- 1 file changed, 19 insertions(+), 22 deletions(-) diff --git a/compiler-rt/test/asan/TestCases/Windows/basic_exception_handling.cpp b/compiler-rt/test/asan/TestCases/Windows/basic_exception_handling.cpp index 94ca4b9bf2df0..f8dd49e64c760 100644 --- a/compiler-rt/test/asan/TestCases/Windows/basic_exception_handling.cpp +++ b/compiler-rt/test/asan/TestCases/Windows/basic_exception_handling.cpp @@ -8,29 +8,26 @@ #include #include -void throwInFunction(){ - throw std::exception("test2"); -} +void throwInFunction() { throw std::exception("test2"); } -int main() -{ - // case 1: direct throw - try { - throw std::exception("test1"); - } catch (const std::exception& ex){ - puts(ex.what()); - // CHECK: test1 - } +int main() { + // case 1: direct throw + try { + throw std::exception("test1"); + } catch (const std::exception &ex) { + puts(ex.what()); + // CHECK: test1 + } - // case 2: throw in function - try { - throwInFunction(); - } catch (const std::exception& ex){ - puts(ex.what()); - // CHECK: test2 - } + // case 2: throw in function + try { + throwInFunction(); + } catch (const std::exception &ex) { + puts(ex.what()); + // CHECK: test2 + } - printf("Success!\n"); - // CHECK: Success! - return 0; + printf("Success!\n"); + // CHECK: Success! + return 0; } \ No newline at end of file From 2326be0809f140351ae1f740a18d9214ca878019 Mon Sep 17 00:00:00 2001 From: David Justo Date: Mon, 22 Sep 2025 20:22:14 -0700 Subject: [PATCH 4/4] optimization: disable catch-parameter instrumentation via a linear pass over function basic blocks --- .../Instrumentation/AddressSanitizer.cpp | 38 +++++++++++-------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp index 986d3c2861af0..598fa8e9421c9 100644 --- a/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/AddressSanitizer.cpp @@ -847,6 +847,7 @@ struct AddressSanitizer { bool maybeInsertAsanInitAtFunctionEntry(Function &F); bool maybeInsertDynamicShadowAtFunctionEntry(Function &F); void markEscapedLocalAllocas(Function &F); + void markCatchParametersAsUninteresting(Function &F); private: friend struct FunctionStackPoisoner; @@ -1397,16 +1398,6 @@ void AddressSanitizer::instrumentMemIntrinsic(MemIntrinsic *MI, MI->eraseFromParent(); } -// Check if an alloca is a catch block parameter -static bool isCatchParameter(const AllocaInst &AI) { - for (const Use &U : AI.uses()) { - if (isa(U.getUser())) { - return true; - } - } - return false; -} - /// Check if we want (and can) handle this alloca. bool AddressSanitizer::isInterestingAlloca(const AllocaInst &AI) { auto [It, Inserted] = ProcessedAllocas.try_emplace(&AI); @@ -1427,11 +1418,7 @@ bool AddressSanitizer::isInterestingAlloca(const AllocaInst &AI) { // swifterror allocas are register promoted by ISel !AI.isSwiftError() && // safe allocas are not interesting - !(SSGI && SSGI->isSafe(AI)) && - // Mitigation for https://github.com/google/sanitizers/issues/749 - // We don't instrument Windows catch-block parameters to avoid - // interfering with exception handling assumptions. - !(TargetTriple.isOSWindows() && isCatchParameter(AI))); + !(SSGI && SSGI->isSafe(AI))); It->second = IsInteresting; return IsInteresting; @@ -2989,6 +2976,24 @@ void AddressSanitizer::markEscapedLocalAllocas(Function &F) { } } } +// Mitigation for https://github.com/google/sanitizers/issues/749 +// We don't instrument Windows catch-block parameters to avoid +// interfering with exception handling assumptions. +void AddressSanitizer::markCatchParametersAsUninteresting(Function &F) { + for (BasicBlock &BB : F) { + for (Instruction &I : BB) { + if (auto *CatchPad = dyn_cast(&I)) { + // Mark the parameters to a catch-block as uninteresting to avoid + // instrumenting them + for (Value *Operand : CatchPad->arg_operands()) { + if (auto *AI = dyn_cast(Operand)) { + ProcessedAllocas[AI] = false; + } + } + } + } + } +} bool AddressSanitizer::suppressInstrumentationSiteForDebug(int &Instrumented) { bool ShouldInstrument = @@ -3032,6 +3037,9 @@ bool AddressSanitizer::instrumentFunction(Function &F, // can be passed to that intrinsic. markEscapedLocalAllocas(F); + if (TargetTriple.isOSWindows()) + markCatchParametersAsUninteresting(F); + // We want to instrument every address only once per basic block (unless there // are calls between uses). SmallPtrSet TempsToInstrument;