selftests, x86: fix how check_cc.sh is being invoked

Add quotes around $(CC) when calling check_cc.sh from a Makefile to
pass the value as a single argument to the script even if it has
several words such as "ccache gcc".  Conversely, remove quotes in
check_cc.sh when calling $CC to make it a command with potentially
several arguments again.

Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture detection")
Tested-by: "kernelci.org bot" <bot@kernelci.org>
Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
---
 tools/testing/selftests/vm/Makefile     | 6 +++---
 tools/testing/selftests/x86/Makefile    | 6 +++---
 tools/testing/selftests/x86/check_cc.sh | 2 +-
 3 files changed, 7 insertions(+), 7 deletions(-)

On 25/02/2022 13:15, Guillaume Tucker wrote:
> Add quotes around $(CC) when calling check_cc.sh from a Makefile to
> pass the value as a single argument to the script even if it has
> several words such as "ccache gcc".  Conversely, remove quotes in
> check_cc.sh when calling $CC to make it a command with potentially
> several arguments again.
> 
> Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture detection")
> Tested-by: "kernelci.org bot" <bot@kernelci.org>
> Signed-off-by: Guillaume Tucker <guillaume.tucker@collabora.com>
> ---
>  tools/testing/selftests/vm/Makefile     | 6 +++---
>  tools/testing/selftests/x86/Makefile    | 6 +++---
>  tools/testing/selftests/x86/check_cc.sh | 2 +-
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/vm/Makefile b/tools/testing/selftests/vm/Makefile
> index 1607322a112c..d934f026ebb5 100644
> --- a/tools/testing/selftests/vm/Makefile
> +++ b/tools/testing/selftests/vm/Makefile
> @@ -49,9 +49,9 @@ TEST_GEN_FILES += split_huge_page_test
>  TEST_GEN_FILES += ksm_tests
>  
>  ifeq ($(MACHINE),x86_64)
> -CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh $(CC) ../x86/trivial_32bit_program.c -m32)
> -CAN_BUILD_X86_64 := $(shell ./../x86/check_cc.sh $(CC) ../x86/trivial_64bit_program.c)
> -CAN_BUILD_WITH_NOPIE := $(shell ./../x86/check_cc.sh $(CC) ../x86/trivial_program.c -no-pie)
> +CAN_BUILD_I386 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_32bit_program.c -m32)
> +CAN_BUILD_X86_64 := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_64bit_program.c)
> +CAN_BUILD_WITH_NOPIE := $(shell ./../x86/check_cc.sh "$(CC)" ../x86/trivial_program.c -no-pie)
>  
>  TARGETS := protection_keys
>  BINARIES_32 := $(TARGETS:%=%_32)
> diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile
> index 8a1f62ab3c8e..53df7d3893d3 100644
> --- a/tools/testing/selftests/x86/Makefile
> +++ b/tools/testing/selftests/x86/Makefile
> @@ -6,9 +6,9 @@ include ../lib.mk
>  .PHONY: all all_32 all_64 warn_32bit_failure clean
>  
>  UNAME_M := $(shell uname -m)
> -CAN_BUILD_I386 := $(shell ./check_cc.sh $(CC) trivial_32bit_program.c -m32)
> -CAN_BUILD_X86_64 := $(shell ./check_cc.sh $(CC) trivial_64bit_program.c)
> -CAN_BUILD_WITH_NOPIE := $(shell ./check_cc.sh $(CC) trivial_program.c -no-pie)
> +CAN_BUILD_I386 := $(shell ./check_cc.sh "$(CC)" trivial_32bit_program.c -m32)
> +CAN_BUILD_X86_64 := $(shell ./check_cc.sh "$(CC)" trivial_64bit_program.c)
> +CAN_BUILD_WITH_NOPIE := $(shell ./check_cc.sh "$(CC)" trivial_program.c -no-pie)
>  
>  TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt test_mremap_vdso \
>  			check_initial_reg_state sigreturn iopl ioperm \
> diff --git a/tools/testing/selftests/x86/check_cc.sh b/tools/testing/selftests/x86/check_cc.sh
> index 3e2089c8cf54..aff2c15018b5 100755
> --- a/tools/testing/selftests/x86/check_cc.sh
> +++ b/tools/testing/selftests/x86/check_cc.sh
> @@ -7,7 +7,7 @@ CC="$1"
>  TESTPROG="$2"
>  shift 2
>  
> -if "$CC" -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then
> +if $CC -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then
>      echo 1
>  else
>      echo 0

I see the change in check_cc.sh is already covered by Usama's patch:

  selftests/x86: Add validity check and allow field splitting

-if "$CC" -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then
+if [ -n "$CC" ] && $CC -o /dev/null "$TESTPROG" -O0 "$@" 2>/dev/null; then


However, the rest of this patch in the Makefiles still needs to
be applied.  Let me know if I should rebase it on top of Usama's.

Thanks,
Guillaume

On Fri, 25 Feb 2022 13:15:43 +0000 Guillaume Tucker <guillaume.tucker@collabora.com> wrote:

> Add quotes around $(CC) when calling check_cc.sh from a Makefile to
> pass the value as a single argument to the script even if it has
> several words such as "ccache gcc".  Conversely, remove quotes in
> check_cc.sh when calling $CC to make it a command with potentially
> several arguments again.

This changelog describes the fix, but it fails to describe the problem
which the patch is fixing!

Presumably, we're hitting some form of runtime failure under
undescribed circumstances when running selftests.  But that's just me
reverse-engineering the patch description.  And me reverse-engineering
stuff is a gloriously unreliable thing.  Please spell out the problem.

On 26/02/2022 01:03, Andrew Morton wrote:
> On Fri, 25 Feb 2022 13:15:43 +0000 Guillaume Tucker <guillaume.tucker@collabora.com> wrote:
> 
>> Add quotes around $(CC) when calling check_cc.sh from a Makefile to
>> pass the value as a single argument to the script even if it has
>> several words such as "ccache gcc".  Conversely, remove quotes in
>> check_cc.sh when calling $CC to make it a command with potentially
>> several arguments again.
> 
> This changelog describes the fix, but it fails to describe the problem
> which the patch is fixing!
> 
> Presumably, we're hitting some form of runtime failure under
> undescribed circumstances when running selftests.  But that's just me
> reverse-engineering the patch description.  And me reverse-engineering
> stuff is a gloriously unreliable thing.  Please spell out the problem.

Thanks for the review.  I've just sent a v2 which is rebased on
other changes in linux-next and with a reworked commit message
which should hopefully be clearer.  The issue was seen when
building some kselftest binaries and $CC is defined with multiple
arguments such as "ccache gcc".

Guillaume