[PATCH] RISC-V: Fix the VDSO symbol generaton for binutils-2.34+

[PATCH] RISC-V: Fix the VDSO symbol generaton for binutils-2.34+

[Palmer Dabbelt]

We were relying on GNU ld's ability to re-link executable files in order
to extract our VDSO symbols.  This behavior was deemed a bug as of
binutils-2.34 (specifically the binutils-gdb commit a87e1817a4 ("Have
the linker fail if any attempt to link in an executable is made."),
which IIUC landed in 2.34), which recently installed itself on my build
setup.

The previous version of this was a bit of a mess: we were linking a
static executable version of the VDSO, containing only a subset of the
input symbols, which we then linked into the kernel.  This worked, but
certainly wasn't a supported path through the toolchain.  Instead this
new version parses the textual output of nm to produce a symbol table.
Both rely on near-zero addresses being linkable, but as we rely on weak
undefined symbols being linkable elsewhere I don't view this as a major
issue.

Fixes: e2c0cdfba7f6 ("RISC-V: User-facing API")
Cc: stable@vger.kernel.org
Cc: clang-built-linux@googlegroups.com
Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
---
 arch/riscv/kernel/vdso/.gitignore |  1 +
 arch/riscv/kernel/vdso/Makefile   | 19 +++++++++----------
 arch/riscv/kernel/vdso/so2s.sh    |  7 +++++++
 3 files changed, 17 insertions(+), 10 deletions(-)
 create mode 100755 arch/riscv/kernel/vdso/so2s.sh

diff --git a/arch/riscv/kernel/vdso/.gitignore b/arch/riscv/kernel/vdso/.gitignore
index 11ebee9e4c1d..3a19def868ec 100644
--- a/arch/riscv/kernel/vdso/.gitignore
+++ b/arch/riscv/kernel/vdso/.gitignore
@@ -1,3 +1,4 @@
 # SPDX-License-Identifier: GPL-2.0-only
 vdso.lds
 *.tmp
+vdso-syms.S
diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
index 478e7338ddc1..2e02958f6224 100644
--- a/arch/riscv/kernel/vdso/Makefile
+++ b/arch/riscv/kernel/vdso/Makefile
@@ -43,19 +43,14 @@ $(obj)/vdso.o: $(obj)/vdso.so
 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 -Wl,--hash-style=both
 
 # 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
 # refer to these symbols in the kernel code rather than hand-coded addresses.
-
-SYSCFLAGS_vdso.so.dbg = -shared -s -Wl,-soname=linux-vdso.so.1 \
-	-Wl,--build-id -Wl,--hash-style=both
-$(obj)/vdso-dummy.o: $(src)/vdso.lds $(obj)/rt_sigreturn.o FORCE
-	$(call if_changed,vdsold)
-
-LDFLAGS_vdso-syms.o := -r --just-symbols
-$(obj)/vdso-syms.o: $(obj)/vdso-dummy.o FORCE
-	$(call if_changed,ld)
+$(obj)/vdso-syms.S: $(obj)/vdso.so FORCE
+	$(call if_changed,so2s)
 
 # strip rule for the .so file
 $(obj)/%.so: OBJCOPYFLAGS := -S
@@ -68,11 +63,15 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE
 # 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 && \
+                           -Wl,-T,$(filter-out FORCE,$^) -o $@.tmp -Wl,-Map,$@.map && \
                    $(CROSS_COMPILE)objcopy \
                            $(patsubst %, -G __vdso_%, $(vdso-syms)) $@.tmp $@ && \
                    rm $@.tmp
 
+# Extracts
+quiet_cmd_so2s = SO2S    $@
+      cmd_so2s = $(CROSS_COMPILE)nm -D $< | $(src)/so2s.sh > $@
+
 # install commands for the unstripped file
 quiet_cmd_vdso_install = INSTALL $@
       cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
diff --git a/arch/riscv/kernel/vdso/so2s.sh b/arch/riscv/kernel/vdso/so2s.sh
new file mode 100755
index 000000000000..7862866b5ebb
--- /dev/null
+++ b/arch/riscv/kernel/vdso/so2s.sh
@@ -0,0 +1,6 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0+
+# Copyright 2020 Palmer Dabbelt <palmerdabbelt@google.com>
+
+sed 's!\([0-9a-f]*\) T \([a-z0-9_]*\)@@LINUX_4.15!.global \2\n.set \2,0x\1!' \
+| grep '^\.'
-- 
2.29.0.rc1.297.gfa9743e501-goog

Re: [PATCH] RISC-V: Fix the VDSO symbol generaton for binutils-2.34+

[Nick Desaulniers]

On Fri, Oct 16, 2020 at 5:44 PM 'Palmer Dabbelt' via Clang Built Linux
<clang-built-linux@googlegroups.com> wrote:
>
> We were relying on GNU ld's ability to re-link executable files in order
> to extract our VDSO symbols.  This behavior was deemed a bug as of
> binutils-2.34 (specifically the binutils-gdb commit a87e1817a4 ("Have
> the linker fail if any attempt to link in an executable is made."),
> which IIUC landed in 2.34), which recently installed itself on my build
> setup.
>
> The previous version of this was a bit of a mess: we were linking a
> static executable version of the VDSO, containing only a subset of the
> input symbols, which we then linked into the kernel.  This worked, but
> certainly wasn't a supported path through the toolchain.  Instead this
> new version parses the textual output of nm to produce a symbol table.
> Both rely on near-zero addresses being linkable, but as we rely on weak
> undefined symbols being linkable elsewhere I don't view this as a major
> issue.
>
> Fixes: e2c0cdfba7f6 ("RISC-V: User-facing API")
> Cc: stable@vger.kernel.org
> Cc: clang-built-linux@googlegroups.com
> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>

Ah, I do see a build failure to link the vdso with:
$ riscv64-linux-gnu-ld --version
GNU ld (GNU Binutils for Debian) 2.34.90.20200706

riscv64-linux-gnu-ld: cannot use executable file
'arch/riscv/kernel/vdso/vdso-dummy.o' as input to a link

This patch fixes that for me, but there's a problem related to related
to `nm` below.

After this, there's two other things we might want to fix up related
to the build of the vdso:
1. it looks like $(CC) is being used to link the vdso, rather than
$(LD).  While it's generally fine to use the compiler as the driver
for building a linked object file, it does not respect the set $(LD).
`-fuse-ld=` needs to be passed to invoke the linker the user
specified.  See also:
https://lore.kernel.org/linux-kbuild/20201013033947.2257501-1-natechancellor@gmail.com/T/#u
(this has popped up in a few places when trying to do hermetic builds
with LLD).
2. I observe the warning when building with clang: `argument unused
during compilation: '-no-pie' [-Wunused-command-line-argument]`. IIRC,
the top level Makefile sets `-Qunused-arguments` for builds with
clang.  `cmd_vdsold` may need that, but it's curious why it's unused
and makes me wonder why/if `-no-pie` is necessary?  That also might be
fixed by fixing 1.

> ---
>  arch/riscv/kernel/vdso/.gitignore |  1 +
>  arch/riscv/kernel/vdso/Makefile   | 19 +++++++++----------
>  arch/riscv/kernel/vdso/so2s.sh    |  7 +++++++
>  3 files changed, 17 insertions(+), 10 deletions(-)
>  create mode 100755 arch/riscv/kernel/vdso/so2s.sh
>
> diff --git a/arch/riscv/kernel/vdso/.gitignore b/arch/riscv/kernel/vdso/.gitignore
> index 11ebee9e4c1d..3a19def868ec 100644
> --- a/arch/riscv/kernel/vdso/.gitignore
> +++ b/arch/riscv/kernel/vdso/.gitignore
> @@ -1,3 +1,4 @@
>  # SPDX-License-Identifier: GPL-2.0-only
>  vdso.lds
>  *.tmp
> +vdso-syms.S
> diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
> index 478e7338ddc1..2e02958f6224 100644
> --- a/arch/riscv/kernel/vdso/Makefile
> +++ b/arch/riscv/kernel/vdso/Makefile
> @@ -43,19 +43,14 @@ $(obj)/vdso.o: $(obj)/vdso.so
>  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 -Wl,--hash-style=both
>
>  # 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
>  # refer to these symbols in the kernel code rather than hand-coded addresses.
> -
> -SYSCFLAGS_vdso.so.dbg = -shared -s -Wl,-soname=linux-vdso.so.1 \
> -       -Wl,--build-id -Wl,--hash-style=both
> -$(obj)/vdso-dummy.o: $(src)/vdso.lds $(obj)/rt_sigreturn.o FORCE
> -       $(call if_changed,vdsold)
> -
> -LDFLAGS_vdso-syms.o := -r --just-symbols
> -$(obj)/vdso-syms.o: $(obj)/vdso-dummy.o FORCE
> -       $(call if_changed,ld)
> +$(obj)/vdso-syms.S: $(obj)/vdso.so FORCE
> +       $(call if_changed,so2s)
>
>  # strip rule for the .so file
>  $(obj)/%.so: OBJCOPYFLAGS := -S
> @@ -68,11 +63,15 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE
>  # 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 && \
> +                           -Wl,-T,$(filter-out FORCE,$^) -o $@.tmp -Wl,-Map,$@.map && \
>                     $(CROSS_COMPILE)objcopy \
>                             $(patsubst %, -G __vdso_%, $(vdso-syms)) $@.tmp $@ && \
>                     rm $@.tmp
>
> +# Extracts
> +quiet_cmd_so2s = SO2S    $@
> +      cmd_so2s = $(CROSS_COMPILE)nm -D $< | $(src)/so2s.sh > $@

This should use `$(NM)` rather than `$(CROSS_COMPILE)nm` which
hardcodes the use of GNU nm from GNU binutils.

> +
>  # install commands for the unstripped file
>  quiet_cmd_vdso_install = INSTALL $@
>        cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
> diff --git a/arch/riscv/kernel/vdso/so2s.sh b/arch/riscv/kernel/vdso/so2s.sh
> new file mode 100755
> index 000000000000..7862866b5ebb
> --- /dev/null
> +++ b/arch/riscv/kernel/vdso/so2s.sh
> @@ -0,0 +1,6 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright 2020 Palmer Dabbelt <palmerdabbelt@google.com>
> +
> +sed 's!\([0-9a-f]*\) T \([a-z0-9_]*\)@@LINUX_4.15!.global \2\n.set \2,0x\1!' \
> +| grep '^\.'
> --

-- 
Thanks,
~Nick Desaulniers

Re: [PATCH] RISC-V: Fix the VDSO symbol generaton for binutils-2.34+

[Fangrui Song]

On 2020-10-19, 'Nick Desaulniers' via Clang Built Linux wrote:
>On Fri, Oct 16, 2020 at 5:44 PM 'Palmer Dabbelt' via Clang Built Linux
><clang-built-linux@googlegroups.com> wrote:
>>
>> We were relying on GNU ld's ability to re-link executable files in order
>> to extract our VDSO symbols.  This behavior was deemed a bug as of
>> binutils-2.34 (specifically the binutils-gdb commit a87e1817a4 ("Have
>> the linker fail if any attempt to link in an executable is made."),
>> which IIUC landed in 2.34), which recently installed itself on my build
>> setup.

I filed the issue https://sourceware.org/bugzilla/show_bug.cgi?id=26047
The commit is in 2.35 but not in the released 2.34 branch.

>> The previous version of this was a bit of a mess: we were linking a
>> static executable version of the VDSO, containing only a subset of the
>> input symbols, which we then linked into the kernel.  This worked, but
>> certainly wasn't a supported path through the toolchain.  Instead this
>> new version parses the textual output of nm to produce a symbol table.
>> Both rely on near-zero addresses being linkable, but as we rely on weak
>> undefined symbols being linkable elsewhere I don't view this as a major
>> issue.
>>
>> Fixes: e2c0cdfba7f6 ("RISC-V: User-facing API")
>> Cc: stable@vger.kernel.org
>> Cc: clang-built-linux@googlegroups.com
>> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
>
>Ah, I do see a build failure to link the vdso with:
>$ riscv64-linux-gnu-ld --version
>GNU ld (GNU Binutils for Debian) 2.34.90.20200706
>
>riscv64-linux-gnu-ld: cannot use executable file
>'arch/riscv/kernel/vdso/vdso-dummy.o' as input to a link
>
>This patch fixes that for me, but there's a problem related to related
>to `nm` below.
>
>After this, there's two other things we might want to fix up related
>to the build of the vdso:
>1. it looks like $(CC) is being used to link the vdso, rather than
>$(LD).  While it's generally fine to use the compiler as the driver
>for building a linked object file, it does not respect the set $(LD).
>`-fuse-ld=` needs to be passed to invoke the linker the user
>specified.  See also:
>https://lore.kernel.org/linux-kbuild/20201013033947.2257501-1-natechancellor@gmail.com/T/#u
>(this has popped up in a few places when trying to do hermetic builds
>with LLD).
>2. I observe the warning when building with clang: `argument unused
>during compilation: '-no-pie' [-Wunused-command-line-argument]`. IIRC,
>the top level Makefile sets `-Qunused-arguments` for builds with
>clang.  `cmd_vdsold` may need that, but it's curious why it's unused
>and makes me wonder why/if `-no-pie` is necessary?  That also might be
>fixed by fixing 1.
>
>> ---
>>  arch/riscv/kernel/vdso/.gitignore |  1 +
>>  arch/riscv/kernel/vdso/Makefile   | 19 +++++++++----------
>>  arch/riscv/kernel/vdso/so2s.sh    |  7 +++++++
>>  3 files changed, 17 insertions(+), 10 deletions(-)
>>  create mode 100755 arch/riscv/kernel/vdso/so2s.sh
>>
>> diff --git a/arch/riscv/kernel/vdso/.gitignore b/arch/riscv/kernel/vdso/.gitignore
>> index 11ebee9e4c1d..3a19def868ec 100644
>> --- a/arch/riscv/kernel/vdso/.gitignore
>> +++ b/arch/riscv/kernel/vdso/.gitignore
>> @@ -1,3 +1,4 @@
>>  # SPDX-License-Identifier: GPL-2.0-only
>>  vdso.lds
>>  *.tmp
>> +vdso-syms.S
>> diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
>> index 478e7338ddc1..2e02958f6224 100644
>> --- a/arch/riscv/kernel/vdso/Makefile
>> +++ b/arch/riscv/kernel/vdso/Makefile
>> @@ -43,19 +43,14 @@ $(obj)/vdso.o: $(obj)/vdso.so
>>  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 -Wl,--hash-style=both
>>
>>  # 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
>>  # refer to these symbols in the kernel code rather than hand-coded addresses.
>> -
>> -SYSCFLAGS_vdso.so.dbg = -shared -s -Wl,-soname=linux-vdso.so.1 \
>> -       -Wl,--build-id -Wl,--hash-style=both
>> -$(obj)/vdso-dummy.o: $(src)/vdso.lds $(obj)/rt_sigreturn.o FORCE
>> -       $(call if_changed,vdsold)
>> -
>> -LDFLAGS_vdso-syms.o := -r --just-symbols
>> -$(obj)/vdso-syms.o: $(obj)/vdso-dummy.o FORCE
>> -       $(call if_changed,ld)
>> +$(obj)/vdso-syms.S: $(obj)/vdso.so FORCE
>> +       $(call if_changed,so2s)
>>
>>  # strip rule for the .so file
>>  $(obj)/%.so: OBJCOPYFLAGS := -S
>> @@ -68,11 +63,15 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE
>>  # 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 && \
>> +                           -Wl,-T,$(filter-out FORCE,$^) -o $@.tmp -Wl,-Map,$@.map && \

Is -Wl,-Map,$@.map needed?

>>                     $(CROSS_COMPILE)objcopy \
>>                             $(patsubst %, -G __vdso_%, $(vdso-syms)) $@.tmp $@ && \
>>                     rm $@.tmp
>>
>> +# Extracts
>> +quiet_cmd_so2s = SO2S    $@
>> +      cmd_so2s = $(CROSS_COMPILE)nm -D $< | $(src)/so2s.sh > $@
>
>This should use `$(NM)` rather than `$(CROSS_COMPILE)nm` which
>hardcodes the use of GNU nm from GNU binutils.
>
>> +
>>  # install commands for the unstripped file
>>  quiet_cmd_vdso_install = INSTALL $@
>>        cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
>> diff --git a/arch/riscv/kernel/vdso/so2s.sh b/arch/riscv/kernel/vdso/so2s.sh
>> new file mode 100755
>> index 000000000000..7862866b5ebb
>> --- /dev/null
>> +++ b/arch/riscv/kernel/vdso/so2s.sh
>> @@ -0,0 +1,6 @@
>> +#!/bin/sh
>> +# SPDX-License-Identifier: GPL-2.0+
>> +# Copyright 2020 Palmer Dabbelt <palmerdabbelt@google.com>
>> +
>> +sed 's!\([0-9a-f]*\) T \([a-z0-9_]*\)@@LINUX_4.15!.global \2\n.set \2,0x\1!' \
>> +| grep '^\.'
>> --
>
>-- 
>Thanks,
>~Nick Desaulniers
>
>-- 
>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/CAKwvOdnsRHA1WMb7OWi-jV662xLrBBBZ%3DzBbB1gvfpBqVFeSfQ%40mail.gmail.com.

Re: [PATCH] RISC-V: Fix the VDSO symbol generaton for binutils-2.34+

[Palmer Dabbelt]

On Mon, 19 Oct 2020 13:41:21 PDT (-0700), maskray@google.com wrote:
> On 2020-10-19, 'Nick Desaulniers' via Clang Built Linux wrote:
>>On Fri, Oct 16, 2020 at 5:44 PM 'Palmer Dabbelt' via Clang Built Linux
>><clang-built-linux@googlegroups.com> wrote:
>>>
>>> We were relying on GNU ld's ability to re-link executable files in order
>>> to extract our VDSO symbols.  This behavior was deemed a bug as of
>>> binutils-2.34 (specifically the binutils-gdb commit a87e1817a4 ("Have
>>> the linker fail if any attempt to link in an executable is made."),
>>> which IIUC landed in 2.34), which recently installed itself on my build
>>> setup.
>
> I filed the issue https://sourceware.org/bugzilla/show_bug.cgi?id=26047
> The commit is in 2.35 but not in the released 2.34 branch.

Thanks.  Looks like it was backported to the Debian 2.34, I guess I was off by
a release when looking.  Regardless, I think the fix in binutils is reasonable
and that we just shouldn't be doing this sort of thing.

>>> The previous version of this was a bit of a mess: we were linking a
>>> static executable version of the VDSO, containing only a subset of the
>>> input symbols, which we then linked into the kernel.  This worked, but
>>> certainly wasn't a supported path through the toolchain.  Instead this
>>> new version parses the textual output of nm to produce a symbol table.
>>> Both rely on near-zero addresses being linkable, but as we rely on weak
>>> undefined symbols being linkable elsewhere I don't view this as a major
>>> issue.
>>>
>>> Fixes: e2c0cdfba7f6 ("RISC-V: User-facing API")
>>> Cc: stable@vger.kernel.org
>>> Cc: clang-built-linux@googlegroups.com
>>> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
>>
>>Ah, I do see a build failure to link the vdso with:
>>$ riscv64-linux-gnu-ld --version
>>GNU ld (GNU Binutils for Debian) 2.34.90.20200706
>>
>>riscv64-linux-gnu-ld: cannot use executable file
>>'arch/riscv/kernel/vdso/vdso-dummy.o' as input to a link
>>
>>This patch fixes that for me, but there's a problem related to related
>>to `nm` below.
>>
>>After this, there's two other things we might want to fix up related
>>to the build of the vdso:
>>1. it looks like $(CC) is being used to link the vdso, rather than
>>$(LD).  While it's generally fine to use the compiler as the driver
>>for building a linked object file, it does not respect the set $(LD).
>>`-fuse-ld=` needs to be passed to invoke the linker the user
>>specified.  See also:
>>https://lore.kernel.org/linux-kbuild/20201013033947.2257501-1-natechancellor@gmail.com/T/#u
>>(this has popped up in a few places when trying to do hermetic builds
>>with LLD).
>>2. I observe the warning when building with clang: `argument unused
>>during compilation: '-no-pie' [-Wunused-command-line-argument]`. IIRC,
>>the top level Makefile sets `-Qunused-arguments` for builds with
>>clang.  `cmd_vdsold` may need that, but it's curious why it's unused
>>and makes me wonder why/if `-no-pie` is necessary?  That also might be
>>fixed by fixing 1.
>>
>>> ---
>>>  arch/riscv/kernel/vdso/.gitignore |  1 +
>>>  arch/riscv/kernel/vdso/Makefile   | 19 +++++++++----------
>>>  arch/riscv/kernel/vdso/so2s.sh    |  7 +++++++
>>>  3 files changed, 17 insertions(+), 10 deletions(-)
>>>  create mode 100755 arch/riscv/kernel/vdso/so2s.sh
>>>
>>> diff --git a/arch/riscv/kernel/vdso/.gitignore b/arch/riscv/kernel/vdso/.gitignore
>>> index 11ebee9e4c1d..3a19def868ec 100644
>>> --- a/arch/riscv/kernel/vdso/.gitignore
>>> +++ b/arch/riscv/kernel/vdso/.gitignore
>>> @@ -1,3 +1,4 @@
>>>  # SPDX-License-Identifier: GPL-2.0-only
>>>  vdso.lds
>>>  *.tmp
>>> +vdso-syms.S
>>> diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
>>> index 478e7338ddc1..2e02958f6224 100644
>>> --- a/arch/riscv/kernel/vdso/Makefile
>>> +++ b/arch/riscv/kernel/vdso/Makefile
>>> @@ -43,19 +43,14 @@ $(obj)/vdso.o: $(obj)/vdso.so
>>>  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 -Wl,--hash-style=both
>>>
>>>  # 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
>>>  # refer to these symbols in the kernel code rather than hand-coded addresses.
>>> -
>>> -SYSCFLAGS_vdso.so.dbg = -shared -s -Wl,-soname=linux-vdso.so.1 \
>>> -       -Wl,--build-id -Wl,--hash-style=both
>>> -$(obj)/vdso-dummy.o: $(src)/vdso.lds $(obj)/rt_sigreturn.o FORCE
>>> -       $(call if_changed,vdsold)
>>> -
>>> -LDFLAGS_vdso-syms.o := -r --just-symbols
>>> -$(obj)/vdso-syms.o: $(obj)/vdso-dummy.o FORCE
>>> -       $(call if_changed,ld)
>>> +$(obj)/vdso-syms.S: $(obj)/vdso.so FORCE
>>> +       $(call if_changed,so2s)
>>>
>>>  # strip rule for the .so file
>>>  $(obj)/%.so: OBJCOPYFLAGS := -S
>>> @@ -68,11 +63,15 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE
>>>  # 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 && \
>>> +                           -Wl,-T,$(filter-out FORCE,$^) -o $@.tmp -Wl,-Map,$@.map && \
>
> Is -Wl,-Map,$@.map needed?

Nope, that's there be accident.  I've fixed it for the v2.

Thanks!

>
>>>                     $(CROSS_COMPILE)objcopy \
>>>                             $(patsubst %, -G __vdso_%, $(vdso-syms)) $@.tmp $@ && \
>>>                     rm $@.tmp
>>>
>>> +# Extracts
>>> +quiet_cmd_so2s = SO2S    $@
>>> +      cmd_so2s = $(CROSS_COMPILE)nm -D $< | $(src)/so2s.sh > $@
>>
>>This should use `$(NM)` rather than `$(CROSS_COMPILE)nm` which
>>hardcodes the use of GNU nm from GNU binutils.
>>
>>> +
>>>  # install commands for the unstripped file
>>>  quiet_cmd_vdso_install = INSTALL $@
>>>        cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
>>> diff --git a/arch/riscv/kernel/vdso/so2s.sh b/arch/riscv/kernel/vdso/so2s.sh
>>> new file mode 100755
>>> index 000000000000..7862866b5ebb
>>> --- /dev/null
>>> +++ b/arch/riscv/kernel/vdso/so2s.sh
>>> @@ -0,0 +1,6 @@
>>> +#!/bin/sh
>>> +# SPDX-License-Identifier: GPL-2.0+
>>> +# Copyright 2020 Palmer Dabbelt <palmerdabbelt@google.com>
>>> +
>>> +sed 's!\([0-9a-f]*\) T \([a-z0-9_]*\)@@LINUX_4.15!.global \2\n.set \2,0x\1!' \
>>> +| grep '^\.'
>>> --
>>
>>--
>>Thanks,
>>~Nick Desaulniers
>>
>>--
>>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/CAKwvOdnsRHA1WMb7OWi-jV662xLrBBBZ%3DzBbB1gvfpBqVFeSfQ%40mail.gmail.com.

Re: [PATCH] RISC-V: Fix the VDSO symbol generaton for binutils-2.34+

[Palmer Dabbelt]

On Mon, 19 Oct 2020 12:24:02 PDT (-0700), Nick Desaulniers wrote:
> On Fri, Oct 16, 2020 at 5:44 PM 'Palmer Dabbelt' via Clang Built Linux
> <clang-built-linux@googlegroups.com> wrote:
>>
>> We were relying on GNU ld's ability to re-link executable files in order
>> to extract our VDSO symbols.  This behavior was deemed a bug as of
>> binutils-2.34 (specifically the binutils-gdb commit a87e1817a4 ("Have
>> the linker fail if any attempt to link in an executable is made."),
>> which IIUC landed in 2.34), which recently installed itself on my build
>> setup.
>>
>> The previous version of this was a bit of a mess: we were linking a
>> static executable version of the VDSO, containing only a subset of the
>> input symbols, which we then linked into the kernel.  This worked, but
>> certainly wasn't a supported path through the toolchain.  Instead this
>> new version parses the textual output of nm to produce a symbol table.
>> Both rely on near-zero addresses being linkable, but as we rely on weak
>> undefined symbols being linkable elsewhere I don't view this as a major
>> issue.
>>
>> Fixes: e2c0cdfba7f6 ("RISC-V: User-facing API")
>> Cc: stable@vger.kernel.org
>> Cc: clang-built-linux@googlegroups.com
>> Signed-off-by: Palmer Dabbelt <palmerdabbelt@google.com>
>
> Ah, I do see a build failure to link the vdso with:
> $ riscv64-linux-gnu-ld --version
> GNU ld (GNU Binutils for Debian) 2.34.90.20200706
>
> riscv64-linux-gnu-ld: cannot use executable file
> 'arch/riscv/kernel/vdso/vdso-dummy.o' as input to a link

Ya, looks like it was actually a backport.

> This patch fixes that for me, but there's a problem related to related
> to `nm` below.
>
> After this, there's two other things we might want to fix up related
> to the build of the vdso:
> 1. it looks like $(CC) is being used to link the vdso, rather than
> $(LD).  While it's generally fine to use the compiler as the driver
> for building a linked object file, it does not respect the set $(LD).
> `-fuse-ld=` needs to be passed to invoke the linker the user
> specified.  See also:
> https://lore.kernel.org/linux-kbuild/20201013033947.2257501-1-natechancellor@gmail.com/T/#u
> (this has popped up in a few places when trying to do hermetic builds
> with LLD).

It's probably just to make the argument handling easier -- I generally avoid
invoking the linker directly and instead just always use CC because I can never
remember the linker arguments.

> 2. I observe the warning when building with clang: `argument unused
> during compilation: '-no-pie' [-Wunused-command-line-argument]`. IIRC,
> the top level Makefile sets `-Qunused-arguments` for builds with
> clang.  `cmd_vdsold` may need that, but it's curious why it's unused
> and makes me wonder why/if `-no-pie` is necessary?  That also might be
> fixed by fixing 1.

That seems odd: vdsold is only used for a link, and -no-pie is necessary for
linking.

>
>> ---
>>  arch/riscv/kernel/vdso/.gitignore |  1 +
>>  arch/riscv/kernel/vdso/Makefile   | 19 +++++++++----------
>>  arch/riscv/kernel/vdso/so2s.sh    |  7 +++++++
>>  3 files changed, 17 insertions(+), 10 deletions(-)
>>  create mode 100755 arch/riscv/kernel/vdso/so2s.sh
>>
>> diff --git a/arch/riscv/kernel/vdso/.gitignore b/arch/riscv/kernel/vdso/.gitignore
>> index 11ebee9e4c1d..3a19def868ec 100644
>> --- a/arch/riscv/kernel/vdso/.gitignore
>> +++ b/arch/riscv/kernel/vdso/.gitignore
>> @@ -1,3 +1,4 @@
>>  # SPDX-License-Identifier: GPL-2.0-only
>>  vdso.lds
>>  *.tmp
>> +vdso-syms.S
>> diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
>> index 478e7338ddc1..2e02958f6224 100644
>> --- a/arch/riscv/kernel/vdso/Makefile
>> +++ b/arch/riscv/kernel/vdso/Makefile
>> @@ -43,19 +43,14 @@ $(obj)/vdso.o: $(obj)/vdso.so
>>  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 -Wl,--hash-style=both
>>
>>  # 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
>>  # refer to these symbols in the kernel code rather than hand-coded addresses.
>> -
>> -SYSCFLAGS_vdso.so.dbg = -shared -s -Wl,-soname=linux-vdso.so.1 \
>> -       -Wl,--build-id -Wl,--hash-style=both
>> -$(obj)/vdso-dummy.o: $(src)/vdso.lds $(obj)/rt_sigreturn.o FORCE
>> -       $(call if_changed,vdsold)
>> -
>> -LDFLAGS_vdso-syms.o := -r --just-symbols
>> -$(obj)/vdso-syms.o: $(obj)/vdso-dummy.o FORCE
>> -       $(call if_changed,ld)
>> +$(obj)/vdso-syms.S: $(obj)/vdso.so FORCE
>> +       $(call if_changed,so2s)
>>
>>  # strip rule for the .so file
>>  $(obj)/%.so: OBJCOPYFLAGS := -S
>> @@ -68,11 +63,15 @@ $(obj)/%.so: $(obj)/%.so.dbg FORCE
>>  # 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 && \
>> +                           -Wl,-T,$(filter-out FORCE,$^) -o $@.tmp -Wl,-Map,$@.map && \
>>                     $(CROSS_COMPILE)objcopy \
>>                             $(patsubst %, -G __vdso_%, $(vdso-syms)) $@.tmp $@ && \
>>                     rm $@.tmp
>>
>> +# Extracts
>> +quiet_cmd_so2s = SO2S    $@
>> +      cmd_so2s = $(CROSS_COMPILE)nm -D $< | $(src)/so2s.sh > $@
>
> This should use `$(NM)` rather than `$(CROSS_COMPILE)nm` which
> hardcodes the use of GNU nm from GNU binutils.

Thanks, this is fixed in the v2.  Presumably we should be using $(OBJCOPY) as well?

>
>> +
>>  # install commands for the unstripped file
>>  quiet_cmd_vdso_install = INSTALL $@
>>        cmd_vdso_install = cp $(obj)/$@.dbg $(MODLIB)/vdso/$@
>> diff --git a/arch/riscv/kernel/vdso/so2s.sh b/arch/riscv/kernel/vdso/so2s.sh
>> new file mode 100755
>> index 000000000000..7862866b5ebb
>> --- /dev/null
>> +++ b/arch/riscv/kernel/vdso/so2s.sh
>> @@ -0,0 +1,6 @@
>> +#!/bin/sh
>> +# SPDX-License-Identifier: GPL-2.0+
>> +# Copyright 2020 Palmer Dabbelt <palmerdabbelt@google.com>
>> +
>> +sed 's!\([0-9a-f]*\) T \([a-z0-9_]*\)@@LINUX_4.15!.global \2\n.set \2,0x\1!' \
>> +| grep '^\.'
>> --