[PATCH] D117190: [GlobalValue] Make dso_local function work with comdat nodeduplicate

[Fangrui Song via Phabricator via llvm-commits]

MaskRay created this revision.
MaskRay added reviewers: leonardchan, ljmf00, sfertile.
Herald added subscribers: dexonsmith, pengfei, hiraditya.
MaskRay requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This fixes -fno-semantic-interposition -fsanitize-coverage incompatibility.

-fPIC -fno-semantic-interposition may add dso_local to an external linkage
function. -fsanitize-coverage instrumentation does not clear dso_local when
adding comdat nodeduplicate. This causes a compatibility issue: the function
symbol may be referenced by a PC-relative relocation without using the local
alias. In -shared mode, ld will report a relocation error.

The fix is to either clear dso_local when adding comdat nodeduplicate, or
supporting comdat nodeduplicate. The latter seems appropriate to me.

The comdat condition was originally added by D77429 <https://reviews.llvm.org/D77429> to not use local alias for a
hidden external linkage function. The condition has been unused since the code was
refactored to only use local alias for default visibility symbols.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D117190

Files:
  llvm/lib/IR/Globals.cpp
  llvm/test/CodeGen/X86/linux-preemption.ll


Index: llvm/test/CodeGen/X86/linux-preemption.ll
===================================================================
--- llvm/test/CodeGen/X86/linux-preemption.ll
+++ llvm/test/CodeGen/X86/linux-preemption.ll
@@ -252,6 +252,24 @@
 ; STATIC: movq external_preemptable_function at GOTPCREL(%rip), %rax
 ; CHECK32: movl external_preemptable_function at GOT(%eax), %eax
 
+$comdat_nodeduplicate_local = comdat nodeduplicate
+$comdat_nodeduplicate_preemptable = comdat nodeduplicate
+
+;; -fpic -fno-semantic-interposition may add dso_local. Some instrumentation
+;; may add comdat nodeduplicate. We should use local aliases to make the symbol
+;; non-preemptible in the linker.
+define dso_local i8* @comdat_nodeduplicate_local() comdat {
+  ret i8* bitcast (i8* ()* @comdat_nodeduplicate_local to i8*)
+}
+; CHECK: leaq .Lcomdat_nodeduplicate_local$local(%rip), %rax
+; STATIC: movl $comdat_nodeduplicate_local, %eax
+
+define dso_preemptable i8* @comdat_nodeduplicate_preemptable() comdat {
+  ret i8* bitcast (i8* ()* @comdat_nodeduplicate_preemptable to i8*)
+}
+; CHECK: movq comdat_nodeduplicate_preemptable at GOTPCREL(%rip), %rax
+; STATIC: movq comdat_nodeduplicate_preemptable at GOTPCREL(%rip), %rax
+
 !llvm.module.flags = !{!0}
 !0 = !{i32 7, !"PIC Level", i32 2}
 
Index: llvm/lib/IR/Globals.cpp
===================================================================
--- llvm/lib/IR/Globals.cpp
+++ llvm/lib/IR/Globals.cpp
@@ -108,7 +108,7 @@
   // See AsmPrinter::getSymbolPreferLocal().
   return hasDefaultVisibility() &&
          GlobalObject::isExternalLinkage(getLinkage()) && !isDeclaration() &&
-         !isa<GlobalIFunc>(this) && !hasComdat();
+         !isa<GlobalIFunc>(this);
 }
 
 unsigned GlobalValue::getAddressSpace() const {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D117190.399562.patch
Type: text/x-patch
Size: 1739 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20220113/e0d728b6/attachment.bin>