This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj] - Merge `gnu-symbols.test` to `symbols.test` and cleanup.
ClosedPublic

Authored by grimar on Dec 19 2019, 3:12 AM.

Details

Summary

This cleans up and merges gnu-symbols.test to symbols.test.
Initially gnu-symbols.test tested the following things:

  1. How symbols are printed in GNU style. It does not make sense to have a separate file for such tests.
  2. It tried to test proc-specific symbol indexes. The test was incomplete and also we already have symbol-shndx.test for that, so this part was removed.
  3. It tested --dyn-symbols and --symbols correlation. All following cases were moved to symbols.test: a) That --dyn-symbols does not trigger showing regular symbols.. b) That --symbols triggers --dyn-symbols implicitly. c) That --dyn-symbols and --symbols works fine together.

Diff Detail

Event Timeline

grimar created this revision.Dec 19 2019, 3:12 AM
Herald added a project: Restricted Project. · View Herald Transcript
jhenderson added inline comments.Dec 19 2019, 6:04 AM
llvm/test/tools/llvm-readobj/ELF/symbols.test
7

You probably want --match-full-lines here and elsewhere.

69

I think this should be "is an llvm-readobj".

MaskRay added inline comments.Dec 20 2019, 12:19 AM
llvm/test/tools/llvm-readobj/ELF/symbols.test
69

(Slightly OT: I use "an R_*_IRELATIVE". I hope I used it correctly:) )

jhenderson added inline comments.Dec 20 2019, 12:37 AM
llvm/test/tools/llvm-readobj/ELF/symbols.test
69

(Slightly OT: I use "an R_*_IRELATIVE". I hope I used it correctly:) )

(I think you do. I think the rule is that words starting with a vowel sound, which includes words where you spell out a letter at the start individually use "an". Example - you'd say "ar_..." for "R_*_IRELATIVE, or "el-el-vee-em" for llvm-*)

grimar marked an inline comment as done.Dec 20 2019, 1:12 AM
grimar added inline comments.
llvm/test/tools/llvm-readobj/ELF/symbols.test
69
grimar updated this revision to Diff 234898.Dec 20 2019, 7:25 AM
grimar marked an inline comment as done.
  • Addressed review comments.
MaskRay accepted this revision.Dec 24 2019, 3:15 PM
MaskRay added inline comments.
llvm/test/tools/llvm-readobj/ELF/symbols.test
83

Rather than "trigger ... implicitly", you may say --symbols implies --dyn-symbols for llvm-readelf.

89

{{^}} can be deleted.

The fact that DynamicSymbols [ does not follow a {{^}} made me wonder why {{^}} is not here...

This revision is now accepted and ready to land.Dec 24 2019, 3:15 PM
grimar marked 3 inline comments as done.Dec 25 2019, 4:30 AM
grimar added inline comments.
llvm/test/tools/llvm-readobj/ELF/symbols.test
83

gr8! :)

89

{{^}} can be deleted.

No. --dyn-symbols triggers DynamicSymbols [, so I can't remove {{^}}

This revision was automatically updated to reflect the committed changes.
grimar marked an inline comment as done.