[Bug c/98965] New: assingment to a struct with an atomic member not atomic

[Bug c/98965] New: assingment to a struct with an atomic member not atomic

[msebor at gcc dot gnu.org]

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

            Bug ID: 98965
           Summary: assingment to a struct with an atomic member not
                    atomic
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
          Assignee: unassigned at gcc dot gnu.org
          Reporter: msebor at gcc dot gnu.org
  Target Milestone: ---

When a struct with an atomic member is assigned to, the assignment is treated
the same as one to a struct with all ordinary (non-atomic) members.  For
example, in the test case below, GCC emits an atomic assignment for the one in
f() but a series of orinary assignments in g().  Either both assignments should
be atomic, or the second one should trigger a diagnostic.

$ cat t.c && gcc -O2 -S -Wall -Wextra -Wpedantic
-fdump-tree-optimized=/dev/stdout t.c
struct A { char a[33]; };
typedef _Atomic struct A AA;

AA a;

void f (const struct A *p)
{
  a = *p;   // atomic
}

struct B { AA a; } b;

void g (const struct B *p)
{
  b = *p;   // non-atomic
}


;; Function f (f, funcdef_no=0, decl_uid=1947, cgraph_uid=1, symbol_order=1)

void f (const struct A * p)
{
  struct A D.1949;

  <bb 2> [local count: 1073741824]:
  D.1949 = *p_2(D);
  __atomic_store (33, &a, &D.1949, 5);
  return;

}



;; Function g (g, funcdef_no=1, decl_uid=1954, cgraph_uid=2, symbol_order=3)

void g (const struct B * p)
{
  <bb 2> [local count: 1073741824]:
  b = *p_2(D);
  return;

}

[Bug c/98965] assingment to a struct with an atomic member not atomic

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
Since (if) assigning to the whole struct in one thread is valid while another
thread assigns to just the member this is wrong-code.  Otherwise, if the
semantics of such a pair of assignments aren't defined, issuing a warning would
be helpful.

[Bug c/98965] assingment to a struct with an atomic member not atomic

[rguenth at gcc dot gnu.org]

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

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

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

--- Comment #2 from Richard Biener <rguenth at gcc dot gnu.org> ---
I bet that this is undefined territory in the C standard as much as a struct
with (partially) volatile (bit-)fields.

[Bug c/98965] assignment to a struct with an atomic member not atomic

[msebor at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |diagnostic

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
The topic came up in a discussion on the WG14 reflector.  For waht it's worth,
my impression is that it's simply an oversight, a result of an incomplete
integration of atomics into the core language.  It would be hard to justify
making this (deliberately) undefined.  The member type information is readily
available at the point of the assignment so if it's not meant to be supported
the standard could easily require a diagnostic without imposing a great burden
on implementations.

Until or unless the standard is clarified, unless we choose to implement the
assignment atomically, issuing a warning would help prevent bugs.

As a data point, of the compilers on Godbolt only Clang and GCC support _Atomic
structs and neither locks the member.

[Bug c/98965] assignment to a struct with an atomic member not atomic

[joseph at codesourcery dot com]

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

--- Comment #4 from joseph at codesourcery dot com <joseph at codesourcery dot com> ---
The difficulty with making such an assignment atomic is that atomic 
operations for different sizes of atomic access don't interoperate on the 
same memory; if the struct contains an _Atomic int, that's accessed 
atomically via appropriate int-sized atomic operations, while a larger 
structure may use locking for atomic access in libatomic, and if both 
mechanisms are used on the same memory the result doesn't properly follow 
the memory model.

So you'd need to split struct assignment up into assignment of members to 
access a member atomically.  But that couldn't work at all when you have a 
union with atomic members, because in the union case the compiler can't 
know which is the currently active member that would determine the size of 
the atomic access.

[Bug c/98965] assignment to a struct with an atomic member not atomic

[msebor at gcc dot gnu.org]

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

--- Comment #5 from Martin Sebor <msebor at gcc dot gnu.org> ---
I was thinking of the latter, i.e., splitting the assignment into a series of
ordinary and atomic, the latter per member.  The union case would be diagnosed
(it too should be straightforward to detect).

But C++ rejects such mixed assignments, so the easiest solution would be to do
the same in C (or at least warn).