Re: [patch, rfc] Semantics of attribute (aligned), misscompilation of crtstuff

["Richard Guenther" <richard dot guenther at gmail dot com>]

On 10/12/07, Zdenek Dvorak <rakdver@kam.mff.cuni.cz> wrote:
> Hello,
>
> crtbegin/end contain the following definitions:
>
> STATIC func_ptr __CTOR_LIST__[1]
>   __attribute__ ((__unused__, section(".ctors"), aligned(sizeof(func_ptr))))
>     = { (func_ptr) (-1) };
>
> STATIC func_ptr __CTOR_END__[1]
>   __attribute__((section(".ctors"), aligned(sizeof(func_ptr))))
>     = { (func_ptr) 0 };
>
> and between these two symbols, references to constructors are added,
> forming a table starting with __CTOR_LIST__ and ending with
> __CTOR_END__.  This table is then traversed using the following
> construction:
>
> func_ptr *p;
> for (p = __CTOR_END__ - 1; *p != (func_ptr) -1; p--)
>   (*p) ();
>
> When vectorizer is run, the alignment of __CTOR_LIST__ and __CTOR_END__ is
> increased, which efectively inserts a NULL pointer at __CTOR_END__[-1].
> This makes the cycle segfault, as it starts with calling
> __CTOR_END__[-1].
>
> I am not quite sure whether what vectorizer does is wrong --
> documentation seems to suggest that attribute(aligned) specifies the
> minimal alignment for the variable, so increasing it seems valid, and
> the assumption of crtstuff that there will be no padding seems wrong.
> If that were the case, crtstuff would need to be fixed.
>
> On the other hand, if attribute (aligned) specifies exact alignment,
> then the crtstuff hack would be ok, and the patch below to prevent
> us from increasing the alignment would be needed.
>
> Which interpretation is correct?

It certainly specifies minimal alignment, so I think crtstuff should be
fixed to not rely on an implementation detail that no longer holds true.

Richard.

> Zdenek
>
> Index: tree-vectorizer.c
> ===================================================================
> *** tree-vectorizer.c   (revision 129237)
> --- tree-vectorizer.c   (working copy)
> *************** increase_alignment (void)
> *** 2563,2568 ****
> --- 2563,2572 ----
>         if (DECL_ALIGN (decl) >= alignment)
>         continue;
>
> +       /* If the user explicitly asked for some alignment, do not change it.  */
> +       if (DECL_USER_ALIGN (decl))
> +       continue;
> +
>         if (vect_can_force_dr_alignment_p (decl, alignment))
>         {
>           DECL_ALIGN (decl) = TYPE_ALIGN (vectype);
>