[PATCH] D81585: [AIX][XCOFF][Patch1] Decode trace back table information for xcoff object file for llvm-objdump -d

[Digger via Phabricator via llvm-commits]

DiggerLin marked 5 inline comments as done.
DiggerLin added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:56
+bool doesXCOFFTracebackTableBegin(ArrayRef<uint8_t> Bytes) {
+  assert(Bytes.size() == 4 && "Traceback table started with 4 bytes zero.");
+  return support::endian::read32be(Bytes.data()) == 0;
----------------
jhenderson wrote:
> I think `return false` would be more appropriate in this case. That way you could, for example, pass in an empty array. Clearly in that situation, the bytes can't represent the start of a traceback table.
> 
> I'd also be inclined to change this name to simply `isXCOFFTracebackTable`. I don't think you need the `Begin` bit, but I don't feel strongly about this, especially since I don't have much context in this area.
> 
> I also think this function better belongs down near your other added code in this file to do with the traceback table.
when the caller call  doesXCOFFTracebackTableBegin(), it should guarantee , there is four bytes . we call assert here , just make sure  there is four bytes.

if there is  four bytes zero in the text section object file, it means the tracebacktable information will follow the fours bytes zero.

I keep the function name as  doesXCOFFTracebackTableBegin, if you strong oppose it, I can change.

  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81585