Page MenuHomePhabricator

[llvm-objcopy]Preserve data in segments not covered by sections
ClosedPublic

Authored by jhenderson on Mar 18 2019, 5:37 AM.

Details

Summary

llvm-objcopy previously knew nothing about data in segments that wasn't covered by section headers, meaning that it wrote zeroes instead of what was there. As it is possible for this data to be useful to the loader, this patch causes llvm-objcopy to start preserving this data. Data in sections that are explicitly removed continues to be written as zeroes.

This fixes https://bugs.llvm.org/show_bug.cgi?id=41005.

The test is possibly more complicated than the current implementation warrants, but did pick up several problems with an earlier implementation, so I'd prefer to keep the test cases rather than reduce it down.

Diff Detail

Repository
rL LLVM

Event Timeline

jhenderson created this revision.Mar 18 2019, 5:37 AM

Fix the test to use a correct regex pattern, and add content to the "blob" sections to show that sections are set to zero post-removal, as desired.

The code looks good to me but the test is going to take me a while to review. I don't think I'll get to this today.

The code looks good to me but the test is going to take me a while to review. I don't think I'll get to this today.

Yeah, sorry about that. Let me know if you need any help in deciphering it!

rupprecht added inline comments.Mar 19 2019, 11:55 AM
test/tools/llvm-objcopy/ELF/preserve-segment-contents.test
26 ↗(On Diff #191121)

You could probably make this shorter just this in a loop, e.g.

with open('%/t.in', 'r+') as input:
  for offset in [
    0x2000, 0x2008, ...,
    ..., 0x5038]:
      input.seek(offset); input.write('\xDE\xAD\xBE\xEF');

Update test as suggested.

Ok, I think how the test functions is fine and it's probably a good test. It certainly tests every case I'd consider. I haven't checked that all the offsets are correct but they don't look wrong at a glance.

LGTM.

rupprecht accepted this revision.Mar 22 2019, 2:42 PM
This revision is now accepted and ready to land.Mar 22 2019, 2:42 PM

Fix an issue where the ELF header and program header table were not being updated if they were covered by a segment. This was fixed by doing the writing of segment data before writing the ELF header and program header table. Also add a test dedicated to showing that this is no longer an issue.

jhenderson requested review of this revision.Mar 25 2019, 6:45 AM

@rupprecht, @jakehehrlich, would you take a look at this again, please. I found a bug in it whilst testing that resulted in the ELF header and program header table not being updated.

rupprecht accepted this revision.Mar 25 2019, 9:20 AM
This revision is now accepted and ready to land.Mar 25 2019, 9:20 AM
This revision was automatically updated to reflect the committed changes.