[PATCH] D75131: [XCOFF][AIX] Enable -r option for llvm-objdump

[Jason Liu via Phabricator via llvm-commits]

jasonliu marked an inline comment as done.
jasonliu added inline comments.
Herald added a subscriber: MaskRay.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:322
 relocation_iterator XCOFFObjectFile::section_rel_begin(DataRefImpl Sec) const {
-  llvm_unreachable("Not yet implemented!");
-  return relocation_iterator(RelocationRef());
+  assert(!is64Bit() && "64-bit support not implemented yet.");
+  const XCOFFSectionHeader32* SectionEntPtr = toSection32(Sec);
----------------
jhenderson wrote:
> jasonliu wrote:
> > jhenderson wrote:
> > > Does other code further up the stack check to make sure the file is not 64 bits? If not, this isn't a valid assertion, since the program could reach this point. What would happen then if assertions are disabled?
> > > 
> > > Same comment goes for all the other instances of this.
> > I agree. I will make them all report_fatal_error instead of assertion. 
> I consider `report_fatal_error` to be as bad as an assertion for code that can get to here, as the error is useless and implies an internal problem in llvm, which isn't true if it's only hit due to malformed input. If at all possible, I'd recommend using `llvm::Expected` to report errors, if it's possible within the interface (what do other ObjectFile variations do?).
 I think the report_fatal_error is appropriate here. In here, we do imply there is an internal problem in llvm for XCOFF object file, as our tooling do not support parsing 64 bit object yet (not that the input object file is malformed input).




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75131/new/

https://reviews.llvm.org/D75131