[PATCH] D75131: [XCOFF][AIX] Enable -r option for llvm-objdump
Hubert Tong via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Mar 17 06:52:26 PDT 2020
hubert.reinterpretcast added inline comments.
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:324
+ if (is64Bit())
+ report_fatal_error("64-bit support not implemented yet.");
+ const XCOFFSectionHeader32* SectionEntPtr = toSection32(Sec);
----------------
jhenderson wrote:
> Here and all report_fatal_error calls: remove the trailing full stop.
We have been trying to full stops for XCOFF work for invocations of report_fatal_error, llvm_unreachable, and assert in order to be consistent within our patches and after having observed that such uses are not uncommon.
For example:
```
llvm/lib/Object/COFFObjectFile.cpp: report_fatal_error("Section was outside of section table.");
llvm/lib/Object/COFFObjectFile.cpp: report_fatal_error("Aux Symbol data was outside of symbol table.");
llvm/lib/Object/MachOObjectFile.cpp: report_fatal_error("Malformed MachO file.");
llvm/lib/Object/MachOObjectFile.cpp: report_fatal_error("Requested symbol index is out of range.");
```
================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:325
+ report_fatal_error("64-bit support not implemented yet.");
+ const XCOFFSectionHeader32* SectionEntPtr = toSection32(Sec);
+ auto RelocationsOrErr = relocations(*SectionEntPtr);
----------------
jhenderson wrote:
> hubert.reinterpretcast wrote:
> > Please replace `Typename*` binding with `Declarator *binding`.
> Isn't this handled by clang-format?
It does for me now. I'm not certain it always did so in the past.
================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:24-25
+ if (SymI == Obj->symbol_end())
+ return make_error<GenericBinaryError>("Could not get relocation symbol.",
+ object_error::parse_failed);
+
----------------
jhenderson wrote:
> Errors usually have lower case first letter and no trailing full stop.
Thanks for pointing this out. We will correct this.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75131/new/
https://reviews.llvm.org/D75131
More information about the llvm-commits
mailing list