Re: [PATCH][4.2/trunk] Fix PR30252 (4.2) and SPEC2k6 dealII miscompile (trunk)

[Richard Guenther <rguenther at suse dot de>]

On Mon, 18 Jun 2007, Daniel Berlin wrote:

> On 6/18/07, Richard Guenther <rguenther@suse.de> wrote:
> > On Tue, 5 Jun 2007, Richard Guenther wrote:
> >
> > >
> > > This (controversical as of Danny) patch fixes two problems in the
> > > points-to solver that cause PR30252 on the 4.2 branch and dealII to
> > > be miscompiled on the trunk.
> > >
> > > The Problem with PR30252 is that we
> > > are faced with a C++ struct hierarchy that has empty bases as first
> > > members which are addressable (and actually addressed by the testcase).
> > > We cannot represent a correct points-to set for this, so we miscompute
> > > offsetting the pointer by a component reference later.  This is fixed
> > > by creating variable infos for the empty bases so the solver can point
> > > to them.
> > >
> > > The Problem with the dealII miscompile is that taking the address of
> > > a substructure can in some cases cause the wrong points-to set being
> > > computed from a previous solution if the substructure access was through
> > > a pointer.  This looks to us like a problem in solution_set_add, which
> > > relies on all aliases being computable by adding an offset to each of
> > > the old solution vars.  This is not true, so to fix this we simply make
> > > sure to not need to "compute" what an offset points to, as only vars in
> > > the previous solution are valid members of the offsetted solution.
> > >
> > > Bootstrapped and tested on x86_64-unknown-linux-gnu.  Danny, this is
> > > now the chance for you to come up with the "correct" solution to these
> > > two problems.  Otherwise I'll make sure to apply this to the branch
> > > before the 4.2.1 release.
> > >
> > > (no runtime testcase for the dealII miscompile, but the audit trail
> > > of the bugzilla contains a testcase that has wrong points-to sets
> > > computed)
> > >
> > > Thanks,
> > > Richard.
> >
> > Ping!
> >
> > We're half-way to the promised 4.2.1 release candidate.  To get a
> > little bit more testing coverage, is the following patch ok for
> > the 4.2 branch (and the mainline)?
> >
> 
> I have no problem putting this in 4.2.
> 
> I've explained why i believe it is the wrong solution in general (the
> constraints must be wrong for this to have any effect)
> 
> Please don't apply to mainline, I have another solution coming (in the
> pta-dev repository) that simply stops trying to use the points-to set
> of "foo" when trying to figure out what "foo->p" accesses.
> 
> Even your patch is not going to fix the array case you pointed out
> when there are SFT's involved.
> We need to stop handling pointer arithmetic for arrays with SFT's, or
> ensure we can handle absolutely every offset properly for an array
> pointer and mark those we couldn't (to collapse like we collapse other
> things).

I have applied the patch to the 4.2 branch and will shortly push the
testcase and the handle_ptr_arith change to mainline.

Thanks,
Richard.