From 1e6f25c517d8d1adeeaf59f826141efdcad8f05a Mon Sep 17 00:00:00 2001 From: Kerry McLaughlin Date: Fri, 6 Sep 2024 10:13:33 +0000 Subject: [PATCH 1/6] [Clang] Emit error for duplicate mangled names within a lambda When functions are passed as arguments to a lambda, it's possible for the mangled names of these functions to be the same despite the prototypes being different. For example: int non_streaming_fn(int); int streaming_fn(int) __arm_streaming; auto lambda_fn = [](const auto &f) { return f(); }; return lambda_fn(non_streaming_fn) + lambda_fn(streaming_fn); Only one function will be generated for the lambda above and the __arm_streaming attribute from streaming_fn will be incorrectly applied to non_streaming_fn. With this change, an error will be emitted when a duplicate mangled name is found but the function prototypes do not match. --- clang/lib/CodeGen/CodeGenModule.cpp | 35 ++++++++++++++----- .../aarch64-sme-lambda-attributes.cpp | 28 +++++++++++++++ 2 files changed, 54 insertions(+), 9 deletions(-) create mode 100644 clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index df4c13c9ad97aa..b07090e3de6dfd 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -4638,8 +4638,8 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction( // Lookup the entry, lazily creating it if necessary. llvm::GlobalValue *Entry = GetGlobalValue(MangledName); if (Entry) { + const FunctionDecl *FD = cast_or_null(D); if (WeakRefReferences.erase(Entry)) { - const FunctionDecl *FD = cast_or_null(D); if (FD && !FD->hasAttr()) Entry->setLinkage(llvm::Function::ExternalLinkage); } @@ -4652,18 +4652,35 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction( // If there are two attempts to define the same mangled name, issue an // error. - if (IsForDefinition && !Entry->isDeclaration()) { - GlobalDecl OtherGD; - // Check that GD is not yet in DiagnosedConflictingDefinitions is required - // to make sure that we issue an error only once. - if (lookupRepresentativeDecl(MangledName, OtherGD) && - (GD.getCanonicalDecl().getDecl() != - OtherGD.getCanonicalDecl().getDecl()) && - DiagnosedConflictingDefinitions.insert(GD).second) { + GlobalDecl OtherGD; + // Check that GD is not yet in DiagnosedConflictingDefinitions is required + // to make sure that we issue an error only once. + if (GD && lookupRepresentativeDecl(MangledName, OtherGD) && + (GD.getCanonicalDecl().getDecl() != + OtherGD.getCanonicalDecl().getDecl()) && + DiagnosedConflictingDefinitions.insert(GD).second) { + if (IsForDefinition && !Entry->isDeclaration()) { getDiags().Report(D->getLocation(), diag::err_duplicate_mangled_name) << MangledName; getDiags().Report(OtherGD.getDecl()->getLocation(), diag::note_previous_definition); + } else { + // For lambdas, it's possible to create the same mangled name from + // different function prototypes. For example, two FPTs may have + // identical types but incompatible function attributes which we should + // not allow. + auto *MD = dyn_cast(D); + if (MD && MD->getParent()->isLambda()) { + const FunctionDecl *OtherFD = + cast_or_null(OtherGD.getDecl()); + if (FD && FD->hasPrototype() && OtherFD && OtherFD->hasPrototype()) { + if (FD->getType()->getAs() != + OtherFD->getType()->getAs()) + getDiags().Report(D->getLocation(), + diag::err_duplicate_mangled_name) + << MangledName; + } + } } } diff --git a/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp b/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp new file mode 100644 index 00000000000000..bce3e657001679 --- /dev/null +++ b/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp @@ -0,0 +1,28 @@ +// REQUIRES: aarch64-registered-target + +// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2 -emit-llvm -o - %s -verify -DTEST1 +// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2 -emit-llvm -o - %s -verify -DTEST2 + +int normal_fn(int); +int streaming_fn(int) __arm_streaming; +int streaming_compatible_fn(int) __arm_streaming_compatible; + +#ifdef TEST1 + +// expected-error@+2 {{definition with same mangled name '_ZZ32function_params_normal_streamingvENK3$_0clIFiiEEEDaRT_' as another definition}} +int function_params_normal_streaming() { + auto a = [](auto &fn) { return fn(42); }; + return a(normal_fn) + a(streaming_fn); +} + +#endif + +#ifdef TEST2 + +// expected-error@+2 {{definition with same mangled name '_ZZ36function_params_streaming_compatiblevENK3$_0clIFiiEEEDaRT_' as another definition}} +int function_params_streaming_compatible() { + auto a = [](auto &fn) { return fn(42); }; + return a(streaming_fn) + a(streaming_compatible_fn); +} + +#endif From fc1394eec345702eea7f3c664e5fc33ce3c525d0 Mon Sep 17 00:00:00 2001 From: Kerry McLaughlin Date: Fri, 13 Sep 2024 08:58:02 +0000 Subject: [PATCH 2/6] - Rewrite changes to GetOrCreateLLVMFunction --- clang/lib/CodeGen/CodeGenModule.cpp | 37 ++++++------------- .../aarch64-sme-lambda-attributes.cpp | 6 ++- 2 files changed, 15 insertions(+), 28 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index b07090e3de6dfd..e0cfbdf57534c7 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -4638,8 +4638,8 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction( // Lookup the entry, lazily creating it if necessary. llvm::GlobalValue *Entry = GetGlobalValue(MangledName); if (Entry) { - const FunctionDecl *FD = cast_or_null(D); if (WeakRefReferences.erase(Entry)) { + const FunctionDecl *FD = cast_or_null(D); if (FD && !FD->hasAttr()) Entry->setLinkage(llvm::Function::ExternalLinkage); } @@ -4652,35 +4652,20 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction( // If there are two attempts to define the same mangled name, issue an // error. - GlobalDecl OtherGD; - // Check that GD is not yet in DiagnosedConflictingDefinitions is required - // to make sure that we issue an error only once. - if (GD && lookupRepresentativeDecl(MangledName, OtherGD) && - (GD.getCanonicalDecl().getDecl() != - OtherGD.getCanonicalDecl().getDecl()) && - DiagnosedConflictingDefinitions.insert(GD).second) { - if (IsForDefinition && !Entry->isDeclaration()) { + auto *MD = dyn_cast_or_null(D); + bool IsLambda = MD && MD->getParent()->isLambda(); + if (IsForDefinition && (!Entry->isDeclaration() || IsLambda)) { + GlobalDecl OtherGD; + // Check that GD is not yet in DiagnosedConflictingDefinitions is required + // to make sure that we issue an error only once. + if (lookupRepresentativeDecl(MangledName, OtherGD) && + (GD.getCanonicalDecl().getDecl() != + OtherGD.getCanonicalDecl().getDecl()) && + DiagnosedConflictingDefinitions.insert(GD).second) { getDiags().Report(D->getLocation(), diag::err_duplicate_mangled_name) << MangledName; getDiags().Report(OtherGD.getDecl()->getLocation(), diag::note_previous_definition); - } else { - // For lambdas, it's possible to create the same mangled name from - // different function prototypes. For example, two FPTs may have - // identical types but incompatible function attributes which we should - // not allow. - auto *MD = dyn_cast(D); - if (MD && MD->getParent()->isLambda()) { - const FunctionDecl *OtherFD = - cast_or_null(OtherGD.getDecl()); - if (FD && FD->hasPrototype() && OtherFD && OtherFD->hasPrototype()) { - if (FD->getType()->getAs() != - OtherFD->getType()->getAs()) - getDiags().Report(D->getLocation(), - diag::err_duplicate_mangled_name) - << MangledName; - } - } } } diff --git a/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp b/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp index bce3e657001679..d7667bf3343e75 100644 --- a/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp +++ b/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp @@ -9,7 +9,8 @@ int streaming_compatible_fn(int) __arm_streaming_compatible; #ifdef TEST1 -// expected-error@+2 {{definition with same mangled name '_ZZ32function_params_normal_streamingvENK3$_0clIFiiEEEDaRT_' as another definition}} +// expected-error@+3 {{definition with same mangled name '_ZZ32function_params_normal_streamingvENK3$_0clIFiiEEEDaRT_' as another definition}} +// expected-note@+2 {{previous definition is here}} int function_params_normal_streaming() { auto a = [](auto &fn) { return fn(42); }; return a(normal_fn) + a(streaming_fn); @@ -19,7 +20,8 @@ int function_params_normal_streaming() { #ifdef TEST2 -// expected-error@+2 {{definition with same mangled name '_ZZ36function_params_streaming_compatiblevENK3$_0clIFiiEEEDaRT_' as another definition}} +// expected-error@+3 {{definition with same mangled name '_ZZ36function_params_streaming_compatiblevENK3$_0clIFiiEEEDaRT_' as another definition}} +// expected-note@+2 {{previous definition is here}} int function_params_streaming_compatible() { auto a = [](auto &fn) { return fn(42); }; return a(streaming_fn) + a(streaming_compatible_fn); From 33443e62afea03ebe22dd27ad2815f668b8da293 Mon Sep 17 00:00:00 2001 From: Kerry McLaughlin Date: Mon, 16 Sep 2024 10:54:03 +0000 Subject: [PATCH 3/6] - Removed second SME attributes test - Added a test using calling convention attributes --- .../aarch64-sme-lambda-attributes.cpp | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp b/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp index d7667bf3343e75..9e9a3d09de24fd 100644 --- a/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp +++ b/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp @@ -1,30 +1,31 @@ // REQUIRES: aarch64-registered-target -// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2 -emit-llvm -o - %s -verify -DTEST1 +// RUN: %clang_cc1 -triple aarch64 -emit-llvm -o - %s -verify -DTEST1 // RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2 -emit-llvm -o - %s -verify -DTEST2 int normal_fn(int); int streaming_fn(int) __arm_streaming; -int streaming_compatible_fn(int) __arm_streaming_compatible; +int vector_pcs_fn(int) __attribute__((aarch64_vector_pcs)); #ifdef TEST1 -// expected-error@+3 {{definition with same mangled name '_ZZ32function_params_normal_streamingvENK3$_0clIFiiEEEDaRT_' as another definition}} -// expected-note@+2 {{previous definition is here}} -int function_params_normal_streaming() { +// expected-error@+4 {{definition with same mangled name '_ZZ23function_pcs_attributesvENK3$_0clIFiiEEEDaRT_' as another definition}} +// expected-note@+3 {{previous definition is here}} +__attribute__((aarch64_vector_pcs)) +int function_pcs_attributes() { auto a = [](auto &fn) { return fn(42); }; - return a(normal_fn) + a(streaming_fn); + return a(normal_fn) + a(vector_pcs_fn); } #endif #ifdef TEST2 -// expected-error@+3 {{definition with same mangled name '_ZZ36function_params_streaming_compatiblevENK3$_0clIFiiEEEDaRT_' as another definition}} +// expected-error@+3 {{definition with same mangled name '_ZZ32function_params_normal_streamingvENK3$_0clIFiiEEEDaRT_' as another definition}} // expected-note@+2 {{previous definition is here}} -int function_params_streaming_compatible() { +int function_params_normal_streaming() { auto a = [](auto &fn) { return fn(42); }; - return a(streaming_fn) + a(streaming_compatible_fn); + return a(normal_fn) + a(streaming_fn); } #endif From 4b6d327fe6b6a552fe16e27f7b8b15ce5d2055da Mon Sep 17 00:00:00 2001 From: Kerry McLaughlin Date: Mon, 16 Sep 2024 16:22:41 +0000 Subject: [PATCH 4/6] - Renamed aarch64-sme-lambda-attributes.cpp to lambda-instantiation-mangling-conflicts.cpp - Replaced ifdefs in test with use of split-file --- ...mbda-instantiation-mangling-conflicts.cpp} | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) rename clang/test/CodeGenCXX/{aarch64-sme-lambda-attributes.cpp => lambda-instantiation-mangling-conflicts.cpp} (78%) diff --git a/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp b/clang/test/CodeGenCXX/lambda-instantiation-mangling-conflicts.cpp similarity index 78% rename from clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp rename to clang/test/CodeGenCXX/lambda-instantiation-mangling-conflicts.cpp index 9e9a3d09de24fd..433f616c8bb637 100644 --- a/clang/test/CodeGenCXX/aarch64-sme-lambda-attributes.cpp +++ b/clang/test/CodeGenCXX/lambda-instantiation-mangling-conflicts.cpp @@ -1,14 +1,16 @@ // REQUIRES: aarch64-registered-target -// RUN: %clang_cc1 -triple aarch64 -emit-llvm -o - %s -verify -DTEST1 -// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2 -emit-llvm -o - %s -verify -DTEST2 +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2 -emit-llvm -o - %t/a.cpp -verify +// RUN: %clang_cc1 -triple aarch64 -target-feature +sme -target-feature +sme2 -emit-llvm -o - %t/b.cpp -verify + +;--- a.cpp int normal_fn(int); -int streaming_fn(int) __arm_streaming; int vector_pcs_fn(int) __attribute__((aarch64_vector_pcs)); -#ifdef TEST1 - // expected-error@+4 {{definition with same mangled name '_ZZ23function_pcs_attributesvENK3$_0clIFiiEEEDaRT_' as another definition}} // expected-note@+3 {{previous definition is here}} __attribute__((aarch64_vector_pcs)) @@ -17,9 +19,10 @@ int function_pcs_attributes() { return a(normal_fn) + a(vector_pcs_fn); } -#endif +;--- b.cpp -#ifdef TEST2 +int normal_fn(int); +int streaming_fn(int) __arm_streaming; // expected-error@+3 {{definition with same mangled name '_ZZ32function_params_normal_streamingvENK3$_0clIFiiEEEDaRT_' as another definition}} // expected-note@+2 {{previous definition is here}} @@ -27,5 +30,3 @@ int function_params_normal_streaming() { auto a = [](auto &fn) { return fn(42); }; return a(normal_fn) + a(streaming_fn); } - -#endif From 00e7b0494c831b3f7405d34fa9819a271b77eb50 Mon Sep 17 00:00:00 2001 From: Kerry McLaughlin Date: Tue, 15 Oct 2024 15:37:56 +0000 Subject: [PATCH 5/6] - Moved error to EmitGlobal and removed specific check for a lambda --- clang/lib/CodeGen/CodeGenModule.cpp | 21 +++++++++++++------ ...ambda-instantiation-mangling-conflicts.cpp | 6 ++---- .../CodeGenCXX/mangle-lambdas-gh88906.cpp | 9 ++------ 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index e0cfbdf57534c7..550454abdd1ef2 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -3911,9 +3911,20 @@ void CodeGenModule::EmitGlobal(GlobalDecl GD) { assert(!MayBeEmittedEagerly(Global)); addDeferredDeclToEmit(GD); } else { - // Otherwise, remember that we saw a deferred decl with this name. The - // first use of the mangled name will cause it to move into - // DeferredDeclsToEmit. + // Otherwise, remember that we saw a deferred decl with this name. + auto DDI = DeferredDecls.find(MangledName); + const auto *FD = dyn_cast(Global); + if (FD && DDI != DeferredDecls.end()) { + bool IsDefinitionAvailableExternally = + getContext().GetGVALinkageForFunction(FD) == GVA_AvailableExternally; + if (!IsDefinitionAvailableExternally && !FD->isMultiVersion() && + !FD->hasAttr()) + getDiags().Report(Global->getLocation(), + diag::err_duplicate_mangled_name) + << MangledName; + } + // The first use of the mangled name will cause it to move + // into DeferredDeclsToEmit. DeferredDecls[MangledName] = GD; } } @@ -4652,9 +4663,7 @@ llvm::Constant *CodeGenModule::GetOrCreateLLVMFunction( // If there are two attempts to define the same mangled name, issue an // error. - auto *MD = dyn_cast_or_null(D); - bool IsLambda = MD && MD->getParent()->isLambda(); - if (IsForDefinition && (!Entry->isDeclaration() || IsLambda)) { + if (IsForDefinition && !Entry->isDeclaration()) { GlobalDecl OtherGD; // Check that GD is not yet in DiagnosedConflictingDefinitions is required // to make sure that we issue an error only once. diff --git a/clang/test/CodeGenCXX/lambda-instantiation-mangling-conflicts.cpp b/clang/test/CodeGenCXX/lambda-instantiation-mangling-conflicts.cpp index 433f616c8bb637..3a5adc5cb7c7d0 100644 --- a/clang/test/CodeGenCXX/lambda-instantiation-mangling-conflicts.cpp +++ b/clang/test/CodeGenCXX/lambda-instantiation-mangling-conflicts.cpp @@ -11,8 +11,7 @@ int normal_fn(int); int vector_pcs_fn(int) __attribute__((aarch64_vector_pcs)); -// expected-error@+4 {{definition with same mangled name '_ZZ23function_pcs_attributesvENK3$_0clIFiiEEEDaRT_' as another definition}} -// expected-note@+3 {{previous definition is here}} +// expected-error@+3 {{definition with same mangled name '_ZZ23function_pcs_attributesvENK3$_0clIFiiEEEDaRT_' as another definition}} __attribute__((aarch64_vector_pcs)) int function_pcs_attributes() { auto a = [](auto &fn) { return fn(42); }; @@ -24,8 +23,7 @@ int function_pcs_attributes() { int normal_fn(int); int streaming_fn(int) __arm_streaming; -// expected-error@+3 {{definition with same mangled name '_ZZ32function_params_normal_streamingvENK3$_0clIFiiEEEDaRT_' as another definition}} -// expected-note@+2 {{previous definition is here}} +// expected-error@+2 {{definition with same mangled name '_ZZ32function_params_normal_streamingvENK3$_0clIFiiEEEDaRT_' as another definition}} int function_params_normal_streaming() { auto a = [](auto &fn) { return fn(42); }; return a(normal_fn) + a(streaming_fn); diff --git a/clang/test/CodeGenCXX/mangle-lambdas-gh88906.cpp b/clang/test/CodeGenCXX/mangle-lambdas-gh88906.cpp index e7592cec5da776..6130756af0e618 100644 --- a/clang/test/CodeGenCXX/mangle-lambdas-gh88906.cpp +++ b/clang/test/CodeGenCXX/mangle-lambdas-gh88906.cpp @@ -1,5 +1,5 @@ // RUN: %clang_cc1 -triple x86_64-linux-gnu %s -emit-llvm -mconstructor-aliases -o - | FileCheck %s -// RUN: %clang_cc1 -triple x86_64-linux-gnu -fclang-abi-compat=18 %s -emit-llvm -mconstructor-aliases -o - | FileCheck --check-prefix=CLANG18 %s +// RUN: not %clang_cc1 -triple x86_64-linux-gnu -fclang-abi-compat=18 %s -emit-llvm -mconstructor-aliases 2>&1 | FileCheck --check-prefix=CLANG18 %s // RUN: %clang_cc1 -triple i386-pc-win32 %s -emit-llvm -mconstructor-aliases -o - | FileCheck --check-prefix=MSABI %s @@ -29,12 +29,7 @@ void GH88906(){ // CHECK-LABEL: define internal void @_ZN4funcC2IN7GH889064Test1bMUlvE_EEET_ // CHECK-LABEL: define internal void @_ZN4funcC2IN7GH889064Test1cMUlvE_EEET_ -// CLANG18-LABEL: define internal void @_ZZ7GH88906vEN4TestC2Ev -// CLANG18: call void @_ZN4funcC2IZ7GH88906vEN4TestUlvE_EZ7GH88906vENS1_UlvE0_EEET_T0_ -// CLANG18: call void @_ZN4funcC2IZ7GH88906vEN4TestUlvE_EEET_ -// CLANG18: call void @_ZN4funcC2IZ7GH88906vEN4TestUlvE_EEET_ - - +// CLANG18: error: definition with same mangled name '_ZN4funcC1IZ7GH88906vEN4TestUlvE_EEET_' as another definition // MSABI-LABEL: define internal x86_thiscallcc noundef ptr @"??0Test@?1??GH88906@@YAXXZ@QAE@XZ" // MSABI: call x86_thiscallcc noundef ptr @"??$?0V@a@Test@?1??GH88906@@YAXXZ@V@12?1??3@YAXXZ@@func@@QAE@V@a@Test@?1??GH88906@@YAXXZ@V@23?1??4@YAXXZ@@Z" From feda5d45b3c4270e1d598ec86ed3e80176f09b23 Mon Sep 17 00:00:00 2001 From: Kerry McLaughlin Date: Mon, 21 Oct 2024 09:20:25 +0000 Subject: [PATCH 6/6] - Add test without lambdas to duplicate-mangled-name.cpp --- clang/test/CodeGenCXX/duplicate-mangled-name.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/clang/test/CodeGenCXX/duplicate-mangled-name.cpp b/clang/test/CodeGenCXX/duplicate-mangled-name.cpp index 04e6fee506ebd7..cfa70623de53cd 100644 --- a/clang/test/CodeGenCXX/duplicate-mangled-name.cpp +++ b/clang/test/CodeGenCXX/duplicate-mangled-name.cpp @@ -2,6 +2,7 @@ // RUN: %clang_cc1 -triple %itanium_abi_triple-only %s -verify -DTEST2 -emit-llvm -o - | FileCheck %s // RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only %s -verify -DTEST3 // RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only %s -verify -DTEST4 +// RUN: %clang_cc1 -triple %itanium_abi_triple -emit-llvm-only %s -verify -DTEST5 #ifdef TEST1 @@ -70,6 +71,13 @@ float foo() { return _ZN2nm3abcE + nm::abc; } +#elif TEST5 + +inline void f(int) asm("foo"); +inline void f(int) {} +inline void f() asm("foo"); +inline void f(){} // expected-error {{definition with same mangled name 'foo' as another definition}} + #else #error Unknown test