Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)

[Richard Biener <richard dot guenther at gmail dot com>]

On Mon, May 5, 2014 at 10:27 PM, Marek Polacek <polacek@redhat.com> wrote:
> In this PR the issue is that we reject (valid) code such as
> _Alignas (long long) long long foo;
> with -m32, because we trip this condition:
>
>    alignas_align = 1U << declspecs->align_log;
>    if (alignas_align < TYPE_ALIGN_UNIT (type))
>      {
>        if (name)
>          error_at (loc, "%<_Alignas%> specifiers cannot reduce "
>                    "alignment of %qE", name);
>
> and error later on, since alignas_align is 4 (correct, see PR52023 for
> why), but TYPE_ALIGN_UNIT of long long is 8.  I think TYPE_ALIGN_UNIT
> is wrong here as that won't give us minimal alignment required.
> In c_sizeof_or_alignof_type we already have the code to compute such
> minimal alignment so I just moved the code to a separate function
> and used that instead of TYPE_ALIGN_UNIT.

Hmm, but isn't TYPE_ALIGN_UNIT wrong then?

Richard.

>
> Note that the test is run only on i?86 and x86_64, because we can't (?)
> easily determine which target requires what alignment.
>
> Regtested/bootstrapped on x86_64-unknown-linux-gnu and
> powerpc64-unknown-linux-gnu, ok for trunk?
>
> 2014-05-05  Marek Polacek  <polacek@redhat.com>
>
>         PR c/61053
> c-family/
>         * c-common.c (min_align_of_type): New function factored out from...
>         (c_sizeof_or_alignof_type): ...here.
>         * c-common.h (min_align_of_type): Declare.
> c/
>         * c-decl.c (grokdeclarator): Use min_align_of_type instead of
>         TYPE_ALIGN_UNIT.
> testsuite/
>         * gcc.dg/pr61053.c: New test.
>
> diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
> index 0ad955d..6d4440e 100644
> --- gcc/c-family/c-common.c
> +++ gcc/c-family/c-common.c
> @@ -4931,6 +4931,26 @@ c_common_get_alias_set (tree t)
>    return -1;
>  }
>
> +/* Return the least alignment required for type TYPE.  */
> +
> +unsigned int
> +min_align_of_type (tree type)
> +{
> +  unsigned int align = TYPE_ALIGN (type);
> +  align = MIN (align, BIGGEST_ALIGNMENT);
> +#ifdef BIGGEST_FIELD_ALIGNMENT
> +  align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
> +#endif
> +  unsigned int field_align = align;
> +#ifdef ADJUST_FIELD_ALIGN
> +  tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
> +                          type);
> +  field_align = ADJUST_FIELD_ALIGN (field, field_align);
> +#endif
> +  align = MIN (align, field_align);
> +  return align / BITS_PER_UNIT;
> +}
> +
>  /* Compute the value of 'sizeof (TYPE)' or '__alignof__ (TYPE)', where
>     the IS_SIZEOF parameter indicates which operator is being applied.
>     The COMPLAIN flag controls whether we should diagnose possibly
> @@ -5009,21 +5029,7 @@ c_sizeof_or_alignof_type (location_t loc,
>                                 size_int (TYPE_PRECISION (char_type_node)
>                                           / BITS_PER_UNIT));
>        else if (min_alignof)
> -       {
> -         unsigned int align = TYPE_ALIGN (type);
> -         align = MIN (align, BIGGEST_ALIGNMENT);
> -#ifdef BIGGEST_FIELD_ALIGNMENT
> -         align = MIN (align, BIGGEST_FIELD_ALIGNMENT);
> -#endif
> -         unsigned int field_align = align;
> -#ifdef ADJUST_FIELD_ALIGN
> -         tree field = build_decl (UNKNOWN_LOCATION, FIELD_DECL, NULL_TREE,
> -                                  type);
> -         field_align = ADJUST_FIELD_ALIGN (field, field_align);
> -#endif
> -         align = MIN (align, field_align);
> -         value = size_int (align / BITS_PER_UNIT);
> -       }
> +       value = size_int (min_align_of_type (type));
>        else
>         value = size_int (TYPE_ALIGN_UNIT (type));
>      }
> diff --git gcc/c-family/c-common.h gcc/c-family/c-common.h
> index 57b7dce..d34d2bb 100644
> --- gcc/c-family/c-common.h
> +++ gcc/c-family/c-common.h
> @@ -758,6 +758,7 @@ extern tree c_wrap_maybe_const (tree, bool);
>  extern tree c_save_expr (tree);
>  extern tree c_common_truthvalue_conversion (location_t, tree);
>  extern void c_apply_type_quals_to_decl (int, tree);
> +extern unsigned int min_align_of_type (tree);
>  extern tree c_sizeof_or_alignof_type (location_t, tree, bool, bool, int);
>  extern tree c_alignof_expr (location_t, tree);
>  /* Print an error message for invalid operands to arith operation CODE.
> diff --git gcc/c/c-decl.c gcc/c/c-decl.c
> index 6e7c589..9f7419a 100644
> --- gcc/c/c-decl.c
> +++ gcc/c/c-decl.c
> @@ -5931,7 +5931,7 @@ grokdeclarator (const struct c_declarator *declarator,
>        else if (declspecs->align_log != -1)
>         {
>           alignas_align = 1U << declspecs->align_log;
> -         if (alignas_align < TYPE_ALIGN_UNIT (type))
> +         if (alignas_align < min_align_of_type (type))
>             {
>               if (name)
>                 error_at (loc, "%<_Alignas%> specifiers cannot reduce "
> diff --git gcc/testsuite/gcc.dg/pr61053.c gcc/testsuite/gcc.dg/pr61053.c
> index e69de29..e3db534 100644
> --- gcc/testsuite/gcc.dg/pr61053.c
> +++ gcc/testsuite/gcc.dg/pr61053.c
> @@ -0,0 +1,75 @@
> +/* PR c/61053 */
> +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
> +/* { dg-options "-std=c11 -pedantic-errors" } */
> +
> +_Alignas (char) char cc;
> +_Alignas (short int) char cs;
> +_Alignas (int) char ci;
> +_Alignas (long int) char cl;
> +_Alignas (long long int) char cll;
> +_Alignas (float) char cf;
> +_Alignas (double) char cd;
> +_Alignas (long double) char cld;
> +
> +_Alignas (char) short int sc; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) short int ss;
> +_Alignas (int) short int si;
> +_Alignas (long int) short int sl;
> +_Alignas (long long int) short int sll;
> +_Alignas (float) short int sf;
> +_Alignas (double) short int sd;
> +_Alignas (long double) short int sld;
> +
> +_Alignas (char) int ic; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) int is; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) int ii;
> +_Alignas (long int) int il;
> +_Alignas (long long int) int ill;
> +_Alignas (float) int if_;
> +_Alignas (double) int id;
> +_Alignas (long double) int ild;
> +
> +_Alignas (char) long int lic; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) long int lis; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) long int lii; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long int) long int lil;
> +_Alignas (long long int) long int lill;
> +_Alignas (float) long int lif; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (double) long int lid;
> +_Alignas (long double) long int lild;
> +
> +_Alignas (char) long long int llic; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) long long int llis; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) long long int llii; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long int) long long int llil;
> +_Alignas (long long int) long long int llill;
> +_Alignas (float) long long int llif; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (double) long long int llid;
> +_Alignas (long double) long long int llild;
> +
> +_Alignas (char) float fc; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) float fs; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) float fi;
> +_Alignas (long int) float fl;
> +_Alignas (long long int) float fll;
> +_Alignas (float) float ff;
> +_Alignas (double) float fd;
> +_Alignas (long double) float fld;
> +
> +_Alignas (char) double dc; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) double ds; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) double di; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long int) double dl;
> +_Alignas (long long int) double dll;
> +_Alignas (float) double df; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (double) double dd;
> +_Alignas (long double) double dld;
> +
> +_Alignas (char) long double ldc; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (short int) long double lds; /* { dg-error "cannot reduce alignment" } */
> +_Alignas (int) long double ldi; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long int) long double ldl; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long long int) long double ldll; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (float) long double ldf; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (double) long double ldd; /* { dg-error "cannot reduce alignment" "" { target { lp64 } } } */
> +_Alignas (long double) long double ldld;
>
>         Marek