diff mbox series

c, c++: alignas and alignof void [PR104944]

Message ID 20220324154922.478547-1-polacek@redhat.com
State New
Headers show
Series c, c++: alignas and alignof void [PR104944] | expand

Commit Message

Marek Polacek March 24, 2022, 3:49 p.m. UTC
I started looking into this PR because in GCC 4.9 we were able to
detect the invalid

  struct alignas(void) S{};

but I broke it in r210262.

It's ill-formed code in C++:
[dcl.align]/3: "An alignment-specifier of the form alignas(type-id) has
the same effect as alignas(alignof(type-id))", and [expr.align]/1:
"The operand shall be a type-id representing a complete object type,
or an array thereof, or a reference to one of those types." and void
is not a complete type.

It's also invalid in C:
6.7.5: _Alignas(type-name) is equivalent to _Alignas(_Alignof(type-name))
6.5.3.4: "The _Alignof operator shall not be applied to a function type
or an incomplete type."

We have a GNU extension whereby we treat sizeof(void) as 1, but I assume
it doesn't apply to alignof, so I'd like to reject it in C too.

(We still say "invalid application of '__alignof__'" rather than
'alignas' but I felt that fixing that may not be suitable as part of this
patch.)

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

	PR c++/104944

gcc/c-family/ChangeLog:

	* c-common.cc (c_sizeof_or_alignof_type): Do not allow alignof(void).

gcc/cp/ChangeLog:

	* typeck.cc (cxx_alignas_expr): Call cxx_sizeof_or_alignof_type with
	complain == true.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/alignas20.C: New test.
	* gcc.dg/c11-align-10.c: New test.
---
 gcc/c-family/c-common.cc               | 20 +++++++++++++++-----
 gcc/cp/typeck.cc                       | 14 ++++++++------
 gcc/testsuite/g++.dg/cpp0x/alignas20.C | 24 ++++++++++++++++++++++++
 gcc/testsuite/gcc.dg/c11-align-10.c    | 18 ++++++++++++++++++
 4 files changed, 65 insertions(+), 11 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/alignas20.C
 create mode 100644 gcc/testsuite/gcc.dg/c11-align-10.c


base-commit: a3f78748fab6b24e3d4a8b319afd3f8afa17248f

Comments

Jason Merrill March 24, 2022, 4:02 p.m. UTC | #1
On 3/24/22 11:49, Marek Polacek wrote:
> I started looking into this PR because in GCC 4.9 we were able to
> detect the invalid
> 
>    struct alignas(void) S{};
> 
> but I broke it in r210262.
> 
> It's ill-formed code in C++:
> [dcl.align]/3: "An alignment-specifier of the form alignas(type-id) has
> the same effect as alignas(alignof(type-id))", and [expr.align]/1:
> "The operand shall be a type-id representing a complete object type,
> or an array thereof, or a reference to one of those types." and void
> is not a complete type.
> 
> It's also invalid in C:
> 6.7.5: _Alignas(type-name) is equivalent to _Alignas(_Alignof(type-name))
> 6.5.3.4: "The _Alignof operator shall not be applied to a function type
> or an incomplete type."
> 
> We have a GNU extension whereby we treat sizeof(void) as 1, but I assume
> it doesn't apply to alignof, so I'd like to reject it in C too.

That makes sense to me in principle, but we've allowed it since the 
beginning of version control, back when c_alignof was a separate 
function.  Changing that seems questionable for a regression fix.

> (We still say "invalid application of '__alignof__'" rather than
> 'alignas' but I felt that fixing that may not be suitable as part of this
> patch.)
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> 	PR c++/104944
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-common.cc (c_sizeof_or_alignof_type): Do not allow alignof(void).
> 
> gcc/cp/ChangeLog:
> 
> 	* typeck.cc (cxx_alignas_expr): Call cxx_sizeof_or_alignof_type with
> 	complain == true.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/alignas20.C: New test.
> 	* gcc.dg/c11-align-10.c: New test.
> ---
>   gcc/c-family/c-common.cc               | 20 +++++++++++++++-----
>   gcc/cp/typeck.cc                       | 14 ++++++++------
>   gcc/testsuite/g++.dg/cpp0x/alignas20.C | 24 ++++++++++++++++++++++++
>   gcc/testsuite/gcc.dg/c11-align-10.c    | 18 ++++++++++++++++++
>   4 files changed, 65 insertions(+), 11 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/alignas20.C
>   create mode 100644 gcc/testsuite/gcc.dg/c11-align-10.c
> 
> diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
> index d034837bb5b..8b76eba64c2 100644
> --- a/gcc/c-family/c-common.cc
> +++ b/gcc/c-family/c-common.cc
> @@ -3849,7 +3849,7 @@ c_common_get_alias_set (tree t)
>      the IS_SIZEOF parameter indicates which operator is being applied.
>      The COMPLAIN flag controls whether we should diagnose possibly
>      ill-formed constructs or not.  LOC is the location of the SIZEOF or
> -   TYPEOF operator.  If MIN_ALIGNOF, the least alignment required for
> +   ALIGNOF operator.  If MIN_ALIGNOF, the least alignment required for
>      a type in any context should be returned, rather than the normal
>      alignment for that type.  */
>   
> @@ -3891,10 +3891,20 @@ c_sizeof_or_alignof_type (location_t loc,
>       }
>     else if (type_code == VOID_TYPE || type_code == ERROR_MARK)
>       {
> -      if (type_code == VOID_TYPE
> -	  && complain && warn_pointer_arith)
> -	pedwarn (loc, OPT_Wpointer_arith,
> -		 "invalid application of %qs to a void type", op_name);
> +      if (type_code == VOID_TYPE && complain)
> +	{
> +	  /* sizeof(void) is a GNU extension.  */
> +	  if (is_sizeof)
> +	    pedwarn (loc, OPT_Wpointer_arith,
> +		     "invalid application of %qs to a void type", op_name);
> +	  /* But alignof(void) is not.  */
> +	  else
> +	    {
> +	      error_at (loc, "invalid application of %qs to a void type",
> +			op_name);
> +	      return error_mark_node;
> +	    }
> +	}
>         else if (!complain)
>           return error_mark_node;
>         value = size_one_node;
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index 516fa574ef6..96653c4f96e 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -1873,9 +1873,9 @@ compparms (const_tree parms1, const_tree parms2)
>   }
>   
>   
> -/* Process a sizeof or alignof expression where the operand is a
> -   type. STD_ALIGNOF indicates whether an alignof has C++11 (minimum alignment)
> -   or GNU (preferred alignment) semantics; it is ignored if op is
> +/* Process a sizeof or alignof expression where the operand is a type.
> +   STD_ALIGNOF indicates whether an alignof has C++11 (minimum alignment)
> +   or GNU (preferred alignment) semantics; it is ignored if OP is
>      SIZEOF_EXPR.  */
>   
>   tree
> @@ -2132,11 +2132,13 @@ cxx_alignas_expr (tree e)
>       /* [dcl.align]/3:
>          
>   	   When the alignment-specifier is of the form
> -	   alignas(type-id ), it shall have the same effect as
> -	   alignas(alignof(type-id )).  */
> +	   alignas(type-id), it shall have the same effect as
> +	   alignas(alignof(type-id)).  */
>   
>       return cxx_sizeof_or_alignof_type (input_location,
> -				       e, ALIGNOF_EXPR, true, false);
> +				       e, ALIGNOF_EXPR,
> +				       /*std_alignof=*/true,
> +				       /*complain=*/true);
>     
>     /* If we reach this point, it means the alignas expression if of
>        the form "alignas(assignment-expression)", so we should follow
> diff --git a/gcc/testsuite/g++.dg/cpp0x/alignas20.C b/gcc/testsuite/g++.dg/cpp0x/alignas20.C
> new file mode 100644
> index 00000000000..f04f27927e2
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/alignas20.C
> @@ -0,0 +1,24 @@
> +// PR c++/104944
> +// { dg-do compile { target c++11 } }
> +
> +struct inc;
> +
> +struct alignas(inc) S1 { }; // { dg-error "invalid application" }
> +struct alignas(void) S2 { }; // { dg-error "invalid application" }
> +
> +template <typename T>
> +struct alignas(T) S4 {}; // { dg-error "invalid application" }
> +
> +template <typename T>
> +struct alignas(T) S5 {}; // { dg-error "invalid application" }
> +
> +S4<void> s1;
> +S5<inc> s2;
> +
> +void
> +g ()
> +{
> +  auto s1 = alignof(void); // { dg-error "invalid application" }
> +  auto s2 = alignof(const void); // { dg-error "invalid application" }
> +  auto s3 = alignof(inc); // { dg-error "invalid application" }
> +}
> diff --git a/gcc/testsuite/gcc.dg/c11-align-10.c b/gcc/testsuite/gcc.dg/c11-align-10.c
> new file mode 100644
> index 00000000000..9943113fda2
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/c11-align-10.c
> @@ -0,0 +1,18 @@
> +/* PR c++/104944 */
> +/* { dg-do compile } */
> +/* { dg-options "-std=c11 -pedantic-errors" } */
> +
> +extern int a[];
> +
> +struct S {
> +  _Alignas(void) int arr1[10]; /* { dg-error "invalid application" } */
> +  _Alignas(__typeof(a)) int arr2[10]; /* { dg-error "invalid application" } */
> +};
> +
> +void
> +g ()
> +{
> +  int s1 = _Alignof(void); /* { dg-error "invalid application" } */
> +  int s2 = _Alignof(const void); /* { dg-error "invalid application" } */
> +  int s3 = _Alignof(__typeof(a)); /* { dg-error "invalid application" } */
> +}
> 
> base-commit: a3f78748fab6b24e3d4a8b319afd3f8afa17248f
diff mbox series

Patch

diff --git a/gcc/c-family/c-common.cc b/gcc/c-family/c-common.cc
index d034837bb5b..8b76eba64c2 100644
--- a/gcc/c-family/c-common.cc
+++ b/gcc/c-family/c-common.cc
@@ -3849,7 +3849,7 @@  c_common_get_alias_set (tree t)
    the IS_SIZEOF parameter indicates which operator is being applied.
    The COMPLAIN flag controls whether we should diagnose possibly
    ill-formed constructs or not.  LOC is the location of the SIZEOF or
-   TYPEOF operator.  If MIN_ALIGNOF, the least alignment required for
+   ALIGNOF operator.  If MIN_ALIGNOF, the least alignment required for
    a type in any context should be returned, rather than the normal
    alignment for that type.  */
 
@@ -3891,10 +3891,20 @@  c_sizeof_or_alignof_type (location_t loc,
     }
   else if (type_code == VOID_TYPE || type_code == ERROR_MARK)
     {
-      if (type_code == VOID_TYPE
-	  && complain && warn_pointer_arith)
-	pedwarn (loc, OPT_Wpointer_arith,
-		 "invalid application of %qs to a void type", op_name);
+      if (type_code == VOID_TYPE && complain)
+	{
+	  /* sizeof(void) is a GNU extension.  */
+	  if (is_sizeof)
+	    pedwarn (loc, OPT_Wpointer_arith,
+		     "invalid application of %qs to a void type", op_name);
+	  /* But alignof(void) is not.  */
+	  else
+	    {
+	      error_at (loc, "invalid application of %qs to a void type",
+			op_name);
+	      return error_mark_node;
+	    }
+	}
       else if (!complain)
         return error_mark_node;
       value = size_one_node;
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 516fa574ef6..96653c4f96e 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -1873,9 +1873,9 @@  compparms (const_tree parms1, const_tree parms2)
 }
 
 
-/* Process a sizeof or alignof expression where the operand is a
-   type. STD_ALIGNOF indicates whether an alignof has C++11 (minimum alignment)
-   or GNU (preferred alignment) semantics; it is ignored if op is
+/* Process a sizeof or alignof expression where the operand is a type.
+   STD_ALIGNOF indicates whether an alignof has C++11 (minimum alignment)
+   or GNU (preferred alignment) semantics; it is ignored if OP is
    SIZEOF_EXPR.  */
 
 tree
@@ -2132,11 +2132,13 @@  cxx_alignas_expr (tree e)
     /* [dcl.align]/3:
        
 	   When the alignment-specifier is of the form
-	   alignas(type-id ), it shall have the same effect as
-	   alignas(alignof(type-id )).  */
+	   alignas(type-id), it shall have the same effect as
+	   alignas(alignof(type-id)).  */
 
     return cxx_sizeof_or_alignof_type (input_location,
-				       e, ALIGNOF_EXPR, true, false);
+				       e, ALIGNOF_EXPR,
+				       /*std_alignof=*/true,
+				       /*complain=*/true);
   
   /* If we reach this point, it means the alignas expression if of
      the form "alignas(assignment-expression)", so we should follow
diff --git a/gcc/testsuite/g++.dg/cpp0x/alignas20.C b/gcc/testsuite/g++.dg/cpp0x/alignas20.C
new file mode 100644
index 00000000000..f04f27927e2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/alignas20.C
@@ -0,0 +1,24 @@ 
+// PR c++/104944
+// { dg-do compile { target c++11 } }
+
+struct inc;
+
+struct alignas(inc) S1 { }; // { dg-error "invalid application" }
+struct alignas(void) S2 { }; // { dg-error "invalid application" }
+
+template <typename T>
+struct alignas(T) S4 {}; // { dg-error "invalid application" }
+
+template <typename T>
+struct alignas(T) S5 {}; // { dg-error "invalid application" }
+
+S4<void> s1;
+S5<inc> s2;
+
+void
+g ()
+{
+  auto s1 = alignof(void); // { dg-error "invalid application" }
+  auto s2 = alignof(const void); // { dg-error "invalid application" }
+  auto s3 = alignof(inc); // { dg-error "invalid application" }
+}
diff --git a/gcc/testsuite/gcc.dg/c11-align-10.c b/gcc/testsuite/gcc.dg/c11-align-10.c
new file mode 100644
index 00000000000..9943113fda2
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/c11-align-10.c
@@ -0,0 +1,18 @@ 
+/* PR c++/104944 */
+/* { dg-do compile } */
+/* { dg-options "-std=c11 -pedantic-errors" } */
+
+extern int a[];
+
+struct S {
+  _Alignas(void) int arr1[10]; /* { dg-error "invalid application" } */
+  _Alignas(__typeof(a)) int arr2[10]; /* { dg-error "invalid application" } */
+};
+
+void
+g ()
+{
+  int s1 = _Alignof(void); /* { dg-error "invalid application" } */
+  int s2 = _Alignof(const void); /* { dg-error "invalid application" } */
+  int s3 = _Alignof(__typeof(a)); /* { dg-error "invalid application" } */
+}