[SPARC] Repair GOT references to internal symbols.

[jyknight]

Summary

They had been getting emitted as a section + offset reference, which
is bogus since the value needs to be the offset within the GOT, not
the actual address of the symbol's object.

[• rafael]

LGTM

test/CodeGen/SPARC/obj-relocs.ll

1 ↗

(On Diff #27674)

Could this be changed to used llvm-mc? If not, what asm parsing feature that is missing?

It can be done in another commit, but we should get to it.

[jyknight]

test/CodeGen/SPARC/obj-relocs.ll

1 ↗

(On Diff #27674)

llvm-mc is tested in the other test case changed here. But, having a case for directly emitting relocs separate from going through textual asm is valuable, at the least because the "%hi" asm directive can parse into 3 different relocation types, depending on context. It'd be entirely plausible to have a bug that outputs an incorrect relocation in direct object writing, but have it turn out right after going through asm serialization/parsing.

[• rafael]

Could this be changed to used llvm-mc? If not, what asm parsing feature that is missing?

It can be done in another commit, but we should get to it.

llvm-mc is tested in the other test case changed here. But, having a case for directly emitting relocs separate from going through textual asm is valuable, at the least because the "%hi" asm directive can parse into 3 different relocation types, depending on context. It'd be entirely plausible to have a bug that outputs an incorrect relocation in direct object writing, but have it turn out right after going through asm serialization/parsing.

How? If it prints the correct assembly (and we have a test for that)
and the correct assembly produces the correct .o (and we have a test
for that), what code path is missed that has a bug when producing a .o
directly?

Cheers,
Rafael

llvm/trunk/include/llvm/Support/ELFRelocs/Sparc.def

 ELF_RELOC(R_SPARC_TLS_LE_LOX10,   73)
 ELF_RELOC(R_SPARC_TLS_DTPMOD32,   74)
 ELF_RELOC(R_SPARC_TLS_DTPMOD64,   75)
 ELF_RELOC(R_SPARC_TLS_DTPOFF32,   76)
 ELF_RELOC(R_SPARC_TLS_DTPOFF64,   77)
 ELF_RELOC(R_SPARC_TLS_TPOFF32,    78)
 ELF_RELOC(R_SPARC_TLS_TPOFF64,    79)
 ELF_RELOC(R_SPARC_GOTDATA_HIX22,  80)
-ELF_RELOC(R_SPARC_GOTDATA_LOX22,  81)
+ELF_RELOC(R_SPARC_GOTDATA_LOX10,  81)
 ELF_RELOC(R_SPARC_GOTDATA_OP_HIX22,  82)
-ELF_RELOC(R_SPARC_GOTDATA_OP_LOX22,  83)
+ELF_RELOC(R_SPARC_GOTDATA_OP_LOX10,  83)
 ELF_RELOC(R_SPARC_GOTDATA_OP,     84)

llvm/trunk/lib/Target/Sparc/AsmParser/SparcAsmParser.cpp

if (name.substr(0, 1).equals_lower("r")
       RegNo = IntRegs[intVal];
       RegKind = SparcOperand::rk_IntReg;
       return true;
     }
   }
   return false;
 }
 
+// Determine if an expression contains a reference to the symbol
+// "_GLOBAL_OFFSET_TABLE_".
 static bool hasGOTReference(const MCExpr *Expr) {
   switch (Expr->getKind()) {
   case MCExpr::Target:
     if (const SparcMCExpr *SE = dyn_cast<SparcMCExpr>(Expr))
       return hasGOTReference(SE->getSubExpr());
     break;
 
   case MCExpr::Constant:
bool SparcAsmParser::matchSparcAsmModifiers(const MCExpr *&EVal,
 
   Parser.Lex(); // Eat the LParen token.
   const MCExpr *subExpr;
   if (Parser.parseParenExpression(subExpr, EndLoc))
     return false;
 
   bool isPIC = getContext().getObjectFileInfo()->getRelocM() == Reloc::PIC_;
 
+  // Ugly: if a sparc assembly expression says "%hi(...)" but the
+  // expression within contains _GLOBAL_OFFSET_TABLE_, it REALLY means
+  // %pc22. Same with %lo -> %pc10. Worse, if it doesn't contain that,
+  // the meaning depends on whether the assembler was invoked with
+  // -KPIC or not: if so, it really means %got22/%got10; if not, it
+  // actually means what it said! Sigh, historical mistakes...
+
   switch(VK) {
   default: break;
   case SparcMCExpr::VK_Sparc_LO:
     VK =  (hasGOTReference(subExpr)
            ? SparcMCExpr::VK_Sparc_PC10
            : (isPIC ? SparcMCExpr::VK_Sparc_GOT10 : VK));
     break;
   case SparcMCExpr::VK_Sparc_HI:

llvm/trunk/lib/Target/Sparc/MCTargetDesc/SparcELFObjectWriter.cpp

SparcELFObjectWriter(bool Is64Bit, uint8_t OSABI)
                                 Is64Bit ?  ELF::EM_SPARCV9 : ELF::EM_SPARC,
                                 /*HasRelocationAddend*/ true) {}
 
     ~SparcELFObjectWriter() override {}
 
   protected:
     unsigned GetRelocType(const MCValue &Target, const MCFixup &Fixup,
                           bool IsPCRel) const override;
+
+    bool needsRelocateWithSymbol(const MCSymbol &Sym,
+                                 unsigned Type) const override;
+
   };
 }
 
 unsigned SparcELFObjectWriter::GetRelocType(const MCValue &Target,
                                             const MCFixup &Fixup,
                                             bool IsPCRel) const {
 
   if (const SparcMCExpr *SExpr = dyn_cast<SparcMCExpr>(Fixup.getValue())) {
unsigned SparcELFObjectWriter::GetRelocType(const MCValue &Target,
   case Sparc::fixup_sparc_tls_ie_add:    return ELF::R_SPARC_TLS_IE_ADD;
   case Sparc::fixup_sparc_tls_le_hix22:  return ELF::R_SPARC_TLS_LE_HIX22;
   case Sparc::fixup_sparc_tls_le_lox10:  return ELF::R_SPARC_TLS_LE_LOX10;
   }
 
   return ELF::R_SPARC_NONE;
 }
 
+bool SparcELFObjectWriter::needsRelocateWithSymbol(const MCSymbol &Sym,
+                                                 unsigned Type) const {
+  switch (Type) {
+    default:
+      return false;
+
+    // All relocations that use a GOT need a symbol, not an offset, as
+    // the offset of the symbol within the section is irrelevant to
+    // where the GOT entry is. Don't need to list all the TLS entries,
+    // as they're all marked as requiring a symbol anyways.
+    case ELF::R_SPARC_GOT10:
+    case ELF::R_SPARC_GOT13:
+    case ELF::R_SPARC_GOT22:
+    case ELF::R_SPARC_GOTDATA_HIX22:
+    case ELF::R_SPARC_GOTDATA_LOX10:
+    case ELF::R_SPARC_GOTDATA_OP_HIX22:
+    case ELF::R_SPARC_GOTDATA_OP_LOX10:
+      return true;
+  }
+}
+
 MCObjectWriter *llvm::createSparcELFObjectWriter(raw_pwrite_stream &OS,
                                                  bool Is64Bit,
                                                  bool IsLittleEndian,
                                                  uint8_t OSABI) {
   MCELFObjectTargetWriter *MOTW = new SparcELFObjectWriter(Is64Bit, OSABI);
   return createELFObjectWriter(MOTW, OS, IsLittleEndian);
 }

llvm/trunk/test/CodeGen/SPARC/obj-relocs.ll

 ; RUN: llc < %s -march=sparcv9 -filetype=obj --relocation-model=static | llvm-readobj -r | FileCheck %s --check-prefix=CHECK-ABS
 ; RUN: llc < %s -march=sparcv9 -filetype=obj --relocation-model=pic    | llvm-readobj -r | FileCheck %s --check-prefix=CHECK-PIC
 
-;CHECK-ABS: Relocations [
-;CHECK-ABS:    0x{{[0-9,A-F]+}} R_SPARC_H44 AGlobalVar 0x0
-;CHECK-ABS:    0x{{[0-9,A-F]+}} R_SPARC_M44 AGlobalVar 0x0
-;CHECK-ABS:    0x{{[0-9,A-F]+}} R_SPARC_L44 AGlobalVar 0x0
-;CHECK-ABS:    0x{{[0-9,A-F]+}} R_SPARC_WDISP30 bar 0x0
-;CHECK-ABS:]
+;CHECK-ABS:      Relocations [
+;CHECK-ABS:         0x{{[0-9,A-F]+}} R_SPARC_H44 AGlobalVar 0x0
+;CHECK-ABS-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_M44 AGlobalVar 0x0
+;CHECK-ABS-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_L44 AGlobalVar 0x0
+;CHECK-ABS-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_H44 .rodata.str1.1 0x0
+;CHECK-ABS-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_M44 .rodata.str1.1 0x0
+;CHECK-ABS-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_WDISP30 bar 0x0
+;CHECK-ABS-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_L44 .rodata.str1.1 0x0
+;CHECK-ABS:      ]
 
-; CHECK-PIC: Relocations [
-; CHECK-PIC:    0x{{[0-9,A-F]+}} R_SPARC_PC22 _GLOBAL_OFFSET_TABLE_ 0x4
-; CHECK-PIC:    0x{{[0-9,A-F]+}} R_SPARC_PC10 _GLOBAL_OFFSET_TABLE_ 0x8
-; CHECK-PIC:    0x{{[0-9,A-F]+}} R_SPARC_GOT22 AGlobalVar 0x0
-; CHECK-PIC:    0x{{[0-9,A-F]+}} R_SPARC_GOT10 AGlobalVar 0x0
-; CHECK-PIC:    0x{{[0-9,A-F]+}} R_SPARC_WPLT30 bar 0x0
-; CHECK-PIC: ]
+; CHECK-PIC:      Relocations [
+; CHECK-PIC:         0x{{[0-9,A-F]+}} R_SPARC_PC22 _GLOBAL_OFFSET_TABLE_ 0x4
+; CHECK-PIC-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_PC10 _GLOBAL_OFFSET_TABLE_ 0x8
+; CHECK-PIC-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_GOT22 AGlobalVar 0x0
+; CHECK-PIC-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_GOT10 AGlobalVar 0x0
+; CHECK-PIC-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_GOT22 .L.mystr 0x0
+; CHECK-PIC-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_GOT10 .L.mystr 0x0
+; CHECK-PIC-NEXT:    0x{{[0-9,A-F]+}} R_SPARC_WPLT30 bar 0x0
+; CHECK-PIC:      ]
 
 
 @AGlobalVar = global i64 0, align 8
+@.mystr = private unnamed_addr constant [6 x i8] c"hello\00", align 1
 
 define i64 @foo(i64 %a) {
 entry:
   %0 = load i64, i64* @AGlobalVar, align 4
   %1 = add i64 %a, %0
-  %2 = call i64 @bar(i64 %1)
+  %2 = call i64 @bar(i8* getelementptr inbounds ([6 x i8], [6 x i8]* @.mystr, i32 0, i32 0), i64 %1)
   ret i64 %2
 }
 
 
-declare i64 @bar(i64)
+declare i64 @bar(i8*, i64)

llvm/trunk/test/MC/Sparc/sparc-pic.s

 ! RUN: llvm-mc %s -arch=sparcv9 --relocation-model=pic -filetype=obj | llvm-readobj -r | FileCheck %s
 
 
 ! CHECK:      Relocations [
 ! CHECK-NOT:    0x{{[0-9,A-F]+}} R_SPARC_WPLT30 .text 0xC
 ! CHECK:        0x{{[0-9,A-F]+}} R_SPARC_PC22 _GLOBAL_OFFSET_TABLE_ 0x4
 ! CHECK-NEXT:   0x{{[0-9,A-F]+}} R_SPARC_PC10 _GLOBAL_OFFSET_TABLE_ 0x8
 ! CHECK-NEXT:   0x{{[0-9,A-F]+}} R_SPARC_GOT22 AGlobalVar 0x0
 ! CHECK-NEXT:   0x{{[0-9,A-F]+}} R_SPARC_GOT10 AGlobalVar 0x0
+! CHECK-NEXT:   0x{{[0-9,A-F]+}} R_SPARC_GOT22 .LC0 0x0
+! CHECK-NEXT:   0x{{[0-9,A-F]+}} R_SPARC_GOT10 .LC0 0x0
 ! CHECK-NEXT:   0x{{[0-9,A-F]+}} R_SPARC_WPLT30 bar 0x0
 ! CHECK:      ]
 
+        .section        ".rodata"
+        .align 8
+.LC0:
+        .asciz   "string"
+        .section ".text"
         .text
         .globl  foo
         .align  4
         .type   foo,@function
 foo:
         .cfi_startproc
         save %sp, -176, %sp
         .cfi_def_cfa_register %fp
         .cfi_window_save
         .cfi_register 15, 31
 .Ltmp4:
         call .Ltmp5
 .Ltmp6:
         sethi %hi(_GLOBAL_OFFSET_TABLE_+(.Ltmp6-.Ltmp4)), %i1
 .Ltmp5:
         or %i1, %lo(_GLOBAL_OFFSET_TABLE_+(.Ltmp5-.Ltmp4)), %i1
         add %i1, %o7, %i1
         sethi %hi(AGlobalVar), %i2
         add %i2, %lo(AGlobalVar), %i2
-        ldx [%i1+%i2], %i1
-        ldx [%i1], %i1
+        ldx [%i1+%i2], %i3
+        ldx [%i3], %i3
+        sethi %hi(.LC0), %i2
+        add %i2, %lo(.LC0), %i2
+        ldx [%i1+%i2], %i4
         call bar
         add %i0, %i1, %o0
         ret
         restore %g0, %o0, %o0
 .Ltmp7:
         .size   foo, .Ltmp7-foo
         .cfi_endproc
 
         .type   AGlobalVar,@object      ! @AGlobalVar
         .section        .bss,#alloc,#write
         .globl  AGlobalVar
         .align  8
 AGlobalVar:
         .xword  0                       ! 0x0
         .size   AGlobalVar, 8
-