[PATCH] D85835: [llvm-readobj/elf] - Refine testing of broken Android's packed relocation sections.

Mon Aug 17 04:25:46 PDT 2020

grimar added inline comments.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/packed-relocs-errors.s:8
+
+# RUN: llvm-mc -filetype=obj -triple=x86_64 %t/asm1.s -o %t1.o
+# RUN: llvm-readobj --relocations %t1.o 2>&1 | FileCheck %s -DFILE=%t1.o --check-prefix=ERR-HEADER
----------------
jhenderson wrote:
> I could easily be misunderstanding how the -triple argument works, but don't we need a `-linux` suffix on here to ensure an ELF object is produced (as opposed to e.g. COFF or Mach-O) on all platforms, regardless of their default triple?
Hmm, good question. Honestly I did not know the answer before, but just few points I'd like to mention:
1) I had no intention to change the tripple here, original objects are using `x86_64-pc-linux-gnu` and I think that
    just truncated the tripple, because copy pasted it from one of LLD(ELF) test cases.
2) There are multiple LLD ELF tests that are using `-triple=x86_64`. I've verified that llvm-mc produces ELF inputs when running 
     from windows environment. (It was very expected, since all these tests are known to pass on windows, but anyways..)
3) Finally, I've debugged how tripples are parsed and the default OS for this case is always `ELF`:
     https://github.com/llvm/llvm-project/blob/master/llvm/lib/Support/Triple.cpp#L665
    I.e. a host OS doesn't matter here and when no OS is specified it fall backs to the default `ELF` one.

But I think that would be indeed slightly cleaner to specify an expected target OS in the tripple. So I am going to update this invocation to specify the `x86_64-unknown-linux`, which is also very commonly used by LLD tests (another popular variant is `x86_64-pc-linux`, btw). I think it might be slightly better than original `x86_64-pc-linux-gnu`, since there is nothing `GNU` specific in this tests.

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

https://reviews.llvm.org/D85835