On systems that set -Wl,--as-needed, putting the -lsodium in the wrong
place on the command line means we get a linker error:

      CC       vdso_test_chacha
    /usr/bin/ld: /tmp/ccKpjnSM.o: in function `main':
    vdso_test_chacha.c:(.text+0x276): undefined reference to `crypto_stream_chacha20'
    collect2: error: ld returned 1 exit status

Fix this by passing pkg-config's --libs output to the LDFLAGS field
instead of the CFLAGS field, as is customary.

Reported-by: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
---
 tools/testing/selftests/vDSO/Makefile | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index 180854eb9fec..834aa862ba2c 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -1,7 +1,8 @@
 # SPDX-License-Identifier: GPL-2.0
 uname_M := $(shell uname -m 2>/dev/null || echo not)
 ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
-SODIUM := $(shell pkg-config --libs --cflags libsodium 2>/dev/null)
+SODIUM_LIBS := $(shell pkg-config --libs libsodium 2>/dev/null)
+SODIUM_CFLAGS := $(shell pkg-config --cflags libsodium 2>/dev/null)
 
 TEST_GEN_PROGS := vdso_test_gettimeofday
 TEST_GEN_PROGS += vdso_test_getcpu
@@ -13,7 +14,7 @@ endif
 TEST_GEN_PROGS += vdso_test_correctness
 ifeq ($(uname_M),x86_64)
 TEST_GEN_PROGS += vdso_test_getrandom
-ifneq ($(SODIUM),)
+ifneq ($(SODIUM_LIBS),)
 TEST_GEN_PROGS += vdso_test_chacha
 endif
 endif
@@ -41,8 +42,9 @@ $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
                                          -isystem $(top_srcdir)/include/uapi
 
 $(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(ARCH)/vdso/vgetrandom-chacha.S
+$(OUTPUT)/vdso_test_chacha: LDLIBS += $(SODIUM_LIBS)
 $(OUTPUT)/vdso_test_chacha: CFLAGS += -idirafter $(top_srcdir)/tools/include \
                                       -idirafter $(top_srcdir)/arch/$(ARCH)/include \
                                       -idirafter $(top_srcdir)/include \
                                       -D__ASSEMBLY__ -DBULID_VDSO -DCONFIG_FUNCTION_ALIGNMENT=0 \
-                                      -Wa,--noexecstack $(SODIUM)
+                                      -Wa,--noexecstack $(SODIUM_CFLAGS)
-- 
2.46.0

On 27/08/24 11:41, Jason A. Donenfeld wrote:
> On systems that set -Wl,--as-needed, putting the -lsodium in the wrong
> place on the command line means we get a linker error:
> 
>       CC       vdso_test_chacha
>     /usr/bin/ld: /tmp/ccKpjnSM.o: in function `main':
>     vdso_test_chacha.c:(.text+0x276): undefined reference to `crypto_stream_chacha20'
>     collect2: error: ld returned 1 exit status
> 
> Fix this by passing pkg-config's --libs output to the LDFLAGS field
> instead of the CFLAGS field, as is customary.
> 
> Reported-by: Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> ---
>  tools/testing/selftests/vDSO/Makefile | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
> index 180854eb9fec..834aa862ba2c 100644
> --- a/tools/testing/selftests/vDSO/Makefile
> +++ b/tools/testing/selftests/vDSO/Makefile
> @@ -1,7 +1,8 @@
>  # SPDX-License-Identifier: GPL-2.0
>  uname_M := $(shell uname -m 2>/dev/null || echo not)
>  ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
> -SODIUM := $(shell pkg-config --libs --cflags libsodium 2>/dev/null)
> +SODIUM_LIBS := $(shell pkg-config --libs libsodium 2>/dev/null)
> +SODIUM_CFLAGS := $(shell pkg-config --cflags libsodium 2>/dev/null)
>  
>  TEST_GEN_PROGS := vdso_test_gettimeofday
>  TEST_GEN_PROGS += vdso_test_getcpu
> @@ -13,7 +14,7 @@ endif
>  TEST_GEN_PROGS += vdso_test_correctness
>  ifeq ($(uname_M),x86_64)
>  TEST_GEN_PROGS += vdso_test_getrandom
> -ifneq ($(SODIUM),)
> +ifneq ($(SODIUM_LIBS),)
>  TEST_GEN_PROGS += vdso_test_chacha
>  endif
>  endif
> @@ -41,8 +42,9 @@ $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
>                                           -isystem $(top_srcdir)/include/uapi
>  
>  $(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(ARCH)/vdso/vgetrandom-chacha.S
> +$(OUTPUT)/vdso_test_chacha: LDLIBS += $(SODIUM_LIBS)
>  $(OUTPUT)/vdso_test_chacha: CFLAGS += -idirafter $(top_srcdir)/tools/include \
>                                        -idirafter $(top_srcdir)/arch/$(ARCH)/include \
>                                        -idirafter $(top_srcdir)/include \
>                                        -D__ASSEMBLY__ -DBULID_VDSO -DCONFIG_FUNCTION_ALIGNMENT=0 \

This seems to be wrong '-DBULID_VDSO'.

> -                                      -Wa,--noexecstack $(SODIUM)
> +                                      -Wa,--noexecstack $(SODIUM_CFLAGS)

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

On Tue, Aug 27, 2024 at 4:45 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
> >                                        -D__ASSEMBLY__ -DBULID_VDSO -DCONFIG_FUNCTION_ALIGNMENT=0 \
>
> This seems to be wrong '-DBULID_VDSO'.

Holy smokes, thanks. It turns out that wasn't needed, and neither is
CONFIG_FUNCTION_ALIGNMENT now. I'll send a patch  removing these.