This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by rupprecht on Aug 30 2018, 9:58 AM.

Details

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.

Diff Detail

Event Timeline

rupprecht created this revision.Aug 30 2018, 9:58 AM
tools/llvm-objcopy/Object.cpp
396

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 added inline comments.Sep 4 2018, 10:31 AM
tools/llvm-objcopy/Object.cpp
396

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 accepted this revision.Sep 4 2018, 10:34 AM
jakehehrlich added inline comments.
test/tools/llvm-objcopy/reloc-no-symtab.test
26

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.

This revision is now accepted and ready to land.Sep 4 2018, 10:34 AM
jakehehrlich requested changes to this revision.Sep 4 2018, 10:35 AM

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.

This revision now requires changes to proceed.Sep 4 2018, 10:35 AM
rupprecht updated this revision to Diff 163897.Sep 4 2018, 1:29 PM
rupprecht marked 3 inline comments as done.
  • 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

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.

Did you confirm that yaml2obj respects the explicit Link field?

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)

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.

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

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

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 accepted this revision.Sep 4 2018, 3:19 PM

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

This revision is now accepted and ready to land.Sep 4 2018, 3:19 PM
This revision was automatically updated to reflect the committed changes.