[PATCH] riscv: Use $(LD) instead of $(CC) to link vDSO

[Nathan Chancellor]

Currently, the VDSO is being linked through $(CC). This does not match
how the rest of the kernel links objects, which is through the $(LD)
variable.

When linking with clang, there are a couple of warnings about flags that
will not be used during the link:

clang-12: warning: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument]
clang-12: warning: argument unused during compilation: '-pg' [-Wunused-command-line-argument]

'-no-pie' was added in commit 85602bea297f ("RISC-V: build vdso-dummy.o
with -no-pie") to override '-pie' getting added to the ld command from
distribution versions of GCC that enable PIE by default. It is
technically no longer needed after commit c2c81bb2f691 ("RISC-V: Fix the
VDSO symbol generaton for binutils-2.35+"), which removed vdso-dummy.o
in favor of generating vdso-syms.S from vdso.so with $(NM) but this also
resolves the issue in case it ever comes back due to having full control
over the $(LD) command. '-pg' is for function tracing, it is not used
during linking as clang states.

These flags could be removed/filtered to fix the warnings but it is
easier to just match the rest of the kernel and use $(LD) directly for
linking. See commits

fe00e50b2db8 ("ARM: 8858/1: vdso: use $(LD) instead of $(CC) to link VDSO")
691efbedc60d ("arm64: vdso: use $(LD) instead of $(CC) to link VDSO")
2ff906994b6c ("MIPS: VDSO: Use $(LD) instead of $(CC) to link VDSO")
2b2a25845d53 ("s390/vdso: Use $(LD) instead of $(CC) to link vDSO")

for more information.

The flags are converted to linker flags and '--eh-frame-hdr' is added to
match what is added by GCC implicitly, which can be seen by adding '-v'
to GCC's invocation.

Additionally, since this area is being modified, use the $(OBJCOPY)
variable instead of an open coded $(CROSS_COMPILE)objcopy so that the
user's choice of objcopy binary is respected.

Link: https://github.com/ClangBuiltLinux/linux/issues/803
Link: https://github.com/ClangBuiltLinux/linux/issues/970
Signed-off-by: Nathan Chancellor <nat...@kernel.org>
---
arch/riscv/kernel/vdso/Makefile | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
index 71a315e73cbe..ca2b40dfd24b 100644
--- a/arch/riscv/kernel/vdso/Makefile
+++ b/arch/riscv/kernel/vdso/Makefile
@@ -41,11 +41,10 @@ KASAN_SANITIZE := n
$(obj)/vdso.o: $(obj)/vdso.so

# link rule for the .so file, .lds has to be first
-SYSCFLAGS_vdso.so.dbg = $(c_flags)
$(obj)/vdso.so.dbg: $(src)/vdso.lds $(obj-vdso) FORCE
$(call if_changed,vdsold)
-SYSCFLAGS_vdso.so.dbg = -shared -s -Wl,-soname=linux-vdso.so.1 \
- -Wl,--build-id=sha1 -Wl,--hash-style=both
+LDFLAGS_vdso.so.dbg = -shared -s -soname=linux-vdso.so.1 \
+ --build-id=sha1 --hash-style=both --eh-frame-hdr

# We also create a special relocatable object that should mirror the symbol
# table and layout of the linked DSO. With ld --just-symbols we can then
@@ -60,13 +59,10 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE

# actual build commands
# The DSO images are built using a special linker script
-# Add -lgcc so rv32 gets static muldi3 and lshrdi3 definitions.
# Make sure only to export the intended __vdso_xxx symbol offsets.
quiet_cmd_vdsold = VDSOLD $@
- cmd_vdsold = $(CC) $(KBUILD_CFLAGS) $(call cc-option, -no-pie) -nostdlib -nostartfiles $(SYSCFLAGS_$(@F)) \
- -Wl,-T,$(filter-out FORCE,$^) -o $@.tmp && \
- $(CROSS_COMPILE)objcopy \
- $(patsubst %, -G __vdso_%, $(vdso-syms)) $@.tmp $@ && \
+ cmd_vdsold = $(LD) $(ld_flags) -T $(filter-out FORCE,$^) -o $@.tmp && \
+ $(OBJCOPY) $(patsubst %, -G __vdso_%, $(vdso-syms)) $@.tmp $@ && \
rm $@.tmp

# Extracts symbol offsets from the VDSO, converting them into an assembly file
--
2.31.0

[kernel test robot]

Hi Nathan,

I love your patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.12-rc4 next-20210326]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Nathan-Chancellor/riscv-Use-LD-instead-of-CC-to-link-vDSO/20210326-055421
base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 002322402dafd846c424ffa9240a937f49b48c42
config: riscv-randconfig-r032-20210326 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project f490a5969bd52c8a48586f134ff8f02ccbb295b3)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# install riscv cross compiling tool for clang build
# apt-get install binutils-riscv64-linux-gnu
# https://github.com/0day-ci/linux/commit/dfdcaf93f40f0d15ffc3f25128442c1688e612d6
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Nathan-Chancellor/riscv-Use-LD-instead-of-CC-to-link-vDSO/20210326-055421
git checkout dfdcaf93f40f0d15ffc3f25128442c1688e612d6
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <l...@intel.com>

All errors (new ones prefixed by >>):

>> riscv64-linux-gnu-objcopy: 'arch/riscv/kernel/vdso/vdso.so.dbg': No such file

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuil...@lists.01.org

[Nathan Chancellor]

On Sat, Mar 27, 2021 at 12:05:34AM +0800, kernel test robot wrote:
> Hi Nathan,
>
> I love your patch! Yet something to improve:
>
> [auto build test ERROR on linus/master]
> [also build test ERROR on v5.12-rc4 next-20210326]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
>
> url: https://github.com/0day-ci/linux/commits/Nathan-Chancellor/riscv-Use-LD-instead-of-CC-to-link-vDSO/20210326-055421
> base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 002322402dafd846c424ffa9240a937f49b48c42
> config: riscv-randconfig-r032-20210326 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project f490a5969bd52c8a48586f134ff8f02ccbb295b3)
> reproduce (this is a W=1 build):
> wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
> chmod +x ~/bin/make.cross
> # install riscv cross compiling tool for clang build
> # apt-get install binutils-riscv64-linux-gnu
> # https://github.com/0day-ci/linux/commit/dfdcaf93f40f0d15ffc3f25128442c1688e612d6
> git remote add linux-review https://github.com/0day-ci/linux
> git fetch --no-tags linux-review Nathan-Chancellor/riscv-Use-LD-instead-of-CC-to-link-vDSO/20210326-055421
> git checkout dfdcaf93f40f0d15ffc3f25128442c1688e612d6
> # save the attached .config to linux build tree
> COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=riscv

For the record, I tried to use this script to reproduce but it has a
couple of bugs:

1. It does not download the right version of clang. This report says
that it is clang-13 but the one that the script downloaded is clang-12.

2. It does not download it to the right location. The script expects
~/0day/clang-latest but it is downloaded to ~/0day/clang it seems. I
symlinked it to get around it.

> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <l...@intel.com>
>
> All errors (new ones prefixed by >>):
>
> >> riscv64-linux-gnu-objcopy: 'arch/riscv/kernel/vdso/vdso.so.dbg': No such file

This error only occurs because of errors before it that are not shown
due to a denylist:

ld.lld: error: arch/riscv/kernel/vdso/rt_sigreturn.o:(.text+0x0): relocation R_RISCV_ALIGN requires unimplemented linker relaxation; recompile with -mno-relax
ld.lld: error: arch/riscv/kernel/vdso/getcpu.o:(.text+0x0): relocation R_RISCV_ALIGN requires unimplemented linker relaxation; recompile with -mno-relax
ld.lld: error: arch/riscv/kernel/vdso/flush_icache.o:(.text+0x0): relocation R_RISCV_ALIGN requires unimplemented linker relaxation; recompile with -mno-relax

My patch only adds another occurrence of this error because we move from
$(CC)'s default linker (in clang's case, ld.bfd) to $(LD), which in the
case of 0day appears to be ld.lld. ld.lld should not be used with RISC-V
in its current form due to errors of this nature, which happen without
my patch as well:

https://github.com/ClangBuiltLinux/linux/issues/1020

Linker relaxation in ld.lld for RISC-V is an ongoing debate/process.
Please give RISC-V the current treatment as s390 with ld.lld for the
time being to get meaningful reports. We will reach out once that issue
has been resolved.

TL;DR: Patch exposes existing issue with LD=ld.lld that would have
happened without it in different areas, the report can be ignored.

Cheers!
Nathan

[Fangrui Song]

Yes, lkp frequently reports this error. It can be suppressed by using
-mno-relax... if ld.lld is picked.

Hmm. This motivated me to file
https://github.com/riscv/riscv-elf-psabi-doc/issues/183
R_RISCV_ALIGN friendly to linkers not supporting relaxation (riscv_relax_delete_bytes).

>Cheers!
>Nathan
>
>--
>You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
>To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.
>To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210326235839.zgfvmtfxrb3hy6i4%40archlinux-ax161.

[Fangrui Song]

On 2021-03-25, Nathan Chancellor wrote:
>Currently, the VDSO is being linked through $(CC). This does not match
>how the rest of the kernel links objects, which is through the $(LD)
>variable.
>
>When linking with clang, there are a couple of warnings about flags that
>will not be used during the link:
>
>clang-12: warning: argument unused during compilation: '-no-pie' [-Wunused-command-line-argument]
>clang-12: warning: argument unused during compilation: '-pg' [-Wunused-command-line-argument]
>
>'-no-pie' was added in commit 85602bea297f ("RISC-V: build vdso-dummy.o
>with -no-pie") to override '-pie' getting added to the ld command from
>distribution versions of GCC that enable PIE by default. It is
>technically no longer needed after commit c2c81bb2f691 ("RISC-V: Fix the
>VDSO symbol generaton for binutils-2.35+"), which removed vdso-dummy.o
>in favor of generating vdso-syms.S from vdso.so with $(NM) but this also
>resolves the issue in case it ever comes back due to having full control
>over the $(LD) command. '-pg' is for function tracing, it is not used
>during linking as clang states.

Looks good.

-pg affects the link action: it changes crt1.o to gcrt1.o.
Since the Makefile uses -nostdlib, crt1.o is suppressed, so -pg
is entirely unneeded.
(-nostdlib implies -nostartfiles so the previous usage has a redundant
option.)

>These flags could be removed/filtered to fix the warnings but it is
>easier to just match the rest of the kernel and use $(LD) directly for
>linking. See commits
>
> fe00e50b2db8 ("ARM: 8858/1: vdso: use $(LD) instead of $(CC) to link VDSO")
> 691efbedc60d ("arm64: vdso: use $(LD) instead of $(CC) to link VDSO")
> 2ff906994b6c ("MIPS: VDSO: Use $(LD) instead of $(CC) to link VDSO")
> 2b2a25845d53 ("s390/vdso: Use $(LD) instead of $(CC) to link vDSO")
>
>for more information.
>
>The flags are converted to linker flags and '--eh-frame-hdr' is added to
>match what is added by GCC implicitly, which can be seen by adding '-v'
>to GCC's invocation.

Another minor change which may be shipped together: --hash-style=both
can be --hash-style=gnu. We don't need sysv .hash . The glibc/musl
support for .gnu.hash has been there for years. .gnu.hash is often
smaller than .hash .

Reviewed-by: Fangrui Song <mas...@google.com>

>--
>You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
>To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-li...@googlegroups.com.

>To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210325215156.1986901-1-nathan%40kernel.org.

[Rong Chen]

Hi Nathan,

Sorry for the inconvenience, we'll fix both asap.

Best Regards,
Rong Chen

>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <l...@intel.com>
>>
>> All errors (new ones prefixed by >>):
>>
>>>> riscv64-linux-gnu-objcopy: 'arch/riscv/kernel/vdso/vdso.so.dbg': No such file
> This error only occurs because of errors before it that are not shown
> due to a denylist:
>
> ld.lld: error: arch/riscv/kernel/vdso/rt_sigreturn.o:(.text+0x0): relocation R_RISCV_ALIGN requires unimplemented linker relaxation; recompile with -mno-relax
> ld.lld: error: arch/riscv/kernel/vdso/getcpu.o:(.text+0x0): relocation R_RISCV_ALIGN requires unimplemented linker relaxation; recompile with -mno-relax
> ld.lld: error: arch/riscv/kernel/vdso/flush_icache.o:(.text+0x0): relocation R_RISCV_ALIGN requires unimplemented linker relaxation; recompile with -mno-relax
>
> My patch only adds another occurrence of this error because we move from
> $(CC)'s default linker (in clang's case, ld.bfd) to $(LD), which in the
> case of 0day appears to be ld.lld. ld.lld should not be used with RISC-V
> in its current form due to errors of this nature, which happen without
> my patch as well:
>
> https://github.com/ClangBuiltLinux/linux/issues/1020
>
> Linker relaxation in ld.lld for RISC-V is an ongoing debate/process.
> Please give RISC-V the current treatment as s390 with ld.lld for the
> time being to get meaningful reports. We will reach out once that issue
> has been resolved.
>
> TL;DR: Patch exposes existing issue with LD=ld.lld that would have
> happened without it in different areas, the report can be ignored.
>
> Cheers!
> Nathan

> _______________________________________________
> kbuild-all mailing list -- kbuil...@lists.01.org
> To unsubscribe send an email to kbuild-a...@lists.01.org

[Palmer Dabbelt]

Thanks, this is on for-next.