[PATCH] selftests/arm64: Prevent build warnings from -Wmaybe-uninitialized

Anshuman Khandual posted 1 patch 3 months, 2 weeks ago
tools/testing/selftests/arm64/abi/tpidr2.c    | 2 +-
tools/testing/selftests/arm64/fp/za-fork.c    | 2 +-
tools/testing/selftests/arm64/gcs/basic-gcs.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
[PATCH] selftests/arm64: Prevent build warnings from -Wmaybe-uninitialized
Posted by Anshuman Khandual 3 months, 2 weeks ago
Arguments passed into WEXITSTATUS() should have been initialized earlier.
Otherwise following warning show up while building platform selftests on
arm64. Hence just zero out all the relevant local variables to avoid the
build warning.

Warning: ‘status’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Will Deacon <will@kernel.org>
Cc: Shuah Khan <shuah@kernel.org>
Cc: Mark Brown <broonie@kernel.org>
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kselftest@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com>
---
This applies on v6.16-rc3

 tools/testing/selftests/arm64/abi/tpidr2.c    | 2 +-
 tools/testing/selftests/arm64/fp/za-fork.c    | 2 +-
 tools/testing/selftests/arm64/gcs/basic-gcs.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/arm64/abi/tpidr2.c b/tools/testing/selftests/arm64/abi/tpidr2.c
index eb19dcc37a755..389a60e5feabf 100644
--- a/tools/testing/selftests/arm64/abi/tpidr2.c
+++ b/tools/testing/selftests/arm64/abi/tpidr2.c
@@ -96,7 +96,7 @@ static int write_sleep_read(void)
 static int write_fork_read(void)
 {
 	pid_t newpid, waiting, oldpid;
-	int status;
+	int status = 0;
 
 	set_tpidr2(getpid());
 
diff --git a/tools/testing/selftests/arm64/fp/za-fork.c b/tools/testing/selftests/arm64/fp/za-fork.c
index 587b946482226..6098beb3515a0 100644
--- a/tools/testing/selftests/arm64/fp/za-fork.c
+++ b/tools/testing/selftests/arm64/fp/za-fork.c
@@ -24,7 +24,7 @@ int verify_fork(void);
 int fork_test_c(void)
 {
 	pid_t newpid, waiting;
-	int child_status, parent_result;
+	int child_status = 0, parent_result;
 
 	newpid = fork();
 	if (newpid == 0) {
diff --git a/tools/testing/selftests/arm64/gcs/basic-gcs.c b/tools/testing/selftests/arm64/gcs/basic-gcs.c
index 3fb9742342a34..2b350b6b7e12c 100644
--- a/tools/testing/selftests/arm64/gcs/basic-gcs.c
+++ b/tools/testing/selftests/arm64/gcs/basic-gcs.c
@@ -240,7 +240,7 @@ static bool map_guarded_stack(void)
 static bool test_fork(void)
 {
 	unsigned long child_mode;
-	int ret, status;
+	int ret, status = 0;
 	pid_t pid;
 	bool pass = true;
 
-- 
2.30.2

Re: [PATCH] selftests/arm64: Prevent build warnings from -Wmaybe-uninitialized
Posted by Thomas Weißschuh 3 months ago
Hi Anshuman,

On Wed, Jun 25, 2025 at 03:01:38AM +0100, Anshuman Khandual wrote:
> Arguments passed into WEXITSTATUS() should have been initialized earlier.
> Otherwise following warning show up while building platform selftests on
> arm64. Hence just zero out all the relevant local variables to avoid the
> build warning.
> 
> Warning: ‘status’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Can you give the following patch a try instead?

https://lore.kernel.org/lkml/20250707-nolibc-waitpid-uninitialized-v1-1-dcd4e70bcd8f@linutronix.de/


Thomas
Re: [PATCH] selftests/arm64: Prevent build warnings from -Wmaybe-uninitialized
Posted by Mark Brown 3 months, 2 weeks ago
On Wed, Jun 25, 2025 at 03:01:38AM +0100, Anshuman Khandual wrote:

> @@ -96,7 +96,7 @@ static int write_sleep_read(void)
>  static int write_fork_read(void)
>  {
>  	pid_t newpid, waiting, oldpid;
> -	int status;
> +	int status = 0;
>  
>  	set_tpidr2(getpid());
>  

This will shut the warnings up, but it's a bit of a heavy hammer that
means that the warning can never trigger warnings for that variable
being unused.  Is it possible to fix this by updating the control flow
such that the compiler can tell that the initialisation follows the use?
Re: [PATCH] selftests/arm64: Prevent build warnings from -Wmaybe-uninitialized
Posted by Anshuman Khandual 3 months, 1 week ago

On 25/06/25 3:54 PM, Mark Brown wrote:
> On Wed, Jun 25, 2025 at 03:01:38AM +0100, Anshuman Khandual wrote:
> 
>> @@ -96,7 +96,7 @@ static int write_sleep_read(void)
>>  static int write_fork_read(void)
>>  {
>>  	pid_t newpid, waiting, oldpid;
>> -	int status;
>> +	int status = 0;
>>  
>>  	set_tpidr2(getpid());
>>  
> 
> This will shut the warnings up, but it's a bit of a heavy hammer that
> means that the warning can never trigger warnings for that variable
> being unused.  Is it possible to fix this by updating the control flow
> such that the compiler can tell that the initialisation follows the use?

The problem might not exist in reality. In the test function test_fork()
in the file tools/testing/selftests/arm64/gcs/basic-gcs.c there does not
seem to be a path where WIFEXITED(status) might get called when 'status'
has not been initialized as there is a preceding waitpid() which would
ensure 'status' gets set. Similar scenarios are present in fork_test_c()
and write_fork_read() as well. 

But the compiler still throws these build warnings. Seems to be false
positives and this fix just works around that.
Re: [PATCH] selftests/arm64: Prevent build warnings from -Wmaybe-uninitialized
Posted by Mark Brown 3 months, 1 week ago
On Fri, Jun 27, 2025 at 04:15:09PM +0530, Anshuman Khandual wrote:
> On 25/06/25 3:54 PM, Mark Brown wrote:

> >> -	int status;
> >> +	int status = 0;

> > This will shut the warnings up, but it's a bit of a heavy hammer that
> > means that the warning can never trigger warnings for that variable
> > being unused.  Is it possible to fix this by updating the control flow
> > such that the compiler can tell that the initialisation follows the use?

> The problem might not exist in reality. In the test function test_fork()
> in the file tools/testing/selftests/arm64/gcs/basic-gcs.c there does not
> seem to be a path where WIFEXITED(status) might get called when 'status'
> has not been initialized as there is a preceding waitpid() which would
> ensure 'status' gets set. Similar scenarios are present in fork_test_c()
> and write_fork_read() as well. 

Yes, it's a fairly common false positive pattern (in this case it'll be
that the compiler can't understand the constraints with pulling things
out of the exit status).

> But the compiler still throws these build warnings. Seems to be false
> positives and this fix just works around that.

Right, the problem is that it works around it with a big hammer that
just shuts the warning up completely meaning that if we do cause issues
in the future then the compiler won't be able to tell us about them.  It
would be better to suppess it by changing the control flow such that the
compiler can follow it.