diff mbox

c/68966 - atomic_fetch_* on atomic_bool not diagnosed

Message ID 567A271B.2090403@gmail.com
State New
Headers show

Commit Message

Martin Sebor Dec. 23, 2015, 4:46 a.m. UTC
The attached patch rejects invocations of atomic fetch_op intrinsics
on objects of _Bool type as discussed in the context of PR c/68908.

Tested on x86_64.

Martin

Comments

Jeff Law Jan. 2, 2016, 7:43 a.m. UTC | #1
On 12/22/2015 09:46 PM, Martin Sebor wrote:
> The attached patch rejects invocations of atomic fetch_op intrinsics
> on objects of _Bool type as discussed in the context of PR c/68908.
>
> Tested on x86_64.
>
> Martin
>
> gcc-68966.patch
>
>
> gcc/testsuite/ChangeLog:
> 2015-12-22  Martin Sebor<msebor@redhat.com>
>
> 	PR c/68966
> 	* gcc.dg/atomic-fetch-bool.c: New test.
> 	* gcc.dg/sync-fetch-bool.c: Same.
>
> gcc/ChangeLog:
> 2015-12-22  Martin Sebor<msebor@redhat.com>
>
> 	PR c/68966
> 	* doc/extend.texi (__atomic Builtins, __sync Builtins): Document
> 	constraint on the type of arguments.
>
> gcc/c-family/ChangeLog:
> 2015-12-22  Martin Sebor<msebor@redhat.com>
>
> 	PR c/68966
> 	* c-common.c (sync_resolve_size): Reject first argument when it's
> 	a pointer to _Bool.
>
> Index: gcc/c-family/c-common.c
> ===================================================================
> --- gcc/c-family/c-common.c	(revision 231903)
> +++ gcc/c-family/c-common.c	(working copy)
> @@ -7667,6 +7667,6 @@
>
>     if (error_operand_p (align))
>       return -1;
>     if (TREE_CODE (align) != INTEGER_CST
>         || !INTEGRAL_TYPE_P (TREE_TYPE (align)))
>       {
No changes in this hunk.  I assume you hand-edited the patch to remove 
something that you really didn't intend to submit and left the above 
useless hunk.

The patch itself is fine for the trunk.

Thanks,
Jeff
diff mbox

Patch

gcc/testsuite/ChangeLog:
2015-12-22  Martin Sebor  <msebor@redhat.com>

	PR c/68966
	* gcc.dg/atomic-fetch-bool.c: New test.
	* gcc.dg/sync-fetch-bool.c: Same.

gcc/ChangeLog:
2015-12-22  Martin Sebor  <msebor@redhat.com>

	PR c/68966
	* doc/extend.texi (__atomic Builtins, __sync Builtins): Document
	constraint on the type of arguments.

gcc/c-family/ChangeLog:
2015-12-22  Martin Sebor  <msebor@redhat.com>

	PR c/68966
	* c-common.c (sync_resolve_size): Reject first argument when it's
	a pointer to _Bool.

Index: gcc/c-family/c-common.c
===================================================================
--- gcc/c-family/c-common.c	(revision 231903)
+++ gcc/c-family/c-common.c	(working copy)
@@ -7667,6 +7667,6 @@ 
 
   if (error_operand_p (align))
     return -1;
   if (TREE_CODE (align) != INTEGER_CST
       || !INTEGRAL_TYPE_P (TREE_TYPE (align)))
     {
@@ -10657,11 +10658,15 @@ 
 /* A helper function for resolve_overloaded_builtin in resolving the
    overloaded __sync_ builtins.  Returns a positive power of 2 if the
    first operand of PARAMS is a pointer to a supported data type.
-   Returns 0 if an error is encountered.  */
+   Returns 0 if an error is encountered.
+   FETCH is true when FUNCTION is one of the _FETCH_ built-ins.  */
 
 static int
-sync_resolve_size (tree function, vec<tree, va_gc> *params)
+sync_resolve_size (tree function, vec<tree, va_gc> *params, bool fetch)
 {
+  /* Type of the argument.  */
+  tree argtype;
+  /* Type the argument points to.  */
   tree type;
   int size;
 
@@ -10671,7 +10676,7 @@ 
       return 0;
     }
 
-  type = TREE_TYPE ((*params)[0]);
+  argtype = type = TREE_TYPE ((*params)[0]);
   if (TREE_CODE (type) == ARRAY_TYPE)
     {
       /* Force array-to-pointer decay for C++.  */
@@ -10686,12 +10691,16 @@ 
   if (!INTEGRAL_TYPE_P (type) && !POINTER_TYPE_P (type))
     goto incompatible;
 
+  if (fetch && TREE_CODE (type) == BOOLEAN_TYPE)
+      goto incompatible;
+
   size = tree_to_uhwi (TYPE_SIZE_UNIT (type));
   if (size == 1 || size == 2 || size == 4 || size == 8 || size == 16)
     return size;
 
  incompatible:
-  error ("incompatible type for argument %d of %qE", 1, function);
+  error ("operand type %qT is incompatible with argument %d of %qE",
+	 argtype, 1, function);
   return 0;
 }
 
@@ -11250,6 +11259,9 @@ 
 			    vec<tree, va_gc> *params)
 {
   enum built_in_function orig_code = DECL_FUNCTION_CODE (function);
+
+  /* Is function is one of the _FETCH_ built-ins?  */
+  bool fetch_op = true;
   bool orig_format = true;
   tree new_return = NULL_TREE;
 
@@ -11325,6 +11337,9 @@ 
     case BUILT_IN_ATOMIC_COMPARE_EXCHANGE_N:
     case BUILT_IN_ATOMIC_LOAD_N:
     case BUILT_IN_ATOMIC_STORE_N:
+      {
+	fetch_op = false;
+      }
     case BUILT_IN_ATOMIC_ADD_FETCH_N:
     case BUILT_IN_ATOMIC_SUB_FETCH_N:
     case BUILT_IN_ATOMIC_AND_FETCH_N:
@@ -11358,7 +11373,7 @@ 
     case BUILT_IN_SYNC_LOCK_TEST_AND_SET_N:
     case BUILT_IN_SYNC_LOCK_RELEASE_N:
       {
-	int n = sync_resolve_size (function, params);
+	int n = sync_resolve_size (function, params, fetch_op);
 	tree new_function, first_param, result;
 	enum built_in_function fncode;
 
Index: gcc/doc/extend.texi
===================================================================
--- gcc/doc/extend.texi	(revision 231903)
+++ gcc/doc/extend.texi	(working copy)
@@ -9246,6 +9246,8 @@ 
 @{ tmp = *ptr; *ptr = ~(tmp & value); return tmp; @}   // nand
 @end smallexample
 
+The object pointed to by the first argument must of integer or pointer type.  It must not be a Boolean type.
+
 @emph{Note:} GCC 4.4 and later implement @code{__sync_fetch_and_nand}
 as @code{*ptr = ~(tmp & value)} instead of @code{*ptr = ~tmp & value}.
 
@@ -9269,6 +9271,8 @@ 
 @{ *ptr = ~(*ptr & value); return *ptr; @}   // nand
 @end smallexample
 
+The same constraints on arguments apply as for the corresponding @code{__sync_op_and_fetch} built-in functions.
+
 @emph{Note:} GCC 4.4 and later implement @code{__sync_nand_and_fetch}
 as @code{*ptr = ~(*ptr & value)} instead of
 @code{*ptr = ~*ptr & value}.
@@ -9515,13 +9519,13 @@ 
 @deftypefnx {Built-in Function} @var{type} __atomic_or_fetch (@var{type} *ptr, @var{type} val, int memorder)
 @deftypefnx {Built-in Function} @var{type} __atomic_nand_fetch (@var{type} *ptr, @var{type} val, int memorder)
 These built-in functions perform the operation suggested by the name, and
-return the result of the operation. That is,
+return the result of the operation.  That is,
 
 @smallexample
 @{ *ptr @var{op}= val; return *ptr; @}
 @end smallexample
 
-All memory orders are valid.
+The object pointed to by the first argument must of integer or pointer type.  It must not be a Boolean type.  All memory orders are valid.
 
 @end deftypefn
 
@@ -9538,7 +9542,7 @@ 
 @{ tmp = *ptr; *ptr @var{op}= val; return tmp; @}
 @end smallexample
 
-All memory orders are valid.
+The same constraints on arguments apply as for the corresponding @code{__atomic_op_fetch} built-in functions.  All memory orders are valid.
 
 @end deftypefn
 
Index: gcc/testsuite/gcc.dg/atomic-fetch-bool.c
===================================================================
--- gcc/testsuite/gcc.dg/atomic-fetch-bool.c	(revision 0)
+++ gcc/testsuite/gcc.dg/atomic-fetch-bool.c	(working copy)
@@ -0,0 +1,26 @@ 
+/* PR c/68966  - atomic_fetch_* on atomic_bool not diagnosed
+   Test to verify that calls to __atomic_fetch_op funcions with a _Bool
+   argument are rejected.  This is necessary because GCC expects that
+   all initialized _Bool objects have a specific representation and
+   allowing atomic operations to change it would break the invariant.  */
+/* { dg-do compile } */
+/* { dg-options "-std=c11" } */
+
+
+void test_atomics (_Atomic _Bool *a)
+{
+  enum { SEQ_CST = __ATOMIC_SEQ_CST };
+  
+  __atomic_fetch_add (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_add." } */
+  __atomic_fetch_sub (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_sub." } */
+  __atomic_fetch_and (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_and." } */
+  __atomic_fetch_xor (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_xor." } */
+  __atomic_fetch_or (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_or." } */
+  __atomic_fetch_nand (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_fetch_nand." } */
+
+  __atomic_add_fetch (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_add_fetch." } */
+  __atomic_sub_fetch (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_sub_fetch." } */
+  __atomic_and_fetch (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_and_fetch." } */
+  __atomic_xor_fetch (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_xor_fetch." } */
+  __atomic_or_fetch (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_or_fetch." } */
+  __atomic_nand_fetch (a, 1, SEQ_CST);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__atomic_nand_fetch." } */
+}
Index: gcc/testsuite/gcc.dg/sync-fetch-bool.c
===================================================================
--- gcc/testsuite/gcc.dg/sync-fetch-bool.c	(revision 0)
+++ gcc/testsuite/gcc.dg/sync-fetch-bool.c	(working copy)
@@ -0,0 +1,24 @@ 
+/* PR c/68966  - atomic_fetch_* on atomic_bool not diagnosed
+   Test to verify that calls to __sync_fetch_op funcions with a _Bool
+   argument are rejected.  This is necessary because GCC expects that
+   all initialized _Bool objects have a specific representation and
+   allowing atomic operations to change it would break the invariant.  */
+/* { dg-do compile } */
+/* { dg-options "-std=c99" } */
+
+
+void test_sync (_Atomic _Bool *a)
+{
+  __sync_fetch_and_add (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_add." } */
+  __sync_fetch_and_sub (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_sub." } */
+  __sync_fetch_and_and (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_and." } */
+  __sync_fetch_and_xor (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_xor." } */
+  __sync_fetch_and_or (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_or." } */
+  __sync_fetch_and_nand (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_fetch_and_nand." } */
+  
+  __sync_add_and_fetch (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_add_and_fetch." } */
+  __sync_sub_and_fetch (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_sub_and_fetch." } */
+  __sync_and_and_fetch (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_and_and_fetch." } */
+  __sync_xor_and_fetch (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_xor_and_fetch." } */
+  __sync_or_and_fetch (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_or_and_fetch." } */
+  __sync_nand_and_fetch (a, 1);   /* { dg-error "operand type ._Atomic _Bool \\*. is incompatible with argument 1 of .__sync_nand_and_fetch." } */
+}