Re: Change warnings for unsupported alignment to errors

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

On Tue, Nov 19, 2013 at 2:45 AM, Joseph S. Myers
<joseph@codesourcery.com> wrote:
> When implementing C11 _Alignas in
> <http://gcc.gnu.org/ml/gcc-patches/2011-11/msg00841.html>, I noted
> that I has omitted checks that alignment was supported (which, as
> constraints in C11, should be errors or pedwarns rather than just
> plain warnings).
>
> The issues with the C11 definition of alignment remain unresolved,
> although there's now N1731 for them (assigned DR numbers 444 and 445).
> However, it seems clearly right to me, and in accord with the intent
> of C11, that declaring an object with an unsupported alignment should
> be an error; in that case, any code generated must be presumed to be
> wrong code as it can't respect the alignment required by the user's
> source code.
>
> Contrary to what I said when implementing _Alignas, arbitrary stack
> alignments are in fact supported on all architectures (PR 33721, fixed
> a year before that patch).  Alignments too big for host unsigned int,
> when measured in bits, are rejected in
> c-common.c:check_user_alignment.  (That's less than ideal - ELF can
> represent alignments of up to 2^31 or 2^63 bytes, not bits, depending
> on whether it's 32-bit or 64-bit ELF.  But no doubt converting
> alignments to be represented in bytes would be a lot of work.)
>
> The alignment checks in varasm.c, however, give warnings rather than
> errors.  This patch changes them to errors.
>
> There's no testcase, as at least for the align_variable case it can't
> actually generate an error on ELF systems, where MAX_OFILE_ALIGNMENT
> is the largest alignment GCC can represent.  (I'm not sure under what
> circumstances assemble_noswitch_variable might generate its
> diagnostic.)
>
> (I do not make an assertion about whether the existing checks for
> supported alignment cover all cases where a requested alignment may
> not be supported; any failure to generate the requested alignment,
> with no diagnostic given, is an ordinary wrong-code bug.)
>
> Bootstrapped with no regressions on x86_64-unknown-linux-gnu.  OK to
> commit?
>
> 2013-11-19  Joseph Myers  <joseph@codesourcery.com>
>
>         * varasm.c (align_variable): Give error instead of warning for
>         unsupported alignment.
>         (assemble_noswitch_variable): Likewise.
>
> Index: gcc/varasm.c
> ===================================================================
> --- gcc/varasm.c        (revision 204948)
> +++ gcc/varasm.c        (working copy)
> @@ -960,9 +960,9 @@ align_variable (tree decl, bool dont_output_data)
>       In particular, a.out format supports a maximum alignment of 4.  */
>    if (align > MAX_OFILE_ALIGNMENT)
>      {
> -      warning (0, "alignment of %q+D is greater than maximum object "
> -               "file alignment.  Using %d", decl,
> -              MAX_OFILE_ALIGNMENT/BITS_PER_UNIT);
> +      error ("alignment of %q+D is greater than maximum object "
> +            "file alignment.  Using %d", decl,
> +            MAX_OFILE_ALIGNMENT/BITS_PER_UNIT);

The "Using %d" part of the diagnostic is now pointless, no?

Ok with removing it or changing it to tell the user the maximum supported
alignment.

Thanks,
Richard.

>        align = MAX_OFILE_ALIGNMENT;
>      }
>
> @@ -1908,8 +1908,8 @@ assemble_noswitch_variable (tree decl, const char
>
>    if (!sect->noswitch.callback (decl, name, size, rounded)
>        && (unsigned HOST_WIDE_INT) (align / BITS_PER_UNIT) > rounded)
> -    warning (0, "requested alignment for %q+D is greater than "
> -            "implemented alignment of %wu", decl, rounded);
> +    error ("requested alignment for %q+D is greater than "
> +          "implemented alignment of %wu", decl, rounded);
>  }
>
>  /* A subroutine of assemble_variable.  Output the label and contents of
>
> --
> Joseph S. Myers
> joseph@codesourcery.com