[PATCH] D65240: [XCOFF][AIX] Generate symbol table entries with llvm-readobj
Hubert Tong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Aug 17 21:39:23 PDT 2019
hubert.reinterpretcast requested changes to this revision.
hubert.reinterpretcast added inline comments.
This revision now requires changes to proceed.
================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:136
+ XCOFF::CFileStringType Type;
+ uint8_t ReservedZero[2];
+ uint8_t AuxType;
----------------
`ReservedZeroes`?
================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:137
+ uint8_t ReservedZero[2];
+ uint8_t AuxType;
+};
----------------
Add at least a comment to indicate that this field is XCOFF64-specific.
================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:269
uint32_t getNumberOfSymbolTableEntries64() const;
+ uint32_t getSymbolIndex(const uintptr_t SymEntPtr) const;
----------------
Stray top-level `const` on a parameter declaration not attached to a function definition.
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:466
+uint32_t XCOFFObjectFile::getSymbolIndex(const uintptr_t SymbolEntPtr) const {
+ uint32_t Offset = reinterpret_cast<const char *>(SymbolEntPtr) -
----------------
Top-level `const` on parameter types are known to cause grief with certain build compilers.
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:467
+uint32_t XCOFFObjectFile::getSymbolIndex(const uintptr_t SymbolEntPtr) const {
+ uint32_t Offset = reinterpret_cast<const char *>(SymbolEntPtr) -
+ reinterpret_cast<const char *>(SymbolTblPtr);
----------------
Type error. The answer fits into 32-bits after being divided by 18, but not necessarily before that.
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:467
+uint32_t XCOFFObjectFile::getSymbolIndex(const uintptr_t SymbolEntPtr) const {
+ uint32_t Offset = reinterpret_cast<const char *>(SymbolEntPtr) -
+ reinterpret_cast<const char *>(SymbolTblPtr);
----------------
hubert.reinterpretcast wrote:
> Type error. The answer fits into 32-bits after being divided by 18, but not necessarily before that.
I suggest moving this to after the error checking.
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:477
+ : getLogicalNumberOfSymbolTableEntries32();
+ if (SymbolEntPtr >
+ reinterpret_cast<uintptr_t>(reinterpret_cast<const char *>(SymbolTblPtr) +
----------------
`>=`. The calculation on the RHS points "one-past-the-end".
================
Comment at: llvm/test/tools/llvm-readobj/xcoff-symbols.test:21
+SYMBOL32-NEXT: Type: XFT_FN (0x0)
+SYMBOL32-NEXT: AuxType: 0x0
+SYMBOL32-NEXT: }
----------------
There is no such field defined for XCOFF32.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D65240/new/
https://reviews.llvm.org/D65240
More information about the llvm-commits
mailing list