Page MenuHomePhabricator

[llvm-readobj][test] - Cleanup testing of the --sections command line option.

Authored by grimar on Jan 30 2020, 2:39 AM.



We have the ELF\sections.test to test --sections.

ELF\sections.test uses precompiled objects, it has a bug (does not test -s alias properly).
Also, we test machine specific section types in ELF\machine-specific-section-types.test,
so we probably do not need to test --sections for a MIPS object in ELF\sections.test.
I think it is enough to test ELF32 and ELF64 (we do not test ELF64 in this test).

Object/readobj-shared-object.test also tests how llvm-readobj handles
--sections. It is location is wrong, it is not complete, it uses precompiled binaries
and it duplicates the ELF\sections.test partially (it tests both ELF32 and ELF64).

We have ELF\readelf-s-alias.test that tests the -s alias for --sections in llvm-readobj
and -s as an alias for --symbols in llvm-readelf.
There is no need to have a separate test for such things.
The test for the -s alias for --sections can be included into the ELF\sections.test.
And the test for -s for llvm-readelf is already included into ELF\symbols.test.

So, this patch:

  1. Removes Object/readobj-shared-object.test.
  2. Removes ELF\readelf-s-alias.test
  3. Rewrites the ELF\sections.test.
  4. Removes ELF/Inputs/trivial.obj.elf-mipsel.

Diff Detail

Event Timeline

grimar created this revision.Jan 30 2020, 2:39 AM
MaskRay accepted this revision.Jan 30 2020, 9:48 AM
This revision is now accepted and ready to land.Jan 30 2020, 9:48 AM
jhenderson accepted this revision.Jan 31 2020, 1:31 AM

LGTM, with two comments.


You can delete "tool" from this sentence.


I'm not sure you need to change this comment. It's clear this test is testing the --sections option, so I don't think you need to be any more verbose than the old comment.

This revision was automatically updated to reflect the committed changes.