[Bug libstdc++/99612] New: Remove "#pragma GCC system_header" from atomic file to warn on incorrect memory order

[Bug libstdc++/99612] New: Remove "#pragma GCC system_header" from atomic file to warn on incorrect memory order

[antoshkka at gmail dot com]

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

            Bug ID: 99612
           Summary: Remove "#pragma GCC system_header" from atomic file to
                    warn on incorrect memory order
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: diagnostic
          Severity: normal
          Priority: P3
         Component: libstdc++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: antoshkka at gmail dot com
  Target Milestone: ---

GCC has -Winvalid-memory-model that warns if wrong memory model is used with
atomic

  auto ret = a.load(std::memory_order_release); // warning
  a.store(10, std::memory_order_acquire); // warning

Unfortunately, that warning does not work by default, because <atomic> header
has a "#pragma GCC system_header" in it.

The only way to get the warning is to use -Wsystem-headers that unleashes all
the warnings from all system headers.

Playground: https://godbolt.org/z/Wca5ef

[Bug libstdc++/99612] Remove "#pragma GCC system_header" from atomic file to warn on incorrect memory order

[redi at gcc dot gnu.org]

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

--- Comment #1 from Jonathan Wakely <redi at gcc dot gnu.org> ---
I'd prefer if the compiler just got it right. This seems like a warning that
should fire even in system headers. Or it should track that the value is a
function parameter and came from a non-system header and so the diagnostic
location should not consider the point of use.

[Bug middle-end/99612] Remove "#pragma GCC system_header" from atomic file to warn on incorrect memory order

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2021-03-16
   Target Milestone|---                         |12.0
           See Also|                            |https://gcc.gnu.org/bugzill
                   |                            |a/show_bug.cgi?id=98871
                 CC|                            |msebor at gcc dot gnu.org
             Status|UNCONFIRMED                 |NEW
          Component|libstdc++                   |middle-end
     Ever confirmed|0                           |1

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
The root cause is the same as the limitation discussed in pr98871.  Warnings
issued from the middle end cannot be controlled with sufficient flexibility. 
The patch attached to the bug should in principle resolve this as well.  Let me
assign this to myself and handle it for GCC 12.

That said, when reporting bugs, please include all the information in the bug
we request in https://gcc.gnu.org/bugs/#need, in particular the full test case,
the command line options used to compile it, and the compiler output.  Links to
third party sites are not sufficient (as GCC on Godbolt changes, so might the
symptoms).

[Bug middle-end/99612] Remove "#pragma GCC system_header" from atomic file to warn on incorrect memory order

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |msebor at gcc dot gnu.org
      Known to fail|                            |11.0

[Bug middle-end/99612] Remove "#pragma GCC system_header" from atomic file to warn on incorrect memory order

[msebor at gcc dot gnu.org]

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

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
The test case:

$ (set -x && cat pr99612.C && g++ -O2 -S -Wall --std=c++20 -O2 -Wall
-Winvalid-memory-model pr99612.C && g++ -O2 -S -Wall --std=c++20 -O2 -Wall
-Winvalid-memory-model -Wsystem-headers pr99612.C)
+ cat pr99612.C
#include <atomic>

std::atomic<int> a;

int f() {
  auto ret = a.load(std::memory_order_release);
  a.store(10, std::memory_order_acquire);
  return ret;
}
+ g++ -O2 -S -Wall --std=c++20 -O2 -Wall -Winvalid-memory-model pr99612.C
+ /g++ -O2 -S -Wall --std=c++20 -O2 -Wall -Winvalid-memory-model
-Wsystem-headers pr99612.C
In file included from
/build/gcc-master/x86_64-pc-linux-gnu/libstdc++-v3/include/atomic:41,
                 from pr99612.C:1:
/build/gcc-master/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/atomic_base.h:
In function ‘int f()’:
/build/gcc-master/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/atomic_base.h:479:31:
warning: invalid memory model for ‘__atomic_load’ [-Winvalid-memory-model]
  479 |         return __atomic_load_n(&_M_i, int(__m));
      |                               ^
/build/gcc-master/x86_64-pc-linux-gnu/libstdc++-v3/include/bits/atomic_base.h:455:25:
warning: invalid memory model for ‘__atomic_store’ [-Winvalid-memory-model]
  455 |         __atomic_store_n(&_M_i, __i, int(__m));
      |                         ^

[Bug middle-end/99612] Remove "#pragma GCC system_header" from atomic file to warn on incorrect memory order

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |patch

--- Comment #4 from Martin Sebor <msebor at gcc dot gnu.org> ---
Patch: https://gcc.gnu.org/pipermail/gcc-patches/2021-December/586402.html

[Bug middle-end/99612] Remove "#pragma GCC system_header" from atomic file to warn on incorrect memory order

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:5a431b60d1f221992e5e9f7a5c032df3b5fa35a5

commit r12-6216-g5a431b60d1f221992e5e9f7a5c032df3b5fa35a5
Author: Martin Sebor <msebor@redhat.com>
Date:   Tue Jan 4 13:44:23 2022 -0700

    Eenable -Winvalid-memory-order for C++ [PR99612].

    Resolves:
    PR middle-end/99612 - Remove "#pragma GCC system_header" from atomic file
to warn on incorrect memory order

    gcc/ChangeLog:

            PR middle-end/99612
            * builtins.c (get_memmodel): Move warning code to
            gimple-ssa-warn-access.cc.
            (expand_builtin_atomic_compare_exchange): Same.
            (expand_ifn_atomic_compare_exchange): Same.
            (expand_builtin_atomic_load): Same.
            (expand_builtin_atomic_store): Same.
            (expand_builtin_atomic_clear): Same.
            * doc/extend.texi (__atomic_exchange_n): Update valid memory
            models.
            * gimple-ssa-warn-access.cc (memmodel_to_uhwi): New function.
            (struct memmodel_pair): New struct.
            (memmodel_name): New function.
            (pass_waccess::maybe_warn_memmodel): New function.
            (pass_waccess::check_atomic_memmodel): New function.
            (pass_waccess::check_atomic_builtin): Handle memory model.
            * input.c (expansion_point_location_if_in_system_header): Return
            original location if expansion location is in a system header.

    gcc/testsuite/ChangeLog:

            PR middle-end/99612
            * c-c++-common/pr83059.c: Adjust text of expected diagnostics.
            * gcc.dg/atomic-invalid-2.c: Same.
            * gcc.dg/atomic-invalid.c: Same.
            * c-c++-common/Winvalid-memory-model.c: New test.
            * g++.dg/warn/Winvalid-memory-model-2.C: New test.
            * g++.dg/warn/Winvalid-memory-model.C: New test.

[Bug middle-end/99612] Remove "#pragma GCC system_header" from atomic file to warn on incorrect memory order

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

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

--- Comment #6 from Martin Sebor <msebor at gcc dot gnu.org> ---
Fixed for GCC 12.  The fix and its dependencies are too intrusive not backport
to GCC 11.