[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-03 Thread Owen Anderson via cfe-commits 

https://github.com/resistor closed 
https://github.com/llvm/llvm-project/pull/125448
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-03 Thread Alexander Shaposhnikov via cfe-commits 

https://github.com/alexander-shaposhnikov approved this pull request.


https://github.com/llvm/llvm-project/pull/125448
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Owen Anderson via cfe-commits 

https://github.com/resistor updated 
https://github.com/llvm/llvm-project/pull/125448

>From 8d2ed55358510e45c03934aaaffccbf12d0bffce Mon Sep 17 00:00:00 2001
From: Owen Anderson 
Date: Mon, 3 Feb 2025 15:41:20 +1300
Subject: [PATCH] [clang] Do not emit template parameter objects as COMDATs
 when they have internal linkage.

Per the ELF spec, section groups may only contain local symbols if those 
symbols are only
referenced from within the section group. [1] In the case of template parameter 
objects,
they can be referenced from outside the group when the type of the object was 
declared
in an anonymous namespace. In that case, we can't place the object in a COMDAT. 
This matches
GCC's linkage behavior on the test input.

[1]: https://www.sco.com/developers/gabi/latest/ch4.sheader.html#section_groups
---
 clang/lib/CodeGen/CodeGenModule.cpp  |  2 +-
 clang/test/CodeGenCXX/template-param-objects.cpp | 10 ++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 05879cd486a8c9..82002b8d8e4d4f 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3765,7 +3765,7 @@ ConstantAddress 
CodeGenModule::GetAddrOfTemplateParamObject(
   auto *GV = new llvm::GlobalVariable(getModule(), Init->getType(),
   /*isConstant=*/true, Linkage, Init, 
Name);
   setGVProperties(GV, TPO);
-  if (supportsCOMDAT())
+  if (supportsCOMDAT() && Linkage == llvm::GlobalValue::LinkOnceODRLinkage)
 GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));
   Emitter.finalize(GV);
 
diff --git a/clang/test/CodeGenCXX/template-param-objects.cpp 
b/clang/test/CodeGenCXX/template-param-objects.cpp
index 11ebd21521e83b..ff6acc438d137a 100644
--- a/clang/test/CodeGenCXX/template-param-objects.cpp
+++ b/clang/test/CodeGenCXX/template-param-objects.cpp
@@ -5,6 +5,9 @@ struct S { char buf[32]; };
 template constexpr const char *begin() { return s.buf; }
 template constexpr const char *end() { return s.buf + 
__builtin_strlen(s.buf); }
 
+namespace { struct T { char buf[32]; }; }
+template constexpr const char* begin_anon() { return t.buf; }
+
 // ITANIUM: 
[[HELLO:@_ZTAXtl1StlA32_cLc104ELc101ELc108ELc108ELc111ELc32ELc119ELc111ELc114ELc108ELc100]]
 // MSABI: 
[[HELLO:@"[?][?]__N2US@@3D0GI@@0GF@@0GM@@0GM@@0GP@@0CA@@0HH@@0GP@@0HC@@0GM@@0GE@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@"]]
 // ITANIUM-SAME: = linkonce_odr constant { <{ [11 x i8], [21 x i8] }> } { <{ 
[11 x i8], [21 x i8] }> <{ [11 x i8] c"hello world", [21 x i8] zeroinitializer 
}> }, comdat
@@ -19,3 +22,10 @@ const char *p = begin();
 // MSABI: @"?q@@3PEBDEB"
 // CHECK-SAME: global ptr getelementptr (i8, ptr [[HELLO]], i64 11)
 const char *q = end();
+
+
+// CHECK: internal constant { <{ [10 x i8], [22 x i8] }> } { <{ [10 x i8], [22 
x i8] }> <{ [10 x i8] c"hello anon", [22 x i8] zeroinitializer }> }
+// CHECK-NOT: comdat
+// ITANIUM: @r
+// MSABI: @"?r@@3PEBDEB"
+const char *r = begin_anon();

___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Owen Anderson via cfe-commits 

https://github.com/resistor updated 
https://github.com/llvm/llvm-project/pull/125448

>From b3078fa4389f6be3f0b8c463ab9e79f1b4a9d23b Mon Sep 17 00:00:00 2001
From: Owen Anderson 
Date: Mon, 3 Feb 2025 15:41:20 +1300
Subject: [PATCH] [clang] Do not emit template parameter objects as COMDATs
 when they have internal linkage.

Per the ELF spec, section groups may only contain local symbols if those 
symbols are only
referenced from within the section group. [1] In the case of template parameter 
objects,
they can be referenced from outside the group when the type of the object was 
declared
in an anonymous namespace. In that case, we can't place the object in a COMDAT. 
This matches
GCC's linkage behavior on the test input.

[1]: https://www.sco.com/developers/gabi/latest/ch4.sheader.html#section_groups
---
 clang/lib/CodeGen/CodeGenModule.cpp  |  2 +-
 clang/test/CodeGenCXX/template-param-objects.cpp | 13 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 05879cd486a8c9..82002b8d8e4d4f 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3765,7 +3765,7 @@ ConstantAddress 
CodeGenModule::GetAddrOfTemplateParamObject(
   auto *GV = new llvm::GlobalVariable(getModule(), Init->getType(),
   /*isConstant=*/true, Linkage, Init, 
Name);
   setGVProperties(GV, TPO);
-  if (supportsCOMDAT())
+  if (supportsCOMDAT() && Linkage == llvm::GlobalValue::LinkOnceODRLinkage)
 GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));
   Emitter.finalize(GV);
 
diff --git a/clang/test/CodeGenCXX/template-param-objects.cpp 
b/clang/test/CodeGenCXX/template-param-objects.cpp
index 11ebd21521e83b..4b78e1e84f8638 100644
--- a/clang/test/CodeGenCXX/template-param-objects.cpp
+++ b/clang/test/CodeGenCXX/template-param-objects.cpp
@@ -5,6 +5,9 @@ struct S { char buf[32]; };
 template constexpr const char *begin() { return s.buf; }
 template constexpr const char *end() { return s.buf + 
__builtin_strlen(s.buf); }
 
+namespace { struct T { char buf[32]; }; }
+template constexpr const char* begin_anon() { return t.buf; }
+
 // ITANIUM: 
[[HELLO:@_ZTAXtl1StlA32_cLc104ELc101ELc108ELc108ELc111ELc32ELc119ELc111ELc114ELc108ELc100]]
 // MSABI: 
[[HELLO:@"[?][?]__N2US@@3D0GI@@0GF@@0GM@@0GM@@0GP@@0CA@@0HH@@0GP@@0HC@@0GM@@0GE@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@"]]
 // ITANIUM-SAME: = linkonce_odr constant { <{ [11 x i8], [21 x i8] }> } { <{ 
[11 x i8], [21 x i8] }> <{ [11 x i8] c"hello world", [21 x i8] zeroinitializer 
}> }, comdat
@@ -19,3 +22,13 @@ const char *p = begin();
 // MSABI: @"?q@@3PEBDEB"
 // CHECK-SAME: global ptr getelementptr (i8, ptr [[HELLO]], i64 11)
 const char *q = end();
+
+
+// ITANIUM: 
[[HELLOANON:@_ZTAXtlN12_GLOBAL__N_11TEtlA32_cLc104ELc101ELc108ELc108ELc111ELc32ELc97ELc110ELc111ELc110]]
+// MSABI: 
[[HELLOANON:@"\?\?__N2UT@\?A0xFD1119C8@@3D0GI@@0GF@@0GM@@0GM@@0GP@@0CA@@0GB@@0GO@@0GP@@0GO@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@"]]
+// CHECK-SAME: internal constant { <{ [10 x i8], [22 x i8] }> } { <{ [10 x 
i8], [22 x i8] }> <{ [10 x i8] c"hello anon", [22 x i8] zeroinitializer }> }
+// ITANIUM: @r
+// MSABI: @"?r@@3PEBDEB"
+// CHECK-SAME: global ptr [[HELLOANON]]
+// CHECK-NOT: comdat
+const char *r = begin_anon();

___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Fangrui Song via cfe-commits 

https://github.com/MaskRay approved this pull request.


https://github.com/llvm/llvm-project/pull/125448
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Owen Anderson via cfe-commits 


@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -emit-llvm -std=c++20 -triple x86_64-unknown-linux-gnu %s 
-o - | FileCheck %s
+
+// COMDAT groups in ELF objects are not permitted to contain local symbols. 
While template parameter
+// objects are normally emitted in COMDATs, we shouldn't do so if they would 
have internal linkage.
+
+extern "C" int printf(...);
+typedef __typeof__(sizeof(0)) size_t;
+
+namespace {
+template
+struct DebugContext
+{
+char value[N];
+constexpr DebugContext(const char (&str)[N]) {

resistor wrote:

Done and folded into existing test.

https://github.com/llvm/llvm-project/pull/125448
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Owen Anderson via cfe-commits 


@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -emit-llvm -std=c++20 -triple x86_64-unknown-linux-gnu %s 
-o - | FileCheck %s

resistor wrote:

Done

https://github.com/llvm/llvm-project/pull/125448
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Owen Anderson via cfe-commits 

https://github.com/resistor updated 
https://github.com/llvm/llvm-project/pull/125448

>From fabd1f091d018e76d59777ce29a9d16ef6abb48f Mon Sep 17 00:00:00 2001
From: Owen Anderson 
Date: Mon, 3 Feb 2025 15:41:20 +1300
Subject: [PATCH] [clang] Do not emit template parameter objects as COMDATs
 when they have internal linkage.

Per the ELF spec, section groups may only contain local symbols if those 
symbols are only
referenced from within the section group. [1] In the case of template parameter 
objects,
they can be referenced from outside the group when the type of the object was 
declared
in an anonymous namespace. In that case, we can't place the object in a COMDAT. 
This matches
GCC's linkage behavior on the test input.

[1]: https://www.sco.com/developers/gabi/latest/ch4.sheader.html#section_groups
---
 clang/lib/CodeGen/CodeGenModule.cpp  |  2 +-
 clang/test/CodeGenCXX/template-param-objects.cpp | 13 +
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 05879cd486a8c9..82002b8d8e4d4f 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3765,7 +3765,7 @@ ConstantAddress 
CodeGenModule::GetAddrOfTemplateParamObject(
   auto *GV = new llvm::GlobalVariable(getModule(), Init->getType(),
   /*isConstant=*/true, Linkage, Init, 
Name);
   setGVProperties(GV, TPO);
-  if (supportsCOMDAT())
+  if (supportsCOMDAT() && Linkage == llvm::GlobalValue::LinkOnceODRLinkage)
 GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));
   Emitter.finalize(GV);
 
diff --git a/clang/test/CodeGenCXX/template-param-objects.cpp 
b/clang/test/CodeGenCXX/template-param-objects.cpp
index 11ebd21521e83b..afe68c034def22 100644
--- a/clang/test/CodeGenCXX/template-param-objects.cpp
+++ b/clang/test/CodeGenCXX/template-param-objects.cpp
@@ -5,6 +5,9 @@ struct S { char buf[32]; };
 template constexpr const char *begin() { return s.buf; }
 template constexpr const char *end() { return s.buf + 
__builtin_strlen(s.buf); }
 
+namespace { struct T { char buf[32]; }; }
+template constexpr const char* begin_anon() { return t.buf; }
+
 // ITANIUM: 
[[HELLO:@_ZTAXtl1StlA32_cLc104ELc101ELc108ELc108ELc111ELc32ELc119ELc111ELc114ELc108ELc100]]
 // MSABI: 
[[HELLO:@"[?][?]__N2US@@3D0GI@@0GF@@0GM@@0GM@@0GP@@0CA@@0HH@@0GP@@0HC@@0GM@@0GE@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@"]]
 // ITANIUM-SAME: = linkonce_odr constant { <{ [11 x i8], [21 x i8] }> } { <{ 
[11 x i8], [21 x i8] }> <{ [11 x i8] c"hello world", [21 x i8] zeroinitializer 
}> }, comdat
@@ -19,3 +22,13 @@ const char *p = begin();
 // MSABI: @"?q@@3PEBDEB"
 // CHECK-SAME: global ptr getelementptr (i8, ptr [[HELLO]], i64 11)
 const char *q = end();
+
+
+// ITANIUM: 
[[HELLOANON:@_ZTAXtlN12_GLOBAL__N_11TEtlA32_cLc104ELc101ELc108ELc108ELc111ELc32ELc97ELc110ELc111ELc110]]
+// MSABI: 
[[HELLOANON:@"[?][?]__N2UT@[?]A0xFD1119C8@@3D0GI@@0GF@@0GM@@0GM@@0GP@@0CA@@0GB@@0GO@@0GP@@0GO@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@@0A@"]]
+// CHECK-SAME: internal constant { <{ [10 x i8], [22 x i8] }> } { <{ [10 x 
i8], [22 x i8] }> <{ [10 x i8] c"hello anon", [22 x i8] zeroinitializer }> }
+// ITANIUM: @r
+// MSABI: @"?r@@3PEBDEB"
+// CHECK-SAME: global ptr [[HELLOANON]]
+// CHECK-NOT: comdat
+const char *r = begin_anon();

___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Owen Anderson via cfe-commits 


@@ -3765,7 +3765,7 @@ ConstantAddress 
CodeGenModule::GetAddrOfTemplateParamObject(
   auto *GV = new llvm::GlobalVariable(getModule(), Init->getType(),
   /*isConstant=*/true, Linkage, Init, 
Name);
   setGVProperties(GV, TPO);
-  if (supportsCOMDAT())
+  if (supportsCOMDAT() && Linkage == llvm::GlobalValue::LinkOnceODRLinkage)

resistor wrote:

Linkage is set to either LinkOnceODRLinkage or InternalLinkage a few lines 
above this.

https://github.com/llvm/llvm-project/pull/125448
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Fangrui Song via cfe-commits 


@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -emit-llvm -std=c++20 -triple x86_64-unknown-linux-gnu %s 
-o - | FileCheck %s

MaskRay wrote:

Probably best to figure out where the primary test of the neighbor lines is and 
adds the extra test there.

https://github.com/llvm/llvm-project/pull/125448
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Fangrui Song via cfe-commits 


@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -emit-llvm -std=c++20 -triple x86_64-unknown-linux-gnu %s 
-o - | FileCheck %s
+
+// COMDAT groups in ELF objects are not permitted to contain local symbols. 
While template parameter
+// objects are normally emitted in COMDATs, we shouldn't do so if they would 
have internal linkage.
+
+extern "C" int printf(...);
+typedef __typeof__(sizeof(0)) size_t;
+
+namespace {
+template
+struct DebugContext
+{
+char value[N];
+constexpr DebugContext(const char (&str)[N]) {

MaskRay wrote:

The test can be reduced.

https://github.com/llvm/llvm-project/pull/125448
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Fangrui Song via cfe-commits 


@@ -3765,7 +3765,7 @@ ConstantAddress 
CodeGenModule::GetAddrOfTemplateParamObject(
   auto *GV = new llvm::GlobalVariable(getModule(), Init->getType(),
   /*isConstant=*/true, Linkage, Init, 
Name);
   setGVProperties(GV, TPO);
-  if (supportsCOMDAT())
+  if (supportsCOMDAT() && Linkage == llvm::GlobalValue::LinkOnceODRLinkage)

MaskRay wrote:

I recall that in some cases WeakODRLinkage is used.  Should the condition be 
something like `!isLocalLinkage()`?

https://github.com/llvm/llvm-project/pull/125448
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread via cfe-commits 

llvmbot wrote:




@llvm/pr-subscribers-clang

Author: Owen Anderson (resistor)


Changes

Per the ELF spec, section groups may only contain local symbols if those 
symbols are only
referenced from within the section group. [1] In the case of template parameter 
objects,
they can be referenced from outside the group when the type of the object was 
declared
in an anonymous namespace. In that case, we can't place the object in a COMDAT. 
This matches
GCC's linkage behavior on the test input.

[1]: https://www.sco.com/developers/gabi/latest/ch4.sheader.html#section_groups


---
Full diff: https://github.com/llvm/llvm-project/pull/125448.diff


2 Files Affected:

- (modified) clang/lib/CodeGen/CodeGenModule.cpp (+1-1) 
- (added) clang/test/CodeGenCXX/nocomdat-local-symbol.cpp (+38) 


``diff
diff --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 05879cd486a8c9..82002b8d8e4d4f 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3765,7 +3765,7 @@ ConstantAddress 
CodeGenModule::GetAddrOfTemplateParamObject(
   auto *GV = new llvm::GlobalVariable(getModule(), Init->getType(),
   /*isConstant=*/true, Linkage, Init, 
Name);
   setGVProperties(GV, TPO);
-  if (supportsCOMDAT())
+  if (supportsCOMDAT() && Linkage == llvm::GlobalValue::LinkOnceODRLinkage)
 GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));
   Emitter.finalize(GV);
 
diff --git a/clang/test/CodeGenCXX/nocomdat-local-symbol.cpp 
b/clang/test/CodeGenCXX/nocomdat-local-symbol.cpp
new file mode 100644
index 00..3beb7d2197dd59
--- /dev/null
+++ b/clang/test/CodeGenCXX/nocomdat-local-symbol.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -emit-llvm -std=c++20 -triple x86_64-unknown-linux-gnu %s 
-o - | FileCheck %s
+
+// COMDAT groups in ELF objects are not permitted to contain local symbols. 
While template parameter
+// objects are normally emitted in COMDATs, we shouldn't do so if they would 
have internal linkage.
+
+extern "C" int printf(...);
+typedef __typeof__(sizeof(0)) size_t;
+
+namespace {
+template
+struct DebugContext
+{
+char value[N];
+constexpr DebugContext(const char (&str)[N]) {
+for (size_t i = 0; i < N; ++i) {
+value[i] = str[i];
+}
+}
+};
+}
+
+template
+struct ConditionalDebug
+{
+public:
+static void log() {
+printf("%s", Context.value);
+}
+};
+
+using Debug = ConditionalDebug<"compartment A">;
+
+void foo() {
+   Debug::log();
+}
+
+// CHECK-NOT: 
$_ZTAXtlN12_GLOBAL__N_112DebugContextILm14EEEtlA14_cLc99ELc111ELc109ELc112ELc97ELc114ELc116ELc109ELc101ELc110ELc116ELc32ELc65
 = comdat any
+// CHECK: 
@_ZTAXtlN12_GLOBAL__N_112DebugContextILm14EEEtlA14_cLc99ELc111ELc109ELc112ELc97ELc114ELc116ELc109ELc101ELc110ELc116ELc32ELc65
 = internal constant %"struct.(anonymous namespace)::DebugContext" { [14 x i8] 
c"compartment A\00" }
\ No newline at end of file

``




https://github.com/llvm/llvm-project/pull/125448
___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang] Do not emit template parameter objects as COMDATs when they have internal linkage. (PR #125448)

2025-02-02 Thread Owen Anderson via cfe-commits 

https://github.com/resistor created 
https://github.com/llvm/llvm-project/pull/125448

Per the ELF spec, section groups may only contain local symbols if those 
symbols are only
referenced from within the section group. [1] In the case of template parameter 
objects,
they can be referenced from outside the group when the type of the object was 
declared
in an anonymous namespace. In that case, we can't place the object in a COMDAT. 
This matches
GCC's linkage behavior on the test input.

[1]: https://www.sco.com/developers/gabi/latest/ch4.sheader.html#section_groups


>From 7aa7a526bc6876cfe9b3c68e09e3e5f84befed03 Mon Sep 17 00:00:00 2001
From: Owen Anderson 
Date: Mon, 3 Feb 2025 15:41:20 +1300
Subject: [PATCH] [clang] Do not emit template parameter objects as COMDATs
 when they have internal linkage.

Per the ELF spec, section groups may only contain local symbols if those 
symbols are only
referenced from within the section group. [1] In the case of template parameter 
objects,
they can be referenced from outside the group when the type of the object was 
declared
in an anonymous namespace. In that case, we can't place the object in a COMDAT. 
This matches
GCC's linkage behavior on the test input.

[1]: https://www.sco.com/developers/gabi/latest/ch4.sheader.html#section_groups
---
 clang/lib/CodeGen/CodeGenModule.cpp   |  2 +-
 .../test/CodeGenCXX/nocomdat-local-symbol.cpp | 38 +++
 2 files changed, 39 insertions(+), 1 deletion(-)
 create mode 100644 clang/test/CodeGenCXX/nocomdat-local-symbol.cpp

diff --git a/clang/lib/CodeGen/CodeGenModule.cpp 
b/clang/lib/CodeGen/CodeGenModule.cpp
index 05879cd486a8c98..82002b8d8e4d4f1 100644
--- a/clang/lib/CodeGen/CodeGenModule.cpp
+++ b/clang/lib/CodeGen/CodeGenModule.cpp
@@ -3765,7 +3765,7 @@ ConstantAddress 
CodeGenModule::GetAddrOfTemplateParamObject(
   auto *GV = new llvm::GlobalVariable(getModule(), Init->getType(),
   /*isConstant=*/true, Linkage, Init, 
Name);
   setGVProperties(GV, TPO);
-  if (supportsCOMDAT())
+  if (supportsCOMDAT() && Linkage == llvm::GlobalValue::LinkOnceODRLinkage)
 GV->setComdat(TheModule.getOrInsertComdat(GV->getName()));
   Emitter.finalize(GV);
 
diff --git a/clang/test/CodeGenCXX/nocomdat-local-symbol.cpp 
b/clang/test/CodeGenCXX/nocomdat-local-symbol.cpp
new file mode 100644
index 000..3beb7d2197dd597
--- /dev/null
+++ b/clang/test/CodeGenCXX/nocomdat-local-symbol.cpp
@@ -0,0 +1,38 @@
+// RUN: %clang_cc1 -emit-llvm -std=c++20 -triple x86_64-unknown-linux-gnu %s 
-o - | FileCheck %s
+
+// COMDAT groups in ELF objects are not permitted to contain local symbols. 
While template parameter
+// objects are normally emitted in COMDATs, we shouldn't do so if they would 
have internal linkage.
+
+extern "C" int printf(...);
+typedef __typeof__(sizeof(0)) size_t;
+
+namespace {
+template
+struct DebugContext
+{
+char value[N];
+constexpr DebugContext(const char (&str)[N]) {
+for (size_t i = 0; i < N; ++i) {
+value[i] = str[i];
+}
+}
+};
+}
+
+template
+struct ConditionalDebug
+{
+public:
+static void log() {
+printf("%s", Context.value);
+}
+};
+
+using Debug = ConditionalDebug<"compartment A">;
+
+void foo() {
+   Debug::log();
+}
+
+// CHECK-NOT: 
$_ZTAXtlN12_GLOBAL__N_112DebugContextILm14EEEtlA14_cLc99ELc111ELc109ELc112ELc97ELc114ELc116ELc109ELc101ELc110ELc116ELc32ELc65
 = comdat any
+// CHECK: 
@_ZTAXtlN12_GLOBAL__N_112DebugContextILm14EEEtlA14_cLc99ELc111ELc109ELc112ELc97ELc114ELc116ELc109ELc101ELc110ELc116ELc32ELc65
 = internal constant %"struct.(anonymous namespace)::DebugContext" { [14 x i8] 
c"compartment A\00" }
\ No newline at end of file

___
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits