[PATCH] D61941: [llvm-objdump]Improve testing of some switches #1

[James Henderson via Phabricator via llvm-commits]

jhenderson marked 4 inline comments as done.
jhenderson added a comment.

In D61941#1504329 <https://reviews.llvm.org/D61941#1504329>, @grimar wrote:

> I only have a minor comment (inlined).
>
> Out of curiosity are you doing some code coverage check on the llvm-objdump to find the places uncovered?
>  Asking because I did that few times for LLD and this gave interesting results that time (IMO). I added many test cases and found few interesting bugs and a dead code pieces.
>  (i.e. if not - I can try to do that and share the results).


I've not been doing code coverage checks as such, but I have been inspecting the code to see what behaviour each feature has (and sometimes cross-referencing with GNU). A number of the test cases in this and the following patches that I'm still preparing are based on an internal test suite we have for a proprietary tool.



================
Comment at: test/tools/llvm-objdump/section-filter.test:66
+
+## Show non-executable sections are not disassembled even if specified,
+## and that only the specified executable sections are disassembled.
----------------
MaskRay wrote:
> objdump -d -j.data  dumps non-executable sections.
Thanks. I'll file a bug for this.


================
Comment at: test/tools/llvm-objdump/section-filter.test:74
+
+## Show that only the specified relocation sections that patch the
+## disassembled sections are dumped.
----------------
MaskRay wrote:
> This is no GNU behavior, either... `objdump -d -r -j.text` dumps `.rela.text`.
> 
> Thanks for the tests. We can improve these aspects...
I agree with this too. See the FIXME above.


================
Comment at: test/tools/llvm-objdump/section-filter.test:213
+
+## Test that an unknown section name causes all section output to be suppressed.
+# RUN: llvm-objdump %t.h.elf -h --section foobar \
----------------
MaskRay wrote:
> Apparently we missed a GNU objdump warning...
Yes, I agree. I'll file a bug for this too.


================
Comment at: test/tools/llvm-objdump/section-filter.test:222
+# NO-SECTION-NOT: .bss
+# NO-SECTION-NOT: foobar
----------------
grimar wrote:
> Will it be better/simpler just to check no output explicitly?
> i.e. something like
> 
> 
> ```
> # NO-SECTION: Sections:
> # NO-SECTION-NOT: {{.*}}
> ```
> 
> (not sure this is valid, but shows the idea)
Yes, probably. I'll try. (Note that `{{.*}}` won't work since that matchese a string with 0 or more characters, i.e. any string - you just want `{{.}}`).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D61941