Under the option --section-headers, we can only print the section types of TEXT, DATA, and BSS for now. This patch adds the DEBUG type.
Details
- Reviewers
jhenderson shchenz MaskRay Higuoxing - Group Reviewers
Restricted Project - Commits
- rGd82f2a123f9c: [llvm-objdump] Print the DEBUG type under `--section-headers`.
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
llvm/test/tools/llvm-objdump/ELF/PowerPC/section-headers-debug.test | ||
---|---|---|
30 ↗ | (On Diff #345798) | You can specify the 'Size' field of the section rather than the 'Content' field, e.g., - Name: .data Type: SHT_PROGBITS Flags: [ SHF_WRITE, SHF_ALLOC ] Size: 0x10 |
43 ↗ | (On Diff #345798) | Nit: remove the blank line. |
llvm/tools/llvm-objdump/llvm-objdump.cpp | ||
1771 | I think we need some tests to check the output when the section has multiple types, e.g., 'DATA' and 'DEBUG'. |
llvm/test/tools/llvm-objdump/ELF/PowerPC/section-headers-debug.test | ||
---|---|---|
1 ↗ | (On Diff #345798) | Rather than having a test dedicated to a specific type field, it would make more sense to have a test that tests all type fields, probably as part of a generic --section-headers test. As such, before you add the DEBUG type, I'd recommend writing a new test/expanding an existing test for --section-headers, to test that option thoroughly (a quick skim of test/tools/llvm-objdump suggests there is no such test there, but there is objdump-sectionheaders.test in llvm/test/Object, which should probably be moved). This would be a separate patch, prior to this one. Another point: the behaviour is not specific to PowerPC, and the test doesn't rely on the PowerPC target being available, so move the test into the ELF directory. |
llvm/test/tools/llvm-objdump/XCOFF/xcoff-section-headers-debug.test | ||
1 ↗ | (On Diff #345798) | Similar comments as above. Don't have a dedicated test for the DEBUG type. Just expand the existing test. Also, it would be preferable to use yaml2obj for this testing in the first place, rather than a canned binary, so rebase this patch on top of the related patches and wait for those to land. |
llvm/tools/llvm-objdump/llvm-objdump.cpp | ||
1771 | +1 - also I think this code is actually slightly incompatible with GNU output, which uses comma separators. It should be fixed to match. |
Address comment.
llvm/test/tools/llvm-objdump/XCOFF/section-headers.test | ||
---|---|---|
1–5 ↗ | (On Diff #347578) | xcoff-long-sec-names.o is useless, but xcoff-section-headers.o is used by other tests. Those tests may be replaced by yaml-docs in the future, which should be another work. |
Seems to me like this is two separate patches now: 1) adding generic DEBUG printing support to llvm-objdump, then 2) adding XCOFF recognition of debug section types.
llvm/test/tools/llvm-objdump/XCOFF/section-headers-truncate.test | ||
---|---|---|
2 ↗ | (On Diff #347588) | You can avoid a new canned binary here by using python to truncate another file. See for example llvm/test/tools/llvm-objcopy/ELF/invalid-e_phoff.test for an example of how to do this. |
llvm/test/tools/llvm-objdump/XCOFF/section-headers.test | ||
24 ↗ | (On Diff #347588) | We probably need to add addresses to some of these too. If memory serves correctly, ELF yaml2obj auto-increments addresses for allocatable sections, if an adderss is not explicitly specified. Perhaps a similar feature could be added to XCOFF yaml2obj in a separate patch? What do you think? In the meantime, the addresses can presumably be done manually here. |
llvm/test/tools/llvm-objdump/section-headers.test | ||
154–155 | I don't think we need an entirely new dedicated test case here. I think you would be better off expanding the first yaml doc in this file to cover the new DEBUG type and a case with multiple types (to pick up the separator change), and then just updating the first set of checks. |
Split the patch and will post the second patch for XCOFF soon.
llvm/test/tools/llvm-objdump/XCOFF/section-headers.test | ||
---|---|---|
24 ↗ | (On Diff #347588) |
Sounds good to me. I will implement the feature for XCOFF yaml2obj in a follow-up work. |
I don't think we need an entirely new dedicated test case here. I think you would be better off expanding the first yaml doc in this file to cover the new DEBUG type and a case with multiple types (to pick up the separator change), and then just updating the first set of checks.