Adds an representation of the section header table to XCOFFObjectFile, and implements enough to dump the section headers with llvm-obdump.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
llvm/include/llvm/Object/XCOFFObjectFile.h | ||
---|---|---|
125 ↗ | (On Diff #195410) | In keeping with the ordering of overriden functions in this file, this declaration should occur between getFeatures and isRelocatableObject as that is the ordering present in the definition of class ObjectFile. |
llvm/lib/Object/XCOFFObjectFile.cpp | ||
40 ↗ | (On Diff #195410) | There is an extra blank line here in comparison to the number of blank lines used between the next few functions below. |
54 ↗ | (On Diff #195410) | This should be static or otherwise made to have internal linkage to avoid ODR violations. |
139 ↗ | (On Diff #195410) | Do we want to use the version that avoids embedded NUL characters in this patch? |
181 ↗ | (On Diff #195410) | It seems (Flags & (XCOFF::STYP_DATA | XCOFF::STYP_TDATA)) is a more common pattern in the codebase. |
186 ↗ | (On Diff #195410) | Same comment as above. |
310 ↗ | (On Diff #195410) | getObject (called below) takes a Size parameter. It can be used to check for the right number of sections without this call to checkSize. |
- Addressed @hubert.reinterpretcast comments
- Added the compiler and source used to generate the object file used in the test.
- Added test coverage of long section names (ie names of length 8 that are not null terminated)
- removed the checkSize function as there were no longer any users.
llvm/include/llvm/Object/XCOFFObjectFile.h | ||
---|---|---|
24 ↗ | (On Diff #195746) | ObjectFile.h already include SymbolicFile.h , I think there do need SymbolicFile.h here. and also |
llvm/include/llvm/Object/XCOFFObjectFile.h | ||
---|---|---|
24 ↗ | (On Diff #195746) | Good point, most of these are already included from ObjectFile.h. I'll clean up our header includes but I'll land that in a separate NFC patch. |
llvm/include/llvm/Object/XCOFFObjectFile.h | ||
---|---|---|
24 ↗ | (On Diff #195746) | Posted: https://reviews.llvm.org/D60885. I plan to commit that Monday or Tuesday and then I'll rebase this patch to pick up the changes. |
LGTM with minor changes (no need to post a new revision).
llvm/include/llvm/Object/XCOFFObjectFile.h | ||
---|---|---|
24 ↗ | (On Diff #195746) | I'd be happy with seeing this committed and having D60885 adjusted instead. |
llvm/test/tools/llvm-objdump/xcoff-section-headers.test | ||
23 ↗ | (On Diff #195746) | Some indication of whether the source is C or C++ (e.g., the base name of the source file) would be helpful. |
29 ↗ | (On Diff #195746) | Add a space after the # on this line (and the next few lines) to match the above lines. |
llvm/test/tools/llvm-objdump/xcoff-section-headers.test | ||
---|---|---|
1 ↗ | (On Diff #195746) | Does it need a REQUIRES: line? |
llvm/test/tools/llvm-objdump/xcoff-section-headers.test | ||
---|---|---|
1 ↗ | (On Diff #195746) | No, this is dumping section headers only (not performing disassembly). |