[PATCH] D139097: [ARM] Add option --print-raw-value to llvm-nm to dump raw symbol values in case of ARM

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I recommend a test with `--numeric-sort` when `--print-raw-values` is used. As I understand it the numeric sort will use the values with thumb/mips bit cleared, which will mean that when printed some symbols will look out of order. This will look a bit weird but is probably better than having different sorting orders.

I don't have any more comments. I think you'll need a LLVM binutils maintainer to approve.



================
Comment at: llvm/include/llvm/Object/ELFObjectFile.h:81
 
+  virtual Expected<uint64_t> getRawSymbolAddress(DataRefImpl Symb) const = 0;
+
----------------
Worth a comment to mention what Raw means. Something like
// As getSymbolAddress, but do not mask the Thumb bit or microMips indicator.


================
Comment at: llvm/include/llvm/Object/ELFObjectFile.h:627
+  if (!SectionAddressOrError)
+    return SectionAddressOrError.takeError();
+
----------------
The code base has many  `// TODO: Test this error.` comments, is it worth replicating them here as this is the same code pattern?

Not a strong opinion. 


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

https://reviews.llvm.org/D139097