[PATCH] D61969: Fix Bugzilla ID 41862 to support checking addresses of disassembled object

[James Henderson via Phabricator via llvm-commits]

jhenderson requested changes to this revision.
jhenderson added a comment.
This revision now requires changes to proceed.

In D61969#1515166 <https://reviews.llvm.org/D61969#1515166>, @xerofoify wrote:

> This is the updated patch. I wasn't sure if the test cases are correct, not sure if the syntax has issues. Otherwise this should be fixed now and thanks for the comments.


I'm not sure what you mean by "not sure if the syntax has issues"? If it's syntactically invalid, the test won't work, I think (indeed, I don't think it is). Have you run the tests?



================
Comment at: llvm/test/tools/llvm-objdump/X86/start-stop-address.test:71
 // RUN: not llvm-objdump -d %t.out --start-address=0x40 --stop-address=0x3f 2>&1 | FileCheck %s --check-prefix ERRMSG
-// ERRMSG: error: Start address should be less than stop address.
+// RUN: not llvm-objdump -d %t.out --start-address=0x40 --stop-address=0x40 2=&1 | FileCheck %s --check-prefix ERRMSG
+// ERRMSG: error: start address should be less than stop address.
----------------
2=&1 is not correct. You should have 2>&1 - that redirects stderr and merges it to stdout, so that all output and error messages are then passed to FileCheck. The syntax is the same as bash etc here.


================
Comment at: llvm/test/tools/llvm-objdump/X86/start-stop-address.test:73
+// ERRMSG: error: start address should be less than stop address.
+// RUN: not llvm-objdump -d %t.out --start-address=0x3f --stop-address=0x40 1>&2 | FileCheck %s --check-prefix ERRMSG
+// ERRMSG: error: the stop address should be after the start address
----------------
1>&2 will redirect stdout to stderr which isn't what you want to do. Also, this behaviour makes no sense.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:1408-1409
+    error("error: start address should be less than stop address");
+  if (StartAddress < StopAddress)
+   error("error: the stop address should be after the start address");
 
----------------
Ummm... what?

We now have:
Start address == stop address -> error
Start address < stop address -> error
Start address > stop address -> error

Under what circumstances do we not emit an error? Why do you need the second if here?


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

https://reviews.llvm.org/D61969