c: Fix up -Wunused-but-set-* warnings for _Atomics [PR99588]

As the following testcases show, compared to -D_Atomic= case we have many
-Wunused-but-set-* warning false positives.
When an _Atomic variable/parameter is read, we call mark_exp_read on it in
convert_lvalue_to_rvalue, but build_atomic_assign does not.
For consistency with the non-_Atomic case where we mark_exp_read the lhs
for lhs op= ... but not for lhs = ..., this patch does that too.
But furthermore we need to pattern match the trees emitted by _Atomic store,
so that _Atomic store itself is not marked as being a variable read, but
when the result of the store is used, we mark it.

2021-03-19  Jakub Jelinek  <jakub@redhat.com>

	PR c/99588
	* c-typeck.c (mark_exp_read): Recognize what build_atomic_assign
	with modifycode NOP_EXPR produces and mark the _Atomic var as read
	if found.
	(build_atomic_assign): For modifycode of NOP_EXPR, use COMPOUND_EXPRs
	rather than STATEMENT_LIST.  Otherwise call mark_exp_read on lhs.
	Set TREE_SIDE_EFFECTS on the TARGET_EXPR.

	* gcc.dg/Wunused-var-5.c: New test.
	* gcc.dg/Wunused-var-6.c: New test.

diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 4e6d369..2685afb 100644 (file)
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -1968,6 +1968,50 @@ mark_exp_read (tree exp)
       mark_exp_read (TREE_OPERAND (exp, 0));
       break;
     case COMPOUND_EXPR:
+      /* Pattern match what build_atomic_assign produces with modifycode
+        NOP_EXPR.  */
+      if (VAR_P (TREE_OPERAND (exp, 1))
+         && DECL_ARTIFICIAL (TREE_OPERAND (exp, 1))
+         && TREE_CODE (TREE_OPERAND (exp, 0)) == COMPOUND_EXPR)
+       {
+         tree t1 = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
+         tree t2 = TREE_OPERAND (TREE_OPERAND (exp, 0), 1);
+         if (TREE_CODE (t1) == TARGET_EXPR
+             && TARGET_EXPR_SLOT (t1) == TREE_OPERAND (exp, 1)
+             && TREE_CODE (t2) == CALL_EXPR)
+           {
+             tree fndecl = get_callee_fndecl (t2);
+             tree arg = NULL_TREE;
+             if (fndecl
+                 && TREE_CODE (fndecl) == FUNCTION_DECL
+                 && fndecl_built_in_p (fndecl, BUILT_IN_NORMAL)
+                 && call_expr_nargs (t2) >= 2)
+               switch (DECL_FUNCTION_CODE (fndecl))
+                 {
+                 case BUILT_IN_ATOMIC_STORE:
+                   arg = CALL_EXPR_ARG (t2, 1);
+                   break;
+                 case BUILT_IN_ATOMIC_STORE_1:
+                 case BUILT_IN_ATOMIC_STORE_2:
+                 case BUILT_IN_ATOMIC_STORE_4:
+                 case BUILT_IN_ATOMIC_STORE_8:
+                 case BUILT_IN_ATOMIC_STORE_16:
+                   arg = CALL_EXPR_ARG (t2, 0);
+                   break;
+                 default:
+                   break;
+                 }
+             if (arg)
+               {
+                 STRIP_NOPS (arg);
+                 if (TREE_CODE (arg) == ADDR_EXPR
+                     && DECL_P (TREE_OPERAND (arg, 0))
+                     && TYPE_ATOMIC (TREE_TYPE (TREE_OPERAND (arg, 0))))
+                   mark_exp_read (TREE_OPERAND (arg, 0));
+               }
+           }
+       }
+      /* FALLTHRU */
     case C_MAYBE_CONST_EXPR:
       mark_exp_read (TREE_OPERAND (exp, 1));
       break;
@@ -4065,7 +4109,7 @@ build_atomic_assign (location_t loc, tree lhs, enum tree_code modifycode,
   vec<tree, va_gc> *params;
   tree val, nonatomic_lhs_type, nonatomic_rhs_type, newval, newval_addr;
   tree old, old_addr;
-  tree compound_stmt;
+  tree compound_stmt = NULL_TREE;
   tree stmt, goto_stmt;
   tree loop_label, loop_decl, done_label, done_decl;
 
@@ -4086,7 +4130,15 @@ build_atomic_assign (location_t loc, tree lhs, enum tree_code modifycode,
 
   /* Create a compound statement to hold the sequence of statements
      with a loop.  */
-  compound_stmt = c_begin_compound_stmt (false);
+  if (modifycode != NOP_EXPR)
+    {
+      compound_stmt = c_begin_compound_stmt (false);
+
+      /* For consistency with build_modify_expr on non-_Atomic,
+        mark the lhs as read.  Also, it would be very hard to match
+        such expressions in mark_exp_read.  */
+      mark_exp_read (lhs);
+    }
 
   /* Remove any excess precision (which is only present here in the
      case of compound assignments).  */
@@ -4112,13 +4164,16 @@ build_atomic_assign (location_t loc, tree lhs, enum tree_code modifycode,
   TREE_NO_WARNING (val) = 1;
   rhs = build4 (TARGET_EXPR, nonatomic_rhs_type, val, rhs, NULL_TREE,
                NULL_TREE);
+  TREE_SIDE_EFFECTS (rhs) = 1;
   SET_EXPR_LOCATION (rhs, loc);
-  add_stmt (rhs);
+  if (modifycode != NOP_EXPR)
+    add_stmt (rhs);
 
   /* NOP_EXPR indicates it's a straight store of the RHS. Simply issue
      an atomic_store.  */
   if (modifycode == NOP_EXPR)
     {
+      compound_stmt = rhs;
       /* Build __atomic_store (&lhs, &val, SEQ_CST)  */
       rhs = build_unary_op (loc, ADDR_EXPR, val, false);
       fndecl = builtin_decl_explicit (BUILT_IN_ATOMIC_STORE);
@@ -4126,10 +4181,9 @@ build_atomic_assign (location_t loc, tree lhs, enum tree_code modifycode,
       params->quick_push (rhs);
       params->quick_push (seq_cst);
       func_call = c_build_function_call_vec (loc, vNULL, fndecl, params, NULL);
-      add_stmt (func_call);
 
-      /* Finish the compound statement.  */
-      compound_stmt = c_end_compound_stmt (loc, compound_stmt, false);
+      compound_stmt = build2 (COMPOUND_EXPR, void_type_node,
+                             compound_stmt, func_call);
 
       /* VAL is the value which was stored, return a COMPOUND_STMT of
         the statement and that value.  */

diff --git a/gcc/testsuite/gcc.dg/Wunused-var-5.c b/gcc/testsuite/gcc.dg/Wunused-var-5.c
new file mode 100644 (file)
index 0000000..cc5bbf5
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wunused-var-5.c
@@ -0,0 +1,23 @@
+/* PR c/99588 */
+/* { dg-do compile } */
+/* { dg-options "-std=c11 -Wunused-but-set-variable" } */
+
+void bar (int, ...);
+void f1 (void) { static _Atomic int x = 0; bar (0, x); }
+void f2 (void) { static _Atomic int x = 0; bar (0, x += 1); }
+void f3 (void) { static _Atomic int x = 0; bar (x); }
+void f4 (void) { static _Atomic int x = 0; bar (x += 1); }
+void f5 (void) { static _Atomic int x = 0; bar (x = 1); }
+void f6 (void) { static _Atomic int x = 0; x = 1; }    /* { dg-warning "variable 'x' set but not used" } */
+void f7 (void) { static _Atomic int x = 0; x += 3; }
+void f8 (void) { _Atomic int x = 0; bar (0, x); }
+void f9 (void) { _Atomic int x = 0; bar (0, x += 1); }
+void f10 (void) { _Atomic int x = 0; bar (x); }
+void f11 (void) { _Atomic int x = 0; bar (x += 1); }
+void f12 (void) { _Atomic int x = 0; bar (x = 1); }
+void f13 (void) { _Atomic int x = 0; x = 1; }          /* { dg-warning "variable 'x' set but not used" } */
+void f14 (void) { _Atomic int x = 0; x += 3; }
+void f15 (void) { _Atomic int x = 0; int y = 3; x += y; }
+void f16 (void) { _Atomic int x = 0; int y = 3; bar (x += y); }
+void f17 (void) { _Atomic int x = 0; int y = 3; x = y; }       /* { dg-warning "variable 'x' set but not used" } */
+void f18 (void) { _Atomic int x = 0; int y = 3; bar (x = y); }

diff --git a/gcc/testsuite/gcc.dg/Wunused-var-6.c b/gcc/testsuite/gcc.dg/Wunused-var-6.c
new file mode 100644 (file)
index 0000000..f48a455
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/Wunused-var-6.c
@@ -0,0 +1,14 @@
+/* PR c/99588 */
+/* { dg-do compile } */
+/* { dg-options "-std=c11 -Wunused-but-set-variable" } */
+
+void bar (int, ...);
+struct S { int a, b, c; };
+typedef _Atomic struct S T;
+
+void
+foo (void)
+{
+  static T x = (struct S) { 0, 0, 0 }; /* { dg-bogus "set but not used" } */
+  bar (0, x = (struct S) { 1, 1, 1 });
+}