[PATCH] D67002: [llvm-nm] - Add a test case for case when we dump a symbol that belongs to a section with a broken sh_name.

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 2 03:27:02 PDT 2019


grimar added inline comments.


================
Comment at: test/tools/llvm-nm/format-sysv-section.test:42
+# RUN: yaml2obj --docnum=2 %s > %t2.o
+# RUN: llvm-nm %t2.o --format=sysv | FileCheck %s --check-prefix=ERR
+
----------------
jhenderson wrote:
> Do we get any warning message here? If we do, I think we should check it. If we don't, I feel like we should.
No, we don't have a warning, because the error is silently consumed:
https://github.com/llvm-mirror/llvm/blob/master/tools/llvm-nm/llvm-nm.cpp#L1102

There are 16 places around in total where errors are ignored in the same way.

I did a quick test to check what GNU nm do. It prints nothing, returns 1 and reports an error:

```
umb at ubuntu:~/tests/66$ nm format-sysv-section.test.tmp2.o --format=sysv
bfd plugin: format-sysv-section.test.tmp2.o: ELF section name out of range
nm: format-sysv-section.test.tmp2.o: invalid string offset 65535 >= 35 for section `.shstrtab'
nm: format-sysv-section.test.tmp2.o: invalid string offset 65535 >= 35 for section `.shstrtab'
nm: format-sysv-section.test.tmp2.o: bad value
umb at ubuntu:~/tests/66$ echo $?
1
```

Given that difference I do not think we should add warnings/errors in this patch, since it only adds a test case for
a mistype fixed and used to check we don't fail with LLVM_ENABLE_ABI_BREAKING_CHECKS.
We should probably decide what the behavior we want to have on errors in llvm-nm first?


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

https://reviews.llvm.org/D67002





More information about the llvm-commits mailing list