[PATCH 0/2] selftests: sgx: Fix build of test_sgx

[PATCH 0/2] selftests: sgx: Fix build of test_sgx

[Muhammad Usama Anjum]

CC can have multiple sub-strings like "ccache gcc". Build system of sgx
selftest is unable to support this use case and test_sgx isn't built.
Then rsync gives an erorr:

rsync: [sender] link_stat "//linux/build/kselftest/sgx/test_sgx" failed: No such file or directory (2)
rsync error: some files/attrs were not transferred (see previous errors) (code 23) at main.c(1333) [sender=3.2.3]

This can be fixed if CC is considered to have one string while passing to
check_cc.sh script and inside this script, CC is expanded before execution.

Following build tests have been performed to confirm that these patches
don't break anything else. The verification has also been done through
KernelCI scripts locally as KernelCI has caught this bug:
1) cd tools/testing/selftests && make
2) make -C tools/testing/selftests
3) make -C tools/testing/selftests O=build
4) make -C tools/testing/selftests O=build CC="ccache gcc"
5) ./kci_build make_kselftest

Muhammad Usama Anjum (2):
  selftests: x86: allow expansion of $(CC)
  selftests: sgx: Treat CC as one argument

 tools/testing/selftests/sgx/Makefile    | 2 +-
 tools/testing/selftests/x86/check_cc.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.30.2

[PATCH 1/2] selftests: x86: allow expansion of $(CC)

[Muhammad Usama Anjum]

CC can have multiple sub-strings like "ccache gcc". Erorr pops up if
it is treated as single string and double quote are used around it.
This can be fixed by removing the quotes and not treating CC a single
string.

Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture detection")
Reported-by: "kernelci.org bot" <bot@kernelci.org>
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/x86/check_cc.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/x86/check_cc.sh b/tools/testing/selftests/x86/check_cc.sh
index 3e2089c8cf549..aff2c15018b53 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
-- 
2.30.2

Re: [PATCH 1/2] selftests: x86: allow expansion of $(CC)

[Shuah Khan]

On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote:
> CC can have multiple sub-strings like "ccache gcc". Erorr pops up if
> it is treated as single string and double quote are used around it.
> This can be fixed by removing the quotes and not treating CC a single
> string.
> 
> Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture detection")
> Reported-by: "kernelci.org bot" <bot@kernelci.org>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>   tools/testing/selftests/x86/check_cc.sh | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/x86/check_cc.sh b/tools/testing/selftests/x86/check_cc.sh
> index 3e2089c8cf549..aff2c15018b53 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
> 

The intent is testing if $CC is set. Does this change work when
$CC is not set?

thanks,
-- Shuah

Re: [PATCH 1/2] selftests: x86: allow expansion of $(CC)

[Muhammad Usama Anjum]

On 2/11/22 1:51 AM, Shuah Khan wrote:
> On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote:
>> CC can have multiple sub-strings like "ccache gcc". Erorr pops up if
>> it is treated as single string and double quote are used around it.
>> This can be fixed by removing the quotes and not treating CC a single
>> string.
>>
>> Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture
>> detection")
>> Reported-by: "kernelci.org bot" <bot@kernelci.org>
>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>> ---
>>   tools/testing/selftests/x86/check_cc.sh | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/x86/check_cc.sh
>> b/tools/testing/selftests/x86/check_cc.sh
>> index 3e2089c8cf549..aff2c15018b53 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
>>
> 
> The intent is testing if $CC is set. Does this change work when
> $CC is not set?
> 
Yeah, it works. I've added a debug variable inside sgx/Makefile and it
is detecting empty argument correctly as well.


--- a/tools/testing/selftests/sgx/Makefile
+++ b/tools/testing/selftests/sgx/Makefile
@@ -6,7 +6,7 @@ include ../lib.mk

 CAN_BUILD_X86_64 := $(shell ../x86/check_cc.sh "$(CC)" \
                            ../x86/trivial_64bit_program.c)
-
+$(info $$CAN_BUILD_X86_64 is [${CAN_BUILD_X86_64}])


Wrong examples:
➜  sgx (next-20220210_) ✗ make CC=""
$CAN_BUILD_X86_64 is [0]
➜  sgx (next-20220210_) ✗ make CC="cache gcc"
$CAN_BUILD_X86_64 is [0]

Correct examples:
➜  sgx (next-20220210_) ✗ make CC=gcc
$CAN_BUILD_X86_64 is [1]
➜  sgx (next-20220210_) ✗ make
$CAN_BUILD_X86_64 is [1]
➜  sgx (next-20220210_) ✗ make CC="ccache gcc"
$CAN_BUILD_X86_64 is [1]
➜  sgx (next-20220210_) ✗ make CC="gcc"
$CAN_BUILD_X86_64 is [1]
➜  sgx (next-20220210_) ✗ make CC="clang"
$CAN_BUILD_X86_64 is [1]
➜  sgx (next-20220210_) ✗ make CC="ccache clang"
$CAN_BUILD_X86_64 is [1]

Re: [PATCH 1/2] selftests: x86: allow expansion of $(CC)

[Shuah Khan]

On 2/10/22 3:41 PM, Muhammad Usama Anjum wrote:
> On 2/11/22 1:51 AM, Shuah Khan wrote:
>> On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote:
>>> CC can have multiple sub-strings like "ccache gcc". Erorr pops up if
>>> it is treated as single string and double quote are used around it.
>>> This can be fixed by removing the quotes and not treating CC a single
>>> string.
>>>
>>> Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture
>>> detection")
>>> Reported-by: "kernelci.org bot" <bot@kernelci.org>
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> ---
>>>    tools/testing/selftests/x86/check_cc.sh | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/x86/check_cc.sh
>>> b/tools/testing/selftests/x86/check_cc.sh
>>> index 3e2089c8cf549..aff2c15018b53 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
>>>
>>
>> The intent is testing if $CC is set. Does this change work when
>> $CC is not set?
>>
> Yeah, it works. I've added a debug variable inside sgx/Makefile and it
> is detecting empty argument correctly as well.
> 

Sounds good.

thanks,
-- Shuah

Re: [PATCH 1/2] selftests: x86: allow expansion of $(CC)

[Shuah Khan]

On 2/11/22 9:47 AM, David Laight wrote:
> From: Shuah Khan
>> Sent: 10 February 2022 20:52
>>
>> On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote:
>>> CC can have multiple sub-strings like "ccache gcc". Erorr pops up if
>>> it is treated as single string and double quote are used around it.
>>> This can be fixed by removing the quotes and not treating CC a single
>>> string.
>>>
>>> Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture detection")
>>> Reported-by: "kernelci.org bot" <bot@kernelci.org>
>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>> ---
>>>    tools/testing/selftests/x86/check_cc.sh | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/x86/check_cc.sh b/tools/testing/selftests/x86/check_cc.sh
>>> index 3e2089c8cf549..aff2c15018b53 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
>>>
>>
>> The intent is testing if $CC is set. Does this change work when
>> $CC is not set?
> 
> More by luck than judgement. Before and after.
> If $CC might be empty you probably want:
> 
> [ -n "$CC" ] && { echo 0; return; }
> 
> The subject is also wrong. Should be "allow field splitting' of ${CC}.
> (no brace or curly braces, not round ones.)
> 

Good points. It would be good enhancement to add the check - since the
current logic doesn't handle the null CC

thanks,
-- Shuah

Re: [PATCH 1/2] selftests: x86: allow expansion of $(CC)

[Muhammad Usama Anjum]

On 2/11/22 10:13 PM, Shuah Khan wrote:
> On 2/11/22 9:47 AM, David Laight wrote:
>> From: Shuah Khan
>>> Sent: 10 February 2022 20:52
>>>
>>> On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote:
>>>> CC can have multiple sub-strings like "ccache gcc". Erorr pops up if
>>>> it is treated as single string and double quote are used around it.
>>>> This can be fixed by removing the quotes and not treating CC a single
>>>> string.
>>>>
>>>> Fixes: e9886ace222e ("selftests, x86: Rework x86 target architecture
>>>> detection")
>>>> Reported-by: "kernelci.org bot" <bot@kernelci.org>
>>>> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
>>>> ---
>>>>    tools/testing/selftests/x86/check_cc.sh | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/tools/testing/selftests/x86/check_cc.sh
>>>> b/tools/testing/selftests/x86/check_cc.sh
>>>> index 3e2089c8cf549..aff2c15018b53 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
>>>>
>>>
>>> The intent is testing if $CC is set. Does this change work when
>>> $CC is not set?
>>
>> More by luck than judgement. Before and after.
>> If $CC might be empty you probably want:
>>
>> [ -n "$CC" ] && { echo 0; return; }
>>
>> The subject is also wrong. Should be "allow field splitting' of ${CC}.
>> (no brace or curly braces, not round ones.)
>>
> 
> Good points. It would be good enhancement to add the check - since the
> current logic doesn't handle the null CC
> 
I'll send a V2.
> thanks,
> -- Shuah

[PATCH 2/2] selftests: sgx: Treat CC as one argument

[Muhammad Usama Anjum]

CC can have multiple sub-strings like "ccache gcc". For check_cc.sh,
CC needs to be treated like one argument. Put double quotes around it to
make CC one string and hence one argument.

Fixes: 2adcba79e69d ("selftests/x86: Add a selftest for SGX")
Reported-by: "kernelci.org bot" <bot@kernelci.org>
Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
---
 tools/testing/selftests/sgx/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
index 2956584e1e37f..75af864e07b65 100644
--- a/tools/testing/selftests/sgx/Makefile
+++ b/tools/testing/selftests/sgx/Makefile
@@ -4,7 +4,7 @@ include ../lib.mk
 
 .PHONY: all clean
 
-CAN_BUILD_X86_64 := $(shell ../x86/check_cc.sh $(CC) \
+CAN_BUILD_X86_64 := $(shell ../x86/check_cc.sh "$(CC)" \
 			    ../x86/trivial_64bit_program.c)
 
 ifndef OBJCOPY
-- 
2.30.2

Re: [PATCH 2/2] selftests: sgx: Treat CC as one argument

[Shuah Khan]

On 2/10/22 12:06 PM, Muhammad Usama Anjum wrote:
> CC can have multiple sub-strings like "ccache gcc". For check_cc.sh,
> CC needs to be treated like one argument. Put double quotes around it to
> make CC one string and hence one argument.
> 
> Fixes: 2adcba79e69d ("selftests/x86: Add a selftest for SGX")
> Reported-by: "kernelci.org bot" <bot@kernelci.org>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>   tools/testing/selftests/sgx/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
> index 2956584e1e37f..75af864e07b65 100644
> --- a/tools/testing/selftests/sgx/Makefile
> +++ b/tools/testing/selftests/sgx/Makefile
> @@ -4,7 +4,7 @@ include ../lib.mk
>   
>   .PHONY: all clean
>   
> -CAN_BUILD_X86_64 := $(shell ../x86/check_cc.sh $(CC) \
> +CAN_BUILD_X86_64 := $(shell ../x86/check_cc.sh "$(CC)" \
>   			    ../x86/trivial_64bit_program.c)
>   
>   ifndef OBJCOPY
> 

The intent is testing if $CC is set. Does this change work when
$CC is not set?

thanks,
-- Shuah

Re: [PATCH 2/2] selftests: sgx: Treat CC as one argument

[Jarkko Sakkinen]

On Fri, Feb 11, 2022 at 12:06:41AM +0500, Muhammad Usama Anjum wrote:
> CC can have multiple sub-strings like "ccache gcc". For check_cc.sh,
> CC needs to be treated like one argument. Put double quotes around it to
> make CC one string and hence one argument.
> 
> Fixes: 2adcba79e69d ("selftests/x86: Add a selftest for SGX")
> Reported-by: "kernelci.org bot" <bot@kernelci.org>
> Signed-off-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> ---
>  tools/testing/selftests/sgx/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/sgx/Makefile b/tools/testing/selftests/sgx/Makefile
> index 2956584e1e37f..75af864e07b65 100644
> --- a/tools/testing/selftests/sgx/Makefile
> +++ b/tools/testing/selftests/sgx/Makefile
> @@ -4,7 +4,7 @@ include ../lib.mk
>  
>  .PHONY: all clean
>  
> -CAN_BUILD_X86_64 := $(shell ../x86/check_cc.sh $(CC) \
> +CAN_BUILD_X86_64 := $(shell ../x86/check_cc.sh "$(CC)" \
>  			    ../x86/trivial_64bit_program.c)
>  
>  ifndef OBJCOPY
> -- 
> 2.30.2
> 


Tested-by: Jarkko Sakkinen <jarkko@kernel.org>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

BR, Jarkko