Page MenuHomePhabricator

[llvm-readobj][ELF] Teach llvm-readobj to show arch specific ELF section's flags
ClosedPublic

Authored by atanasyan on Jan 17 2016, 3:11 AM.

Details

Summary

Some architecture specific ELF section flags might have the same value (for example SHF_X86_64_LARGE and SHF_HEX_GPREL) and we have to check machine architectures to select an appropriate set of possible flags.

The patch selects architecture specific flags into separate arrays ElfxxxSectionFlags and combines ElfSectionFlags and ElfxxxSectionFlags before pass to the StreamWriter::printFlags() method.

Diff Detail

Repository
rL LLVM

Event Timeline

atanasyan updated this revision to Diff 45097.Jan 17 2016, 3:11 AM
atanasyan retitled this revision from to [llvm-readobj][ELF] Teach llvm-readobj to show arch specific ELF section's flags.
atanasyan updated this object.
atanasyan added a reviewer: davide.
atanasyan set the repository for this revision to rL LLVM.
atanasyan added a subscriber: llvm-commits.
davide edited edge metadata.Jan 17 2016, 1:35 PM

The readobj changes look fine, modulo comment.

tools/llvm-readobj/ELFDumper.cpp
1161

Missing default case I guess?

Just one thing: I don't know AMDGPU target at all. I think the changes there are correct but I'd be happier if Tom or Matt can sign-off the AMDGPU specific bits of this patch.

atanasyan added inline comments.Jan 17 2016, 2:05 PM
tools/llvm-readobj/ELFDumper.cpp
1161

By default we do not need to add any additional flags to the SectionFlags container. Is it okay to put the following a bit redundant code here?

default:
  break;
davide added inline comments.Jan 17 2016, 2:11 PM
tools/llvm-readobj/ELFDumper.cpp
1161

I'm fine with less code (and happier, in general) but please make sure neither gcc or clang will complain with "non all cases covered in switch statement".

Tom, Matt do you have any notes/objections on AMDGPU part of the patch?

AMDGPU changes LGTM.

rafael accepted this revision.Jan 20 2016, 7:07 AM
rafael added a reviewer: rafael.
rafael added a subscriber: rafael.

LGTM assuming no warnings.

This revision is now accepted and ready to land.Jan 20 2016, 7:07 AM
This revision was automatically updated to reflect the committed changes.