linux-test-project / ltp

Linux Test Project (mailing list: https://lists.linux.it/listinfo/ltp)
https://linux-test-project.readthedocs.io/
GNU General Public License v2.0
2.28k stars 1k forks source link

kvm: use $(LD) instead of hardcoding ld #948

Closed rossburton closed 2 years ago

rossburton commented 2 years ago

In cross-compiled builds the host ld may not know the required ELF format, so ensure we use $(LD) which will be the cross-capable ld binary.

Signed-off-by: Ross Burton ross.burton@arm.com

metan-ucw commented 2 years ago

@mdoucha can you have a look please?

pevik commented 2 years ago

Reviewed-by: Petr Vorel <pvorel@suse.cz>

pevik commented 2 years ago

@rossburton FYI (for next time) we prefer patches over mailing list https://lists.linux.it/listinfo/ltp https://patchwork.ozlabs.org/project/ltp/list/ https://lore.kernel.org/ltp/

pevik commented 2 years ago

@rossburton patch does not apply on current master. Although it's trivial to apply your changes on current master, it'd be better rebase before submitting patch.

mdoucha commented 2 years ago

Looks good to me. I was originally worried that non-GNU linker might not support wrapping arbitrary files into a resource object but I guess cross-compilation support is more important.

Reviewed-by: Martin Doucha <mdoucha@suse.cz>

pevik commented 2 years ago

Going to merge. FYI I tested compilation of the original code with hardcoded ld in Buildroot distro (which uses cross compilation), but they don't set '$(LD)', because it's set by configure triplet. That's why we haven't detected it.

shr-project commented 2 years ago

FWIW: this PR breaks build with gold:

martin@jama /OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/ltp/20220527-r0/git/testcases/kernel/kvm $ make V=1 -j 1
ccache x86_64-oe-linux-gcc  -m64 -march=core2 -mtune=core2 -msse3 -mfpmath=sse -Wl,-O1 -Wl,--hash-style=gnu -Wl,--as-needed -fmacro-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/ltp/20220527-r0=/usr/src/debug/ltp/20220527-
r0                      -fdebug-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/ltp/20220527-r0=/usr/src/debug/ltp/20220527-r0                      -fdebug-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/ltp/20
220527-r0/recipe-sysroot=                      -fdebug-prefix-map=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/ltp/20220527-r0/recipe-sysroot-native=  -Wl,-z,relro,-z,now -fuse-ld=bfd   -O2 -D_FORTIFY_SOURCE=2 -Wformat -Wformat-secu
rity -Werror=format-security --sysroot=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/ltp/20220527-r0/recipe-sysroot -I/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/ltp/20220527-r0/git/testcases/kernel/kvm/include -I../../../incl
ude -I../../../include -I../../../include/old/ -DCOMPILE_PAYLOAD -ffreestanding -O2 -Wall -fno-asynchronous-unwind-tables -mno-mmx -mno-sse -fno-pie -nostdlib -Wl,--build-id=none -no-pie -Wl,-T/OE/build/oe-core/tmp-glibc/work/core2-64-oe-
linux/ltp/20220527-r0/git/testcases/kernel/kvm/linker/x86_64.lds -o kvm_pagefault01-payload.elf kvm_pagefault01.c lib_guest.o bootstrap_x86_64.o lib_x86.o 
cc1: warning: SSE instruction set disabled, using 387 arithmetics
objcopy -O binary -j .init.boot -j .text -j .data -j .init -j .preinit_array -j .init_array --gap-fill=0 kvm_pagefault01-payload.elf kvm_pagefault01-payload.bin
x86_64-oe-linux-ld --sysroot=/OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/ltp/20220527-r0/recipe-sysroot   -z noexecstack -r -T /OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/ltp/20220527-r0/git/testcases/kernel/kvm/linker/paylo
ad.lds --oformat=elf64-x86-64 -o kvm_pagefault01-payload.o kvm_pagefault01-payload.bin
x86_64-oe-linux-ld: error: kvm_pagefault01-payload.bin:1:1: invalid character
make: *** [Makefile:53: kvm_pagefault01-payload.o] Error 1

martin@jama /OE/build/oe-core/tmp-glibc/work/core2-64-oe-linux/ltp/20220527-r0/git/testcases/kernel/kvm $ x86_64-oe-linux-ld.bfd --version
GNU ld (GNU Binutils) 2.38.20220623
Copyright (C) 2022 Free Software Foundation, Inc.
This program is free software; you may redistribute it under the terms of
the GNU General Public License version 3 or (at your option) a later version.
This program has absolutely no warranty.
pevik commented 2 years ago

@shr-project isn't it a configuration issue? What are $LD and $CC?

pevik commented 2 years ago

OK, not a config issue: https://sourceware.org/bugzilla/show_bug.cgi?id=18097 https://git.openembedded.org/openembedded-core/commit/?id=674fd0c423f3a8911d73b4319ab23bd9b3f1bbac

@shr-project see https://github.com/linux-test-project/ltp/pull/948#issuecomment-1183110153

shr-project commented 2 years ago

Yes, I've sent https://git.openembedded.org/openembedded-core/commit/?id=674fd0c423f3a8911d73b4319ab23bd9b3f1bbac to fix

x86_64-oe-linux/12.1.0/ld: error: ltp/20220527-r0/git/testcases/kernel/kvm/linker/x86_64.lds:36:20: syntax error, unexpected '('
x86_64-oe-linux/12.1.0/ld: fatal error: unable to parse script file ltp/20220527-r0/git/testcases/kernel/kvm/linker/x86_64.lds

but now when the LD from Yocto is respected here as well, it causes this second issue with payload format and because this LD line doesn't use LDFLAGS (and cannot as ld.bfd wouldn't recognize e.g. "-Wl,O1" as used in CXX), I can work around this by explicitly appending ".bfd" in cases where LD is gold (ld-is-gold in DISTRO_FEATURES in Yocto), but that's even more fragile than -fuse-ld=bfd, because people can use lld or something else in LD and then appending .bfd would fail as well.

The same reported by me in oe-core's version of Ross's patch: https://lists.openembedded.org/g/openembedded-core/message/168193

pevik commented 1 year ago

@mdoucha @metan-ucw @wangli5665 @jstancek Could you please have a look?

mdoucha commented 1 year ago

Could you please have a look?

This is exactly the issue I was trying to avoid by calling ld directly. I guess we should add a variable KVM_LD which will default to $(LD) but can be customized.

pevik commented 1 year ago

@mdoucha makes sense. Please send a patch once you have time.

richiejp commented 1 year ago

Merged in 5ef0b7892a17b64040e55e9ad62d36ebb75d33fd

shr-project commented 1 year ago

Thanks, KVM_LD change backported to oe-core with a work around to use ld.bfd when gold is used by default: https://lists.openembedded.org/g/openembedded-core/message/168600