[PATCH] D71651: [llvm-readobj][test] - Refactor mips-st-other.test

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Dec 18 13:13:06 PST 2019


MaskRay added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/ELF/mips-symbols-stother.test:4-5
+# RUN: yaml2obj %s > %t.o
+# RUN: llvm-readobj --symbols %t.o | FileCheck %s --strict-whitespace --check-prefix=MIPS-LLVM
+# RUN: llvm-readelf --symbols %t.o | FileCheck %s --strict-whitespace --check-prefix=MIPS-GNU
+
----------------
jhenderson wrote:
> grimar wrote:
> > jhenderson wrote:
> > > Is --strict-whitespace important for this test? Do we anticipate spacing differences from other non-MIPS other bits?
> > I think it is important for GNU. As output shows, the `Ndx` column is misplaced. Here I document this behavior.
> > We might want to fix it. As far I can see we do not have a test that has sh_other > `0xF`
> > and seems that is why this issue was never noticed. It is only a valid case for MIPS, hence I think it should be here.
> > 
> > For LLVM style perhaps we do not need -`-strict-whitespace`, I've added it just in case and for consistency.
> Okay.
> I think it is important for GNU. As output shows, the Ndx column is misplaced. Here I document this behavior.

If the correct behavior seems straightforward, we can fix it and do not necessarily strictly follow GNU. I've filed bugs about GNU readelf display issues and I find they are willing to fix issues.


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

https://reviews.llvm.org/D71651





More information about the llvm-commits mailing list