[PATCH] D63631: [llvm-objdump] Move --start-address >= --stop-address check out of the -d code.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 21 01:33:27 PDT 2019


jhenderson accepted this revision.
jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-objdump/X86/start-stop-address.test:70-71
 
-// RUN: not llvm-objdump -d %t.out --start-address=0x40 --stop-address=0x3f 2>&1 | FileCheck %s --check-prefix ERRMSG
-// RUN: not llvm-objdump -d %t.out --start-address=0x40 --stop-address=0x40 2>&1 | FileCheck %s --check-prefix ERRMSG
+// RUN: not llvm-objdump %t.out --start-address=0x40 --stop-address=0x40 2>&1 | FileCheck %s --check-prefix ERRMSG
+// RUN: not llvm-objdump %t.out --start-address=0x40 --stop-address=0x3f 2>&1 | FileCheck %s --check-prefix ERRMSG
 // ERRMSG: start address should be less than stop address.
----------------
I think this change should only remove the `-d` part, not change the ordering of the --stop-address values.


================
Comment at: llvm/tools/llvm-objdump/llvm-objdump.cpp:2065
+  if (StartAddress >= StopAddress)
+    error("start address should be less than stop address");
+
----------------
grimar wrote:
> Does it make sence to include the address values here? And/or the option name?
> It is not really clear from the error message text what is wrong it seems.
I agree that it might be nice to improve the error message. I'm okay with that being a separate change though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63631





More information about the llvm-commits mailing list