diff mbox

-Winvalid-memory-model warning not given for stdatomic.h macros (PR c/69104)

Message ID 20160105150057.GF31604@redhat.com
State New
Headers show

Commit Message

Marek Polacek Jan. 5, 2016, 3 p.m. UTC
At present, we fail to warn about wrong memory orders for certain atomic
functions.  This happens for macros in stdatomic.h.  The fix is to use the
expansion point location rather than the input location.

While at it, I made a trivial fix to the wording of one of the warnings: don't
start with a capital and don't end the warning with a fullstop.

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

2016-01-05  Marek Polacek  <polacek@redhat.com>

	PR c/69104
	* builtins.c (get_memmodel): Use expansion point location rather than
	the input location.  Call warning_at rather than warning.
	(expand_builtin_atomic_compare_exchange): Likewise.
	(expand_builtin_atomic_load): Likewise.
	(expand_builtin_atomic_store): Likewise.
	(expand_builtin_atomic_clear): Likewise.

	* gcc.dg/atomic-invalid-2.c: New.


	Marek

Comments

Jeff Law Jan. 5, 2016, 6:27 p.m. UTC | #1
On 01/05/2016 08:00 AM, Marek Polacek wrote:
> At present, we fail to warn about wrong memory orders for certain atomic
> functions.  This happens for macros in stdatomic.h.  The fix is to use the
> expansion point location rather than the input location.
>
> While at it, I made a trivial fix to the wording of one of the warnings: don't
> start with a capital and don't end the warning with a fullstop.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2016-01-05  Marek Polacek  <polacek@redhat.com>
>
> 	PR c/69104
> 	* builtins.c (get_memmodel): Use expansion point location rather than
> 	the input location.  Call warning_at rather than warning.
> 	(expand_builtin_atomic_compare_exchange): Likewise.
> 	(expand_builtin_atomic_load): Likewise.
> 	(expand_builtin_atomic_store): Likewise.
> 	(expand_builtin_atomic_clear): Likewise.
>
> 	* gcc.dg/atomic-invalid-2.c: New.
OK.
jeff
diff mbox

Patch

diff --git gcc/builtins.c gcc/builtins.c
index 63d3190..eec4a58 100644
--- gcc/builtins.c
+++ gcc/builtins.c
@@ -5037,6 +5037,8 @@  get_memmodel (tree exp)
 {
   rtx op;
   unsigned HOST_WIDE_INT val;
+  source_location loc
+    = expansion_point_location_if_in_system_header (input_location);
 
   /* If the parameter is not a constant, it's a run time value so we'll just
      convert it to MEMMODEL_SEQ_CST to avoid annoying runtime checking.  */
@@ -5050,16 +5052,16 @@  get_memmodel (tree exp)
     val = targetm.memmodel_check (val);
   else if (val & ~MEMMODEL_MASK)
     {
-      warning (OPT_Winvalid_memory_model,
-	       "Unknown architecture specifier in memory model to builtin.");
+      warning_at (loc, OPT_Winvalid_memory_model,
+		  "unknown architecture specifier in memory model to builtin");
       return MEMMODEL_SEQ_CST;
     }
 
   /* Should never see a user explicit SYNC memodel model, so >= LAST works. */
   if (memmodel_base (val) >= MEMMODEL_LAST)
     {
-      warning (OPT_Winvalid_memory_model,
-	       "invalid memory model argument to builtin");
+      warning_at (loc, OPT_Winvalid_memory_model,
+		  "invalid memory model argument to builtin");
       return MEMMODEL_SEQ_CST;
     }
 
@@ -5111,23 +5113,25 @@  expand_builtin_atomic_compare_exchange (machine_mode mode, tree exp,
   enum memmodel success, failure;
   tree weak;
   bool is_weak;
+  source_location loc
+    = expansion_point_location_if_in_system_header (input_location);
 
   success = get_memmodel (CALL_EXPR_ARG (exp, 4));
   failure = get_memmodel (CALL_EXPR_ARG (exp, 5));
 
   if (failure > success)
     {
-      warning (OPT_Winvalid_memory_model,
-	       "failure memory model cannot be stronger than success memory "
-	       "model for %<__atomic_compare_exchange%>");
+      warning_at (loc, OPT_Winvalid_memory_model,
+		  "failure memory model cannot be stronger than success "
+		  "memory model for %<__atomic_compare_exchange%>");
       success = MEMMODEL_SEQ_CST;
     }
  
   if (is_mm_release (failure) || is_mm_acq_rel (failure))
     {
-      warning (OPT_Winvalid_memory_model,
-	       "invalid failure memory model for "
-	       "%<__atomic_compare_exchange%>");
+      warning_at (loc, OPT_Winvalid_memory_model,
+		  "invalid failure memory model for "
+		  "%<__atomic_compare_exchange%>");
       failure = MEMMODEL_SEQ_CST;
       success = MEMMODEL_SEQ_CST;
     }
@@ -5188,8 +5192,10 @@  expand_builtin_atomic_load (machine_mode mode, tree exp, rtx target)
   model = get_memmodel (CALL_EXPR_ARG (exp, 1));
   if (is_mm_release (model) || is_mm_acq_rel (model))
     {
-      warning (OPT_Winvalid_memory_model,
-	       "invalid memory model for %<__atomic_load%>");
+      source_location loc
+	= expansion_point_location_if_in_system_header (input_location);
+      warning_at (loc, OPT_Winvalid_memory_model,
+		  "invalid memory model for %<__atomic_load%>");
       model = MEMMODEL_SEQ_CST;
     }
 
@@ -5218,8 +5224,10 @@  expand_builtin_atomic_store (machine_mode mode, tree exp)
   if (!(is_mm_relaxed (model) || is_mm_seq_cst (model)
 	|| is_mm_release (model)))
     {
-      warning (OPT_Winvalid_memory_model,
-	       "invalid memory model for %<__atomic_store%>");
+      source_location loc
+	= expansion_point_location_if_in_system_header (input_location);
+      warning_at (loc, OPT_Winvalid_memory_model,
+		  "invalid memory model for %<__atomic_store%>");
       model = MEMMODEL_SEQ_CST;
     }
 
@@ -5319,8 +5327,10 @@  expand_builtin_atomic_clear (tree exp)
 
   if (is_mm_consume (model) || is_mm_acquire (model) || is_mm_acq_rel (model))
     {
-      warning (OPT_Winvalid_memory_model,
-	       "invalid memory model for %<__atomic_store%>");
+      source_location loc
+	= expansion_point_location_if_in_system_header (input_location);
+      warning_at (loc, OPT_Winvalid_memory_model,
+		  "invalid memory model for %<__atomic_store%>");
       model = MEMMODEL_SEQ_CST;
     }
 
diff --git gcc/testsuite/gcc.dg/atomic-invalid-2.c gcc/testsuite/gcc.dg/atomic-invalid-2.c
index e69de29..c73458e 100644
--- gcc/testsuite/gcc.dg/atomic-invalid-2.c
+++ gcc/testsuite/gcc.dg/atomic-invalid-2.c
@@ -0,0 +1,59 @@ 
+/* PR c/69104.  Test atomic routines for invalid memory model errors.  This
+   only needs to be tested on a single size.  */
+/* { dg-do compile } */
+/* { dg-require-effective-target sync_int_long } */
+
+#include <stdatomic.h>
+
+/* atomic_store_explicit():
+   The order argument shall not be memory_order_acquire,
+   memory_order_consume, nor memory_order_acq_rel.  */
+
+void
+store (atomic_int *i)
+{
+  atomic_store_explicit (i, 0, memory_order_consume); /* { dg-warning "invalid memory model" } */
+  atomic_store_explicit (i, 0, memory_order_acquire); /* { dg-warning "invalid memory model" } */
+  atomic_store_explicit (i, 0, memory_order_acq_rel); /* { dg-warning "invalid memory model" } */
+}
+
+/* atomic_load_explicit():
+   The order argument shall not be memory_order_release nor
+   memory_order_acq_rel.  */
+
+void
+load (atomic_int *i)
+{
+  atomic_int j = atomic_load_explicit (i, memory_order_release); /* { dg-warning "invalid memory model" } */
+  atomic_int k = atomic_load_explicit (i, memory_order_acq_rel); /* { dg-warning "invalid memory model" } */
+}
+
+/* atomic_compare_exchange():
+   The failure argument shall not be memory_order_release nor
+   memory_order_acq_rel.  The failure argument shall be no stronger than the
+   success argument.  */
+
+void
+exchange (atomic_int *i)
+{
+  int r;
+
+  atomic_compare_exchange_strong_explicit (i, &r, 0, memory_order_seq_cst, memory_order_release); /* { dg-warning "invalid failure memory" } */
+  atomic_compare_exchange_strong_explicit (i, &r, 0, memory_order_seq_cst, memory_order_acq_rel); /* { dg-warning "invalid failure memory" } */
+  atomic_compare_exchange_strong_explicit (i, &r, 0, memory_order_relaxed, memory_order_consume); /* { dg-warning "failure memory model cannot be stronger" } */
+
+  atomic_compare_exchange_weak_explicit (i, &r, 0, memory_order_seq_cst, memory_order_release); /* { dg-warning "invalid failure memory" } */
+  atomic_compare_exchange_weak_explicit (i, &r, 0, memory_order_seq_cst, memory_order_acq_rel); /* { dg-warning "invalid failure memory" } */
+  atomic_compare_exchange_weak_explicit (i, &r, 0, memory_order_relaxed, memory_order_consume); /* { dg-warning "failure memory model cannot be stronger" } */
+}
+
+/* atomic_flag_clear():
+   The order argument shall not be memory_order_acquire nor
+   memory_order_acq_rel.  */
+
+void
+clear (atomic_int *i)
+{
+  atomic_flag_clear_explicit (i, memory_order_acquire); /* { dg-warning "invalid memory model" } */
+  atomic_flag_clear_explicit (i, memory_order_acq_rel); /* { dg-warning "invalid memory model" } */
+}