[Bug c/99588] New: variable set but not used warning on static _Atomic assignment

[Bug c/99588] New: variable set but not used warning on static _Atomic assignment

[godmar at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

            Bug ID: 99588
           Summary: variable set but not used warning on static _Atomic
                    assignment
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: godmar at gmail dot com
  Target Milestone: ---

A student shared this program with me:

#include <stdio.h>
#include <stdatomic.h>

void test() {
    static _Atomic int x = 0;
    printf("%d\n", x += 1);
}

int main(int argc, char **argv) {
    for(int i = 0; i < 10; i++) {
        test();
    }
}

which on the current x86_64 trunk (and earlier versions) as per godbolt yields
a warning:

<source>: In function 'test':
<source>:5:24: warning: variable 'x' set but not used
[-Wunused-but-set-variable]
    5 |     static _Atomic int x = 0;
      |                        ^
Compiler returned: 0

If the _Atomic modifier is removed, the warning disappears. Is this kosher?

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[rguenth at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jsm28 at gcc dot gnu.org
   Last reconfirmed|                            |2021-03-15
             Status|UNCONFIRMED                 |NEW
           Keywords|                            |diagnostic
     Ever confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
We're likely confused by the "interesting" IL this emits:

;; Function test (null)
;; enabled by -tree-original


{
  static atomic int x = 0;

    static atomic int x = 0;
  printf ((const char * restrict) "%d\n", TARGET_EXPR <D.2476, 1>;
  TARGET_EXPR <D.2477, (int) __atomic_add_fetch_4 ((volatile void *) &x,
(unsigned int) TARGET_EXPR <D.2476, 1>, 5)>;, D.2477);
}

but clearly 'x' is used in the __atomic_add_fetch_4

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[egallager at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

Eric Gallager <egallager at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Blocks|                            |89180
                 CC|                            |egallager at gcc dot gnu.org,
                   |                            |eggert at gnu dot org

--- Comment #2 from Eric Gallager <egallager at gcc dot gnu.org> ---
I think there's another similar bug against GCC out there that this one is
reminding me of, but I can't seem to remember the number or find it right now
though... I think Paul Eggert either reported or was on cc for it, though, so
maybe he knows? 
(or maybe I'm just imagining things in which case I'm sorry)


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=89180
[Bug 89180] [meta-bug] bogus/missing -Wunused warnings

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
More complete testcase:
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; }
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; }
void f14 (void) { _Atomic int x = 0; x += 3; }

With -D_Atomic= we warn on f6 and f13 and nothing else,
otherwise on everything but f1, f3, f8, f10.
The reason for not warning on the f{1,3,8,10} is
convert_lvalue_to_rvalue:
      /* EXPR is always read.  */
      mark_exp_read (exp.value);
So for consistency with the non-_Atomic x op= expr at least should
mark_exp_read too in build_atomic_assign (i.e. something like:
  if (modifycode != NOP_EXPR)
    mark_exp_read (lhs);
somewhere early.  That results in warning on f{5,6,12,13}, f5 and f12 still
being false positives.

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 50419
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=50419&action=edit
gcc11-pr99588.patch

Untested fix.

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:3279a9a5a9a2e4e17175678cb7b15613495e306e

commit r11-7746-g3279a9a5a9a2e4e17175678cb7b15613495e306e
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 19 22:54:31 2021 +0100

    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.

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far, will wait a little bit before backporting.

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

--- Comment #7 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:b1fc1f1c4b2e9005c40ed476b067577da2d2ce84

commit r10-9619-gb1fc1f1c4b2e9005c40ed476b067577da2d2ce84
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 19 22:54:31 2021 +0100

    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.

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

--- Comment #8 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 10.3 too.

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:803a95e2a0134105dd259d7ccd258744e94c3233

commit r9-9434-g803a95e2a0134105dd259d7ccd258744e94c3233
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 19 22:54:31 2021 +0100

    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.

    (cherry picked from commit b1fc1f1c4b2e9005c40ed476b067577da2d2ce84)

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-8 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:3aa249a941d83bf734173991da6bf53ae79a3d47

commit r8-10897-g3aa249a941d83bf734173991da6bf53ae79a3d47
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Mar 19 22:54:31 2021 +0100

    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.

    (cherry picked from commit b1fc1f1c4b2e9005c40ed476b067577da2d2ce84)

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[jakub at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #11 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

[Bug c/99588] variable set but not used warning on static _Atomic assignment

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99588

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |8.5