[Bug tree-optimization/39612] [4.4/4.5/4.6/4.7 Regression] LIM inserts loads from uninitialized local memory

["rguenther at suse dot de" <gcc-bugzilla at gcc dot gnu dot org>]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39612

--- Comment #16 from rguenther at suse dot de <rguenther at suse dot de> 2012-01-17 10:40:31 UTC ---
On Tue, 17 Jan 2012, rakdver at gcc dot gnu.org wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=39612
> 
> --- Comment #14 from Zdenek Dvorak <rakdver at gcc dot gnu.org> 2012-01-17 10:19:01 UTC ---
> > > Also, the warning is at least morally right.  If R <= 1, the original code will
> > > pass inter to foo uninitialized, which probably is not intended.  So, the right
> > > thing to do could be issuing "may be used uninitialized" warning instead of "is
> > > used uninitialized" one.
> > 
> > Yes, but the point is that without the loop header copy we insert the
> > loads and stores from/to inter in a path that is executed even when
> > R <= 1 and thus the loop is not executed at all, and we warn about
> > the inserted loads - not about the final one.  Modified testcase:
> 
> I realize that; making the warning to point to the right line would be somewhat
> difficult, I guess.

Yeah.

> > For the store data-race consider
> > 
> > int inter[3];
> > void
> > f2 (int R)
> > {
> >   int i;
> > 
> >   for (i = 1; i < R; i++)
> >     {
> >       inter[0] = 1;
> >       inter[1] = 1;
> >       inter[2] = 1;
> >     }
> > }
> > 
> > where inter is protected by a mutex, but only if R > 1.  We still
> > insert loads/stores on paths that are executed when the loop is not
> > entered:
> 
> Which is to be expected, as long as inter is not volatile.  Store motion (and
> cse, ...) will cause this kind of problems, this does not seem to be anything
> specific to the testcase in question.  If you have something like
> 
>   for (i = 1; i < R; i++)
>     {
>        lock ();
>        do something with inter[1]
>        unlock ();
>     }
> 
> LSM will move inter[1] to a temporary variable regardless of the locks, which
> will cause race conditions with other threads (and whether loop header is
> copied or not is irrelevant).

I think for the explicit lock code we are safe because we consider
the lock/unlock calls to alias inter[] so we cannot SM it.  In the
light of the C++11 memory model we probably have to do something
about even non-volatile accesses.

I suppose we cannot easily detect at the moment if the loop has
its header copied, thus, is do {} while style?  We're using
ref_always_accessed_p for the trapping insns issue, we could
extend that to cover all global memory accesses - but I suppose
that would pessimize things if ref_always_accessed_p isn't
very good.