[PATCH] D87612: [llvm-readobj][test] - Improve section-symbols.test

George Rimar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 15 01:36:18 PDT 2020


grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/section-symbols.test:115-118
+## Test --relocations option.
+## FIXME: relocations should print section symbol names when them are not empty.
+# RUN: llvm-readobj %t1 --relocations 2>&1 | FileCheck %s -DFILE=%t1 --check-prefix=LLVM-RELOCS
+# RUN: llvm-readelf %t1 --relocations 2>&1 | FileCheck %s -DFILE=%t1 --check-prefix=GNU-RELOCS
----------------
jhenderson wrote:
> Is there any reason to do this in a separate run? You could do `llvm-readobj --symbols --relocations` at the same time.
The first test contains constructions like `2> %t.llvm.err1` and then checks the stderr output separatelly for `--symbols`
I am not sure how much it is useful honestly. I think more useful is to check an exact output to reveal places where we print warnings.
Are you OK if I change `2> %t.llvm.err1` to `2>&1` and merge these tests then?


================
Comment at: llvm/test/tools/llvm-readobj/ELF/section-symbols.test:116
+## Test --relocations option.
+## FIXME: relocations should print section symbol names when them are not empty.
+# RUN: llvm-readobj %t1 --relocations 2>&1 | FileCheck %s -DFILE=%t1 --check-prefix=LLVM-RELOCS
----------------
jhenderson wrote:
> I also think that if a name can't be looked up, it shouldn't prevent printing of the relocation. That seems unhelpful to me. Possibly another piece of work, and worth highlighting in the FIXME?
I think it is addressed in D87613: all relocations are dumped there and this FIXME is removed.


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

https://reviews.llvm.org/D87612



More information about the llvm-commits mailing list