https://github.com/pdschbrt updated https://github.com/llvm/llvm-project/pull/226942
>From 4d3952b9313cec51867de92f32b354ab5c8f6eec Mon Sep 17 00:00:00 2001 From: Philipp Schubert <[email protected]> Date: Mon, 28 Sep 2026 12:04:53 +0200 Subject: [PATCH 1/2] [clang][CodeGen] Fix stack-use-after-return in deferred annotations CodeGenModule::DeferredAnnotations was keyed by StringRef, but not every mangled name passed to GetOrCreateLLVMFunction outlives the call. CodeGenVTables::maybeEmitThunk mangles the thunk name into a stack-local SmallString and hands it to GetAddrOfThunk, so for an annotated virtual function the map retained a reference into a frame that was gone by the time EmitGlobalAnnotations looked the key up. ASan reports this as a stack-use-after-return in EmitGlobalAnnotations, with the freed frame being maybeEmitThunk's 'Name'. The user-visible effect is that annotations silently disappear from the this-adjusting thunks once the dead frame has been reused. Make the key own its storage, using StringMap<unsigned> as the MapVector map type so lookups still hash a StringRef without allocating. --- clang/docs/ReleaseNotes.md | 4 ++ clang/lib/CodeGen/CodeGenModule.cpp | 4 +- clang/lib/CodeGen/CodeGenModule.h | 5 ++- clang/test/CodeGenCXX/attr-annotate-thunk.cpp | 40 +++++++++++++++++++ 4 files changed, 50 insertions(+), 3 deletions(-) create mode 100644 clang/test/CodeGenCXX/attr-annotate-thunk.cpp diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index 3c6acf353f93f..adc941e287607 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -802,6 +802,10 @@ features cannot lower the translation-unit ABI level; - Fixed an assertion failure when a method or function definition follows an Objective-C `@implementation` that was ended by a nested `@interface`, `@protocol` or `@implementation` before its `@end`. (#GH209503) +- Fixed `annotate` attributes being dropped from the `this`-adjusting thunks of + annotated virtual functions. The deferred-annotation map kept a `StringRef` + into the caller's stack buffer, so the lookup at the end of the translation + unit read a dangling key. ### OpenACC Specific Changes diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 7274a8588670f..277764fa300c3 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -4863,7 +4863,7 @@ void CodeGenModule::EmitGlobal(GlobalDecl GD) { if (FD->hasAttr<AnnotateAttr>()) { StringRef MangledName = getMangledName(GD); if (GetGlobalValue(MangledName)) - DeferredAnnotations[MangledName] = FD; + DeferredAnnotations[MangledName.str()] = FD; } // Forward declarations are emitted lazily on first use. @@ -5784,7 +5784,7 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction( // Store the declaration associated with this function so it is potentially // updated by further declarations or definitions and emitted at the end. if (D && D->hasAttr<AnnotateAttr>()) - DeferredAnnotations[MangledName] = cast<ValueDecl>(D); + DeferredAnnotations[MangledName.str()] = cast<ValueDecl>(D); // If we already created a function with the same mangled name (but different // type) before, take its name and add it to the list of functions to be diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 9e3f073c20f4a..28e9bcf9e866b 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -506,7 +506,10 @@ class CodeGenModule : public CodeGenTypeCache { // Store deferred function annotations so they can be emitted at the end with // most up to date ValueDecl that will have all the inherited annotations. - llvm::MapVector<StringRef, const ValueDecl *> DeferredAnnotations; + // The key owns its storage: not every mangled name handed to + // GetOrCreateLLVMFunction outlives the call. + llvm::MapVector<std::string, const ValueDecl *, llvm::StringMap<unsigned>> + DeferredAnnotations; /// Map used to get unique annotation strings. llvm::StringMap<llvm::Constant*> AnnotationStrings; diff --git a/clang/test/CodeGenCXX/attr-annotate-thunk.cpp b/clang/test/CodeGenCXX/attr-annotate-thunk.cpp new file mode 100644 index 0000000000000..5aa57877c1719 --- /dev/null +++ b/clang/test/CodeGenCXX/attr-annotate-thunk.cpp @@ -0,0 +1,40 @@ +// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-unknown-linux-gnu -o - | FileCheck %s + +// Annotations on a virtual function are deferred to the end of the TU, keyed +// by mangled name. For a this-adjusting thunk that name is mangled into a +// stack buffer in CodeGenVTables::maybeEmitThunk, so the deferred-annotation +// map must own a copy of the key instead of referencing the caller's storage. + +struct A { + virtual void f(); + virtual ~A(); +}; + +struct B { + virtual void g(); + virtual ~B(); +}; + +struct C : A, B { + void f() override; + __attribute__((annotate("annotated_method"))) void g() override; + __attribute__((annotate("annotated_dtor"))) ~C() override; +}; + +void C::f() {} +void C::g() {} +C::~C() {} + +// Each annotation is recorded for the function itself and for the thunk that +// adjusts `this` to the B subobject. + +// CHECK: @[[METHOD:.*]] = private unnamed_addr constant [17 x i8] c"annotated_method\00", section "llvm.metadata" +// CHECK: @[[DTOR:.*]] = private unnamed_addr constant [15 x i8] c"annotated_dtor\00", section "llvm.metadata" +// CHECK: @llvm.global.annotations = appending global [7 x { ptr, ptr, ptr, i32, ptr }] [ +// CHECK-SAME: { ptr @_ZN1C1gEv, ptr @[[METHOD]], +// CHECK-SAME: { ptr @_ZThn8_N1C1gEv, ptr @[[METHOD]], +// CHECK-SAME: { ptr @_ZN1CD2Ev, ptr @[[DTOR]], +// CHECK-SAME: { ptr @_ZN1CD1Ev, ptr @[[DTOR]], +// CHECK-SAME: { ptr @_ZThn8_N1CD1Ev, ptr @[[DTOR]], +// CHECK-SAME: { ptr @_ZN1CD0Ev, ptr @[[DTOR]], +// CHECK-SAME: { ptr @_ZThn8_N1CD0Ev, ptr @[[DTOR]], >From 8932c28eaa9a138b98e6f5e8d501d8ff206128a9 Mon Sep 17 00:00:00 2001 From: Philipp Schubert <[email protected]> Date: Wed, 30 Sep 2026 08:55:24 +0200 Subject: [PATCH 2/2] Address reviewer comments --- clang/docs/ReleaseNotes.md | 5 +-- .../CodeGenCXX/attr-annotate-constructor.cpp | 10 ----- .../CodeGenCXX/attr-annotate-destructor.cpp | 10 ----- ...cpp => attr-annotate-member-functions.cpp} | 37 ++++++++++++++++--- 4 files changed, 32 insertions(+), 30 deletions(-) delete mode 100644 clang/test/CodeGenCXX/attr-annotate-constructor.cpp delete mode 100644 clang/test/CodeGenCXX/attr-annotate-destructor.cpp rename clang/test/CodeGenCXX/{attr-annotate-thunk.cpp => attr-annotate-member-functions.cpp} (52%) diff --git a/clang/docs/ReleaseNotes.md b/clang/docs/ReleaseNotes.md index adc941e287607..c9fcbf41ae9b5 100644 --- a/clang/docs/ReleaseNotes.md +++ b/clang/docs/ReleaseNotes.md @@ -802,10 +802,7 @@ features cannot lower the translation-unit ABI level; - Fixed an assertion failure when a method or function definition follows an Objective-C `@implementation` that was ended by a nested `@interface`, `@protocol` or `@implementation` before its `@end`. (#GH209503) -- Fixed `annotate` attributes being dropped from the `this`-adjusting thunks of - annotated virtual functions. The deferred-annotation map kept a `StringRef` - into the caller's stack buffer, so the lookup at the end of the translation - unit read a dangling key. +- Fixed use-after-free with annotate attribute on a C++ class method with a this-adjusting thunk. ### OpenACC Specific Changes diff --git a/clang/test/CodeGenCXX/attr-annotate-constructor.cpp b/clang/test/CodeGenCXX/attr-annotate-constructor.cpp deleted file mode 100644 index 7a115137f1a67..0000000000000 --- a/clang/test/CodeGenCXX/attr-annotate-constructor.cpp +++ /dev/null @@ -1,10 +0,0 @@ -// RUN: %clang %s -S -emit-llvm -target x86_64-unknown-linux -o - - -// Test annotation attributes on constructors do not crash. - -class Foo { -public: - [[clang::annotate("test")]] Foo() {} -}; - -Foo foo; diff --git a/clang/test/CodeGenCXX/attr-annotate-destructor.cpp b/clang/test/CodeGenCXX/attr-annotate-destructor.cpp deleted file mode 100644 index 4e5a2190a4585..0000000000000 --- a/clang/test/CodeGenCXX/attr-annotate-destructor.cpp +++ /dev/null @@ -1,10 +0,0 @@ -// RUN: %clang_cc1 %s -emit-llvm -triple x86_64-unknown-linux-gnu -o - | FileCheck %s - -// Test annotation attributes on destructors do not crash. - -struct k { - ~k() __attribute__((annotate(""))) {} -}; -void m() { k(); } - -// CHECK: @llvm.global.annotations = appending global [2 x { ptr, ptr, ptr, i32, ptr }] [{ diff --git a/clang/test/CodeGenCXX/attr-annotate-thunk.cpp b/clang/test/CodeGenCXX/attr-annotate-member-functions.cpp similarity index 52% rename from clang/test/CodeGenCXX/attr-annotate-thunk.cpp rename to clang/test/CodeGenCXX/attr-annotate-member-functions.cpp index 5aa57877c1719..9d9490e8a74ce 100644 --- a/clang/test/CodeGenCXX/attr-annotate-thunk.cpp +++ b/clang/test/CodeGenCXX/attr-annotate-member-functions.cpp @@ -1,9 +1,31 @@ // RUN: %clang_cc1 %s -emit-llvm -triple x86_64-unknown-linux-gnu -o - | FileCheck %s +// Test annotation attributes on C++ constructors, destructors and virtual +// member functions. + +// Annotations on a constructor do not crash. + +class Foo { +public: + [[clang::annotate("test")]] Foo() {} +}; + +Foo foo; + +// Annotations on a destructor do not crash. + +struct k { + ~k() __attribute__((annotate(""))) {} +}; + +void m() { k(); } + // Annotations on a virtual function are deferred to the end of the TU, keyed // by mangled name. For a this-adjusting thunk that name is mangled into a // stack buffer in CodeGenVTables::maybeEmitThunk, so the deferred-annotation // map must own a copy of the key instead of referencing the caller's storage. +// Each annotation is recorded for the function itself and for the thunk that +// adjusts `this` to the B subobject. struct A { virtual void f(); @@ -25,12 +47,13 @@ void C::f() {} void C::g() {} C::~C() {} -// Each annotation is recorded for the function itself and for the thunk that -// adjusts `this` to the B subobject. - -// CHECK: @[[METHOD:.*]] = private unnamed_addr constant [17 x i8] c"annotated_method\00", section "llvm.metadata" -// CHECK: @[[DTOR:.*]] = private unnamed_addr constant [15 x i8] c"annotated_dtor\00", section "llvm.metadata" -// CHECK: @llvm.global.annotations = appending global [7 x { ptr, ptr, ptr, i32, ptr }] [ +// CHECK: @[[TEST:[.a-z0-9_]+]] = private unnamed_addr constant [5 x i8] c"test\00", section "llvm.metadata" +// CHECK: @[[EMPTY:[.a-z0-9_]+]] = private unnamed_addr constant [1 x i8] zeroinitializer, section "llvm.metadata" +// CHECK: @[[METHOD:[.a-z0-9_]+]] = private unnamed_addr constant [17 x i8] c"annotated_method\00", section "llvm.metadata" +// CHECK: @[[DTOR:[.a-z0-9_]+]] = private unnamed_addr constant [15 x i8] c"annotated_dtor\00", section "llvm.metadata" +// CHECK: @llvm.global.annotations = appending global [11 x { ptr, ptr, ptr, i32, ptr }] [ +// CHECK-SAME: { ptr @_ZN3FooC1Ev, ptr @[[TEST]], +// CHECK-SAME: { ptr @_ZN1kD1Ev, ptr @[[EMPTY]], // CHECK-SAME: { ptr @_ZN1C1gEv, ptr @[[METHOD]], // CHECK-SAME: { ptr @_ZThn8_N1C1gEv, ptr @[[METHOD]], // CHECK-SAME: { ptr @_ZN1CD2Ev, ptr @[[DTOR]], @@ -38,3 +61,5 @@ C::~C() {} // CHECK-SAME: { ptr @_ZThn8_N1CD1Ev, ptr @[[DTOR]], // CHECK-SAME: { ptr @_ZN1CD0Ev, ptr @[[DTOR]], // CHECK-SAME: { ptr @_ZThn8_N1CD0Ev, ptr @[[DTOR]], +// CHECK-SAME: { ptr @_ZN3FooC2Ev, ptr @[[TEST]], +// CHECK-SAME: { ptr @_ZN1kD2Ev, ptr @[[EMPTY]], _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
