Page MenuHomePhabricator

[llvm-objdump] - Implement -z/--disassemble-zeroes
ClosedPublic

Authored by grimar on Dec 26 2018, 4:46 AM.

Details

Summary

This is https://bugs.llvm.org/show_bug.cgi?id=37151,

GNU objdump spec says that "Normally the disassembly output will skip blocks of zeroes.",
but currently, llvm-objdump prints them.

The patch implements the -z/--disassemble-zeroes option and switches the default to always
skip blocks of zeroes.

GNU objdump has a bit more complex heuristic for this option. For example, it tries to
skip the last zero bytes in a section if the amount of them > 3:
https://github.com/CyberGrandChallenge/binutils/blob/master/binutils/objdump.c#L1489
so it tries to avoid disassembling zeroes inserted by section alignment.
I think it is too brittle and probably not very useful. Modern linkers like LLD insert/align sections with
trap instructions and not zeroes. Hence I tried to implement simple and clear rules in this patch
(see my comments in the code).

Diff Detail

Repository
rL LLVM

Event Timeline

grimar created this revision.Dec 26 2018, 4:46 AM
atanasyan added inline comments.Dec 26 2018, 6:17 AM
test/ELF/mips-26.s
9 ↗(On Diff #179507)

Output of llvm-objdump without -z option for at least this test case looks strange:

__start:
   20008:       0c 00 80 00     jal     131072 <bar>
   2000c:       00 00 00 00     nop
   20010:       0c 00 80 10     jal     131136 <foo0+0x20040>
                ...
   20018:       00 00 00 00     nop

loc:
   20018:       00 00 00 00     nop

We skip the first nop at 20014 offset, but show nop at 20018 twice. I apply the patch at r350033.

grimar marked an inline comment as done.Dec 26 2018, 6:23 AM
grimar added inline comments.
test/ELF/mips-26.s
9 ↗(On Diff #179507)

I knew this skips the nops in this test because they are encoded as zero bytes for mips, but did not notice it shows the same offset twice. Will recheck it, thanks for looking!

grimar planned changes to this revision.Dec 27 2018, 2:42 AM
grimar updated this revision to Diff 179621.Dec 28 2018, 4:13 AM
  • Updated the implementation and test cases.
  • Rebased on top of D56123.
This revision is now accepted and ready to land.Dec 28 2018, 5:28 AM

A few suggested comment improvements and one function name from me. Otherwise looks good.

test/MC/X86/disassemble-zeroes.s
5 ↗(On Diff #179621)

and that can force -> and can force

23 ↗(On Diff #179621)

zeroes blocks. -> blocks of zeroes.

tools/llvm-objdump/llvm-objdump.cpp
272–273 ↗(On Diff #179621)
"Do not skip blocks of zeroes when disassembling the blocks of zeroes"

I think this should be simply "Do not skip blocks of zeroes when disassembling"

1309–1311 ↗(On Diff #179621)

I'd reword the second sentence as:

"This function returns the number of zero bytes that can be skipped when dumping the disassembly of the instructions in Buf."

1312 ↗(On Diff #179621)

Since this function isn't actually doing the skipping, perhaps this should be something like "countSkippableZeroBytes" or something along those lines?

1317 ↗(On Diff #179621)

amount -> number

Also, please rename the variable accordingly.

1322–1323 ↗(On Diff #179621)

earlier than -> unless (I think)

This revision was automatically updated to reflect the committed changes.
grimar marked 8 inline comments as done.