[PATCH] D73306: llvm-addr2line: assume addresses on the command line are hexadecimal rather than attempting to guess the base based on the form of the number.

[Jordan Rupprecht via Phabricator via llvm-commits]

rupprecht added a comment.

LGTM after James' comments too. Thanks for spotting this inconsistency.



================
Comment at: llvm/test/tools/llvm-symbolizer/input-base.test:8
+
+llvm-addr2line always assumes hexadecimal, but accepts an optional 0x prefix.
+RUN: llvm-addr2line -e /dev/null -a 0x1234 | FileCheck %s
----------------
Tiny nit: my first time reading through this I read "assumes hex" as "guesses it's hex but falls back in other cases" (e.g. if the `0b` prefix is provided).

Something like "requires hex" (like you have in the docs) might be more straightforward? Same with the comment in the code.


================
Comment at: llvm/test/tools/llvm-symbolizer/input-base.test:13
+RUN: llvm-addr2line -e /dev/null -a 0b1010 | FileCheck %s --check-prefix=HEXADECIMAL-NOT-BINARY
+RUN: llvm-addr2line -e /dev/null -a 0o1234 | FileCheck %s --check-prefix=INVALID-NOT-OCTAL
+
----------------
jhenderson wrote:
> What gets printed instead here? I thought llvm-addr2line's behaviour was the same a llvm-symbolizer and that it prints the input value as-is if the value is not a valid number, so I'm surprised to see this check passes.
> 
> Does reading from /dev/null work on Windows? (I can try it out if you need me to).
> Does reading from /dev/null work on Windows? (I can try it out if you need me to).

Looks like lit handles that: https://github.com/llvm/llvm-project/blob/master/llvm/utils/lit/lit/TestRunner.py#L37


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73306