[Bug c/69404] atomic builtins accept incompatible pointers

[msebor at gcc dot gnu.org]

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

Martin Sebor  changed:

   What|Removed |Added

Summary|atomic builtins silently|atomic builtins accept
   |discard cv-qualifiers   |incompatible pointers

--- Comment #1 from Martin Sebor  ---
Actually, with more testing I see the builtins do even more than that: they
silently convert between pointers to types of the same size.  (Clang diagnoses
those as well.)

$ cat z.c && /home/msebor/build/gcc-trunk-git/gcc/xgcc
-B/home/msebor/build/gcc-trunk-git/gcc -S -Wall -Wextra -Wpedantic -o/dev/null
z.c
int* foo (void (*p)(void))
{
int *q;
__atomic_load (&p, &q, 0);
return q;
}

[Bug c/69404] atomic builtins accept incompatible pointers

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski  ---
So this might be following C++ rules here dealing with qualifiers.

There is no discarding of qualifiers either but rather adding them according to
C++ rules.

Note C and C++ deals with inner qualifiers differently.

The reason why I said it does not discard here is because take:
__atomic_load (&p, &q, 0);


This is basically p = q;  so you need to add const qualifier to the inner most
type.  And __atomic_load supports pointers to pointers that have qualifiers on
the inner type.

[Bug c/69404] atomic builtins accept incompatible pointers

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski  ---
(In reply to Martin Sebor from comment #1)
> Actually, with more testing I see the builtins do even more than that: they
> silently convert between pointers to types of the same size.  (Clang
> diagnoses those as well.)
> 
> $ cat z.c && /home/msebor/build/gcc-trunk-git/gcc/xgcc
> -B/home/msebor/build/gcc-trunk-git/gcc -S -Wall -Wextra -Wpedantic
> -o/dev/null z.c
> int* foo (void (*p)(void))
> {
> int *q;
> __atomic_load (&p, &q, 0);
> return q;
> }

Most likely because it treats it as void* :).

Since this is an GCC extension, GCC can define the behavior any which way it
wants.  If clang is not compatible is not GCC's fault.
Does stdatomic.h defines have the same issue then there is a problem there.

[Bug c/69404] atomic builtins accept incompatible pointers

[pinskia at gcc dot gnu.org]

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

--- Comment #4 from Andrew Pinski  ---
Oh one more thing about qualifiers: _Atomic is a qualifer.

[Bug c/69404] atomic builtins accept incompatible pointers

[msebor at gcc dot gnu.org]

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

--- Comment #5 from Martin Sebor  ---
The GCC manual says the __atomic builtins are meant to be compatible with those
described in the Intel Itanium Processor-specific Application Binary Interface,
section 7.4.  Both the ABI and the GCC manual describe the builtins to operate
on objects of some /type/.  For example, __atomic_load is described like so:

  __atomic_load (type *ptr, type *ret, int memorder)

implying that ptr and ret should be compatible.  (The ABI doesn't describe
__atomic_load but it does describe many of the __atomic builtins.)

GCC rejects the test cases when the __atomic builtins are replaced with the
corresponding C11 atomic generic functions defined in , but only
because these are implemented as macros involving conversions that detect the
incompatibility.  The C11 generics that don't do such conversions are just as
impotent at detecting incompatibilities as the corresponding builtins.  For
example, GCC doesn't diagnose the program below (both Clang and Intel CC do
issue the expected diagnostic -- ICC when the _Atomic keyword is removed as it
doesn't seem to understand it yet).

$ cat z.c && /home/msebor/build/gcc-trunk-git/gcc/xgcc
-B/home/msebor/build/gcc-trunk-git/gcc -S -Wall -Wextra -Wpedantic -o/dev/null
z.c 
#include 

int* foo (void (*p)(void))
{
int* _Atomic q;
atomic_init (&q, 0);

atomic_fetch_add (&q, p);
return q;
}

I certainly agree that GCC can define its own extensions any way it sees fit. 
But I can think of no use for this "extension" except to make it easier to
accidentally write incorrect code.

Incidentally, I came across this problem while testing a patch for bug 64843
(and improving the documentation of the builtins in the GCC manual op resolve
bug 52291).

[Bug c/69404] atomic builtins accept incompatible pointers

[msebor at gcc dot gnu.org]

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

Martin Sebor  changed:

   What|Removed |Added

 CC||eric at efcs dot ca

--- Comment #6 from Martin Sebor  ---
*** Bug 69425 has been marked as a duplicate of this bug. ***

[Bug c/69404] atomic builtins accept incompatible pointers

[msebor at gcc dot gnu.org]

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

Martin Sebor  changed:

   What|Removed |Added

   Last reconfirmed||2016-2-2
  Known to fail||4.9.3, 5.3.0, 6.0

--- Comment #7 from Martin Sebor  ---
This bug also allows the atomic intrinsics to change the representation of
_Bool objects to other than 0 1, violating the GCC invariant.  This is a
loophole that the fix bug 69404 was intended to close (and did for
__atomic_fetch_xxx) but missed for __atomic_load and __atomic_exchange.  As a
result, GCC compiles the following test case without a warning:

$ cat u.c && /home/msebor/build/gcc-trunk-git/gcc/xgcc
-B/home/msebor/build/gcc-trunk-git/gcc -S -Wall -Wextra -Wpedantic -o/dev/null
u.c
void f (char *a, _Bool *b)
{
__atomic_load (a, b, 0);
}