From: Fangrui Song <maskray@google.com>
To: Nathan Chancellor <nathan@kernel.org>
Cc: Palmer Dabbelt <palmer@dabbelt.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Albert Ou <aou@eecs.berkeley.edu>,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
clang-built-linux@googlegroups.com
Subject: Re: [PATCH] riscv: Use $(LD) instead of $(CC) to link vDSO
Date: Fri, 26 Mar 2021 19:02:16 -0700 [thread overview]
Message-ID: <20210327020216.63ayp7uul3ymhlor@google.com> (raw)
In-Reply-To: <20210325215156.1986901-1-nathan@kernel.org>
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 <maskray@google.com>
>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 <nathan@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
>
>--
>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-linux+unsubscribe@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.
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: Fangrui Song <maskray@google.com>
To: Nathan Chancellor <nathan@kernel.org>
Cc: Palmer Dabbelt <palmer@dabbelt.com>,
Paul Walmsley <paul.walmsley@sifive.com>,
Albert Ou <aou@eecs.berkeley.edu>,
linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org,
clang-built-linux@googlegroups.com
Subject: Re: [PATCH] riscv: Use $(LD) instead of $(CC) to link vDSO
Date: Fri, 26 Mar 2021 19:02:16 -0700 [thread overview]
Message-ID: <20210327020216.63ayp7uul3ymhlor@google.com> (raw)
In-Reply-To: <20210325215156.1986901-1-nathan@kernel.org>
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 <maskray@google.com>
>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 <nathan@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
>
>--
>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-linux+unsubscribe@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.
next prev parent reply other threads:[~2021-03-27 2:02 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-25 21:51 [PATCH] riscv: Use $(LD) instead of $(CC) to link vDSO Nathan Chancellor
2021-03-25 21:51 ` Nathan Chancellor
2021-03-26 16:05 ` kernel test robot
2021-03-26 16:05 ` kernel test robot
2021-03-26 16:05 ` kernel test robot
2021-03-26 23:58 ` Nathan Chancellor
2021-03-26 23:58 ` Nathan Chancellor
2021-03-26 23:58 ` Nathan Chancellor
2021-03-27 1:56 ` Fangrui Song
2021-03-27 1:56 ` Fangrui Song
2021-03-27 1:56 ` Fangrui Song
2021-03-29 7:41 ` [kbuild-all] " Rong Chen
2021-03-29 7:41 ` Rong Chen
2021-03-29 7:41 ` Rong Chen
2021-03-27 2:02 ` Fangrui Song [this message]
2021-03-27 2:02 ` Fangrui Song
2021-04-11 21:20 ` Palmer Dabbelt
2021-04-11 21:20 ` Palmer Dabbelt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210327020216.63ayp7uul3ymhlor@google.com \
--to=maskray@google.com \
--cc=aou@eecs.berkeley.edu \
--cc=clang-built-linux@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=nathan@kernel.org \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.