[Bug c/20794] New: [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[Bug c/20794] New: [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[jakub at gcc dot gnu dot org]

extern void abort (void);

typedef int T __attribute__((aligned));
struct S { T a[2]; } s;
void *p[2];

int
main (void)
{
  p[0] = &s.a[0];
  p[1] = &s.a[1];
  if (p[0] == p[1])
    abort ();
  return 0;
}

is miscompiled on at least i386, x86_64, ppc32 and ppc64, at all optimization
levels.  Works correctly in C++.

-- 
           Summary: [4.0/4.1 Regression] Miscompilation with __attribute
                    ((aligned))
           Product: gcc
           Version: 4.0.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P2
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jakub at gcc dot gnu dot org
                CC: gcc-bugs at gcc dot gnu dot org


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-04-06 14:52 -------
I want to say this is a middle-end problem as the trees look ok:
  p[0] = &s.a[0]{lb: 0 sz: 0};
  D.1140 = &s.a[1]{lb: 0 sz: 0};
  p[1] = D.1140;
  D.1141 = p[0];
  D.1142 = p[1];
  if (D.1141 == D.1142) goto <L0>; else goto <L1>;

Confirmed.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
          Component|c                           |middle-end
     Ever Confirmed|                            |1
   Last reconfirmed|0000-00-00 00:00:00         |2005-04-06 14:52:28
               date|                            |
   Target Milestone|---                         |4.0.0


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[jakub at gcc dot gnu dot org]

------- Additional Comments From jakub at gcc dot gnu dot org  2005-04-06 15:05 -------
Doesn't need to be in struct, and the requested alignment must be bigger
than sizeof (int) to reproduce.
extern void abort (void);

typedef int T __attribute__((aligned (8)));
T a[2];
void *p[2];

int
main (void)
{
  p[0] = &a[0];
  p[1] = &a[1];
  if (p[0] == p[1])
    abort ();
  return 0;
}
fails as well.

Note that the requested alignment is apparently honored by neither GCC 3.4 and
earlier in C, nor in G++ (3.4 as well as 4.0) for the individual array members,
only the whole array is aligned as if the attribute was present on the array
itself, not on the member type.

So we probably first need to answer whether that layout is correct or if we want
an ABI change here.

-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[jakub at gcc dot gnu dot org]

------- Additional Comments From jakub at gcc dot gnu dot org  2005-04-07 14:08 -------
This is caused by the big Ada patch that introduced the extra 2 ARRAY_REF
arguments.  Unfortunately, I couldn't find the reason why is the last
ARRAY_REF's argument measured in aligment units and not in bytes
(and what was the reason for adding this argument).

The size_zero_node in last ARRAY_REF's argument is created in
gimplify.c (canonicalize_addr_expr), because the element type has
TYPE_ALIGN_UNIT 16, but TYPE_SIZE_UNIT 4, therefore it is 0 strides and
when array_ref_element_size multiplies it again by TYPE_ALIGN_UNIT, the result
is 0 and not 4.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rth at gcc dot gnu dot org,
                   |                            |kenner at gcc dot gnu dot
                   |                            |org


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[kenner at vlsi1 dot ultra dot nyu dot edu]

------- Additional Comments From kenner at vlsi1 dot ultra dot nyu dot edu  2005-04-07 14:13 -------
Subject: Re:   [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

    This is caused by the big Ada patch that introduced the extra 2
    ARRAY_REF arguments.  Unfortunately, I couldn't find the reason why is
    the last ARRAY_REF's argument measured in aligment units and not in
    bytes (and what was the reason for adding this argument).

It has to do with the way MEM_ALIGN is computed.  What happens is that
when we compute the address of the MEM, we do it by multiplying the index
by the size in some units.  The second operand of that multiply is the only
way that RTL generation can know what the alignment is.  If the units
were in bytes, the multiplication wouldn't show the alignment.  We have
the same issue with DECL_FIELD_OFFSET.

There most definitely needs to be a better way to do this, but this goes back
years and I haven't found one.

    The element type has TYPE_ALIGN_UNIT 16, but TYPE_SIZE_UNIT 4,

That's wrong.  The size always needs to be a multiple of the alignment.
That's fundamental.


-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[jakub at gcc dot gnu dot org]

------- Additional Comments From jakub at gcc dot gnu dot org  2005-04-07 14:26 -------
> That's wrong.  The size always needs to be a multiple of the alignment.
> That's fundamental.

With user defined alignment, size certainly is not necessarily a multiple of the
alignment and with the exception of arrays it shouldn't cause any problems.
In arrays I agree this is a problem.  Now, the question is what to do:
1) issue error whenever a type with alignment bigger than size is used in
   an array
2) align the whole array and change the element type silently to a type
   with alignment equal to the element size


-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[kenner at vlsi1 dot ultra dot nyu dot edu]

------- Additional Comments From kenner at vlsi1 dot ultra dot nyu dot edu  2005-04-07 14:41 -------
Subject: Re:   [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

    With user defined alignment, size certainly is not necessarily a
    multiple of the alignment and with the exception of arrays it
    shouldn't cause any problems.

Well, I always thought that was invalid.  Indeed for Ada, I make a new
type with the larger size.  We've never actually *documented* anywhere
that the size is a multiple of the alignment, but I think lots of stuff
all over the place silently assumes it.

    In arrays I agree this is a problem.  Now, the question is what to do:
    1) issue error whenever a type with alignment bigger than size is used in
       an array
    2) align the whole array and change the element type silently to a type
       with alignment equal to the element size

I think (2) is best.


-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[rth at gcc dot gnu dot org]

------- Additional Comments From rth at gcc dot gnu dot org  2005-04-07 17:52 -------
For the record, this has come up on the lists before in some slightly other
context and Mark has advocated (1).  I'm of a mind to agree.

-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[kenner at vlsi1 dot ultra dot nyu dot edu]

------- Additional Comments From kenner at vlsi1 dot ultra dot nyu dot edu  2005-04-07 17:54 -------
Subject: Re:   [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

    For the record, this has come up on the lists before in some slightly
    other context and Mark has advocated (1).  I'm of a mind to agree.

I should add that I understand both arguments and don't have a very
strong feeling either way.


-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2005-04-07 18:00 -------
Yes, I still think that we ought to declare this invalid code.

If a particular front end (e.g., Ada) wants to adjust the types of the array
elements, etc., that's its business -- but the C/C++ front-ends should not.  And
the middle end should be able to expect that the size of the elements is at
least as large as their alignments.  

I think that once we agree on this, actually issuing the error will be easy to
do.  And, once we agree, we should change this PR to accepts-invalid.

-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[kenner at vlsi1 dot ultra dot nyu dot edu]

------- Additional Comments From kenner at vlsi1 dot ultra dot nyu dot edu  2005-04-07 18:03 -------
Subject: Re:   [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

    And the middle end should be able to expect that the size of the
    elements is at least as large as their alignments.

Are you suggesting that this be true just for *array* elements, for all
elements (including record components), or for all decls?


-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2005-04-07 18:08 -------
Subject: Re:  [4.0/4.1 Regression] Miscompilation with
 __attribute ((aligned))

kenner at vlsi1 dot ultra dot nyu dot edu wrote:
> ------- Additional Comments From kenner at vlsi1 dot ultra dot nyu dot edu  2005-04-07 18:03 -------
> Subject: Re:   [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))
> 
>     And the middle end should be able to expect that the size of the
>     elements is at least as large as their alignments.
> 
> Are you suggesting that this be true just for *array* elements, for all
> elements (including record components), or for all decls?

For array elements.  I don't see the same problem with record components 
or stand-along objects.  The problem comes only in arrays because that 
is where pointer-arithmetic is allowed, and then, either:

1. &a[1] != &a[0] + sizeof (a[0]), or

2. &a[1] is not aligned as per the element type

Both of those invariants are inviolate in C, in my opinion.



-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[sje at cup dot hp dot com]

------- Additional Comments From sje at cup dot hp dot com  2005-04-11 21:34 -------
This also came up in the string starting at 
http://gcc.gnu.org/ml/gcc/2005-03/msg00483.html

I gave a patch in

http://gcc.gnu.org/ml/gcc/2005-03/msg00729.html

but having GCC generate this error gives a bunch of test failures
in the vect tests and in gcc.dg/compat/struct-layout.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |sje at cup dot hp dot com


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[jsm28 at gcc dot gnu dot org]

------- Additional Comments From jsm28 at gcc dot gnu dot org  2005-04-16 16:15 -------
Much the same issue can arise with array references through
pointer-to-aligned-type, and with arithmetic on such pointers, as does with
array-of-aligned-type.  However, having extra alignment on a DECL, as opposed to
on its type, doesn't cause problems.

The obvious options include:

* Make a new type of larger size to match the alignment whenever e.g. an
8-byte-aligned-int is requested.  (Probably breaks too much.)

* Disallow arrays of extra-aligned types, and array references and pointer
arithmetic on such types; either with an error, or with a warning and removal of
the "aligned" attribute (in the case of arrays, attaching it to the array; in
the case of pointers, causing the results of the arithmetic to have the ordinary
type without alignment).

We could also add a target_aligned attribute which can be used to describe the
alignment of a pointer's target, to use for pointers to the start of an array
where the start is aligned but the individual elements aren't.  As well as
allowing e.g. to define functions taking such pointer arguments, this would also
allow refined rules: an array of an extra-aligned type has the alignment passed
to the array (with a warning); pointers to extra-aligned types have the extra
alignment removed from the target type in favour of a target_aligned attribute
on the pointer; extra-aligned-array decays to target_aligned pointer; arithmetic
on target_aligned pointers loses the target_aligned attribute unless GCC can
prove that the value being added is a multiple of the extra alignment.  I think
this, properly implemented and ideally with the problems applying attributes to
array parameters dealt with, would cover all the semantics people might want to
specify except if they really do want alignment attributes to increase the size
of the types being aligned.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jsm28 at gcc dot gnu dot org


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2005-04-17 02:43 -------
Subject: Re:  [4.0/4.1 Regression] Miscompilation with
 __attribute ((aligned))

jsm28 at gcc dot gnu dot org wrote:
> ------- Additional Comments From jsm28 at gcc dot gnu dot org  2005-04-16 16:15 -------
> Much the same issue can arise with array references through
> pointer-to-aligned-type, and with arithmetic on such pointers, as does with
> array-of-aligned-type.

Agreed.

> The obvious options include:
> 
> * Make a new type of larger size to match the alignment whenever e.g. an
> 8-byte-aligned-int is requested.  (Probably breaks too much.)
> 
> * Disallow arrays of extra-aligned types, and array references and pointer
> arithmetic on such types; either with an error, or with a warning and removal of
> the "aligned" attribute (in the case of arrays, attaching it to the array; in
> the case of pointers, causing the results of the arithmetic to have the ordinary
> type without alignment).

As a C front-end maintainer, which of these options do you prefer?  It 
sounds like you, like me, favor the second option, but I'd like to be sure.

> We could also add a target_aligned attribute which can be used to describe the
> alignment of a pointer's target, to use for pointers to the start of an array
> where the start is aligned but the individual elements aren't.

That sounds plausible, but should, IMO, be done after first implementing 
one of your options above.



-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2005-04-17 03:53 -------
It looks like we'll end up deciding that the bug here is in failing to issue an
error message on invalid code.

Target milestone pushed back to 4.0.1.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.0.0                       |4.0.1


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

[Bug middle-end/20794] [4.0/4.1 Regression] Miscompilation with __attribute ((aligned))

[joseph at codesourcery dot com]

------- Additional Comments From joseph at codesourcery dot com  2005-04-17 11:04 -------
Subject: Re:  [4.0/4.1 Regression] Miscompilation with
 __attribute ((aligned))

On Sun, 17 Apr 2005, mark at codesourcery dot com wrote:

> > The obvious options include:
> > 
> > * Make a new type of larger size to match the alignment whenever e.g. an
> > 8-byte-aligned-int is requested.  (Probably breaks too much.)
> > 
> > * Disallow arrays of extra-aligned types, and array references and pointer
> > arithmetic on such types; either with an error, or with a warning and removal of
> > the "aligned" attribute (in the case of arrays, attaching it to the array; in
> > the case of pointers, causing the results of the arithmetic to have the ordinary
> > type without alignment).
> 
> As a C front-end maintainer, which of these options do you prefer?  It 
> sounds like you, like me, favor the second option, but I'd like to be sure.

I'd prefer the second option.



-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Arrays and pointer arithmetic on __attribute ((aligned)) types permitted

[mmitchel at gcc dot gnu dot org]

------- Additional Comments From mmitchel at gcc dot gnu dot org  2005-04-17 17:06 -------
Joseph, RTH, and I all feel this should be invalid code.  I think that's
consensus.  So, I've updated the PR.  Patches to actually issue a diagnostic
would be welcome.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|wrong-code                  |accepts-invalid
            Summary|[4.0/4.1 Regression]        |[4.0/4.1 Regression] Arrays
                   |Miscompilation with         |and pointer arithmetic on
                   |__attribute ((aligned))     |__attribute ((aligned))
                   |                            |types permitted


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

[Bug middle-end/20794] [4.0/4.1 Regression] Arrays and pointer arithmetic on __attribute ((aligned)) types permitted

[sje at cup dot hp dot com]

------- Additional Comments From sje at cup dot hp dot com  2005-04-22 20:22 -------
I submitted a patch, http://gcc.gnu.org/ml/gcc-patches/2005-04/msg02284.html,
but as the mail says it results in a lot of regressions in the compat and
vector tests.

-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Arrays and pointer arithmetic on __attribute ((aligned)) types permitted

[dorit at il dot ibm dot com]

------- Additional Comments From dorit at il dot ibm dot com  2005-04-27 08:59 -------
(In reply to comment #18)
> I submitted a patch, http://gcc.gnu.org/ml/gcc-patches/2005-04/msg02284.html,
> but as the mail says it results in a lot of regressions in the compat and
> vector tests.

I'll see what I can do about the vector tests.



-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Arrays and pointer arithmetic on __attribute ((aligned)) types permitted

[sje at cup dot hp dot com]

------- Additional Comments From sje at cup dot hp dot com  2005-04-27 19:44 -------
It looks like most of the compat tests have been fixed but I still get two
failures.  They are tmpdir-gcc.dg-struct-layout-1/t002 and
tmpdir-gcc.dg-struct-layout-1/t027.  I cut down t002 and wound up with

void * __attribute__((aligned (8))) a[2];

I am not sure if this is supposed to apply the alignment to the array as a whole
or to the elements.  It appears to be applying it to the elements.  If it is
supposed to apply it to the elements then it is an illegal test, if it is
supposed to apply it to the array as a whole then it is a compiler bug.

-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Arrays and pointer arithmetic on __attribute ((aligned)) types permitted

[dorit at il dot ibm dot com]

------- Additional Comments From dorit at il dot ibm dot com  2005-05-02 14:14 -------
see patch: http://gcc.gnu.org/ml/gcc-patches/2005-04/msg02874.html



-- 


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

[Bug middle-end/20794] [4.0/4.1 Regression] Arrays and pointer arithmetic on __attribute ((aligned)) types permitted

[mmitchel at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.0.1                       |4.0.2


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

[Bug middle-end/20794] [4.0/4.1 Regression] Arrays and pointer arithmetic on __attribute ((aligned)) types permitted

[mmitchel at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|4.0.2                       |4.0.3


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