[ELF][RISCV] Create dummy .sdata for __global_pointer$ if .sdata does not exist

[MaskRay]

Summary

If .sdata is absent, linker synthesized __global_pointer$ gets a section index of SHN_ABS.

In -pie/-shared mode, lld complains if a PC relative relocation references an absolute symbol (SHN_ABS) but ld.bfd doesn't:

ld.lld: error: relocation R_RISCV_PCREL_HI20 cannot refer to lute symbol: __global_pointer$

To allow lla gp, __global_pointer$ when .sdata is absent, create a dummy .sdata .

Also, change the visibility from STV_HIDDEN to STV_DEFAULT and don't define the symbol for -shared.
This is what ld.bfd does, though I don't understand why it does so.

[ruiu]

LGTM

[jrtc27]

GNU ld treats it as undefined:

10: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND __global_pointer$

test/ELF/riscv-gp.s

14 ↗

(On Diff #204016)

Should be GLOBAL DEFAULT to match GNU ld.

32 ↗

(On Diff #204016)

GLOBAL DEFAULT again.

[MaskRay]

test/ELF/riscv-gp.s

14 ↗

(On Diff #204016)

glibc/musl Scrt1.o:

.option push
.option norelax
	lla   gp, __global_pointer$ # R_RISCV_PCREL_HI20+R_RISCV_PCREL_LO12_I

.option pop

Do you know why __global_pointer$ is STB_GLOBAL STV_DEFAULT in ld.bfd?

If a shared object can have .sdata and __global_pointer$, and it does the similar lla (pc relative) to load gp, I think a hidden symbol makes more sense.

In a shared object, STB_GLOBAL STV_DEFAULT symbols are preemptable by default. The usage of lla will imply the symbols are non-preemptable, thus hidden visibility will make more sense.

[jrtc27]

test/ELF/riscv-gp.s

14 ↗

(On Diff #204016)

The gp register is global for the entire application; the GPREL relaxations only occur for executables. Shared libraries have no __global_pointer$.

[MaskRay]

GNU ld treats it as undefined:

Yes. That was why I created the patch to fix the musl -fpie -pie link error: ld.lld: error: relocation R_RISCV_PCREL_HI20 cannot refer to absolute symbol: __global_pointer$. This error is lld specific: in -pie/-shared mode, a PC relative relocation to an absolute symbol is not allowed.

In https://groups.google.com/forum/#!searchin/generic-abi/SHN_ABS|sort:date/generic-abi/MC4b4sqnKC0/oQgZxxwMmJAJ, Cary's interpretation of gABI is:

On the contrary, the only interpretation that makes sense to me is that it will not change because of relocation at link time or at load time.

If lld resolves the PC relative relocation statically, the value will be incorrect when the pie or shared object is loaded at runtime because the SHN_ABS symbol's value does not change. The best lld can do is to issue an error.

An alternative is to create a dummy .sdata in musl's Scrt1.o. I don't know how reasonable it is to have lla gp,__global_pointer$ in other code that might not define .sdata.

[jrtc27]

In D63132#1538147, @MaskRay wrote:

GNU ld treats it as undefined:

Yes. That was why I created the patch to fix the musl -fpie -pie link error: ld.lld: error: relocation R_RISCV_PCREL_HI20 cannot refer to absolute symbol: __global_pointer$. This error is lld specific: in -pie/-shared mode, a PC relative relocation to an absolute symbol is not allowed.

In https://groups.google.com/forum/#!searchin/generic-abi/SHN_ABS|sort:date/generic-abi/MC4b4sqnKC0/oQgZxxwMmJAJ, Cary's interpretation of gABI is:

On the contrary, the only interpretation that makes sense to me is that it will not change because of relocation at link time or at load time.

If lld resolves the PC relative relocation statically, the value will be incorrect when the pie or shared object is loaded at runtime because the SHN_ABS symbol's value does not change. The best lld can do is to issue an error.

An alternative is to create a dummy .sdata in musl's Scrt1.o. I don't know how reasonable it is to have lla gp,__global_pointer$ in other code that might not define .sdata.

Ah, so GNU ld sets it to 0/UND for -shared, but for -pie it puts it 0x800 past the end of .got:

[ 7] .got              PROGBITS        0000000000002000 001000 000008 08  WA  0   0  8
  11: 0000000000002808     0 NOTYPE  GLOBAL DEFAULT    7 __global_pointer$

You can see the logic BFD is using in ld/emulparams/elf32lriscv-defs.sh.

[dalias]

I don't immediately see any problem with adding a dummy .sdata, though I think the linker should probably also be able to do this. defining a dummy __global_pointer with SHN_ABS makes no sense. The bfd linker doing this looks like a bug to me, and the bfd linker failing to produce an error when linking it looks like another bug.

[MaskRay]

Repurpose this patch since musl will add a dummy .sdata to its Scrt1.o

[MaskRay]

Synthesize .sdata

_GLOBAL_OFFSET_TABLE_ has a similar behavior: it forces the creation of .got.plt or .got

[MaskRay]

Check if __global_pointer$ is referenced.

[MaskRay]

Don't define __global_pointer$ for -shared

[MaskRay]

@jrtc27 Do this patch and D63076 look good now?

[jrtc27]

In D63132#1541242, @MaskRay wrote:

@jrtc27 Do this patch and D63076 look good now?

I think this patch is fine from my perspective now.

lld/trunk/ELF/Symbols.h

struct ElfSym {
   static Defined *MipsGp;
   static Defined *MipsGpDisp;
   static Defined *MipsLocalGp;
 
   // __rel{,a}_iplt_{start,end} symbols.
   static Defined *RelaIpltStart;
   static Defined *RelaIpltEnd;
 
+  // __global_pointer$ for RISC-V.
+  static Defined *RISCVGlobalPointer;
+
   // _TLS_MODULE_BASE_ on targets that support TLSDESC.
   static Defined *TlsModuleBase;
 };
 
 // A buffer class that is large enough to hold any Symbol-derived
 // object. We allocate memory using this class and instantiate a symbol
 // using the placement new.
 union SymbolUnion {

lld/trunk/ELF/Symbols.cpp

 Defined *ElfSym::End1;
 Defined *ElfSym::End2;
 Defined *ElfSym::GlobalOffsetTable;
 Defined *ElfSym::MipsGp;
 Defined *ElfSym::MipsGpDisp;
 Defined *ElfSym::MipsLocalGp;
 Defined *ElfSym::RelaIpltStart;
 Defined *ElfSym::RelaIpltEnd;
+Defined *ElfSym::RISCVGlobalPointer;
 Defined *ElfSym::TlsModuleBase;
 
 static uint64_t getSymVA(const Symbol &Sym, int64_t &Addend) {
   switch (Sym.kind()) {
   case Symbol::DefinedKind: {
     auto &D = cast<Defined>(Sym);
     SectionBase *IS = D.Section;
 

lld/trunk/ELF/SyntheticSections.h

 class PartitionIndexSection : public SyntheticSection {
 public:
   PartitionIndexSection();
   size_t getSize() const override;
   void finalizeContents() override;
   void writeTo(uint8_t *Buf) override;
 };
 
+// Create a dummy .sdata for __global_pointer$ if .sdata does not exist.
+class RISCVSdataSection final : public SyntheticSection {
+public:
+  RISCVSdataSection();
+  size_t getSize() const override { return 0; }
+  bool isNeeded() const override;
+  void writeTo(uint8_t *Buf) override {}
+};
+
 InputSection *createInterpSection();
 MergeInputSection *createCommentSection();
 template <class ELFT> void splitSections();
 void mergeSections();
 
 template <typename ELFT> void writeEhdr(uint8_t *Buf, Partition &Part);
 template <typename ELFT> void writePhdrs(uint8_t *Buf, Partition &Part);
 
struct InStruct {
   PPC64LongBranchTargetSection *PPC64LongBranchTarget;
   MipsGotSection *MipsGot;
   MipsRldMapSection *MipsRldMap;
   SyntheticSection *PartEnd;
   SyntheticSection *PartIndex;
   PltSection *Plt;
   PltSection *Iplt;
   PPC32Got2Section *PPC32Got2;
+  RISCVSdataSection *RISCVSdata;
   RelocationBaseSection *RelaPlt;
   RelocationBaseSection *RelaIplt;
   StringTableSection *ShStrTab;
   StringTableSection *StrTab;
   SymbolTableBaseSection *SymTab;
   SymtabShndxSection *SymTabShndx;
 };
 
 extern InStruct In;
 
 } // namespace elf
 } // namespace lld
 
 #endif

lld/trunk/ELF/SyntheticSections.cpp

bool PPC64LongBranchTargetSection::isNeeded() const {
   // is too early to determine if this section will be empty or not. We need
   // Finalized to keep the section alive until after thunk creation. Finalized
   // only gets set to true once `finalizeSections()` is called after thunk
   // creation. Becuase of this, if we don't create any long-branch thunks we end
   // up with an empty .branch_lt section in the binary.
   return !Finalized || !Entries.empty();
 }
 
+RISCVSdataSection::RISCVSdataSection()
+    : SyntheticSection(SHF_ALLOC | SHF_WRITE, SHT_PROGBITS, 1, ".sdata") {}
+
+bool RISCVSdataSection::isNeeded() const {
+  if (!ElfSym::RISCVGlobalPointer)
+    return false;
+
+  // __global_pointer$ is defined relative to .sdata . If the section does not
+  // exist, create a dummy one.
+  for (BaseCommand *Base : getParent()->SectionCommands)
+    if (auto *ISD = dyn_cast<InputSectionDescription>(Base))
+      for (InputSection *IS : ISD->Sections)
+        if (IS != this)
+          return false;
+  return true;
+}
+
 static uint8_t getAbiVersion() {
   // MIPS non-PIC executable gets ABI version 1.
   if (Config->EMachine == EM_MIPS) {
     if (!Config->Pic && !Config->Relocatable &&
         (Config->EFlags & (EF_MIPS_PIC | EF_MIPS_CPIC)) == EF_MIPS_CPIC)
       return 1;
     return 0;
   }

lld/trunk/ELF/Writer.cpp

if (Config->EMachine == EM_PPC) {
     Add(In.PPC32Got2);
   }
 
   if (Config->EMachine == EM_PPC64) {
     In.PPC64LongBranchTarget = make<PPC64LongBranchTargetSection>();
     Add(In.PPC64LongBranchTarget);
   }
 
+  if (Config->EMachine == EM_RISCV) {
+    In.RISCVSdata = make<RISCVSdataSection>();
+    Add(In.RISCVSdata);
+  }
+
   In.GotPlt = make<GotPltSection>();
   Add(In.GotPlt);
   In.IgotPlt = make<IgotPltSection>();
   Add(In.IgotPlt);
 
   // _GLOBAL_OFFSET_TABLE_ is defined relative to either .got.plt or .got. Treat
   // it as a relocation and ensure the referenced section is created.
   if (ElfSym::GlobalOffsetTable && Config->EMachine != EM_MIPS) {
if (Main->Dynamic->Parent)
     Symtab->addSymbol(Defined{/*File=*/nullptr, "_DYNAMIC", STB_WEAK,
                               STV_HIDDEN, STT_NOTYPE,
                               /*Value=*/0, /*Size=*/0, Main->Dynamic});
 
   // Define __rel[a]_iplt_{start,end} symbols if needed.
   addRelIpltSymbols();
 
   // RISC-V's gp can address +/- 2 KiB, set it to .sdata + 0x800 if not defined.
-  if (Config->EMachine == EM_RISCV)
-    if (!dyn_cast_or_null<Defined>(Symtab->find("__global_pointer$")))
-      addOptionalRegular("__global_pointer$", findSection(".sdata"), 0x800);
+  // This symbol should only be defined in an executable.
+  if (Config->EMachine == EM_RISCV && !Config->Shared)
+    ElfSym::RISCVGlobalPointer =
+        addOptionalRegular("__global_pointer$", findSection(".sdata"), 0x800,
+                           STV_DEFAULT, STB_GLOBAL);
 
   if (Config->EMachine == EM_X86_64) {
     // On targets that support TLSDESC, _TLS_MODULE_BASE_ is defined in such a
     // way that:
     //
     // 1) Without relaxation: it produces a dynamic TLSDESC relocation that
     // computes 0.
     // 2) With LD->LE relaxation: _TLS_MODULE_BASE_@tpoff = 0 (lowest address in
template <class ELFT> void Writer<ELFT>::finalizeSections() {
   finalizeSynthetic(In.MipsGot);
   finalizeSynthetic(In.IgotPlt);
   finalizeSynthetic(In.GotPlt);
   finalizeSynthetic(In.RelaIplt);
   finalizeSynthetic(In.RelaPlt);
   finalizeSynthetic(In.Plt);
   finalizeSynthetic(In.Iplt);
   finalizeSynthetic(In.PPC32Got2);
+  finalizeSynthetic(In.RISCVSdata);
   finalizeSynthetic(In.PartIndex);
 
   // Dynamic section must be the last one in this list and dynamic
   // symbol table section (DynSymTab) must be the first one.
   for (Partition &Part : Partitions) {
     finalizeSynthetic(Part.ARMExidx);
     finalizeSynthetic(Part.DynSymTab);
     finalizeSynthetic(Part.GnuHashTab);

lld/trunk/test/ELF/riscv-gp-dummy-sdata.s

+# REQUIRES: riscv
+# RUN: llvm-mc -filetype=obj -triple=riscv32 %s -o %t.32.o
+# RUN: ld.lld -pie %t.32.o -o %t.32
+# RUN: llvm-readelf -S %t.32 | FileCheck --check-prefix=SEC %s
+# RUN: llvm-readelf -s %t.32 | FileCheck --check-prefix=SYM %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 %s -o %t.64.o
+# RUN: ld.lld -pie %t.64.o -o %t.64
+# RUN: llvm-readelf -S %t.64 | FileCheck --check-prefix=SEC %s
+# RUN: llvm-readelf -s %t.64 | FileCheck --check-prefix=SYM %s
+
+## If there is an undefined reference to __global_pointer$ but .sdata doesn't
+## exist, create a dummy one.
+
+## __global_pointer$ = .sdata+0x800
+# SEC: [ 7] .sdata PROGBITS {{0*}}00003000
+# SYM: {{0*}}00003800 0 NOTYPE GLOBAL DEFAULT 7 __global_pointer$
+
+## If __global_pointer$ is not used, don't create .sdata .
+
+# RUN: llvm-mc -filetype=obj -triple=riscv32 /dev/null -o %t.32.o
+# RUN: ld.lld -pie %t.32.o -o %t.32
+# RUN: llvm-readelf -S %t.32 | FileCheck --implicit-check-not=.sdata /dev/null
+
+lla gp, __global_pointer$

lld/trunk/test/ELF/riscv-gp.s

+# REQUIRES: riscv
+# RUN: llvm-mc -filetype=obj -triple=riscv32 %s -o %t.32.o
+# RUN: ld.lld -pie %t.32.o -o %t.32
+# RUN: llvm-readelf -s %t.32 | FileCheck --check-prefix=SYM %s
+# RUN: llvm-readelf -S %t.32 | FileCheck --check-prefix=SEC %s
+# RUN: not ld.lld -shared %t.32.o -o /dev/null 2>&1 | FileCheck --check-prefix=ERR %s
+
+# RUN: llvm-mc -filetype=obj -triple=riscv64 %s -o %t.64.o
+# RUN: ld.lld -pie %t.64.o -o %t.64
+# RUN: llvm-readelf -s %t.64 | FileCheck --check-prefix=SYM %s
+# RUN: llvm-readelf -S %t.64 | FileCheck --check-prefix=SEC %s
+# RUN: not ld.lld -shared %t.64.o -o /dev/null 2>&1 | FileCheck --check-prefix=ERR %s
+
+## __global_pointer$ = .sdata+0x800 = 0x3800
+# SEC: [ 7] .sdata PROGBITS {{0*}}00003000
+# SYM: {{0*}}00003800 0 NOTYPE GLOBAL DEFAULT 7 __global_pointer$
+
+## __global_pointer$ - 0x1000 = 4096*3-2048
+# DIS:      1000: auipc gp, 3
+# DIS-NEXT:       addi gp, gp, -2048
+
+# ERR: error: relocation R_RISCV_PCREL_HI20 cannot be used against symbol __global_pointer$; recompile with -fPIC
+
+lla gp, __global_pointer$
+
+.section .sdata,"aw"