[llvm-objdump] Remove unnecessary indentation when dumping ELF data.

[ychen]

Summary

The format string for Byte has a leading space.

Great to have this before https://reviews.llvm.org/D62964

[MaskRay]

You can edit test/tools/llvm-objdump/X86/disassemble-code-data-mix.s to check this. X86/disassemble-align.s is an example to check \t.

[ychen]

In D63393#1545389, @MaskRay wrote:

You can edit test/tools/llvm-objdump/X86/disassemble-code-data-mix.s to check this. X86/disassemble-align.s is an example to check \t.

Thank you! Will do.

[ychen]

• add test

[MaskRay]

llvm/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s

17 ↗

(On Diff #204988)

Assume you'll update this part when committing...

[ychen]

llvm/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s

17 ↗

(On Diff #204988)

I thought I don't need to. Without this change, the modified test should fail.

// CHECK: b: 74 65 73 74 20 73 74 72 test str

^

<stdin>:2:1: note: scanning from here
<stdin>:|file format ELF64-x86-64
^
<stdin>:15:2: note: possible intended match here
b:| 74 65 73 74 20 73 74 72 test str
^

[MaskRay]

llvm/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s

17 ↗

(On Diff #204988)

ok, you probably need FileCheck --strict-whitespace

[jhenderson]

llvm/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s

17 ↗

(On Diff #204988)

There are two related switches here that are important to note. I haven't inspected the test enough to figure out which are the right ones to use here. The default behaviour is for FileCheck to ignore leading and trailing whitespace, and to replace all contiguous whitespace with a single space, both in the CHECK lines and in the text it is checking again. --strict-whitespace overrides the latter of these, but not the former, whilst --match-full-lines overrides the former but not the latter. Using both means you get an exact match of the line, whitespace and everything, which is usually what you want when you are testing formatting (though you can also use regexes to achieve some of the same thing). One gotcha with doing both is that if you do this, you can't have a space character between any CHECKs and the first character you are checking, unless your line starts with a space.

Also, watch out for hard tabs, since editors will often get rid of those. If checking hard tabs, use {{\t}} explicitly (together with --strict-whitespace).

[ychen]

• update test

[ychen]

llvm/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s

17 ↗

(On Diff #204988)

Thank you @MaskRay @jhenderson. That's really helpful.

I used tr '\t' '|' to make sure there are no tabs in the output for this test.

[jhenderson]

I don't think you need the tr bit. If you have any tabs, the FileCheck will fail, because the whitespace in the lines doesn't contain any tabs.

[ychen]

• update

[ychen]

In D63393#1547972, @jhenderson wrote:

I don't think you need the tr bit. If you have any tabs, the FileCheck will fail, because the whitespace in the lines doesn't contain any tabs.

Agree. Test updated.

[jhenderson]

LGTM.

llvm/trunk/test/tools/llvm-objdump/X86/disassemble-code-data-mix.s

-// RUN: llvm-mc %s -filetype=obj -triple=x86_64-pc-linux | llvm-objdump -d - | FileCheck %s
+// RUN: llvm-mc %s -filetype=obj -triple=x86_64-pc-linux | llvm-objdump -d - | FileCheck --strict-whitespace --match-full-lines %s
         .text
         .globl  foo
         .type   foo, @function
 foo:
         pushq   %rbp
         movq    %rsp, %rbp
         movl    $0, %eax
         popq    %rbp
         ret
 
         .globl bar
         .type bar, @object
 bar:
         .string "test string"
 
-// CHECK:        b:        74 65 73 74 20 73 74 72         test str
-// CHECK-NEXT:  13:        69 6e 67 00                     ing.
-
+// CHECK:       b: 74 65 73 74 20 73 74 72         test str
+// CHECK-NEXT:      13: 69 6e 67 00                     ing.

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

 static void dumpELFData(uint64_t SectionAddr, uint64_t Index, uint64_t End,
                         ArrayRef<uint8_t> Bytes) {
   // print out data up to 8 bytes at a time in hex and ascii
   uint8_t AsciiData[9] = {'\0'};
   uint8_t Byte;
   int NumBytes = 0;
 
   for (; Index < End; ++Index) {
-    if (NumBytes == 0) {
+    if (NumBytes == 0)
       outs() << format("%8" PRIx64 ":", SectionAddr + Index);
-      outs() << "\t";
-    }
     Byte = Bytes.slice(Index)[0];
     outs() << format(" %02x", Byte);
     AsciiData[NumBytes] = isPrint(Byte) ? Byte : '.';
 
     uint8_t IndentOffset = 0;
     NumBytes++;
     if (Index == End - 1 || NumBytes > 8) {
       // Indent the space for less than 8 bytes data.