llvmbot wrote:

<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-clang

Author: Owen Anderson (resistor)

<details>
<summary>Changes</summary>

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 00000000000000..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<size_t N>
+struct DebugContext
+{
+    char value[N];
+    constexpr DebugContext(const char (&str)[N]) {
+        for (size_t i = 0; i < N; ++i) {
+            value[i] = str[i];
+        }
+    }
+};
+}
+
+template<DebugContext Context>
+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_cLc99ELc111ELc109ELc112ELc97ELc114ELc116ELc109ELc101ELc110ELc116ELc32ELc65EEEE
 = comdat any
+// CHECK: 
@_ZTAXtlN12_GLOBAL__N_112DebugContextILm14EEEtlA14_cLc99ELc111ELc109ELc112ELc97ELc114ELc116ELc109ELc101ELc110ELc116ELc32ELc65EEEE
 = internal constant %"struct.(anonymous namespace)::DebugContext" { [14 x i8] 
c"compartment A\00" }
\ No newline at end of file

``````````

</details>


https://github.com/llvm/llvm-project/pull/125448
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to