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

[George Rimar via Phabricator via llvm-commits]

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
----------------
grimar wrote:
> 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?
So, I've leaved this place as is for now, but will be happy to merge tests if you think that it is fine.


================
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
----------------
grimar wrote:
> 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.
I've updated the FIXME for now though.


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

https://reviews.llvm.org/D87612