[AIX][XCOFF] print out the traceback info

[DiggerLin]

Summary

Adding a new option -traceback-table to print out the traceback info of xcoff ojbect file.

[jhenderson]

llvm/tools/llvm-objdump/llvm-objdump.cpp

3012–3013

I'd be okay with moving the "if traceback table, enable --disassemble" logic to here, like the other cases, and dropping the "if XCOFF" check. It will simplify the code slightly, as you can then drop the !TracebackTable check from the below if clause. It also means that disassembly is printed for both XCOFF and non-XCOFF objects in the same invocation of llvm-objdump, which is good for consistency.

(If you do this, it might be worth adding a trivial test case showing that disassembly is enabled for non-XCOFF objects with --traceback-table).

[DiggerLin]

address comment

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

1437 ↗

(On Diff #482551)

thanks ,fixed it.

llvm/tools/llvm-objdump/XCOFFDump.cpp

247

thanks, fixed it.

259

thanks

llvm/tools/llvm-objdump/llvm-objdump.cpp

1954

since the patch is a big patch and last for about 3 years, if I added the "XMC_GL" now, I also need to add a test case for "XMC_GL", it will let the patch bigger. Can I have a separate patch to deal with "XMC_GL"?

2006

in the line 1629 ,End already includes the SectionAddr

uint64_t End = std::min<uint64_t>(SectionAddr + SectSize, StopAddress);

3012–3013

Sorry that I can not understand the comment, I do not change code above your comment.

[jhenderson]

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table-warning.test

1 ↗

(On Diff #513737)

4 ↗

(On Diff #513737)

Nit

14 ↗

(On Diff #513737)

Nit: normally, there isn't blank lines between parts of YAML.

35 ↗

(On Diff #513737)

Please a) review the error message content and fix it (I'm not going to point it out - it should be obvious to you with a quick look), b) review the LLVM coding standards for formatting of error and warning messages and fix accordingly. If you're still not sure what needs to be changed, please ask, but I don't want to need to explain everything in detail when things should be obvious.

There's no need for the extra space indentation at the start of the CHECK line, i.e. it should be # WARN: {{.*}} etc. Also, there's no real need for the first {{.*}}: before warning:. Finally, it would be good to check the actual file name after warning:. You can add -D FILE=%t.o to the FileCheck invocation and then use [[FILE]] in place of the {{.*}} for the file path.

llvm/tools/llvm-objdump/llvm-objdump.cpp

3012–3013

In this version of the patch, at line 2834, you add a check for (O->isXCOFF() && TracebackTable) to cause disassembly to be printed. This is different to how other command-line options, such as -S (PrintSource) cause printDisassembly to be called, if --disassemble is specified. I think you should change how --traceback-table causes disassembly to be printed, to be the same as these other options.

By changing this, there are a few effects:

1. You can remove the change you made at line 2834.
2. You can remove the !TracebackTable check at line 3239, since it will be redundant.
3. If you use --traceback-table when you provide a non-XCOFF file, disassembly will be printed, even if the user hasn't specified --disassemble. There should be a new test case to show this behaviour.

Does that make things clearer?

[DiggerLin]

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table-warning.test

1 ↗

(On Diff #513737)

thanks.

llvm/tools/llvm-objdump/llvm-objdump.cpp

3012–3013

thanks a lot for explain.

[DiggerLin]

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table-warning.test

35 ↗

(On Diff #513737)

sorry for above mistake.

[jhenderson]

As a gentle reminder, please could you make sure you do a personal review of your own code before uploading a patch for review (including updates to an existing patch). If you are working in a company, it is generally a good idea to get a second person to take a look at your code within your company before pushing to the open source community for review. This will help reduce the number of silly mistakes, improving the time it takes to get a patch through review, due to fewer iterations of the patch being needed, and increasing the desire from LLVM community reviewers , such as myself, to review your code (which again will improve the time it takes to get a patch reviewed).

llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-clobber-register.ll

49 ↗

(On Diff #514371)

Please make a separate small commit to change the comment here that is emitted.

llvm/test/tools/llvm-objdump/X86/elf-disassemble.test

4 ↗

(On Diff #514371)

I think this case deserves a comment. To avoid confusing the issue, I'd move the RUN line to the end of the test (after the checks), and add a comment saying that this shows that disassembly is enabled by default for --traceback-table, or something to that effect.

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table-warning.test

34 ↗

(On Diff #514371)

I think this line has trailing whitespace you can remove?

llvm/tools/llvm-objdump/llvm-objdump.cpp

3020

You don't need this anymore, because the Disassemble value is checked. See point 2 in my comment above.

[DiggerLin]

1. rebased to latest master , it including change Optional to std::optional. and changing test case based on patch https://reviews.llvm.org/D146071
2. address comment.

[DiggerLin]

In D89049#4276745, @jhenderson wrote:

As a gentle reminder, please could you make sure you do a personal review of your own code before uploading a patch for review (including updates to an existing patch). If you are working in a company, it is generally a good idea to get a second person to take a look at your code within your company before pushing to the open source community for review. This will help reduce the number of silly mistakes, improving the time it takes to get a patch through review, due to fewer iterations of the patch being needed, and increasing the desire from LLVM community reviewers , such as myself, to review your code (which again will improve the time it takes to get a patch reviewed).

I understand your frustration. In fact, every time when I update, I conduct a self-review, but I still have blind spots. I will ask my internal team member to do some review and reduce your work. Sorry for the inconvenient.

llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-clobber-register.ll

49 ↗

(On Diff #514371)

yes, I will do it in a separate patch.

llvm/test/tools/llvm-objdump/X86/elf-disassemble.test

4 ↗

(On Diff #514371)

I put it after the "# RUN: llvm-objdump %t.o -d | FileCheck %s --implicit-check-not=Disassembly" because option "--traceback-table" implies enable -d or "--disassemble" not implies enable "--disassemble-all".

llvm/tools/llvm-objdump/llvm-objdump.cpp

3020

thanks. actually, I take a reivew myself when I tried to update the patch, sorry for I do not find the error. thanks for your time.

[jhenderson]

Mostly looks fine now, apart from a few small things, but someone familiar with XCOFF needs to review this too.

llvm/test/tools/llvm-objdump/X86/elf-disassemble.test

6 ↗

(On Diff #515324)

Sorry, I should have mentioned this before: I reckon having a bit of extra info in the comment would be useful - see the inline edit.

llvm/tools/llvm-objdump/XCOFFDump.cpp

126

I'm going to assume that XCOFFTracebackTable::create actually needs to modify Size for some other use-case, rather than just taking it by value.

Rather than resetting the Size value a few lines down, I think it might be better to create a copy of the Size variable and pass that in, e.g:

// XCOFFTracebackTable::create modifies the size parameter, so ensure Size isn't changed.
uint64_t SizeCopy = End - Address;
Expected<XCOFFTracebackTable> TTOrErr =
    XCOFFTracebackTable::create(Bytes.data() + Index, SizeCopy, Is64Bit);

130

Don't print trailing whitespace.

148

Sorry, I missed this earlier: I don't like how you print the warning using reportWarning, and then go on to add more content to stderr outside the reportWarning function. This bakes in an assumption that reportWarning actually prints the warning to stderr. This is fine for now, but imagine if somebody wanted to add a --suppress-warnings option to llvm-objdump, or another option that changed where warnings are printed to. In those cases, the main warning would be hidden, but not the body of the message.

Better would be to use a single local stream that includes the initial part of the message and to which the rest of the warning content is printed. You then finish off this block with a call to reportWarning, passing in the whole string you built up, with the complete message.

[stephenpeckham]

The changes to the yaml2obj files allow valid XCOFF files to be generated. With the suggested changes, you can run "ld -r" on the generate object files.

Testing for this patch is difficult because the compiler doesn't generate all combinations of optional traceback-table fields.

llvm/lib/Object/XCOFFObjectFile.cpp

1464 ↗

(On Diff #482551)

This is not right. The only potential padding in a traceback table is before the EH value (if TB_EH_INFO is set), because the eh value must be aligned on a word boundary.

Removing DH.skip(2) will affect the tests.

An object with vector information and EH information will be displayed incorrectly.

llvm/test/tools/llvm-objdump/XCOFF/disassemble-invalid-traceback-table.test

24

SectionOrLength: 40
SymbolAlignmentAndType: 0x21

will generate a valid XCOFF file

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table-warning.test

22 ↗

(On Diff #515324)

SymbolAlignmentAndType: 0x21
SetionOrLength: 0x24

31 ↗

(On Diff #515324)

SymbolAlignmentAndType: 0x21

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table.test

29

SymbolAlignmentAndType: 0x21
SectionOrLength:0x24

38

SymbolAlignmentAndType: 0x21
SectionOrLength:0x38

llvm/tools/llvm-objdump/XCOFFDump.cpp

246

It's not critical, but I would use "# " in the output for the array of displacements.

[stephenpeckham]

I['m satisfied with the current state of the code. The updates I suggested to the test cases could be handled in another patch.

llvm/lib/Object/XCOFFObjectFile.cpp

1464 ↗

(On Diff #515324)

After additional investigation, I retract my previous comment. The LLVM compiler emits padding for the vector extension of the traceback table while the legacy compilers do not.

[DiggerLin]

llvm/tools/llvm-objdump/XCOFFDump.cpp

246

I will use '#' for the first one. ControlledStorageInfoDisp[0]

[DiggerLin]

llvm/test/tools/llvm-objdump/XCOFF/disassemble-invalid-traceback-table.test

7

I will delete the trail white space in next update.

28

I will align the warning(back a space) in next update.

[xingxue]

llvm/lib/Object/XCOFFObjectFile.cpp

1479 ↗

(On Diff #515324)

Wouldn't this dereference 0 if !ParmsTypeOrError is true?

llvm/tools/llvm-objdump/XCOFFDump.cpp

328

332

354

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?

llvm/tools/llvm-objdump/llvm-objdump.cpp

2006

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;

[DiggerLin]

address xing's comment

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

1479 ↗

(On Diff #515324)

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.

llvm/tools/llvm-objdump/XCOFFDump.cpp

354

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;
 }

llvm/tools/llvm-objdump/llvm-objdump.cpp

2006

good catch , thanks. I will fix it.

[xingxue]

llvm/tools/llvm-objdump/XCOFFDump.cpp

354

I meant to print ... or ...\t# Paddings to indicate the rest are paddings regardless of whether a padding byte is zero or not. @stephenpeckham @jhenderson, what do you think?

[DiggerLin]

llvm/tools/llvm-objdump/XCOFFDump.cpp

354

Print out no-zero value will letting user know what is non-zero value of the padding(in our llvm xcoff ojbect writer, the padding will always zero for sections, if not zero means ,something wrong, and printing out non-zero will let user get more info)
@hubert.reinterpretcast , do you have any option on above xing's comment ?

[DiggerLin]

reformat padding output of traceback table based on the discussion with Xingxue and stephen packham offline.

[xingxue]

Thanks for working on this feature! LGTM once CI is fixed, which is a clang-format issue.

[jhenderson]

I plan on giving this another look through next week, as by then, I should have got past my high priority stuff at work. If you don't mind, please hold off submitting until then.

[DiggerLin]

In D89049#4348866, @jhenderson wrote:

I plan on giving this another look through next week, as by then, I should have got past my high priority stuff at work. If you don't mind, please hold off submitting until then.

OK, I will hold off submitting, thanks for your time.

[DiggerLin]

gentle ping. @jhenderson

[jhenderson]

Apologies @DiggerLin, I didn't have time to look last week as the critical work I was expecting to have finished by early last week has thrown up several issues that I need to resolve urgently. Due to UK bank holidays and personal time off` this week, it won't be until next week before I have time to look at this (at the earliest). I will try to prioritise this in the upstream reviews that I look at first though.

[DiggerLin]

In D89049#4380103, @jhenderson wrote:

Apologies @DiggerLin, I didn't have time to look last week as the critical work I was expecting to have finished by early last week has thrown up several issues that I need to resolve urgently. Due to UK bank holidays and personal time off` this week, it won't be until next week before I have time to look at this (at the earliest). I will try to prioritise this in the upstream reviews that I look at first though.

Thanks a lot , @jhenderson

[jhenderson]

Pre-merge check is complaining about clang-format. Please make sure to run clang-format on all your changes.

There are a lot of cases where you've used the .value() method of std::optional. I believe operator*()/operator->() are more typical ways of achieving the same thing (and according to cppreference.com, may even be more efficient).

llvm/lib/Object/XCOFFObjectFile.cpp

1435 ↗

(On Diff #521328)

Is this change needed?

1488 ↗

(On Diff #521328)

Is this an option?

llvm/test/tools/llvm-objdump/XCOFF/disassemble-invalid-traceback-table.test

2

Sorry, didn't spot this before.

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table-warning.test

22 ↗

(On Diff #521328)

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table.test

3–4

I don't think the part about llc is relevant. It will also go out-of-date if llc should ever be able to generate it in the future.

20

How much of this text data is actually important to the test? If a significant proportion of it is, it would be worth giving instructions on how to generate the machine code that this represents.

llvm/tools/llvm-objdump/XCOFFDump.cpp

114

As this is specific to XCOFFObjectFile, I think it would make sense to pass in XCOFFObjectFile, rather than ObjectFile, and do the cast outside this function.

280

?

290

?

295

?

[jhenderson]

llvm/tools/llvm-objdump/XCOFFDump.cpp

108

?

244

255

?

322

?

336–337

I think the comment can be simplified, as suggested. Please reflow appropriately.

354

I don't have a strong opinion on whether non-zero padding should be printed or not. I'm happy to defer to others on this.

360

Don't use else after a return.

367

This is more grammatically correct.

392

I'm not sure this comment makes sense (some of the bytes can be zero, just not the first one), but regardless, I don't think it's really needed.

395

Is this supposed to be duplicated?

397

Don't end a block with a blank line.

llvm/tools/llvm-objdump/llvm-objdump.cpp

1953

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

1435 ↗

(On Diff #521328)

yes , it is not necessary .

1488 ↗

(On Diff #521328)

yes. thanks

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table.test

3–4

agree.

llvm/tools/llvm-objdump/XCOFFDump.cpp

255

thanks

395

yes, the code will make the output as

# CHECK-NEXT:      5b: 00               # Padding
# CHECK-NEXT:        ...
# CHECK-NEXT:      68: 01 00 00 00
# CHECK-NEXT:      72: 00 00 00 00
# CHECK-NEXT:        ...
# CHECK-NEXT:      90: 01 00 00 00

without the duplication code, above will be changed to

# CHECK-NEXT:      5b: 00               # Padding
# CHECK-NEXT:        ...
# CHECK-NEXT:      68: 01 00 00 00
# CHECK-NEXT:        ...
# CHECK-NEXT:      90: 01 00 00 00

[DiggerLin]

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table.test

20

there are two functions in the test case. it test different traceback table fields. and it also test different padding format.

[DiggerLin]

address James' comment

[MaskRay]

llvm/tools/llvm-objdump/XCOFFDump.cpp

143

We don't usually add parens for (A + B) > C and ... ? : A + B.

153

Nit: here and throughout, you can use '\n' as it compiles to less machine code and is slightly faster.

[jhenderson]

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table.test

42–45

(Strictly, you could use "data are" and be grammatically correct, but it sounds weird to a native speaker)

llvm/tools/llvm-objdump/XCOFFDump.cpp

332

392

Ping this comment?

395

The logic here is fairly intertwined, so I might be misunderstanding something, but if the aim is to ensure that there is always a set of zeros before the "...", I'm not convinced that this code achieves that. As far as I can tell, the first four bytes are printed and definitely have at least one non-zero byte involved. The second call though could print non-zero bytes too, so you'd end up with, e.g.:

5b: 00               # Padding
  ...
68: 01 00 00 00
72: 00 00 00 01
  ...
90: 01 00 00 00

If the next 12 bytes are then all 0, the ... will still be printed. I think?

395

I've gone back over the logic again and I'm finding it really hard to figure out. I'd like to summarise what my understanding of the goals of this code actually are, to make sure I understand things correctly:

1. Print a representation of the padding bytes from the end of the parsed data to the end of the full data.
2. Data is printed out in blocks of 4 bytes, aligned to the word boundary.
3. The first line may have fewer than 4 bytes, if the parsed data didn't end on a word boundary.
4. Consecutive blocks of 12 zeros or more are skipped and replaced with "...".
5. Any such skipped block must be preceded by a block of 4 bytes that are all zero.

Is this correct?

[DiggerLin]

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table.test

42–45

thanks

llvm/tools/llvm-objdump/XCOFFDump.cpp

153

thanks

392

yes, some bytes can be zero in the code

// Print four bytes which has non-zero value.
 if (Print4BytesOrFewer())
   return;

I delete the comment. thanks

395

it will not print as above.

after print
72: 00 00 00 01

there is code " Index -= NextLineBytesNum;"

while (Index < Size) {
   Index -= NextLineBytesNum;

to reset the index to 0x72, so there is not more than 12 consecutive bytes of zeros.
it will print next line , like

5b: 00               # Padding
  ...
0x68: 01 00 00 00
0x72: 00 00 00 01 
0x76: 00 00 00 00  
   ...
0x90: 01 00 00 00

395

yes. you are correct!

Any such skipped block must be preceded by a block of 4 bytes that are all zero.

except for the first align padding like (which do not need a block of 4 bytes at first align padding)

5b: 00               # Padding
  ...

[jhenderson]

llvm/tools/llvm-objdump/XCOFFDump.cpp

332

You missed the space between "displacement" and "(address)". Unless that's a function call (i.e. a "function" called "displacement" with a "parameter" called "address"), there should always be a space before the opening parenthesis in English prose.

349

Having read through this code a few more times, I think we can simplify it in a few ways. Here's my attempt. There might be bugs/typos in it, but hopefully it gets the gist of what I'm thinking, even if it's not 100%.

Some things that motivated this:

• The initial if (Size - Index <= 4) line looks unnecessary to me. Everything else will just work wtihout it, I believe.
• Without too much difficulty, we can get rid of the special alignment printing.
• Print4BytesOrFewer is just PrintBytes with some extra stuff, so it should probably call PrintBytes.
• I also am not convinced that the Size == Index line belongs in Print4BytesOrFewer, although I see why it's being done. I think it would be more readable in the main code logic to pull it out.
• As Print4BytesOrFewer is specific to the Padding, it might be worth renaming it for clarity, e.g. PrintPaddingLine. However, after rewriting the code to all be in a single loop, I realised that we only needed one call to PrintBytes, so the lambda can disappear completely.

// Print all padding.
const char *LineSuffix = "\t# Padding\n";
while(Index < Size) {
  // Determine how many bytes to print (4, except for the first line, which will be just enough to align to the word boundary).
  uint64_t PrintSize = 4 - Index % 4;
  PrintSize = std::min(PrintSize, Size - Index);

  // Identify if the to-be printed line contains only zero-valued bytes.
  bool LastLinePrintedWasAllZero = true;
  for (size_t I = Index; I < Index + PrintSize; ++Index)
    LastLinePrintedWasAllZero &= Bytes[I] == 0;

  // Print the line.
  PrintBytes(PrintSize);
  OS << LineSuffix;
  LineSuffix = "\n";

  // If three or more consecutive lines would only contain 0-valued bytes, replace all after the first with "...".
  if (LastLinePrintedWasAllZero) {
    // Look to find the next non-zero byte. 
    auto NextNonZero = std::find_if_not(Bytes.begin() + Index, Bytes.begin() + Size, [](uint8_t Byte){ return Byte == 0; });
    // The next two lines won't both be all zero, so proceed as normal.
    if (NextNonZero < Bytes.begin() + Index + 8)
      continue;

    // The next two lines would both be all zero. Skip them and all subsequent zero-valued lines. Make sure we only skip full lines by rounding down to the word boundary (the last line is always considered a full line).
    OS << std::string(8, ' ') << "...\n";
    Index += std::distance(Bytes.begin() + Index, NextNonZero);
    if (Index == Size)
      return;
    Index = Index - Index % 4;
  }
}

371

Don't start blocks with a blank line.

[DiggerLin]

llvm/tools/llvm-objdump/XCOFFDump.cpp

349

thanks, I got some ideal from your code to simplify my code. But I kept most of my code.

for I think some part of my code is more efficient.

for example:

// Identify if the to-be printed line contains only zero-valued bytes.
  bool LastLinePrintedWasAllZero = true;
  for (size_t I = Index; I < Index + PrintSize; ++Index)
    LastLinePrintedWasAllZero &= Bytes[I] == 0;

  // If three or more consecutive lines would only contain 0-valued bytes, replace all after the first with "...".
  if (LastLinePrintedWasAllZero) {
    // Look to find the next non-zero byte. 
    auto NextNonZero = std::find_if_not(Bytes.begin() + Index, Bytes.begin() + Size, [](uint8_t Byte){ return Byte == 0; });

    // The next two lines won't both be all zero, so proceed as normal.
if (NextNonZero < Bytes.begin() + Index + 8)
      continue;

    // The next two lines would both be all zero. Skip them and all subsequent zero-valued lines. Make sure we only skip full lines by rounding down to the word boundary (the last line is always considered a full line).
    OS << std::string(8, ' ') << "...\n";

I use

// Set the 'CurIndex' back to the first byte of the previously printed line.

uint64_t CurIndex = Index - PrintSize;

// Count the number of consecutive bytes of zeros starting from the first
// byte of the previously printed line.
while (Bytes[CurIndex] == 0 && CurIndex < Size)
  ++CurIndex;

// If three or more consecutive lines would only contain 0-valued bytes,
// replace all after the first with "...".
if (CurIndex >= Index + 8) {
   OS << std::string(8, ' ') << "...\n";

and in the code

   // The next two lines won't both be all zero, so proceed as normal.
if (NextNonZero < Bytes.begin() + Index + 8)
      continue;

we already know that "the two line won't both be all zero" , we just need to print out immediately without to check all zero again.

I use code to print out the two lines immediately

while (Index < CurIndex )
   if (Print4BytesOrFewer())
     return;

[DiggerLin]

gentle ping @jhenderson

[jhenderson]

Ah, sorry. I wrote a lengthy comment last week with another suggested approach, but somehow failed to submit it.

llvm/tools/llvm-objdump/XCOFFDump.cpp

349

I see your point, but I'm still not a fan of how the code you've written is structured, as it makes it difficult to understand and see what is going on. How does the following attempt look? This ensures we inspect each line to see if it is all 0 or not once only, but also ensures we only have one location where we call PrintBytes, and only one Index tracker.

// Print all padding.
const char *LineSuffix = "\t# Padding\n";
auto IsWordZero = [&](uint64_t WordPos) {
  if (WordPos >= Size)
    return true;
  uint64_t LineLength = 4 - Index % 4;
  LineLength = std::min(4, Size - WordPos);
  return std::all_of(Bytes + WordPos, Bytes + WordPos + LineLength, [](uint8_t Byte){ return Byte == 0; });
};
bool AreWordsZero = { IsWordZero(Index), IsWordZero(Index + 4), IsWordZero(Index + 8) };
bool ShouldPrintCurrentWord = true;
while(true) {
  // Determine the length of the line (4, except for the first line, which will be just enough to align to the word boundary, and the last line, which will be the remainder of the data).
  uint64_t LineLength = 4 - Index % 4;
  LineLength = std::min(LineLength , Size - Index);
  if (ShouldPrintLine) {
    // Print the line.
    printRawData(Bytes.slice(Index, LineLength), Address + Index, OS, STI);
    OS << LineSuffix;
    LineSuffix = "\n";
  }

  if (Index + LineLength == Size)
    return;

  // For 3 or more consecutive lines of zeros, skip all but the first one, and replace them with "...".
  if (AreWordsZero[0] && AreWordsZero[1] && AreWordsZero[2]) {
    if (ShouldPrintLine)
      OS << std::string(8, ' ') << "...\n";
    ShouldPrintLine = false;
  } else if (!ShouldPrintLine && !AreWordsZero[1]) {
    // We have reached the end of a skipped block of zeros.
    ShouldPrintLine = true;
  }
  AreWordsZero[0] = AreWordsZero[1];
  AreWordsZero[1] = AreWordsZero[2];
  AreWordsZero[2] = IsWordZero(Index + 8);
  Index += LineLength;
}

This essentially uses a sliding window approach to keep track of the upcoming words and whether they are all zero or not, whilst also tracking whether a line should be printed or not. In addition to checking each byte only once to see if it is all zero, this code also only has only one place where Index is updated (hence the use of printRawBytes rather than PrintBytes), and one place where the data is printed (those two points are the main issues I have with your current code).

[DiggerLin]

llvm/tools/llvm-objdump/XCOFFDump.cpp

349

thanks, changed as suggestion.

[MaskRay]

llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-clobber-register.ll

12 ↗

(On Diff #533994)

You may want --strict-whitespace as well, otherwise the space formatting change in the code cannot be tested.

[MaskRay]

I just took a rough look. The new code and test look good to me.

llvm/tools/llvm-objdump/XCOFFDump.cpp

140

delete blank line

141

omit braces

164

move this to the first use site and delete the blank line after the declaration.

actually, it may be better to avoid this used-only-once variable.

255

delete blank line

llvm/tools/llvm-objdump/llvm-objdump.cpp

1953

Is there a risk that SI-1 == size_t(-1) ?

[MaskRay]

1 .print out the traceback info

if there is need of original object file of xcoff-invalid-traceback-table.o and xcoff-traceback-table.o when review, I send the the object file through email.

This patch adds a major feature but the description seems a bit too concise.

[DiggerLin]

address MaskRay's comment

[DiggerLin]

llvm/tools/llvm-objdump/llvm-objdump.cpp

1953

I do not think there is a risk

for (size_t SI = 0, SE = Symbols.size(); SI != SE;) {
    // Advance SI past all the symbols starting at the same address,
    // and make an ArrayRef of them.
    unsigned FirstSI = SI;
    uint64_t Start = Symbols[SI].Addr;
    ArrayRef<SymbolInfoTy> SymbolsHere;
    while (SI != SE && Symbols[SI].Addr == Start)
      ++SI;

while (SI != SE && Symbols[SI].Addr == Start)
  ++SI;

must be run when the code first time enter the loop. so after the ++SI, the SI must be large than 0.

[DiggerLin]

rebase the patch

[jhenderson]

Apart from a couple of nits, LGTM!! Thanks for the patience, and sorry I wasn't able to review particularly efficiently.

Please don't land this without further sign off from @MaskRay and possibly @stephenpeckham.

llvm/test/CodeGen/PowerPC/aix-emit-tracebacktable-clobber-register.ll

12 ↗

(On Diff #533994)

Did you miss this comment?

llvm/tools/llvm-objdump/XCOFFDump.cpp

264

FWIW, I think the blank line that was removed here is okay to be present. I think @MaskRay was suggesting that a variable that is declared and then used in an if statement immediately afterwards should not have a blank line between it and the if statement.

A blank line after a non-trivial if block is usually a good thing, if there is code after the if statement.

[DiggerLin]

add --strict-whitespace for test case aix-emit-tracebacktable-clobber-register.ll @MaskRay

[DiggerLin]

In D89049#4454961, @jhenderson wrote:

Apart from a couple of nits, LGTM!! Thanks for the patience, and sorry I wasn't able to review particularly efficiently.

I appreciate your thorough review and taking the time to provide your inputs. I have addressed the nits you mentioned, and I'm glad to hear that you found the overall work to be acceptable. I completely understand that reviewing is time-consuming, so there's no need to apologize. Your comments and patience are greatly appreciated! , Thanks for your times. @jhenderson

[MaskRay]

I only spot checked this and this looks good.

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table.test

56

79

ditto. align

llvm/tools/llvm-objdump/ObjdumpOpts.td

66 ↗

(On Diff #535398)

I think it's time to use Group<grp_xcoff>;, then you can omit This option is for XCOFF files only since the options will be listed with a --help section named XCOFF. See grp_mach_o for an example.

[jhenderson]

llvm/tools/llvm-objdump/ObjdumpOpts.td

66 ↗

(On Diff #535398)

Makes sense to me (probably it should be a subsequent patch though).

[DiggerLin]

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table.test

56

the original output as

00000000 (idx: 0) .AddNum[PR]:
       0: 94 21 ff c0   stwu 1, -64(1)
       4: 00 00 00 00   # Traceback table start


00000024 (idx: 2) .foo[PR]:
      24: 93 e1 ff fc   stw 31, -4(1)
      28: 00 00 00 00   # Traceback table start
      2c: 00            # Version = 0

00000000 (idx: 0) .AddNum[PR]:
00000024 (idx: 2) .foo[PR]:

are label, there start from column zero.

and the test case use --strict-whitespace
so I have to keep as

CHECK:00000000 (idx: 0) .AddNum[PR]:

llvm/tools/llvm-objdump/ObjdumpOpts.td

66 ↗

(On Diff #535398)

thanks. I will do it in subsequent patch for this comment.

[stephenpeckham]

I've skimmed through the latest changes and I didn't see any additional problems.

llvm/docs/CommandGuide/llvm-objdump.rst

 
 XCOFF ONLY OPTIONS AND COMMANDS
 ---------------------------------
 
 .. option:: --symbol-description
 
   Add symbol description to disassembly output.
 
+.. option:: --traceback-table
+
+  Decode traceback table for disassembly output.
+
 BUGS
 ----
 
 To report bugs, please visit <https://bugs.llvm.org/>.
 
 SEE ALSO
 --------
 
 :manpage:`llvm-nm(1)`, :manpage:`llvm-readelf(1)`, :manpage:`llvm-readobj(1)`

llvm/test/tools/llvm-objdump/XCOFF/disassemble-invalid-traceback-table.test

+# REQUIRES: powerpc-registered-target
+
+# RUN: llvm-objdump -D --traceback-table --symbol-description %p/Inputs/xcoff-invalid-traceback-table.o 2>&1 | \
+# RUN:   FileCheck --implicit-check-not warning: --check-prefixes=WARN %s
+
+## xcoff-invalid-traceback-table.o compiled with IBM XL C/C++ for AIX, V16.1.0
+## compiler command: xlc -o xcoff-invalid-traceback-table.o -c foo.c
+## foo.c:
+## float foo(int i1,float f1,double d1,int i2, float f2) {
+##    return i1+f1+d1+i2+f2;
+##   }
+## And then modify the binary content of traceback table of the object file to make it as invalid trace back table.
+
+#WARN:     {{.*}}llvm-objdump: warning: '{{.*}}xcoff-invalid-traceback-table.o': traceback table parsing failure
+#WARN-NEXT: >>> unexpected end of data at offset 0x100 while reading [0x9a, 0x10d)
+#WARN-NEXT: >>> Raw traceback table data is:
+#WARN-NEXT:      8c: 00 00 22 40
+#WARN-NEXT:      90: 80 01 02 07
+#WARN-NEXT:      94: 5a 00 00 00
+#WARN-NEXT:      98: 00 00 00 88
+#WARN-NEXT:      9c: 00 73 66 6f
+#WARN-NEXT:      a0: 6f 00 00 00
+#WARN-NEXT:         ...
+

llvm/test/tools/llvm-objdump/XCOFF/disassemble-traceback-table.test

+# REQUIRES: powerpc-registered-target
+
+# RUN: llvm-objdump -D --traceback-table  --symbol-description %p/Inputs/xcoff-traceback-table.o | \
+# RUN:   FileCheck %s
+
+## xcoff-traceback-table.o compiled with IBM XL C/C++ for AIX, V16.1.0
+## compiler command: xlc -qaltivec -qarch=pwr6 -o xcoff-traceback-table.o -c foo.c
+## foo.c:
+
+## #include <altivec.h>
+## int AddNum(int a, int b) {
+##   return a+b;
+## }
+## vector float foo(int i1,float f1,double d1,int i2, float f2, vector float vf) {
+##    if (i1+f1+d1+i2+f2 > 0)
+##        return vec_abs(vf);
+##          }
+## And modify the binary content of traceback table of the object file to add some 
+## ControlledStorageInfo, AllocaRegister, externsion table.
+
+CHECK:      Disassembly of section .text:{{[[:space:]] *}}
+CHECK-NEXT: 00000000 (idx: 12) .AddNum:
+CHECK-NEXT:        0: 94 21 ff c0       stwu 1, -64(1)
+CHECK-NEXT:        4: 90 61 00 58       stw 3, 88(1)
+CHECK-NEXT:        8: 90 81 00 5c       stw 4, 92(1)
+CHECK-NEXT:        c: 80 61 00 58       lwz 3, 88(1)
+CHECK-NEXT:       10: 80 81 00 5c       lwz 4, 92(1)
+CHECK-NEXT:       14: 7c 63 22 14       add 3, 3, 4
+CHECK-NEXT:       18: 38 21 00 40       addi 1, 1, 64
+CHECK-NEXT:       1c: 4e 80 00 20       blr
+CHECK-NEXT:       20: 00 00 00 00       # Traceback table begin
+CHECK-NEXT:       24: 00                # Version = 0
+CHECK-NEXT:       25: 00                # Language = C
+CHECK-NEXT:       26: 20                # -isGlobalLinkage, -isOutOfLineEpilogOrPrologue
+CHECK-NEXT:                               +hasTraceBackTableOffset, -isInternalProcedure
+CHECK-NEXT:                               -hasControlledStorage, -isTOCless
+CHECK-NEXT:                               -isFloatingPointPresent
+CHECK-NEXT:                               -isFloatingPointOperationLogOrAbortEnabled
+CHECK-NEXT:       27: 40                # -isInterruptHandler, +isFuncNamePresent, -isAllocaUsed
+CHECK-NEXT:                               OnConditionDirective = 0, -isCRSaved, -isLRSaved
+CHECK-NEXT:       28: 80                # +isBackChainStored, -isFixup, NumOfFPRsSaved = 0
+CHECK-NEXT:       29: 00                # -hasVectorInfo, -hasExtensionTable, NumOfGPRsSaved = 0
+CHECK-NEXT:       2a: 02                # NumberOfFixedParms = 2
+CHECK-NEXT:       2b: 01                # NumberOfFPParms = 0, +hasParmsOnStack
+CHECK-NEXT:       2c: 00 00 00 00       # ParmsType = i, i
+CHECK-NEXT:       30: 00 00 00 20       # TraceBackTableOffset = 32
+CHECK-NEXT:       34: 00 06             # FunctionNameLen = 6
+CHECK-NEXT:       36: 41 64 64 4e       # FunctionName = AddNum
+CHECK-NEXT:       3a: 75 6d
+CHECK-NEXT:       3c: 00 00 00 00       # Paddings
+
+CHECK:       00000040 (idx: 14) .foo:
+CHECK-NEXT:       40: 93 e1 ff fc       stw 31, -4(1)
+CHECK-NEXT:       44: 93 c1 ff f8       stw 30, -8(1)
+CHECK-NEXT:       48: 94 21 ff 80       stwu 1, -128(1)
+CHECK-NEXT:       4c: 83 c2 00 08       lwz 30, 8(2)
+
+CHECK:       f4: 00 00 00 00       # Traceback table begin
+CHECK-NEXT:       f8: 00                # Version = 0
+CHECK-NEXT:       f9: 00                # Language = C
+CHECK-NEXT:       fa: 2a                # -isGlobalLinkage, -isOutOfLineEpilogOrPrologue
+CHECK-NEXT:                               +hasTraceBackTableOffset, -isInternalProcedure
+CHECK-NEXT:                               +hasControlledStorage, -isTOCless
+CHECK-NEXT:                               +isFloatingPointPresent
+CHECK-NEXT:                               -isFloatingPointOperationLogOrAbortEnabled
+CHECK-NEXT:       fb: 60                # -isInterruptHandler, +isFuncNamePresent, +isAllocaUsed
+CHECK-NEXT:                               OnConditionDirective = 0, -isCRSaved, -isLRSaved
+CHECK-NEXT:       fc: 80                # +isBackChainStored, -isFixup, NumOfFPRsSaved = 0
+CHECK-NEXT:       fd: c2                # +hasVectorInfo, +hasExtensionTable, NumOfGPRsSaved = 2
+CHECK-NEXT:       fe: 02                # NumberOfFixedParms = 2
+CHECK-NEXT:       ff: 07                # NumberOfFPParms = 3, +hasParmsOnStack
+CHECK-NEXT:      100: 2c 90 00 00       # ParmsType = i, f, d, i, f, v
+CHECK-NEXT:      104: 00 00 00 b4       # TraceBackTableOffset = 180
+CHECK-NEXT:      108: 00 00 00 03       # NumOfCtlAnchors = 3
+CHECK-NEXT:      10c: 6f 00 00 01         ControlledStorageInfoDisp[0] = 1862270977
+CHECK-NEXT:      110: 00 00 00 0a         ControlledStorageInfoDisp[1] = 10
+CHECK-NEXT:      114: 00 00 01 00         ControlledStorageInfoDisp[2] = 256
+CHECK-NEXT:      118: 00 03             # FunctionNameLen = 3
+CHECK-NEXT:      11a: 66 6f 6f          # FunctionName = foo
+CHECK-NEXT:      11d: 1f                # AllocaRegister = 31
+CHECK-NEXT:      11e: 02                # NumberOfVRSaved = 0, +isVRSavedOnStack, -hasVarArgs
+CHECK-NEXT:      11f: 03                # NumberOfVectorParms = 1, +hasVMXInstruction
+CHECK-NEXT:      120: c0 00 00 00       # VectorParmsInfoString = vf
+CHECK-NEXT:      124: 20                # ExtensionTable = TB_SSP_CANARY
+
+CHECK:      Disassembly of section .data:{{[[:space:]] *}}
+CHECK-NEXT: 00000180 (idx: 24) AddNum[TC]:
+CHECK-NEXT:      180: 00 00 01 8c       <unknown>

llvm/tools/llvm-objdump/XCOFFDump.h

 //===-- XCOFFDump.h ---------------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TOOLS_LLVM_OBJDUMP_XCOFFDUMP_H
 #define LLVM_TOOLS_LLVM_OBJDUMP_XCOFFDUMP_H
 
+#include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Object/XCOFFObjectFile.h"
+#include "llvm/Support/FormattedStream.h"
 
 namespace llvm {
 
 struct SymbolInfoTy;
 
 namespace objdump {
 Optional<XCOFF::StorageMappingClass>
 getXCOFFSymbolCsectSMC(const object::XCOFFObjectFile *Obj,
                        const object::SymbolRef &Sym);
 
 bool isLabel(const object::XCOFFObjectFile *Obj, const object::SymbolRef &Sym);
 
 std::string getXCOFFSymbolDescription(const SymbolInfoTy &SymbolInfo,
                                       StringRef SymbolName);
 
 Error getXCOFFRelocationValueString(const object::XCOFFObjectFile *Obj,
                                     const object::RelocationRef &RelRef,
                                     llvm::SmallVectorImpl<char> &Result);
+
+void dumpTracebackTable(ArrayRef<uint8_t> Bytes, uint64_t Address,
+                        formatted_raw_ostream &OS, uint64_t End,
+                        const MCSubtargetInfo &STI,
+                        const object::ObjectFile *Obj);
 } // namespace objdump
 } // namespace llvm
 #endif

llvm/tools/llvm-objdump/XCOFFDump.cpp

 /// This file implements the XCOFF-specific dumper for llvm-objdump.
 ///
 //===----------------------------------------------------------------------===//
 
 #include "XCOFFDump.h"
 
 #include "llvm-objdump.h"
 #include "llvm/Demangle/Demangle.h"
+#include "llvm/MC/MCInstPrinter.h"
+#include "llvm/Support/Endian.h"
+
+#include <sstream>
 
 using namespace llvm;
 using namespace llvm::object;
+using namespace llvm::XCOFF;
+using namespace llvm::support;
 
 Error objdump::getXCOFFRelocationValueString(const XCOFFObjectFile *Obj,
                                              const RelocationRef &Rel,
                                              SmallVectorImpl<char> &Result) {
   symbol_iterator SymI = Rel.getSymbol();
   if (SymI == Obj->symbol_end())
     return make_error<GenericBinaryError>(
         "invalid symbol reference in relocation entry",
if (SymbolInfo.XCOFFSymInfo.StorageMappingClass &&
       !SymbolInfo.XCOFFSymInfo.IsLabel) {
     const XCOFF::StorageMappingClass Smc =
         SymbolInfo.XCOFFSymInfo.StorageMappingClass.getValue();
     Result.append(("[" + XCOFF::getMappingClassString(Smc) + "]").str());
   }
 
   return Result;
 }
+
+static std::string addOffsetToValueInString(const std::string &StrMsg,
+                                            unsigned Offset) {
+  std::string Res;
+  size_t Pos = 0, PrePos = 0;
+  Pos = StrMsg.find("0x", Pos);
+  while (Pos != std::string::npos) {
+    Pos += 2;
+    Res += std::string(StrMsg, PrePos, Pos - PrePos);
+    PrePos = Pos;
+    while (Pos < StrMsg.size() && isdigit(StrMsg[Pos]))
+      ++Pos;
+    std::string NumString = StrMsg.substr(PrePos, Pos - PrePos);
+    int Num = std::stoi(NumString, 0, 16);
+    std::stringstream Stream;
+    Stream << std::hex << Num + Offset;
+    Res += Stream.str();
+    PrePos = Pos;
+    Pos = StrMsg.find("0x", Pos);
+  }
+
+  if (PrePos < StrMsg.size())
+    Res += std::string(StrMsg, PrePos);
+  return Res;
+}
+
+#define PRINTBOOL(Prefix, Obj, Field)                                          \
+  OS << Prefix << " " << ((Obj.Field()) ? "+" : "-") << #Field
+
+#define PRINTGET(Prefix, Obj, Field)                                           \
+  OS << Prefix << " " << #Field << " = " << (unsigned)(Obj.get##Field())
+
+#define SPLIT                                                                  \
+  OS << "\n";                                                                  \
+  OS.indent(TabStop)
+
+#define PRINTOPTIONAL(Field)                                                   \
+  if (TbTable.get##Field()) {                                                  \
+    OS << "\n";                                                                \
+    printRawData(Bytes.slice(Index, 4), Address + Index, OS, STI);             \
+    Index += 4;                                                                \
+    OS << "\t# " << #Field << " = " << TbTable.get##Field().getValue();        \
+  }
+
+void objdump::dumpTracebackTable(ArrayRef<uint8_t> Bytes, uint64_t Address,
+                                 formatted_raw_ostream &OS, uint64_t End,
+                                 const MCSubtargetInfo &STI,
+                                 const ObjectFile *Obj) {
+  uint64_t Index = 0;
+  unsigned TabStop = getInstStartColumn(STI) - 1;
+  // Print out backtrace table boundary.
+  printRawData(Bytes.slice(Index, 4), Address, OS, STI);
+  OS << "\t# Traceback table begin\n";
+  Index += 4;
+
+  uint64_t Size = End - Address;
+
+  Expected<XCOFFTracebackTable> TTOrErr =
+      XCOFFTracebackTable::create(Bytes.data() + Index, Size);
+
+  if (!TTOrErr) {
+    reportWarning(
+        Twine("traceback table parsing failure\n>>> ") +
+            addOffsetToValueInString(toString(TTOrErr.takeError()), Address) +
+            "\n>>> Raw traceback table data is:",
+        Obj->getFileName());
+
+    uint64_t LastNoZero = Index;
+
+    // The value of Size has been changed in function XCOFFTracebackTable::create()
+    Size = End - Address;
+
+    for (uint64_t I = Index; I < Size; I = I + 4) {
+      if (support::endian::read32be(Bytes.slice(I, 4).data()) != 0)
+        LastNoZero = (I + 4) > Size ? Size : (I + 4);
+    }
+
+    if (Size - LastNoZero <= 4)
+      LastNoZero = Size;
+
+    formatted_raw_ostream FOS(errs());
+    while (Index < LastNoZero) {
+      printRawData(Bytes.slice(Index, 4), Address + Index, FOS, STI);
+      Index += 4;
+      errs() << "\n";
+    }
+
+    // Print out all remaining zero as ...
+    if (Size - LastNoZero >= 8)
+      errs() << "\t\t...\n";
+
+    return;
+  }
+
+  XCOFFTracebackTable TbTable = *TTOrErr;
+
+  auto printOutBytes = [&](uint64_t n) {
+    printRawData(Bytes.slice(Index, n), Address + Index, OS, STI);
+    Index += n;
+  };
+
+  // Print out the first byte of 8 bytes of mandatory fields.
+  printOutBytes(1);
+  OS << format("\t# Version = %i", TbTable.getVersion()) << "\n";
+
+  // Print out the second byte of 8 bytes of mandatory fields.
+  printOutBytes(1);
+  TracebackTable::LanguageID LangId =
+      static_cast<TracebackTable::LanguageID>(TbTable.getLanguageID());
+  OS << "\t# Language = " << getNameForTracebackTableLanguageId(LangId) << "\n";
+
+  // Print out the third byte of 8 bytes of mandatory fields.
+  printOutBytes(1);
+  PRINTBOOL("\t#", TbTable, isGlobalLinkage);
+  PRINTBOOL(",", TbTable, isOutOfLineEpilogOrPrologue);
+  SPLIT;
+  PRINTBOOL("\t ", TbTable, hasTraceBackTableOffset);
+  PRINTBOOL(",", TbTable, isInternalProcedure);
+  SPLIT;
+  PRINTBOOL("\t ", TbTable, hasControlledStorage);
+  PRINTBOOL(",", TbTable, isTOCless);
+  SPLIT;
+  PRINTBOOL("\t ", TbTable, isFloatingPointPresent);
+  SPLIT;
+  PRINTBOOL("\t ", TbTable, isFloatingPointOperationLogOrAbortEnabled);
+  OS << "\n";
+
+  // Print out the 4th byte of 8 bytes of mandatory fields.
+  printOutBytes(1);
+  PRINTBOOL("\t#", TbTable, isInterruptHandler);
+  PRINTBOOL(",", TbTable, isFuncNamePresent);
+  PRINTBOOL(",", TbTable, isAllocaUsed);
+  SPLIT;
+  PRINTGET("\t ", TbTable, OnConditionDirective);
+  PRINTBOOL(",", TbTable, isCRSaved);
+  PRINTBOOL(",", TbTable, isLRSaved);
+  OS << "\n";
+
+  // Print out the 5th byte of 8 bytes of mandatory fields.
+  printOutBytes(1);
+  PRINTBOOL("\t#", TbTable, isBackChainStored);
+  PRINTBOOL(",", TbTable, isFixup);
+  PRINTGET(",", TbTable, NumOfFPRsSaved);
+  OS << "\n";
+
+  // Print out the 6th byte of 8 bytes of mandatory fields.
+  printOutBytes(1);
+  PRINTBOOL("\t#", TbTable, hasVectorInfo);
+  PRINTBOOL(",", TbTable, hasExtensionTable);
+  PRINTGET(",", TbTable, NumOfGPRsSaved);
+  OS << "\n";
+
+  // Print out the 7th byte of 8 bytes of mandatory fields.
+  printOutBytes(1);
+  PRINTGET("\t#", TbTable, NumberOfFixedParms);
+  OS << "\n";
+
+  // Print out the 8th byte of 8 bytes of mandatory fields.
+  printOutBytes(1);
+  PRINTGET("\t#", TbTable, NumberOfFPParms);
+  PRINTBOOL(",", TbTable, hasParmsOnStack);
+
+  PRINTOPTIONAL(ParmsType);
+  PRINTOPTIONAL(TraceBackTableOffset);
+  PRINTOPTIONAL(HandlerMask);
+  PRINTOPTIONAL(NumOfCtlAnchors);
+
+  if (TbTable.getControlledStorageInfoDisp()) {
+    SmallVector<uint32_t, 8> Disp =
+        TbTable.getControlledStorageInfoDisp().getValue();
+    for (unsigned I = 0; I < Disp.size(); ++I) {
+      OS << "\n";
+      printOutBytes(4);
+      OS << "\t  ControlledStorageInfoDisp[" << I << "] = " << Disp[I];
+    }
+  }
+
+  // Print out function name length and function name if there is.
+  if (TbTable.isFuncNamePresent()) {
+    uint16_t FunctionNameLen = TbTable.getFunctionName().getValue().size();
+
+    assert(FunctionNameLen > 0 &&
+           "The length of the function name must be large than zero.");
+
+    OS << "\n";
+    printOutBytes(2);
+    OS << "\t# FunctionNameLen = " << FunctionNameLen;
+
+    uint16_t RemainingBytes = FunctionNameLen;
+    bool HasPrinted = false;
+    while (RemainingBytes > 0) {
+      OS << "\n";
+      uint16_t PrintLen = RemainingBytes >= 4 ? 4 : RemainingBytes;
+      printRawData(Bytes.slice(Index, PrintLen), Address + Index, OS, STI);
+      Index += PrintLen;
+      RemainingBytes -= PrintLen;
+
+      if (!HasPrinted) {
+        OS << "\t# FunctionName = " << TbTable.getFunctionName().getValue();
+        HasPrinted = true;
+      }
+    }
+  }
+
+  if (TbTable.isAllocaUsed()) {
+    OS << "\n";
+    printOutBytes(1);
+    OS << format("\t# AllocaRegister = %u",
+                 TbTable.getAllocaRegister().getValue());
+  }
+
+  if (TbTable.getVectorExt()) {
+    OS << "\n";
+    TBVectorExt VecExt = TbTable.getVectorExt().getValue();
+    // Print first byte of VectorExt.
+    printOutBytes(1);
+    PRINTGET("\t#", VecExt, NumberOfVRSaved);
+    PRINTBOOL(",", VecExt, isVRSavedOnStack);
+    PRINTBOOL(",", VecExt, hasVarArgs);
+    OS << "\n";
+
+    // Print the second byte of VectorExt.
+    printOutBytes(1);
+    PRINTGET("\t#", VecExt, NumberOfVectorParms);
+    PRINTBOOL(",", VecExt, hasVMXInstruction);
+    OS << "\n";
+
+    printOutBytes(4);
+    OS << "\t# VectorParmsInfoString = " << VecExt.getVectorParmsInfo();
+  }
+
+  if (TbTable.getExtensionTable()) {
+    OS << "\n";
+    printOutBytes(1);
+    ExtendedTBTableFlag Flag = static_cast<ExtendedTBTableFlag>(
+        TbTable.getExtensionTable().getValue());
+    OS << "\t# ExtensionTable = " << getExtendedTBTableFlagString(Flag);
+  }
+
+  if (End == Address + Index) {
+    OS << "\n";
+    return;
+  }
+
+  // Print out all padding.
+  OS << "\n";
+  Size = End - Address;
+  uint64_t Remaining = Size - Index;
+  if (Remaining >= 8) {
+    while (Remaining > 0 && Bytes[Size - Remaining] == 0)
+      --Remaining;
+    if (Remaining == 0) {
+      OS << "\t\t...\n";
+      return;
+    }
+  }
+
+  uint16_t AlignmentLen = 4 - Index % 4;
+  printRawData(Bytes.slice(Index, AlignmentLen), Address + Index, OS, STI);
+  OS << "\t# Paddings\n";
+  Index += AlignmentLen;
+  while (Index < End - Address) {
+    printRawData(Bytes.slice(Index, 4), Address + Index, OS, STI);
+    OS << "\t# Padding\n";
+    Index += 4;
+  }
+}
+#undef PRINTBOOL
+#undef PRINTGET
+#undef SPLIT
+#undef PRINTOPTIONAL

llvm/tools/llvm-objdump/llvm-objdump.h

 //===--- llvm-objdump.h -----------------------------------------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_TOOLS_LLVM_OBJDUMP_LLVM_OBJDUMP_H
 #define LLVM_TOOLS_LLVM_OBJDUMP_LLVM_OBJDUMP_H
 
 #include "llvm/ADT/StringSet.h"
 #include "llvm/DebugInfo/DIContext.h"
 #include "llvm/MC/MCDisassembler/MCDisassembler.h"
+#include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/DataTypes.h"
+#include "llvm/Support/FormattedStream.h"
 
 namespace llvm {
 class StringRef;
 class Twine;
 
 namespace object {
 class ELFObjectFileBase;
 class ELFSectionRef;

 extern cl::opt<bool> NoLeadingAddr;
 extern cl::opt<std::string> Prefix;
 extern cl::opt<bool> PrintImmHex;
 extern cl::opt<bool> PrivateHeaders;
 extern cl::opt<bool> Relocations;
 extern cl::opt<bool> SectionHeaders;
 extern cl::opt<bool> SectionContents;
 extern cl::opt<bool> SymbolDescription;
+extern cl::opt<bool> TracebackTable;
 extern cl::opt<bool> SymbolTable;
 extern cl::opt<std::string> TripleName;
 extern cl::opt<bool> UnwindInfo;
 
 extern StringSet<> FoundSectionSet;
 
 typedef std::function<bool(llvm::object::SectionRef const &)> FilterPredicate;
 

 
 std::string getFileNameForError(const object::Archive::Child &C,
                                 unsigned Index);
 SymbolInfoTy createSymbolInfo(const object::ObjectFile *Obj,
                               const object::SymbolRef &Symbol);
 
 } // namespace objdump
 } // end namespace llvm
 
+using namespace llvm;
+unsigned getInstStartColumn(const MCSubtargetInfo &STI);
+void printRawData(llvm::ArrayRef<uint8_t> Bytes, uint64_t Address,
+                  llvm::formatted_raw_ostream &OS,
+                  llvm::MCSubtargetInfo const &STI);
 #endif

llvm/tools/llvm-objdump/llvm-objdump.cpp

 #include "llvm/MC/MCDisassembler/MCDisassembler.h"
 #include "llvm/MC/MCDisassembler/MCRelocationInfo.h"
 #include "llvm/MC/MCInst.h"
 #include "llvm/MC/MCInstPrinter.h"
 #include "llvm/MC/MCInstrAnalysis.h"
 #include "llvm/MC/MCInstrInfo.h"
 #include "llvm/MC/MCObjectFileInfo.h"
 #include "llvm/MC/MCRegisterInfo.h"
-#include "llvm/MC/MCSubtargetInfo.h"
 #include "llvm/MC/MCTargetOptions.h"
 #include "llvm/Object/Archive.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/COFFImportFile.h"
 #include "llvm/Object/ELFObjectFile.h"
 #include "llvm/Object/FaultMapParser.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/MachOUniversal.h"
static cl::alias DisassembleAllShort("D",
                                      cl::aliasopt(DisassembleAll));
 
 cl::opt<bool> objdump::SymbolDescription(
     "symbol-description",
     cl::desc("Add symbol description for disassembly. This "
              "option is for XCOFF files only"),
     cl::init(false), cl::cat(ObjdumpCat));
 
+cl::opt<bool> objdump::TracebackTable(
+    "traceback-table",
+    cl::desc("Decode traceback table for disassembly. This "
+             "option is for XCOFF files only"),
+    cl::init(false), cl::cat(ObjdumpCat));
+
 static cl::list<std::string>
     DisassembleSymbols("disassemble-symbols", cl::CommaSeparated,
                        cl::desc("List of symbols to disassemble. "
                                 "Accept demangled names when --demangle is "
                                 "specified, otherwise accept mangled names"),
                        cl::cat(ObjdumpCat));
 
 static cl::opt<bool> DisassembleZeroes(
if (Type == MachO::X86_64_RELOC_UNSIGNED && Rel.d.a > 0) {
       if (PrevType == MachO::X86_64_RELOC_SUBTRACTOR)
         return true;
     }
   }
 
   return false;
 }
 
-namespace {
-
 /// Get the column at which we want to start printing the instruction
 /// disassembly, taking into account anything which appears to the left of it.
 unsigned getInstStartColumn(const MCSubtargetInfo &STI) {
   return NoShowRawInsn ? 16 : STI.getTargetTriple().isX86() ? 40 : 24;
 }
 
+void printRawData(ArrayRef<uint8_t> Bytes, uint64_t Address,
+                  formatted_raw_ostream &OS, MCSubtargetInfo const &STI) {
+  size_t Start = OS.tell();
+  if (!NoLeadingAddr)
+    OS << format("%8" PRIx64 ":", Address);
+  if (!NoShowRawInsn) {
+    OS << ' ';
+    dumpBytes(Bytes, OS);
+  }
+
+  // The output of printInst starts with a tab. Print some spaces so that
+  // the tab has 1 column and advances to the target tab stop.
+  unsigned TabStop = getInstStartColumn(STI);
+  unsigned Column = OS.tell() - Start;
+  OS.indent(Column < TabStop - 1 ? TabStop - 1 - Column : 7 - Column % 8);
+}
+
+namespace {
+
 /// Stores a single expression representing the location of a source-level
 /// variable, along with the PC range for which that expression is valid.
 struct LiveVariable {
   DWARFLocationExpression LocExpr;
   const char *VarName;
   DWARFUnit *Unit;
   const DWARFDie FuncDie;
 
printInst(MCInstPrinter &IP, const MCInst *MI, ArrayRef<uint8_t> Bytes,
             object::SectionedAddress Address, formatted_raw_ostream &OS,
             StringRef Annot, MCSubtargetInfo const &STI, SourcePrinter *SP,
             StringRef ObjectFilename, std::vector<RelocationRef> *Rels,
             LiveVariablePrinter &LVP) {
     if (SP && (PrintSource || PrintLines))
       SP->printSourceLine(OS, Address, ObjectFilename, LVP);
     LVP.printBetweenInsts(OS, false);
 
-    size_t Start = OS.tell();
-    if (!NoLeadingAddr)
-      OS << format("%8" PRIx64 ":", Address.Address);
-    if (!NoShowRawInsn) {
-      OS << ' ';
-      dumpBytes(Bytes, OS);
-    }
-
-    // The output of printInst starts with a tab. Print some spaces so that
-    // the tab has 1 column and advances to the target tab stop.
-    unsigned TabStop = getInstStartColumn(STI);
-    unsigned Column = OS.tell() - Start;
-    OS.indent(Column < TabStop - 1 ? TabStop - 1 - Column : 7 - Column % 8);
+    printRawData(Bytes, Address.Address, OS, STI);
 
     if (MI) {
       // See MCInstPrinter::printInst. On targets where a PC relative immediate
       // is relative to the next instruction and the length of a MCInst is
       // difficult to measure (x86), this is the address of the next
       // instruction.
       uint64_t Addr =
           Address.Address + (STI.getTargetTriple().isX86() ? Bytes.size() : 0);
case Triple::hexagon:
     return HexagonPrettyPrinterInst;
   case Triple::amdgcn:
     return AMDGCNPrettyPrinterInst;
   case Triple::bpfel:
   case Triple::bpfeb:
     return BPFPrettyPrinterInst;
   }
 }
-}
+} // namespace
 
 static uint8_t getElfSymbolType(const ObjectFile *Obj, const SymbolRef &Sym) {
   assert(Obj->isELF());
   if (auto *Elf32LEObj = dyn_cast<ELF32LEObjectFile>(Obj))
     return unwrapOrError(Elf32LEObj->getSymbol(Sym.getRawDataRefImpl()),
                          Obj->getFileName())
         ->getType();
   if (auto *Elf64LEObj = dyn_cast<ELF64LEObjectFile>(Obj))

 }
 
 SymbolInfoTy objdump::createSymbolInfo(const ObjectFile *Obj,
                                        const SymbolRef &Symbol) {
   const StringRef FileName = Obj->getFileName();
   const uint64_t Addr = unwrapOrError(Symbol.getAddress(), FileName);
   const StringRef Name = unwrapOrError(Symbol.getName(), FileName);
 
-  if (Obj->isXCOFF() && SymbolDescription) {
+  if (Obj->isXCOFF() && (SymbolDescription || TracebackTable)) {
     const auto *XCOFFObj = cast<XCOFFObjectFile>(Obj);
     DataRefImpl SymbolDRI = Symbol.getRawDataRefImpl();
 
     const uint32_t SymbolIndex = XCOFFObj->getSymbolIndex(SymbolDRI.p);
     Optional<XCOFF::StorageMappingClass> Smc =
         getXCOFFSymbolCsectSMC(XCOFFObj, Symbol);
     return SymbolInfoTy(Addr, Name, Smc, SymbolIndex,
                         isLabel(XCOFFObj, Symbol));
   } else
     return SymbolInfoTy(Addr, Name,
                         Obj->isELF() ? getElfSymbolType(Obj, Symbol)
                                      : (uint8_t)ELF::STT_NOTYPE);
 }
 
 static SymbolInfoTy createDummySymbolInfo(const ObjectFile *Obj,
                                           const uint64_t Addr, StringRef &Name,
                                           uint8_t Type) {
-  if (Obj->isXCOFF() && SymbolDescription)
+  if (Obj->isXCOFF() && (SymbolDescription || TracebackTable))
     return SymbolInfoTy(Addr, Name, None, None, false);
   else
     return SymbolInfoTy(Addr, Name, Type);
 }
 
 static void
 collectLocalBranchTargets(ArrayRef<uint8_t> Bytes, const MCInstrAnalysis *MIA,
                           MCDisassembler *DisAsm, MCInstPrinter *IP,
for (unsigned SI = 0, SE = Symbols.size(); SI != SE; ++SI) {
           Index = End;
         }
       }
 
       bool CheckARMELFData = hasMappingSymbols(Obj) &&
                              Symbols[SI].Type != ELF::STT_OBJECT &&
                              !DisassembleAll;
       bool DumpARMELFData = false;
+      bool DumpTracebackTableForXCOFFFunction =
+          Obj->isXCOFF() && Section.isText() && TracebackTable &&
+          Symbols[SI].XCOFFSymInfo.StorageMappingClass &&
+          (Symbols[SI].XCOFFSymInfo.StorageMappingClass.getValue() ==
+           XCOFF::XMC_PR);
+
       formatted_raw_ostream FOS(outs());
 
       std::unordered_map<uint64_t, std::string> AllLabels;
       if (SymbolizeOperands)
         collectLocalBranchTargets(Bytes, MIA, DisAsm, IP, PrimarySTI,
                                   SectionAddr, Index, End, AllLabels);
 
       while (Index < End) {
for (unsigned SI = 0, SE = Symbols.size(); SI != SE; ++SI) {
             if (size_t N =
                     countSkippableZeroBytes(Bytes.slice(Index, MaxOffset))) {
               FOS << "\t\t..." << '\n';
               Index += N;
               continue;
             }
           }
 
+          if (DumpTracebackTableForXCOFFFunction &&
+              doesXCOFFTracebackTableBegin(Bytes.slice(Index, 4))) {
+            dumpTracebackTable(Bytes.slice(Index),
+                               SectionAddr + Index + VMAAdjustment, FOS, End,
+                               *STI, Obj);
+            Index = End;
+            continue;
+          }
+
           // Print local label if there's any.
           auto Iter = AllLabels.find(SectionAddr + Index);
           if (Iter != AllLabels.end())
             FOS << "<" << Iter->second << ">:\n";
 
           // Disassemble a real instruction or a data when disassemble all is
           // provided
           MCInst Inst;
int main(int argc, char **argv) {
   // Removes trailing separators from prefix.
   while (!Prefix.empty() && sys::path::is_separator(Prefix.back()))
     Prefix.pop_back();
 
   if (AllHeaders)
     ArchiveHeaders = FileHeaders = PrivateHeaders = Relocations =
         SectionHeaders = SymbolTable = true;
 
   if (DisassembleAll || PrintSource || PrintLines ||
       !DisassembleSymbols.empty())
     Disassemble = true;
 
   if (!ArchiveHeaders && !Disassemble && DwarfDumpType == DIDT_Null &&
       !DynamicRelocations && !FileHeaders && !PrivateHeaders && !RawClangAST &&
       !Relocations && !SectionHeaders && !SectionContents && !SymbolTable &&
       !DynamicSymbolTable && !UnwindInfo && !FaultMapSection &&
       !(MachOOpt &&
         (Bind || DataInCode || DylibId || DylibsUsed || ExportsTrie ||
          FirstPrivateHeader || IndirectSymbols || InfoPlist || LazyBind ||
          LinkOptHints || ObjcMetaData || Rebase || UniversalHeaders ||
          WeakBind || !FilterSections.empty()))) {
     cl::PrintHelpMessage();
     return 2;
   }
 
   DisasmSymbolSet.insert(DisassembleSymbols.begin(), DisassembleSymbols.end());
 
   llvm::for_each(InputFilenames, dumpInput);
 
   warnOnNoMatchForSections();
 
   return EXIT_SUCCESS;
 }