Message ID | Pine.LNX.4.64.1311190144180.8831@digraph.polyomino.org.uk |
---|---|
State | New |
Headers | show |
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
On Tue, 19 Nov 2013, Richard Biener wrote: > > - 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. I've applied this patch with "... is greater than maximum object file alignment %d" (no ". Using").
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); 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