diff mbox series

[1/5] sim/cgen: mask uninitialized variable warning in cgen-run.c

Message ID aab2ea3326ddac9c919045fd90c54085b1ce4811.1665578246.git.aburgess@redhat.com
State Committed
Commit cb9d1609da6e623158ba5a8cb4a2712bcea4f57f
Headers
Series Silence some build warnings in various simulators |

Commit Message

Andrew Burgess Oct. 12, 2022, 12:38 p.m. UTC 
  I see an uninitialized variable warning (with gcc 9.3.1) from
cgen-run.c, like this:

  /tmp/build/sim/../../src/sim/cris/../common/cgen-run.c: In function ‘sim_resume’:
  /tmp/build/sim/../../src/sim/cris/../common/cgen-run.c:259:5: warning: ‘engine_fns$’ may be used uninitialized in this function [-Wmaybe-uninitialized]
    259 |    (* engine_fns[next_cpu_nr]) (cpu);
        |    ~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  /tmp/build/sim/../../src/sim/cris/../common/cgen-run.c:232:14: note: ‘engine_fns$’ was declared here
    232 |   ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
        |              ^~~~~~~~~~

This is a false positive - we over allocate engine_fn, and then only
initialize the nr_cpus entries which we will later go on to use.

However, we can easily silence this warning by initializing the unused
entries in engine_fns to NULL, this might also help if anyone ever
looks at engine_fns in a debugger, it should now be obvious which
entries are in use, and which are not.

With this change the warning is gone.

There should be no change in behaviour with this commit.
---
 sim/common/cgen-run.c | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Mike Frysinger Oct. 23, 2022, 12:30 p.m. UTC | #1 
On 12 Oct 2022 13:38, Andrew Burgess via Gdb-patches wrote:
> --- a/sim/common/cgen-run.c
> +++ b/sim/common/cgen-run.c
> @@ -242,6 +242,11 @@ engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast
>        prime_cpu (cpu, max_insns);
>      }
>  
> +  /* Ensure the remaining engine_fns slots are initialized, this silences a
> +     compiler warning when engine_fns is used below.  */
> +  for (i = nr_cpus; i < MAX_NR_PROCESSORS; ++i)
> +    engine_fns[i] = NULL;

engine_fns is declared in this func.  why not assign it and let gcc handle
the rest ?
-  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
+  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS] = {};
-mike
Andrew Burgess Oct. 24, 2022, 3:57 p.m. UTC | #2 
Mike Frysinger <vapier@gentoo.org> writes:

> On 12 Oct 2022 13:38, Andrew Burgess via Gdb-patches wrote:
>> --- a/sim/common/cgen-run.c
>> +++ b/sim/common/cgen-run.c
>> @@ -242,6 +242,11 @@ engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast
>>        prime_cpu (cpu, max_insns);
>>      }
>>  
>> +  /* Ensure the remaining engine_fns slots are initialized, this silences a
>> +     compiler warning when engine_fns is used below.  */
>> +  for (i = nr_cpus; i < MAX_NR_PROCESSORS; ++i)
>> +    engine_fns[i] = NULL;
>
> engine_fns is declared in this func.  why not assign it and let gcc handle
> the rest ?
> -  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
> +  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS] = {};

How's the patch below?

Thanks,
Andrew

---

commit 79f4f1d82d1da482e223079deb453eda7b2d2323
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Mon Oct 24 16:55:07 2022 +0100

    sim/cgen: initialize variable at creation in engine_run_n
    
    Zero initialize engine_fns entirely at creation, then override those
    fields we intend to use, rather than zero just initializing the unused
    fields later on.
    
    There should be no user visible changes after this commit.

diff --git a/sim/common/cgen-run.c b/sim/common/cgen-run.c
index a9a493c01b9..1ace067a395 100644
--- a/sim/common/cgen-run.c
+++ b/sim/common/cgen-run.c
@@ -229,7 +229,7 @@ static void
 engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast_p)
 {
   int i;
-  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
+  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS] = {};
 
   SIM_ASSERT (nr_cpus <= MAX_NR_PROCESSORS);
   SIM_ASSERT (next_cpu_nr >= 0 && next_cpu_nr < nr_cpus);
@@ -242,11 +242,6 @@ engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast
       prime_cpu (cpu, max_insns);
     }
 
-  /* Ensure the remaining engine_fns slots are initialized, this silences a
-     compiler warning when engine_fns is used below.  */
-  for (i = nr_cpus; i < MAX_NR_PROCESSORS; ++i)
-    engine_fns[i] = NULL;
-
   while (1)
     {
       SIM_ENGINE_PREFIX_HOOK (sd);
Mike Frysinger Oct. 24, 2022, 3:59 p.m. UTC | #3 
On 24 Oct 2022 16:57, Andrew Burgess wrote:
> Mike Frysinger <vapier@gentoo.org> writes:
> commit 79f4f1d82d1da482e223079deb453eda7b2d2323
> Author: Andrew Burgess <aburgess@redhat.com>
> Date:   Mon Oct 24 16:55:07 2022 +0100
> 
>     sim/cgen: initialize variable at creation in engine_run_n
>     
>     Zero initialize engine_fns entirely at creation, then override those
>     fields we intend to use, rather than zero just initializing the unused
>     fields later on.
>     
>     There should be no user visible changes after this commit.
> 
> diff --git a/sim/common/cgen-run.c b/sim/common/cgen-run.c
> index a9a493c01b9..1ace067a395 100644
> --- a/sim/common/cgen-run.c
> +++ b/sim/common/cgen-run.c
> @@ -229,7 +229,7 @@ static void
>  engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast_p)
>  {
>    int i;
> -  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
> +  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS] = {};
>  
>    SIM_ASSERT (nr_cpus <= MAX_NR_PROCESSORS);
>    SIM_ASSERT (next_cpu_nr >= 0 && next_cpu_nr < nr_cpus);
> @@ -242,11 +242,6 @@ engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast
>        prime_cpu (cpu, max_insns);
>      }
>  
> -  /* Ensure the remaining engine_fns slots are initialized, this silences a
> -     compiler warning when engine_fns is used below.  */

this comment is useful, so i would retain it

otherwise lgtm
-mike
Andrew Burgess Oct. 27, 2022, 3:53 p.m. UTC | #4 
Mike Frysinger <vapier@gentoo.org> writes:

> On 24 Oct 2022 16:57, Andrew Burgess wrote:
>> Mike Frysinger <vapier@gentoo.org> writes:
>> commit 79f4f1d82d1da482e223079deb453eda7b2d2323
>> Author: Andrew Burgess <aburgess@redhat.com>
>> Date:   Mon Oct 24 16:55:07 2022 +0100
>> 
>>     sim/cgen: initialize variable at creation in engine_run_n
>>     
>>     Zero initialize engine_fns entirely at creation, then override those
>>     fields we intend to use, rather than zero just initializing the unused
>>     fields later on.
>>     
>>     There should be no user visible changes after this commit.
>> 
>> diff --git a/sim/common/cgen-run.c b/sim/common/cgen-run.c
>> index a9a493c01b9..1ace067a395 100644
>> --- a/sim/common/cgen-run.c
>> +++ b/sim/common/cgen-run.c
>> @@ -229,7 +229,7 @@ static void
>>  engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast_p)
>>  {
>>    int i;
>> -  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
>> +  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS] = {};
>>  
>>    SIM_ASSERT (nr_cpus <= MAX_NR_PROCESSORS);
>>    SIM_ASSERT (next_cpu_nr >= 0 && next_cpu_nr < nr_cpus);
>> @@ -242,11 +242,6 @@ engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast
>>        prime_cpu (cpu, max_insns);
>>      }
>>  
>> -  /* Ensure the remaining engine_fns slots are initialized, this silences a
>> -     compiler warning when engine_fns is used below.  */
>
> this comment is useful, so i would retain it

I added a variant of this comment back, and pushed this patch.

Thanks,
Andrew

---

commit a09f33be653fb112586be126f3d5ab848aaed095
Author: Andrew Burgess <aburgess@redhat.com>
Date:   Mon Oct 24 16:55:07 2022 +0100

    sim/cgen: initialize variable at creation in engine_run_n
    
    Zero initialize engine_fns entirely at creation, then override those
    fields we intend to use, rather than zero just initializing the unused
    fields later on.
    
    There should be no user visible changes after this commit.

diff --git a/sim/common/cgen-run.c b/sim/common/cgen-run.c
index a9a493c01b9..b6400a69c13 100644
--- a/sim/common/cgen-run.c
+++ b/sim/common/cgen-run.c
@@ -229,7 +229,9 @@ static void
 engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast_p)
 {
   int i;
-  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS];
+  /* Ensure that engine_fns is fully initialized, this silences a compiler
+     warning when engine_fns is used below.  */
+  ENGINE_FN *engine_fns[MAX_NR_PROCESSORS] = {};
 
   SIM_ASSERT (nr_cpus <= MAX_NR_PROCESSORS);
   SIM_ASSERT (next_cpu_nr >= 0 && next_cpu_nr < nr_cpus);
@@ -242,11 +244,6 @@ engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast
       prime_cpu (cpu, max_insns);
     }
 
-  /* Ensure the remaining engine_fns slots are initialized, this silences a
-     compiler warning when engine_fns is used below.  */
-  for (i = nr_cpus; i < MAX_NR_PROCESSORS; ++i)
-    engine_fns[i] = NULL;
-
   while (1)
     {
       SIM_ENGINE_PREFIX_HOOK (sd);
diff mbox series

Patch

diff --git a/sim/common/cgen-run.c b/sim/common/cgen-run.c
index 9a13b0ca416..a9a493c01b9 100644
--- a/sim/common/cgen-run.c
+++ b/sim/common/cgen-run.c
@@ -242,6 +242,11 @@  engine_run_n (SIM_DESC sd, int next_cpu_nr, int nr_cpus, int max_insns, int fast
       prime_cpu (cpu, max_insns);
     }
 
+  /* Ensure the remaining engine_fns slots are initialized, this silences a
+     compiler warning when engine_fns is used below.  */
+  for (i = nr_cpus; i < MAX_NR_PROCESSORS; ++i)
+    engine_fns[i] = NULL;
+
   while (1)
     {
       SIM_ENGINE_PREFIX_HOOK (sd);