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
- rG LLVM Github Monorepo
Event Timeline
llvm/include/llvm/Object/XCOFFObjectFile.h | ||
---|---|---|
126 | 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 | There is an extra blank line here in comparison to the number of blank lines used between the next few functions below. | |
42 | This should be static or otherwise made to have internal linkage to avoid ODR violations. | |
127 | Do we want to use the version that avoids embedded NUL characters in this patch? | |
170 | It seems (Flags & (XCOFF::STYP_DATA | XCOFF::STYP_TDATA)) is a more common pattern in the codebase. | |
175 | Same comment as above. | |
299 | 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 | ObjectFile.h already include SymbolicFile.h , I think there do need SymbolicFile.h here. and also |
llvm/include/llvm/Object/XCOFFObjectFile.h | ||
---|---|---|
24 | 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 | 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 | I'd be happy with seeing this committed and having D60885 adjusted instead. | |
llvm/test/tools/llvm-objdump/xcoff-section-headers.test | ||
23 | Some indication of whether the source is C or C++ (e.g., the base name of the source file) would be helpful. | |
29 | 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 | Does it need a REQUIRES: line? |
llvm/test/tools/llvm-objdump/xcoff-section-headers.test | ||
---|---|---|
1 | No, this is dumping section headers only (not performing disassembly). |
ObjectFile.h already include SymbolicFile.h , I think there do need SymbolicFile.h here. and also
#include "llvm/Object/Binary.h"
#include "llvm/Object/Error.h"
#include "llvm/BinaryFormat/Magic.h"
#include "llvm/ADT/iterator_range.h"