[PATCH] D57549: [llvm-objdump] - llvm-objdump can miss printing bytes at the end of a section.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 1 00:25:21 PST 2019


grimar added a comment.
Herald added a project: LLVM.

Looks correct to me. Have a few comments/nits about the test case.



================
Comment at: test/tools/llvm-objdump/X86/bytes.test:8
+foo:
+.byte 't','h','i', 's',' ','i','s',' '
+.byte 'a',' ','t','e','s','t'
----------------
I would simplify to
`.ascii "this is a test"`


================
Comment at: test/tools/llvm-objdump/X86/bytes.test:11
+# .byte 0x74, 0x68, 0x69, 0x73, 0x20, 0x69, 0x73, 0x20
+# .byte 0x61, 0x20, 0x74, 0x65, 0x73, 0x74
+
----------------
I am not sure this is a useful 2 lines of comments.
You are testing the ASCII output first of all.


================
Comment at: test/tools/llvm-objdump/X86/bytes.test:13
+
+.size foo, . - foo
+
----------------
You do not need this line I think.


================
Comment at: test/tools/llvm-objdump/X86/bytes.test:16
+# CHECK: foo:
+# CHECK:       0:       74 68 69 73 20 69 73 20         this is
+# CHECK:       8:       61 20 74 65 73 74               a test
----------------
So maybe just omit testing the bytes?

```
0:       {{*.}}         this is
8:       {{*.}}         a test
```


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57549





More information about the llvm-commits mailing list