Received-SPF: pass (google.com: domain of linux-kernel+bounces-215603-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Received-SPF: Pass (protection.outlook.com: domain of nvidia.com designates
 216.228.117.161 as permitted sender) receiver=protection.outlook.com;
 client-ip=216.228.117.161; helo=mail.nvidia.com; pr=C
Message-ID: <ab6124eb-88a7-441b-add9-b12176308155@nvidia.com>
Date: Fri, 14 Jun 2024 16:38:47 -0700
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 1/3] selftests/vDSO: fix clang build errors and
 warnings
To: Shuah Khan <shuah@kernel.org>
CC: "Jason A . Donenfeld" <Jason@zx2c4.com>, Andy Lutomirski
	<luto@kernel.org>, Mark Brown <broonie@kernel.org>, Vincenzo Frascino
	<vincenzo.frascino@arm.com>, Colin Ian King <colin.i.king@gmail.com>,
	Valentin Obst <kernel@valentinobst.de>, <linux-kselftest@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>, <llvm@lists.linux.dev>, Carlos Llamas
	<cmllamas@google.com>, Edward Liaw <edliaw@google.com>, Muhammad Usama Anjum
	<usama.anjum@collabora.com>, Thomas Gleixner <tglx@linutronix.de>
References: <20240614233105.265009-1-jhubbard@nvidia.com>
 <20240614233105.265009-2-jhubbard@nvidia.com>
Content-Language: en-US
From: John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <20240614233105.265009-2-jhubbard@nvidia.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Jun 2024 23:39:02.2952
 (UTC)
X-MS-Exchange-CrossTenant-Network-Message-Id: 6c4cbf64-1c16-4d9e-331b-08dc8ccb2c02
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=43083d15-7273-40c1-b7db-39efd9ccc17a;Ip=[216.228.117.161];Helo=[mail.nvidia.com]
X-MS-Exchange-CrossTenant-AuthSource:
	SA2PEPF00003F63.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Anonymous
X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB6161

Also Cc'ing Thomas Gleixner on this one, sorry for the omission.

On 6/14/24 4:31 PM, John Hubbard wrote:
> When building with clang, via:
> 
>      make LLVM=1 -C tools/testing/selftests
> 
> ...there are several warnings, and an error. This fixes all of those and
> allows these tests to run and pass.
> 
> 1. Fix linker error (undefined reference to memcpy) by providing a local
>     version of memcpy.
> 
> 2. clang complains about using this form:
> 
>      if (g = h & 0xf0000000)
> 
> ...so factor out the assignment into a separate step.
> 
> 3. The code is passing a signed const char* to elf_hash(), which expects
>     a const unsigned char *. There are several callers, so fix this at
>     the source by allowing the function to accept a signed argument, and
>     then converting to unsigned operations, once inside the function.
> 
> 4. clang doesn't have __attribute__((externally_visible)) and generates
>     a warning to that effect. Fortunately, gcc 12 and gcc 13 do not seem
>     to require that attribute in order to build, run and pass tests here,
>     so remove it.
> 
> Reviewed-by: Carlos Llamas <cmllamas@google.com>
> Reviewed-by: Edward Liaw <edliaw@google.com>
> Reviewed-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Tested-by: Muhammad Usama Anjum <usama.anjum@collabora.com>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>
> ---
>   tools/testing/selftests/vDSO/parse_vdso.c      | 16 +++++++++++-----
>   .../selftests/vDSO/vdso_standalone_test_x86.c  | 18 ++++++++++++++++--
>   2 files changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/vDSO/parse_vdso.c b/tools/testing/selftests/vDSO/parse_vdso.c
> index 413f75620a35..4ae417372e9e 100644
> --- a/tools/testing/selftests/vDSO/parse_vdso.c
> +++ b/tools/testing/selftests/vDSO/parse_vdso.c
> @@ -55,14 +55,20 @@ static struct vdso_info
>   	ELF(Verdef) *verdef;
>   } vdso_info;
>   
> -/* Straight from the ELF specification. */
> -static unsigned long elf_hash(const unsigned char *name)
> +/*
> + * Straight from the ELF specification...and then tweaked slightly, in order to
> + * avoid a few clang warnings.
> + */
> +static unsigned long elf_hash(const char *name)
>   {
>   	unsigned long h = 0, g;
> -	while (*name)
> +	const unsigned char *uch_name = (const unsigned char *)name;
> +
> +	while (*uch_name)
>   	{
> -		h = (h << 4) + *name++;
> -		if (g = h & 0xf0000000)
> +		h = (h << 4) + *uch_name++;
> +		g = h & 0xf0000000;
> +		if (g)
>   			h ^= g >> 24;
>   		h &= ~g;
>   	}
> diff --git a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> index 8a44ff973ee1..27f6fdf11969 100644
> --- a/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> +++ b/tools/testing/selftests/vDSO/vdso_standalone_test_x86.c
> @@ -18,7 +18,7 @@
>   
>   #include "parse_vdso.h"
>   
> -/* We need a libc functions... */
> +/* We need some libc functions... */
>   int strcmp(const char *a, const char *b)
>   {
>   	/* This implementation is buggy: it never returns -1. */
> @@ -34,6 +34,20 @@ int strcmp(const char *a, const char *b)
>   	return 0;
>   }
>   
> +/*
> + * The clang build needs this, although gcc does not.
> + * Stolen from lib/string.c.
> + */
> +void *memcpy(void *dest, const void *src, size_t count)
> +{
> +	char *tmp = dest;
> +	const char *s = src;
> +
> +	while (count--)
> +		*tmp++ = *s++;
> +	return dest;
> +}
> +
>   /* ...and two syscalls.  This is x86-specific. */
>   static inline long x86_syscall3(long nr, long a0, long a1, long a2)
>   {
> @@ -70,7 +84,7 @@ void to_base10(char *lastdig, time_t n)
>   	}
>   }
>   
> -__attribute__((externally_visible)) void c_main(void **stack)
> +void c_main(void **stack)
>   {
>   	/* Parse the stack */
>   	long argc = (long)*stack;

thanks,
-- 
John Hubbard
NVIDIA