Re: [RFC PATCH] Fix pointer diff (was: -fsanitize=pointer-overflow support (PR sanitizer/80998))

[Richard Biener <rguenther@suse.de>]

On Thu, 22 Jun 2017, Marc Glisse wrote:

> On Thu, 22 Jun 2017, Richard Biener wrote:
> 
> > > If we consider pointers as unsigned, with a subtraction that has a signed
> > > result with the constraint that overflow is undefined, we cannot model
> > > that
> > > optimally with just the usual signed/unsigned operations, so I am in favor
> > > of
> > > POINTER_DIFF, at least in the long run (together with having a signed
> > > second
> > > argument for POINTER_PLUS, etc). For 64-bit platforms it might have been
> > > easier to declare that the upper half (3/4 ?) of the address space doesn't
> > > exist...
> > 
> > I repeatedly thought of POINTER_DIFF_EXPR but adding such a basic tree
> > code is quite a big job.
> 
> Yes :-(
> It is probably not realistic to introduce it just to avoid a couple
> regressions while fixing a bug.
> 
> > So we'd have POINTER_DIFF_EXPR take two pointer typed args and produce
> > ptrdiff_t.  What's the advantage of having this?
> 
> It represents q-p with one statement instead of 3 (long)q-(long)p or 4
> (long)((ulong)q-(ulong)p). It allows us to stay in the pointer world, so
> (q-p)>0 is equivalent to p<q, not just (long)p<(long)q. It properly models
> what (undefined) overflow means for pointers.
> 
> Of course it is hard to know in advance if that's significant or
> negligible, maybe size_t finds its way in too many places anyway.

As with all those experiments ...

Well, if I would sell this as a consultant to somebody I'd estimate
3 man months for this work which realistically means you have to
start now otherwise you won't make it this stage 1.

A smaller job would be to make POINTER_PLUS_EXPR take ptrdiff_t
as offset operand.  But the fallout is likely similar.  A lame(?)
half-way transition would allow for both unsigned and signed
ptrdiff_t (note sizetype -> [u]ptrdiff_t is already a transition
for some embedded archs eventually).  Maybe allowing both signed
and unsigned offsets is desirable (you of course get interesting
effects when combining those in GIMPLE where signedness matters).

Richard.

> > And yes, I agree that POINTER_PLUS_EXPR should take
> > ptrdiff_t rather than sizetype offset -- changing one without the
> > other will lead to awkwardness in required patterns involving
> > both like (p - q) + q.
> > 
> > As said, it's a big job with likely all sorts of (testsuite) fallout.
> > 
> > > > The third one is
> > > >        if (&a[b] - &a[c] != b - c)
> > > >                link_error();
> > > > where fold already during generic folding used to be able to cope with
> > > > it,
> > > > but now we have:
> > > > (long int) (((long unsigned int) b - (long unsigned int) c) * 4) /[ex] 4
> > > > !=
> > > > b - c
> > > > which we don't fold.
> > > 
> > > Once we have this last expression, we have lost, we need to know that the
> > > multiplication cannot overflow for this. When the size multiplications are
> > > done in a signed type in the future (?), it might help.
> > 
> > Not sure where the unsigned multiply comes from -- I guess we fold
> > it inside the cast ...
> 
> We usually do those multiplications in an unsigned type. I experimented
> with changing one such place in
> https://gcc.gnu.org/ml/gcc-patches/2017-05/msg01641.html , there is
> probably at least another one in the middle-end.
> 
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)