[PATCH] D73531: [llvm-objdump] avoid crash disassembling unknown instruction

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/ARM/unknown-instr.test:1
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objdump -D -triple=thumbv8.1m.main-none-eabi -mattr=+mve %t | FileCheck %s
----------------
jhenderson wrote:
> jhenderson wrote:
> > To be clear, does this test result in a crash without your fix?
> > 
> > It would be a good idea to add a comment to the top of this test to explain what is going on, why you use the switches you do, and why you use the content you do.
> I'd still like a comment explaining what this test is doing.
Oops, you got in as I posted this :)


================
Comment at: llvm/test/tools/llvm-objdump/ARM/unknown-instr.test:5
+# This is a test case with "random" data/instructions, checking that llvm-objdump
+# doesn't crash. Disassembly of instructions can fail when it e.g. is not given
+# the right set of architecture features, for example when the source is compiled
----------------
I'd change "doesn't crash" -> handles such instructions cleanly.

Use two '#' characters for comments (i.e. "## This is...") to make them more clearly stand out as comments instead of lit/FileCheck directives.


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

https://reviews.llvm.org/D73531