Re: [PATCH] Revert "Correctly determine libc.so 'OUTPUT_FORMAT' when cross-compiling."

[Vineet Gupta <vineetg@rivosinc.com>]

Hi Ludovic,

On 11/26/22 06:57, Ludovic Courtès wrote:
> Hi,
>
> Vineet Gupta <vineetg@rivosinc.com> skribis:
>
>> This reverts commit 361d6454c034a920f2c96517c277990d390b9652.
>>
>> This trips up riscv gnu toolchain builds [1]
>>
>> riscv ld segfaults when linking libgcc because libc.so linker script
>> contains `OUTPUT_FORMAT(elf32-little)` vs. `OUTPUT_FORMAT(elf32-littleriscv)`.
>>
>> This patch causes builds to lookup riscv32* prefixed objdump and failing
>> to find it falls back to host objdump which is the root of the issue.
>> The host objdump in turn generates `OUTPUT_FORMAT(elf32-little)`
>>
>> riscv glibc multilib builds lack riscv32 prefix binaries. They have a
>> single set of "riscv64" prefixed binaries supporting both 32 and 64-bit
>> abis: ilp32/ilp32d/lp64/lp64d using -march/-mabi.
>>
>> FWIW I'm not sure how this patch fixed a real problem to begin with.
> The rationale was described in the context of cross-compilation to
> aarch64-linux-gnu:
>
>    https://sourceware.org/pipermail/libc-alpha/2021-July/128333.html

Yes I understood that clearly before posting this patch.

> We observed a similar issue with objcopy when cross-compiling to
> powerpc64le-linux-gnu:
>
>    https://issues.guix.gnu.org/49417

Right but they both are for the guix build env. Can you show that 
problem exists in standard glibc build env and that this patch fixes 
some issue there. FWIW at above page the full log link is broken.

Anyhow if you read the details in my revert changelog, I mentioned that 
for the cross toolchain, there are 2 objdump binaries. One with the 
prefix and one w/o.

| $ find TC_INSTALL -name *objdump -exec md5sum {} \;
| 5fd0b967d4977a4f8bc3a7b7a9318b1d  ./bin/riscv64-unknown-linux-gnu-objdump
| 5fd0b967d4977a4f8bc3a7b7a9318b1d  ./riscv64-unknown-linux-gnu/bin/objdump

W/o the patch, sure glibc build picks up the non-prefixed one.

| /scratch/vineetg/gnu/TC_INSTALL/lib/gcc/riscv64-unknown-linux-gnu/12.2.0/../../../../riscv64-unknown-linux-gnu/bin/objdump -f /scratch/vineetg/gnu/toolchain-src/build-glibc-linux-rv32imac-ilp32/format.lds.so | sed -n 's/.*file format \(.*\)/OUTPUT_FORMAT(\1)/;T;p' > /scratch/vineetg/gnu/toolchain-src/build-glibc-linux-rv32imac-ilp32/format.lds

And the non-prefixed objdump is functionally exactly same as prefixed one (as evident from md5sum above). Obviously they both produce the desired/correct "file format" - again from my changelog posted.

|
| $ ./bin/riscv64-unknown-linux-gnu-objdump -f ./xx.lds.so  | grep "file format"
| ./xx.lds.so:     file format elf32-littleriscv
|
| $ ./riscv64-unknown-linux-gnu/bin/objdump -f ./xx.lds.so  | grep "file format"


The issue is this patch makes an assumption that a triplet prefixed 
cross binary exists, which may not always happen in multilib setups.


> Maybe we should see, in your case, why “riscv32-linux-gnu-objdump -f”
> reports “elf32-little” instead of “elf32-littleriscv”?

Again if you read my changelog carefully I mention that in my setup 
riscv32-* binaries don't exist, as explained above.
So lets not add that assumption in tooling.

I'm not aware of how guix build env is setup. Can you check if PATH is 
not clobbered there - if the tooling uses -print-prog-name=objdump it 
should pick the correct cross objdump, not host binary.

-Vineet