[llvm-strip] Allow copying relocation sections without symbol tables.

[rupprecht]

Summary

Fixes the error "Link field value 0 in section .rela.plt is invalid" when copying/stripping certain binaries. Minimal repro:

$ cat /tmp/a.c
int main() { return 0; }
$ clang -static /tmp/a.c -o /tmp/a
$ llvm-strip /tmp/a -o /tmp/b
llvm-strip: error: Link field value 0 in section .rela.plt is invalid.

[alexander-shaposhnikov]

tools/llvm-objcopy/Object.cpp

396 ↗

(On Diff #163359)

to double check - Link is not getting set at all (if Symbols == nullptr), just wondering what the value will be in this case ? 0 or garbage ? probably i'd do what binutils objcopy does. Asking just in case.

[jakehehrlich]

tools/llvm-objcopy/Object.cpp

396 ↗

(On Diff #163359)

It will be whatever it originally was in the file but it can only be SHN_UNDEF if Symbols == nullptr. That said I think I'd slightly prefer explicitly setting it to zero here because that's correct and it only just happens to be the case that this will always work correctly.

[jakehehrlich]

test/tools/llvm-objcopy/reloc-no-symtab.test

26 ↗

(On Diff #163359)

It has been my experience that yaml2obj does not respect the Link field but instead silently sets it to the symbol table that it always constructs. You should check to see if the produced binary in fact has a link of zero for that section.

[jakehehrlich]

Sorry, I didn't mean to accept this change. I was about to and then thought about the Link issue I mentioned in the test. I don't know how to cancel an approval in phabricator so I'm marking it as "request changes". Nothing is critically wrong here, I just want to make sure the test works as intended.

[rupprecht]

• Explicitly set Link = 0 when there's no symbol, and update test case to check Link = 0 in the input file as well.

tools/llvm-objcopy/Object.cpp

396 ↗

(On Diff #163359)

It's expected to be 0 and AFAICT it will always be 0 the way it's currently written, but there's no reason to not just be explicit about it.

[alexander-shaposhnikov]

LG, thanks

[jakehehrlich]

Did you confirm that yaml2obj respects the explicit Link field?

[rupprecht]

In D51493#1223839, @jakehehrlich wrote:

Did you confirm that yaml2obj respects the explicit Link field?

Yes, I added that to the test case.

I also manually verified that changing it to some other number is respected in the output file (running yaml2obj <test file> && llvm-readobj -sections)

[jakehehrlich]

When I run that yaml2obj test I actually get the following error rather than a silent failure.

error: Unknown section referenced: '0' at YAML section '.rela.plt'.

You might need to modify yaml2obj to test this or just convince me that this doesn't need testing. Alternatively you can upload a binary to the repo in Inputs that reproduces the issue.

[jakehehrlich]

Hmm...this is odd...maybe I missed a change to yaml2obj. I'm rebuilding now.

[rupprecht]

In D51493#1223877, @jakehehrlich wrote:

When I run that yaml2obj test I actually get the following error rather than a silent failure.

error: Unknown section referenced: '0' at YAML section '.rela.plt'.

You might need to modify yaml2obj to test this or just convince me that this doesn't need testing. Alternatively you can upload a binary to the repo in Inputs that reproduces the issue.

I'm using yaml2obj from head -- maybe there's a recent change that fixed this?

$ bin/yaml2obj ~/src/llvm/test/tools/llvm-objcopy/reloc-no-symtab.test | bin/llvm-readobj -sections | grep -A 10 .rela.plt
    Name: .rela.plt (16)
    Type: SHT_RELA (0x4)
    Flags [ (0x42)
      SHF_ALLOC (0x2)
      SHF_INFO_LINK (0x40)
    ]
    Address: 0x0
    Offset: 0x240
    Size: 0
    Link: 0
    Info: 4

[rupprecht]

In D51493#1223898, @rupprecht wrote:

In D51493#1223877, @jakehehrlich wrote:

When I run that yaml2obj test I actually get the following error rather than a silent failure.

error: Unknown section referenced: '0' at YAML section '.rela.plt'.

You might need to modify yaml2obj to test this or just convince me that this doesn't need testing. Alternatively you can upload a binary to the repo in Inputs that reproduces the issue.

I'm using yaml2obj from head -- maybe there's a recent change that fixed this?

Looks like r339873 changed/fixed this.

$ bin/yaml2obj ~/src/llvm/test/tools/llvm-objcopy/reloc-no-symtab.test | bin/llvm-readobj -sections | grep -A 10 .rela.plt
    Name: .rela.plt (16)
    Type: SHT_RELA (0x4)
    Flags [ (0x42)
      SHF_ALLOC (0x2)
      SHF_INFO_LINK (0x40)
    ]
    Address: 0x0
    Offset: 0x240
    Size: 0
    Link: 0
    Info: 4

[jakehehrlich]

Confirmed, using head solves this issue; I was just using a very stale checkout.

llvm/trunk/test/tools/llvm-objcopy/reloc-no-symtab.test

+# Regression test for the following case:
+# $ cat /tmp/a.c
+# int main() { return 0; }
+# $ clang -static /tmp/a.c -o /tmp/a
+# $ llvm-strip /tmp/a -o /tmp/b
+# llvm-strip: error: Link field value 0 in section .rela.plt is invalid.
+
+# RUN: yaml2obj %s > %t.original
+# RUN: llvm-strip %t.original -o %t.stripped
+# RUN: llvm-readobj -sections %t.original | FileCheck %s
+# RUN: llvm-readobj -sections %t.stripped | FileCheck %s
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:          .text
+    Type:          SHT_PROGBITS
+    Flags:         [ SHF_ALLOC, SHF_EXECINSTR ]
+  - Name:          .rela.plt
+    Type:          SHT_RELA
+    Flags:         [ SHF_ALLOC, SHF_INFO_LINK ]
+    Info:          .got.plt
+    Link:          0
+  - Name:          .plt
+    Type:          SHT_PROGBITS
+    Flags:         [ SHF_ALLOC, SHF_EXECINSTR ]
+  - Name:          .got.plt
+    Type:          SHT_PROGBITS
+    Flags:         [ SHF_ALLOC, SHF_WRITE ]
+
+# CHECK:      Name: .rela.plt
+# CHECK-NEXT: Type: SHT_RELA
+# CHECK-NEXT: Flags [
+# CHECK-NEXT:   SHF_ALLOC
+# CHECK-NEXT:   SHF_INFO_LINK
+# CHECK-NEXT: ]
+# CHECK-NEXT: Address:
+# CHECK-NEXT: Offset:
+# CHECK-NEXT: Size: 0
+# CHECK-NEXT: Link: 0
+# CHECK-NEXT: Info: 4

llvm/trunk/tools/llvm-objcopy/Object.cpp

error("Symbol table " + Symbols->Name +
           "section " +
           this->Name);
   }
 }
 
 template <class SymTabType>
 void RelocSectionWithSymtabBase<SymTabType>::initialize(
     SectionTableRef SecTable) {
+  if (Link != SHN_UNDEF)
     setSymTab(SecTable.getSectionOfType<SymTabType>(
         Link,
-      "Link field value " + Twine(Link) + " in section " + Name + " is invalid",
+        "Link field value " + Twine(Link) + " in section " + Name +
+            " is invalid",
         "Link field value " + Twine(Link) + " in section " + Name +
             " is not a symbol table"));
 
   if (Info != SHN_UNDEF)
     setSection(SecTable.getSection(Info, "Info field value " + Twine(Info) +
                                              " in section " + Name +
                                              " is invalid"));
   else
     setSection(nullptr);
 }
 
 template <class SymTabType>
 void RelocSectionWithSymtabBase<SymTabType>::finalize() {
-  this->Link = Symbols->Index;
+  this->Link = Symbols ? Symbols->Index : 0;
+
   if (SecToApplyRel != nullptr)
     this->Info = SecToApplyRel->Index;
 }
 
 template <class ELFT>
 static void setAddend(Elf_Rel_Impl<ELFT, false> &Rel, uint64_t Addend) {}
 
 template <class ELFT>