[PPC64] Preserve LocalEntry when linking

[luporl]

Summary

On PowerPC64, it is necessary to keep the LocalEntry bits in st_other,
especially when -r is used. Otherwise, when the resulting object is used
in a posterior linking, LocalEntry info will be unavailable and functions may
be called through the wrong entrypoint.

[ruiu]

Sorry for the belated response. I missed this one for some reason.

Can you add a test?

ELF/SyntheticSections.cpp

2003 ↗

(On Diff #182027)

I think you need to define a new function that is so trivial. I'd hardcode & 0xe0 with an explanation what that is.

[MaskRay]

ELF/Target.h

182 ↗

(On Diff #182027)

Perhaps add a comment referencing Arch/PPC64.cpp getPPC64GlobalEntryToLocalEntryOffset

The 3 most significant bits of st_other are used by OpenPOWER ABI.

[MaskRay]

ELF/Target.h

182 ↗

(On Diff #182027)

Sorry, the comment should be near line 2003 :)

[luporl]

• Addressed review's comments
• Added a test for the fix

[luporl]

In D56782#1394013, @ruiu wrote:

Sorry for the belated response. I missed this one for some reason.

Can you add a test?

Yes, I've added the ppc64-local-entry.s test.

[ruiu]

LGTM

[luporl]

Thank you @ruiu. Can you commit this change for me?

lld/trunk/ELF/SyntheticSections.cpp

for (SymbolTableEntry &Ent : Symbols) {
     ESym->st_other = 0;
     if (Sym->isLocal()) {
       ESym->setBindingAndType(STB_LOCAL, Sym->Type);
     } else {
       ESym->setBindingAndType(Sym->computeBinding(), Sym->Type);
       ESym->setVisibility(Sym->Visibility);
     }
 
+    // The 3 most significant bits of st_other are used by OpenPOWER ABI.
+    // See getPPC64GlobalEntryToLocalEntryOffset() for more details.
+    if (Config->EMachine == EM_PPC64)
+      ESym->st_other |= Sym->StOther & 0xe0;
+
     ESym->st_name = Ent.StrTabOffset;
     ESym->st_shndx = getSymSectionIndex(Ent.Sym);
 
     // Copy symbol size if it is a defined symbol. st_size is not significant
     // for undefined symbols, so whether copying it or not is up to us if that's
     // the case. We'll leave it as zero because by not setting a value, we can
     // get the exact same outputs for two sets of input files that differ only
     // in undefined symbol size in DSOs.

lld/trunk/test/ELF/ppc64-local-entry.s

+# REQUIRES: ppc
+
+# RUN: llvm-mc -filetype=obj -triple=powerpc64 %s -o %t
+# RUN: ld.lld -r %t -o %t2
+# RUN: llvm-objdump -s -section=.symtab %t2 | FileCheck %s
+
+.text
+.abiversion 2
+.globl  _start
+.p2align  2
+.type   _start,@function
+
+_start:
+.Lfunc_begin0:
+.Lfunc_gep0:
+  addis 2, 12, .TOC.-.Lfunc_gep0@ha
+  addi 2, 2, .TOC.-.Lfunc_gep0@l
+.Lfunc_lep0:
+  .localentry _start, .Lfunc_lep0-.Lfunc_gep0
+  # The code below is not important, it just needs to access some
+  # global data or function, in order to use the TOC.
+  # In this case, it performs the following:
+  # g += 10;
+  # Also note that this code is not intended to be run, but only
+  # to check if the linker will preserve the localentry info.
+  addis 3, 2, g@toc@ha
+  addi 3, 3, g@toc@l
+  lwz 4, 0(3)
+  addi 4, 4, 10
+  stw 4, 0(3)
+  blr
+  .long 0
+  .quad 0
+.Lfunc_end0:
+  .size   _start, .Lfunc_end0-.Lfunc_begin0
+
+  .type g,@object               # @g
+  .lcomm  g,4,4
+
+// We expect the st_other byte to be 0x60:
+// localentry = 011 (gep + 2 instructions), reserved = 000,
+// visibility = 00 (STV_DEFAULT)
+// Currently, llvm-objdump does not support displaying
+// st_other's PPC64 specific flags, thus we check the
+// result of the hexdump of .symtab section.
+
+// CHECK: 0070 00000000 00000000 00000009 12600001