This is an archive of the discontinued LLVM Phabricator instance.

[ELF][test] Add some additional .eh_frame/.eh_frame_hdr testing
ClosedPublic

Authored by jhenderson on Jun 30 2020, 3:21 AM.

Details

Summary

This patch adds a few extra cases to the existing testing for eh_frame and eh_frame_hdr behaviour in LLD. They all come from a private testsuite we are trying to migrate to lit.

Diff Detail

Event Timeline

jhenderson created this revision.Jun 30 2020, 3:21 AM
Herald added a project: Restricted Project. · View Herald Transcript
Herald added a subscriber: emaste. · View Herald Transcript
grimar added inline comments.Jun 30 2020, 4:08 AM
lld/test/ELF/eh-frame-hdr-comdat.s
15

Perhaps show the full output? (it is not that clear what is the "garbage" you're reffering to. Given that there is no garbage in the output, showing the full output is probably better).

Contents of section .eh_frame_hdr:
 200158 011b033b 14000000 01000000 4c100000
 200168 30000000

Or may be it is even better to use llvm-readobj -u, which shows both size and the number of FDEs (and explains all fields):

EHFrameHeader {
  Address: 0x200158
  Offset: 0x158
  Size: 0x14
  Corresponding Section: .eh_frame_hdr
  Header {
    version: 1
    eh_frame_ptr_enc: 0x1b
    fde_count_enc: 0x3
    table_enc: 0x3b
    eh_frame_ptr: 0x200170
    fde_count: 1
    entry 0 {
      initial_location: 0x2011a4
      address: 0x200188
    }
  }
}
lld/test/ELF/eh-frame-type.test
1–12

Use yaml2obj -DTYPE=SHT_X86_64_UNWIND instead?

jhenderson marked 2 inline comments as done.Jun 30 2020, 4:22 AM
jhenderson added inline comments.
lld/test/ELF/eh-frame-hdr-comdat.s
15

This is basically a copy of eh-frame-hdr-icf.s, with a different inptu, hence the style. I don't mind updating it though.

lld/test/ELF/eh-frame-type.test
1–12

Makes sense. Not sure why I didn't think of that before!

I made a rough check that this does not duplicate existing tests. So the addition is good.

lld/test/ELF/eh-frame-hdr-comdat.s
17

,unique,0 is not needed.

lld/test/ELF/eh-frame-merge.s
6

///

MaskRay edited reviewers, added: psmith; removed: peter.smith.Jun 30 2020, 5:31 PM
jhenderson updated this revision to Diff 274759.Jul 1 2020, 4:34 AM
jhenderson marked 4 inline comments as done.

Address review comments:

  • Use full output for eh_frame_hdr check (also match up addresses to show the output is correct).
  • Changed comment marker.
  • Used yaml2obj -D option.
  • Removed extraneous unique directive caused by copy/paste.
grimar accepted this revision.Jul 1 2020, 4:53 AM

LGTM

This revision is now accepted and ready to land.Jul 1 2020, 4:53 AM
MaskRay accepted this revision.Jul 1 2020, 4:55 PM

Thanks!

lld/test/ELF/eh-frame-type.test
13

Might be good adding another test ld.lld %t1.o %t2.o - check the two types can be mixed (clang assembled .o and GNU as assembled .o can be mixed)

Context: D76151

MaskRay retitled this revision from [ELF][test] Add some additional eh_frame/eh_frame_hdr testing to [ELF][test] Add some additional .eh_frame/.eh_frame_hdr testing.Jul 1 2020, 4:56 PM
MaskRay added inline comments.
lld/test/ELF/eh-frame-hdr-comdat.s
4

-unknown-linux can be omitted (we are testing generic ELF behavior, not specific to Linux)

jhenderson updated this revision to Diff 275095.Jul 2 2020, 6:41 AM
jhenderson marked 2 inline comments as done.

Tidy up YAML, add additional case for mixing .eh_frame of different types and simplify llvm-mc call.

This revision was automatically updated to reflect the committed changes.