search

Quuxplusone / LLVMBugzillaTest

0 stars 0 forks source link

clang c11 atomics are non-conforming #24107

Open Quuxplusone opened 9 years ago

Quuxplusone commented 9 years ago
Bugzilla Link PR24108
Status NEW
Importance P normal
Reported by Stefan Teleman (stefan.teleman@oracle.com)
Reported on 2015-07-13 13:45:11 -0700
Last modified on 2018-12-04 09:45:21 -0800
Version 3.6
Hardware PC Linux
CC ditaliano@apple.com, hfinkel@anl.gov, llvm-bugs@lists.llvm.org, richard-llvm@metafoo.co.uk, shawn.walker@oracle.com, stefan.teleman@oracle.com
Fixed by commit(s)
Attachments at.c (900 bytes, text/plain)
Blocks
Blocked by
See also 
Created attachment 14584
c11 atomics test program

The following test program:

/* at.c */
#include <stdio.h>
#include <stdlib.h>
#include <stdatomic.h>

struct Test {
  int i;
  _Atomic int ai;
  double d;
  _Atomic double ad;
};

int main()
{
  struct Test T;
  _Atomic struct Test AT;
  _Atomic struct Test *TP = ATOMIC_VAR_INIT(NULL);

  T.i = 1;
  AT.i = 0;
  T.ai = ATOMIC_VAR_INIT(1);
  AT.ai = ATOMIC_VAR_INIT(0);
  T.d = 2.5;
  AT.d = 1.5;
  T.ad = ATOMIC_VAR_INIT(2.5);
  AT.ad = ATOMIC_VAR_INIT(1.5);

  (void) fprintf(stdout, "T.i=%i, T.ai=%i, T.d=%lf, T.ad=%lf\n",
                 T.i, T.ai, T.d, T.ad);
  (void) fprintf(stdout, "AT.i=%i, AT.ai=%i, AT.d=%lf, AT.ad=%lf\n",
                 AT.i, AT.ai, AT.d, AT.ad);

  atomic_store(&AT, T);

  (void) fprintf(stdout, "T.i=%i, T.ai=%i, T.d=%lf, T.ad=%lf\n",
                 T.i, T.ai, T.d, T.ad);
  (void) fprintf(stdout, "AT.i=%i, AT.ai=%i, AT.d=%lf, AT.ad=%lf\n",
                 AT.i, AT.ai, AT.d, AT.ad);

  return 0;
}

----------------------------------------------------------------------------

When compiled on Fedora 21 x86_64 with clang 3.6.1, using the official
Fedora binaries from llvm.org:

/usr/local/clang-361/bin/clang --version
clang version 3.6.1 (tags/RELEASE_361/final)
Target: x86_64-unknown-linux-gnu
Thread model: posix
--------------------------------------------------------------------------
/usr/local/clang-361/bin/clang -m64 -O0 -std=c11 -Wall -emit-llvm -S at.c -o
at.ll
--------------------------------------------------------------------------
at.c:19:5: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
  AT.i = 0;
  ~~^~
at.c:21:5: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
  AT.ai = ATOMIC_VAR_INIT(0);
  ~~^~~
at.c:23:5: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
  AT.d = 1.5;
  ~~^~
at.c:25:5: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
  AT.ad = ATOMIC_VAR_INIT(1.5);
  ~~^~~
at.c:30:20: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
                 AT.i, AT.ai, AT.d, AT.ad);
                 ~~^~
at.c:30:26: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
                 AT.i, AT.ai, AT.d, AT.ad);
                       ~~^~~
at.c:30:33: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
                 AT.i, AT.ai, AT.d, AT.ad);
                              ~~^~
at.c:30:39: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
                 AT.i, AT.ai, AT.d, AT.ad);
                                    ~~^~~
at.c:37:20: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
                 AT.i, AT.ai, AT.d, AT.ad);
                 ~~^~
at.c:37:26: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
                 AT.i, AT.ai, AT.d, AT.ad);
                       ~~^~~
at.c:37:33: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
                 AT.i, AT.ai, AT.d, AT.ad);
                              ~~^~
at.c:37:39: error: member reference base type '_Atomic(struct Test)' is not a
structure or union
                 AT.i, AT.ai, AT.d, AT.ad);
                                    ~~^~~
12 errors generated.

--------------------------------------------------------------------------

When compiled with GCC 4.9.2 in Fedora 21:

/usr/bin/gcc --version
gcc (GCC) 4.9.2 20150212 (Red Hat 4.9.2-6)
Copyright (C) 2014 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
---------------------------------------------------------------------------
/usr/bin/gcc -m64 -O0 -std=c11 -Wall -c at.c -o at.o
---------------------------------------------------------------------------
at.c: In function 'main':
at.c:16:24: warning: unused variable 'TP' [-Wunused-variable]
   _Atomic struct Test *TP = ATOMIC_VAR_INIT(NULL);

---------------------------------------------------------------------------
Quuxplusone commented 9 years ago

Attached at.c (900 bytes, text/plain): c11 atomics test program

Quuxplusone commented 9 years ago

All your member accesses on the atomic struct object have undefined behavior, per C11 6.5.2.3/5. We should probably accept them with a warning.

Quuxplusone commented 9 years ago 
Correct, but undefined behavior in Standardese does not mean "Verboten". It
only means that the Standard does not provide a normative behavior.

GCC allows access to atomic members without warning, even with -pedantic:

/usr/bin/gcc -m64 -O0 -std=c11 -pedantic -Wall -c at.c -o at.o
at.c: In function 'main':
at.c:16:24: warning: unused variable 'TP' [-Wunused-variable]
   _Atomic struct Test *TP = ATOMIC_VAR_INIT(NULL);

I believe GCC's implementation is a better approach, as it goes beyond the
minimum requirement of the Standard. It is less error-prone for developers,
and it avoids unnecessary copying between an atomic datum and a non-atomic
equivalent.
Quuxplusone commented 9 years ago

Take. I'm working on this, and I'll submit a codereview soon.