diff mbox series

c-family: check qualifiers of arguments to __atomic built-ins (PR 95378)

Message ID 20200616221111.GA1542265@redhat.com
State New
Headers show
Series c-family: check qualifiers of arguments to __atomic built-ins (PR 95378) | expand

Commit Message

Jonathan Wakely June 16, 2020, 10:11 p.m. UTC 
Currently the __atomic_{load,store,exchange,compare_exchange} built-ins
will happily store values through pointers to const, or use pointers to
volatile as the input and output arguments. This patch ensures that any
pointer that will be written through does not point to a const object,
and only the pointer to the atomic variable can be volatile.

This differs slightly from Clang, which allows the third argument to
__atomic_exchange (the one that is used to return the old value) to be
volatile if and only if the first argument is volatile. That doesn't
seem useful.

For C++ emit errors, but for C use pedwarns that are controlled by
-Wincompatible-pointer-types.

gcc/c-family/ChangeLog:

	* c-common.c (get_atomic_generic_size): Check cv-qualifiers in
	pointer arguments.

gcc/testsuite/ChangeLog:

	* c-c++-common/pr95378.c: New test.


Tested powerpc64le-linux.

OK for master?
commit 1212f9c14f7e5c55dc49b95c81c57e637e1bcb60
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Mon Jun 1 23:42:50 2020 +0100

    c-family: check qualifiers of arguments to __atomic built-ins (PR 95378)
    
    Currently the __atomic_{load,store,exchange,compare_exchange} built-ins
    will happily store values through pointers to const, or use pointers to
    volatile as the input and output arguments. This patch ensures that any
    pointer that will be written through does not point to a const object,
    and only the pointer to the atomic variable can be volatile.
    
    This differs slightly from Clang, which allows the third argument to
    __atomic_exchange (the one that is used to return the old value) to be
    volatile if and only if the first argument is volatile. That doesn't
    seem useful.
    
    For C++ emit errors, but for C use pedwarns that are controlled by
    -Wincompatible-pointer-types.
    
    gcc/c-family/ChangeLog:
    
            * c-common.c (get_atomic_generic_size): Check cv-qualifiers in
            pointer arguments.
    
    gcc/testsuite/ChangeLog:
    
            * c-c++-common/pr95378.c: New test.

Comments

Joseph Myers June 17, 2020, 5:53 p.m. UTC | #1 
On Tue, 16 Jun 2020, Jonathan Wakely via Gcc-patches wrote:

> Currently the __atomic_{load,store,exchange,compare_exchange} built-ins
> will happily store values through pointers to const, or use pointers to
> volatile as the input and output arguments. This patch ensures that any
> pointer that will be written through does not point to a const object,
> and only the pointer to the atomic variable can be volatile.
> 
> This differs slightly from Clang, which allows the third argument to
> __atomic_exchange (the one that is used to return the old value) to be
> volatile if and only if the first argument is volatile. That doesn't
> seem useful.
> 
> For C++ emit errors, but for C use pedwarns that are controlled by
> -Wincompatible-pointer-types.
> 
> gcc/c-family/ChangeLog:
> 
> 	* c-common.c (get_atomic_generic_size): Check cv-qualifiers in
> 	pointer arguments.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* c-c++-common/pr95378.c: New test.
> 
> 
> Tested powerpc64le-linux.
> 
> OK for master?

OK.
diff mbox series

Patch

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index b1379faa412..b73ad2ea3d2 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -6903,6 +6903,7 @@  get_atomic_generic_size (location_t loc, tree function,
 {
   unsigned int n_param;
   unsigned int n_model;
+  unsigned int outputs = 0; // bitset of output parameters
   unsigned int x;
   int size_0;
   tree type_0;
@@ -6913,15 +6914,22 @@  get_atomic_generic_size (location_t loc, tree function,
     case BUILT_IN_ATOMIC_EXCHANGE:
       n_param = 4;
       n_model = 1;
+      outputs = 5;
       break;
     case BUILT_IN_ATOMIC_LOAD:
+      n_param = 3;
+      n_model = 1;
+      outputs = 2;
+      break;
     case BUILT_IN_ATOMIC_STORE:
       n_param = 3;
       n_model = 1;
+      outputs = 1;
       break;
     case BUILT_IN_ATOMIC_COMPARE_EXCHANGE:
       n_param = 6;
       n_model = 2;
+      outputs = 3;
       break;
     default:
       gcc_unreachable ();
@@ -7010,6 +7018,39 @@  get_atomic_generic_size (location_t loc, tree function,
 		    function);
 	  return 0;
 	}
+
+      {
+	auto_diagnostic_group d;
+	int quals = TYPE_QUALS (TREE_TYPE (type));
+	/* Must not write to an argument of a const-qualified type.  */
+	if (outputs & (1 << x) && quals & TYPE_QUAL_CONST)
+	  {
+	    if (c_dialect_cxx ())
+	      {
+		error_at (loc, "argument %d of %qE must not be a pointer to "
+			  "a %<const%> type", x + 1, function);
+		return 0;
+	      }
+	    else
+	      pedwarn (loc, OPT_Wincompatible_pointer_types, "argument %d "
+		       "of %qE discards %<const%> qualifier", x + 1,
+		       function);
+	  }
+	/* Only the first argument is allowed to be volatile.  */
+	if (x > 0 && quals & TYPE_QUAL_VOLATILE)
+	  {
+	    if (c_dialect_cxx ())
+	      {
+		error_at (loc, "argument %d of %qE must not be a pointer to "
+			  "a %<volatile%> type", x + 1, function);
+		return 0;
+	      }
+	    else
+	      pedwarn (loc, OPT_Wincompatible_pointer_types, "argument %d "
+		       "of %qE discards %<volatile%> qualifier", x + 1,
+		       function);
+	  }
+      }
     }
 
   /* Check memory model parameters for validity.  */
diff --git a/gcc/testsuite/c-c++-common/pr95378.c b/gcc/testsuite/c-c++-common/pr95378.c
new file mode 100644
index 00000000000..a547d1febf5
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr95378.c
@@ -0,0 +1,111 @@ 
+/* { dg-do compile } */
+
+#define seq_cst __ATOMIC_SEQ_CST
+
+extern int *i;
+extern int *j;
+extern const int *c;
+extern volatile int *v;
+extern const volatile int *cv;
+
+void
+load()
+{
+  __atomic_load(c, i, seq_cst);
+  __atomic_load(cv, i, seq_cst);
+
+  __atomic_load(i, c, seq_cst);
+  /* { dg-error "argument 2 of '__atomic_load' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 2 of '__atomic_load' discards 'const' qualifier" "" { target c } .-2 } */
+  __atomic_load(i, v, seq_cst);
+  /* { dg-error "argument 2 of '__atomic_load' must not be a pointer to a 'volatile' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 2 of '__atomic_load' discards 'volatile' qualifier" "" { target c } .-2 } */
+  __atomic_load(i, cv, seq_cst);
+  /* { dg-error "argument 2 of '__atomic_load' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 2 of '__atomic_load' discards 'const' qualifier" "" { target c } .-2 } */
+  /* { dg-warning "argument 2 of '__atomic_load' discards 'volatile' qualifier" "" { target c } .-3 } */
+}
+
+void
+store()
+{
+  __atomic_store(i, c, seq_cst);
+  __atomic_store(v, c, seq_cst);
+
+  __atomic_store(c, i, seq_cst);
+  /* { dg-error "argument 1 of '__atomic_store' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 1 of '__atomic_store' discards 'const' qualifier" "" { target c } .-2 } */
+  __atomic_store(cv, i, seq_cst);
+  /* { dg-error "argument 1 of '__atomic_store' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 1 of '__atomic_store' discards 'const' qualifier" "" { target c } .-2 } */
+
+  __atomic_store(i, v, seq_cst);
+  /* { dg-error "argument 2 of '__atomic_store' must not be a pointer to a 'volatile' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 2 of '__atomic_store' discards 'volatile' qualifier" "" { target c } .-2 } */
+}
+
+void
+exchange()
+{
+  __atomic_exchange(i, c, j, seq_cst);
+  __atomic_exchange(v, i, j, seq_cst);
+  __atomic_exchange(v, c, j, seq_cst);
+
+  __atomic_exchange(c, i, j, seq_cst);
+  /* { dg-error "argument 1 of '__atomic_exchange' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 1 of '__atomic_exchange' discards 'const' qualifier" "" { target c } .-2 } */
+  __atomic_exchange(cv, i, j, seq_cst);
+  /* { dg-error "argument 1 of '__atomic_exchange' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 1 of '__atomic_exchange' discards 'const' qualifier" "" { target c } .-2 } */
+
+  __atomic_exchange(i, v, j, seq_cst);
+  /* { dg-error "argument 2 of '__atomic_exchange' must not be a pointer to a 'volatile' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 2 of '__atomic_exchange' discards 'volatile' qualifier" "" { target c } .-2 } */
+  __atomic_exchange(i, cv, j, seq_cst);
+  /* { dg-error "argument 2 of '__atomic_exchange' must not be a pointer to a 'volatile' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 2 of '__atomic_exchange' discards 'volatile' qualifier" "" { target c } .-2 } */
+
+  __atomic_exchange(i, j, c, seq_cst);
+  /* { dg-error "argument 3 of '__atomic_exchange' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 3 of '__atomic_exchange' discards 'const' qualifier" "" { target c } .-2 } */
+  __atomic_exchange(i, j, v, seq_cst);
+  /* { dg-error "argument 3 of '__atomic_exchange' must not be a pointer to a 'volatile' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 3 of '__atomic_exchange' discards 'volatile' qualifier" "" { target c } .-2 } */
+  __atomic_exchange(i, j, cv, seq_cst);
+  /* { dg-error "argument 3 of '__atomic_exchange' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 3 of '__atomic_exchange' discards 'const' qualifier" "" { target c } .-2 } */
+  /* { dg-warning "argument 3 of '__atomic_exchange' discards 'volatile' qualifier" "" { target c } .-3 } */
+}
+
+void
+compare_exchange()
+{
+  __atomic_compare_exchange(i, j, c, 1, seq_cst, seq_cst);
+  __atomic_compare_exchange(v, i, j, 1, seq_cst, seq_cst);
+  __atomic_compare_exchange(v, i, c, 1, seq_cst, seq_cst);
+
+  __atomic_compare_exchange(c, i, j, 1, seq_cst, seq_cst);
+  /* { dg-error "argument 1 of '__atomic_compare_exchange' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 1 of '__atomic_compare_exchange' discards 'const' qualifier" "" { target c } .-2 } */
+  __atomic_compare_exchange(cv, i, j, 1, seq_cst, seq_cst);
+  /* { dg-error "argument 1 of '__atomic_compare_exchange' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 1 of '__atomic_compare_exchange' discards 'const' qualifier" "" { target c } .-2 } */
+
+  __atomic_compare_exchange(i, c, j, 1, seq_cst, seq_cst);
+  /* { dg-error "argument 2 of '__atomic_compare_exchange' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 2 of '__atomic_compare_exchange' discards 'const' qualifier" "" { target c } .-2 } */
+  __atomic_compare_exchange(i, v, j, 1, seq_cst, seq_cst);
+  /* { dg-error "argument 2 of '__atomic_compare_exchange' must not be a pointer to a 'volatile' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 2 of '__atomic_compare_exchange' discards 'volatile' qualifier" "" { target c } .-2 } */
+  __atomic_compare_exchange(i, cv, j, 1, seq_cst, seq_cst);
+  /* { dg-error "argument 2 of '__atomic_compare_exchange' must not be a pointer to a 'const' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 2 of '__atomic_compare_exchange' discards 'const" "" { target c } .-2 } */
+  /* { dg-warning "argument 2 of '__atomic_compare_exchange' discards 'volatile' qualifier" "" { target c } .-3 } */
+
+  __atomic_compare_exchange(i, j, v, 1, seq_cst, seq_cst);
+  /* { dg-error "argument 3 of '__atomic_compare_exchange' must not be a pointer to a 'volatile' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 3 of '__atomic_compare_exchange' discards 'volatile' qualifier" "" { target c } .-2 } */
+  __atomic_compare_exchange(i, j, cv, 1, seq_cst, seq_cst);
+  /* { dg-error "argument 3 of '__atomic_compare_exchange' must not be a pointer to a 'volatile' type" "" { target c++ } .-1 } */
+  /* { dg-warning "argument 3 of '__atomic_compare_exchange' discards 'volatile' qualifier" "" { target c } .-2 } */
+}