This is an archive of the discontinued LLVM Phabricator instance.

[llvm-objdump] - Fix the indentation when printing dynamic tags.
ClosedPublic

Authored by grimar on Jan 16 2020, 5:26 AM.

Details

Summary

We have a bug currently: printed tag names might overlap the
value column. It happens for MIPS now.

This patch adds a logic to calculate the size of indentation on fly
to fix such issues.

Diff Detail

Event Timeline

grimar created this revision.Jan 16 2020, 5:26 AM
jhenderson added inline comments.Jan 16 2020, 6:24 AM
llvm/test/tools/llvm-objdump/elf-dynamic-section.test
434

"a values row" -> "the value column"
"a minimal reasonable indentation" -> "the minimal possible indentation"

llvm/tools/llvm-objdump/ELFDump.cpp
166

value row -> value column

grimar edited the summary of this revision. (Show Details)Jan 17 2020, 2:38 AM
grimar updated this revision to Diff 238725.EditedJan 17 2020, 2:45 AM
grimar marked 2 inline comments as done.
  • Addressed review comments.
  • Refined the comment in the test.
This revision is now accepted and ready to land.Jan 17 2020, 5:28 AM
MaskRay accepted this revision.Jan 19 2020, 11:28 AM
MaskRay added inline comments.
llvm/test/tools/llvm-objdump/elf-dynamic-section.test
437

> -> -o

I think we tend to avoid output redirection if that can be achieved with small efforts.

441

Delete --strict-whitespace.

Without --match-full-lines, the line beginning is not enforced by the "strict whitespace" rule.

443

# INDENT: {{^}}Dynamic Section:

444

# INDENT: {{^}} NEEDED 0x

grimar marked an inline comment as done.Jan 20 2020, 1:22 AM
grimar added inline comments.
llvm/test/tools/llvm-objdump/elf-dynamic-section.test
441

Actually this test checks the indentation between "NEEDED" and "0x", not the
indentation at the line beginning (which is tested above). So it was correct I think.

I do not mind testing the indentation at the beginning just in case, so I've applied
your suggestion partially, but I can't remove --strict-whitespace as it breaks the
test, because allows having any indentation between "NEEDED" and "0x".

This revision was automatically updated to reflect the committed changes.