[PATCH] D89049: [AIX][XCOFF] print out the traceback info

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:1479
     if (!ParmsTypeOrError) {
       Err = ParmsTypeOrError.takeError();
       return;
----------------
xingxue wrote:
> Wouldn't this dereference 0 if `!ParmsTypeOrError` is true?
ParmsTypeOrError is an object of class Expected.

In https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/Support/Error.h#L559

there is definition of 

```
  /// Return false if there is an error.
  explicit operator bool() {
#if LLVM_ENABLE_ABI_BREAKING_CHECKS
    Unchecked = HasError;
#endif
    return !HasError;
  }
```

!ParmsTypeOrError) means there is an error.


================
Comment at: llvm/tools/llvm-objdump/XCOFFDump.cpp:378
+  if (Remaining >= 8) {
+    while (Remaining > 0 && Bytes[Size - Remaining] == 0)
+      --Remaining;
----------------
xingxue wrote:
> If the remaining bytes are paddings, why do we differentiate if a padding is zero or not? Can we just print `...` or `... # Paddings` for paddings?
it just to print out the "... # Paddings" if all the remaining is zero and there are 8  or more bytes remaining in the end. 

otherwise, it will print all the bytes out with following code.


```
 uint16_t AlignmentLen = 4 - Index % 4;
  printRawData(Bytes.slice(Index, AlignmentLen), Address + Index, OS, STI);
  OS << "\t# Padding\n";
  Index += AlignmentLen;
  while (Index < End - Address) {
    printRawData(Bytes.slice(Index, 4), Address + Index, OS, STI);
    OS << "\t# Padding\n";
    Index += 4;
  }
```



================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1793
+            dumpTracebackTable(Bytes.slice(Index),
+                               SectionAddr + Index + VMAAdjustment, FOS, End,
+                               *STI, &Obj);
----------------
xingxue wrote:
> DiggerLin wrote:
> > stephenpeckham wrote:
> > > Programs are usually linked with a non-zero .text origin, so I think you need SectionAddr + End instead of End.  
> > in the line 1629 ,End already includes the SectionAddr
> > 
> >  uint64_t End = std::min<uint64_t>(**SectionAddr + SectSize,** StopAddress);
> > in the line 1629 ,End already includes the SectionAddr
> > 
> >  uint64_t End = std::min<uint64_t>(**SectionAddr + SectSize,** StopAddress);
> 
> @stephenpeckham's comment is still valid. Although `End` was initialized with `SectionAddr` added as you pointed out, `SectionAddr` was later subtracted from it in line 1649.
> ```
>  End -= SectionAddr;
> ```
> 
good catch , thanks. I will fix it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89049