[PATCH] D85774: [XCOFF][AIX] Enable tooling support for 64 bit symbol table parsing
Digger via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 13 11:40:30 PDT 2020
DiggerLin added a comment.
I just wonder whether we can implement two separate structure XCOFFSymbolEntry32 and XCOFFSymbolEntry64 without so much union be used on currently implement.
================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:116
+ support::ubig32_t Value; // Symbol value; storage class-dependent.
+ } Entry32Type;
+
----------------
the type name Entry32Type is not easy to understand.
change to the SymNameAndValue32 ? it means the first 12 bytes are related to symbol name and symbol value .
================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:153
+ // For SectionOrLength32 in 32-bit, SectionOrLengthLowByte64 and
+ // SectionOrLengthHighByte64 in 64-bit:
+ // If the symbol type is XTY_SD or XTY_CM, the csect length.
----------------
there only SectionOrLengthLowByte64 in the union, the SectionOrLengthHighByte64 can deleted in the comment
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:744
+ CurOffset = Obj->is64Bit() ? Obj->getSymbolTableOffset64()
+ : Obj->getSymbolTableOffset32();
+ uint64_t SymbolTableSize = static_cast<uint64_t>(sizeof(XCOFFSymbolEntry)) *
----------------
can we a member function as
uint64_t XCOFFObjectFile::getSymbolTableOffset() const {
return is64Bit() ? fileHeader64()->SymbolTableOffset
: fileHeader32()->SymbolTableOffset;
}
add
CurOffset = getSymbolTableOffset() here;
================
Comment at: llvm/tools/llvm-readobj/XCOFFDumper.cpp:338
W.printHex(GetSymbolValueName(SymbolEntPtr->StorageClass),
- SymbolEntPtr->Value);
+ Obj.is64Bit() ? SymbolEntPtr->Obj64.Value
+ : SymbolEntPtr->Obj32.Value);
----------------
can we a new member function in the XCOFFSymbolRef
getValue()
and we will not see any Obj.is64Bit() here.
XCOFFSymRef->getValue()
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85774/new/
https://reviews.llvm.org/D85774
More information about the llvm-commits
mailing list