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

[Digger via Phabricator via llvm-commits]

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


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:882
+
+XCOFFTracebackTable::XCOFFTracebackTable(const uint8_t *Ptr, uint64_t Size,
+                                         Error &Err)
----------------
jasonliu wrote:
> DiggerLin wrote:
> > jasonliu wrote:
> > > We passed in a pointer and a size to this constructor function and create function, so caller knows how big the traceback table is. 
> > > I think in general we need to have a way to tell caller some of the content is not parsed when that happens. 
> > > i.e. everything in the traceback looks good, but it has some extra bytes in the end we do not recognized. 
> > > So that caller maybe able to just dump the raw bytes out if they want, and print out a general message saying:" Some of the traceback table content is not recognized and here are the raw bytes."
> > I added a data member "TBTSize" to store the valid traceback table size and API for it.
> > When the caller want  to print out the table content is not recognized , it can not the api the get the valid size.
> Sounds like we are not planning to treat extra bytes at the end of Traceback Table as an error, which kind of makes sense as we already parsed all the traceback table entries we know and it looks good. 
> Regarding on how we return the parsed traceback table size to the caller, instead of using a member function getTBTSize, have we thought about modify the create/constructor to have `uint64_t *Size` instead of `uint64_t Size`, so that the size could be passed back via the parameter? And we would need extra documentations for that as well. An example would be what we used here for `DataExtractor`: https://llvm.org/doxygen/DataExtractor_8h_source.html#l00100 
> Also we would want to have a test to test this behavior. 
yes. we do not want to  planning to treat extra bytes at the end of Traceback Table as an error(the Caller need to deal it , the caller can decide to look it a warning or error. the api also provide API). 

if return the parsed traceback table size to the caller through the construct, it means the caller can not construct the XCOFFTracebackTable from a constant . and If the caller do not want the size to be modified, the caller has to has local copy variable first  and use the 

And I think we already to document here 
 uint64_t TBTSize; // The size of the XCOFFTracebackTable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81585