On Wed, Oct 6, 2021 at 5:03 PM Martin Sebor <mse...@gmail.com> wrote:
>
> On 10/6/21 7:47 AM, Aldy Hernandez via Gcc-patches wrote:
> -Wmaybe-uninitialized certainly suffers from a high rate of false
> positives (I count 40 open PRs). Even after all this time debugging
> it I still struggle with the code for it but in all the bug reports
> I've reviewed, only a small subset seems to be due to bugs or even
> limitations in it. Most are inherent in its design goal to warn
> for reads from variables that cannot be proven to have been
> initialized.
>
> If this one is a bug with some chance of getting fixed it would
> be good to distill it to a small test case (even though it's not
> unlikely that there already is an existing report with the same
> root cause).
>
> That said, from from your description and the IL above it sounds
> to me like the uninitialized read here is possible (it takes place
> when _881 != 0), and so -Wmaybe-uininitialized triggers as it's
> supposed to.
>
> Either way, rather than suppressing the warning for the whole file
> I would suggest to consider initializing the local variable instead.
> Looking at the code in calls.c, I can't convince myself that
> the variable cannot, in fact, be used uninitialized.
>
> Martin
FWIW, libgomp/team.c suffers from the same problem if you remove the
stop-gap in gomp_team_start():
struct gomp_thread_start_data *start_data = NULL;
The use of start_data in the block predicated by "if (nested)" is the
only path that can use start_data without passing through its
initialization. But the only way to elide the initialization is from:
if (!nested)
{
...
....
if (i == nthreads)
goto do_release;
}
So the use of start_data either crosses the gomp_alloca
initialization, or is predicated by if(nested).
And the gimple shows a very similar pattern to what I described earlier:
<bb 163> [local count: 59055800]:
# start_data_876 = PHI <start_data_770(162), start_data_258(288),
start_data_359(307)>
do_release:
if (_1 != 0)
goto <bb 164>; [55.90%]
else
goto <bb 289>; [44.10%]
<bb 289> [local count: 26046541]:
goto <bb 165>; [100.00%]
<bb 164> [local count: 33009259]:
_223 = &team_417(D)->barrier;
gomp_barrier_wait (_223);
goto <bb 166>; [100.00%]
<bb 265> [local count: 6962719]:
<bb 165> [local count: 33009259]:
# start_data_781 = PHI <start_data_876(289), start_data_518(D)(265)>
# old_threads_used_887 = PHI <old_threads_used_782(289),
old_threads_used_454(265)>
# affinity_count_825 = PHI <affinity_count_885(289), affinity_count_343(265)>
# affinity_thr_904 = PHI <affinity_thr_867(289), 0B(265)>
# force_display_840 = PHI <force_display_612(289), force_display_192(265)>
_589 = &MEM[(struct gomp_simple_barrier_t *)pool_410 + 64B].bar;
gomp_barrier_wait (_589);
<bb 166> [local count: 66018519]:
# start_data_870 = PHI <start_data_876(164), start_data_781(165)>
The use of start_data_781 coming in from BB265 (start_data_518(D))
would be uninitialized if the read wasn't guarded by "if (_1 != 0)".
It seems uninit has issues seeing this pattern.
Unfortunately reducing this has been, ahem...challenging, but the
problem is still there, both within calls.c on aarch64 and on x86-64
for libgomp/team.c.
Aldy
