Found one (the?) ARM bug: (Pointer) aliasing issues

[Iain Buclaw]

On Nov 8, 2013 7:20 PM, "Johannes Pfau" <nospam at example.com> wrote:
>
> Am Fri, 8 Nov 2013 13:12:23 +0000
> schrieb Iain Buclaw <ibuclaw at ubuntu.com>:
>
> > On 7 November 2013 18:05, Johannes Pfau <nospam at example.com> wrote:
> >
> > > Am Thu, 7 Nov 2013 16:14:59 +0000
> > > schrieb Iain Buclaw <ibuclaw at ubuntu.com>:
> > > type here. It would allow code like this to work: "char[] a; char*
> > > b = (cast(char*)&a)" but I don't see why this should work, it's
> > > illegal anyway?
> > >
> > >
> > That would be seen as two distinct alias sets that would break strict
> > aliasing in that example.
> >
> > Though, will have to implement -Wstrict-aliasing in the front-end to
> > get any feel for what could potentially be utterly wrong.  But the
> > idea is that for dynamic arrays, telling gcc to not rely on
> > structural equality to determine whether or not two dynamic arrays
> > are part of the same alias set.
> >
> > eg:
> > byte[] a, long[] b = *(cast (long[]*)&a) should be seen as being
> > different alias sets, and so *will* be about breaking strict aliasing.
> >
> > In contrast, string[] a, char[] b = *cast(string[]*)&a) should be
> > seen as being part of the same alias set, and so the compiler must
> > know that the two types (which are distinct structures to the
> > backend) could potentially be referencing the same slice of memory,
> > as to not cause any problems.
>
> Now I get what you mean. AFAIK the backend already strips type
> qualifiers when building alias sets, so char[] and string[] should
> already have the same alias set. But if you want to explicitly
> enforce this in GDC this makes sense.
>

I believe the difference is char* vs immutable(char)* - which a) is two
distinct struct types in the back end and b) the strip mod stuff would see
both as 'nothing to do' as the pointer has no qualifiers directly attached
to it.

Now I stop and think about dynamic arrays, I believe there's some work I
need to do on improving debugging of them (both in gdb and when debugging
gcc) - for instance when printing the type of an &array, is it still shown
as an unnamed 'struct*' ? :)

> >
> > For people trying to work around the cast system for dynamic arrays,
> > IMO they should be punished for it, and told to do it in the correct
> > way that invokes _d_arraycopy, or do their unsafe work through unions.
> >
>
> OK. Although then we have to be very careful when implementing
> _d_arraycast (but maybe we can just check if we're in _d_arraycast and
> disable aliasing? Is this possible?).
>

Should be ok if we give special treatment for void[]. I can't remember the
exact reason why there is a dereferenced cast in that function, might just
be a pointer copy... (when I think array cast,  I only see array.length =
(length * F.sizeof) / T.sizeof;

> > >
> > >
> > > // Permit type-punning when accessing a union
> > >
> > > Isn't that already guaranteed by GCC? See:
> > > http://code.metager.de/source/xref/gnu/gcc/gcc/alias.c#982
> > > [...]
> > >
> > >
> > There is no harm enforcing it in the front-end as well, [...]
> >
> OK
>
>
> >
> > Not that I'm aware of (other than Ada boasting it's use).  But I'd
> > like to push the opinion of - although it isn't in the spec, D should
> > be strict aliasing.  And people should be aware of the problems
> > breaking strict aliasing (see:
> >
http://cellperformance.beyond3d.com/articles/2006/06/understanding-strict-aliasing.html
)
>
> I only found this:
> http://docs.lib.purdue.edu/ecetr/123/
>
> They list performance improvements in the range of 0-6% depending on
> architecture. It's testing GCC from 2004 so some things may have
> changed, but 0-6% is not bad.
>
> > But first... plan of attack:
> >
> > - We first disable strict aliasing entirely (lang_hook.get_alias_set
> > => 0)
> > - Implement -Wstrict-aliasing
> > - Start turning on strict aliasing for types guaranteed not to be
> > referencing the same memory location as other types (eg: TypeBasic,
> > TypeDelegate, TypeSArray, TypeVector).
> > - Identify gdc problems with implicit code generation that could break
> > strict aliasing (these are our bugs).
> > - Identify frontend/library problems that could break strict aliasing
> > (these are the dmd/phobos developer's bugs).
> > - Turn on strict aliasing for the remaining types.  For those that
> > cause problems, we can define a TYPE_LANG_FLAG macro to allow us to
> > tell the backend if the type can alias any other types.
>
> Sounds good. If we can convince Walter to do this step-by-step
> (especially -Wstrict-aliasing needs to be in all compilers) this should
> work. But if GDC started enforcing strict aliasing and DMD didn't people
> probably wouldn't fix their code.
>
> Maybe the ARM port will be finished when we start working on this list
> so we have one more architecture to verify. ~5 phobos unit tests
> remaining ;-) (test suite already passes)
>
> >
> > Other possible considerations:
> >
> > - Most D code pretty much assumes that any object may be accessed via
> > a void[] or void*.
>
> Probably ubyte*, ubyte[] as well.
>
> >
> > - Infact, for the consideration of std.math.  It we could go one step
> > further and simply build up an alias set list based on the type size
> > over type distinction.  In this model
> > double/long/byte[8]/short[4]/int[2] would all be considered as types
> > that could be referencing each other.
> >
>
> I don't think that really helps much. Think of real with 16bytes. This
> may also negatively impact possible optimizations (I'd guess many types
> are word-sized. Another place where statistics would be nice)

reals are special in that they are flexible in size. std.math is able to
cope with this, but assumes that the target uses IEEE format. See my
implementation of floor() or lrint() for instance which uses different code
paths for each support float.

Regards
-- 
Iain Buclaw

*(p < e ? p++ : p) = (c & 0x0f) + '0';
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.puremagic.com/pipermail/d.gnu/attachments/20131109/09eadd49/attachment.html>