[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

2018-05-05 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

Committed in r331598, with an additional test case.

(Accepting officially on behalf of @rjmccall so that I can close the review.)


https://reviews.llvm.org/D45944



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

2018-05-04 Thread John McCall via Phabricator via cfe-commits
rjmccall added a comment.

LGTM, although you might consider testing a broader set of builtins.  Maybe at 
least one from the `__atomic_*` family?


https://reviews.llvm.org/D45944



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

2018-05-04 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added reviewers: rjmccall, efriedma.
aaron.ballman added a comment.

Adding a few more potential reviewers.


https://reviews.llvm.org/D45944



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

2018-05-02 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

Ping


https://reviews.llvm.org/D45944



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

2018-04-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment.

In https://reviews.llvm.org/D45944#1075055, @lebedev.ri wrote:

> Please always upload all patches with full context (`-U9`)


I'm not keen on uploading patches with that much context. For instance, it 
disables syntax highlighting in Phab for many of our files. However, perhaps 
100 lines of context was a bit optimistic.


https://reviews.llvm.org/D45944



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

2018-04-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 143526.
aaron.ballman added a comment.

Updated with more context.


https://reviews.llvm.org/D45944

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/builtins.c


Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -248,3 +248,7 @@
 
 return buf;
 }
+
+void test21(const int *ptr) {
+  __sync_fetch_and_add(ptr, 1); // expected-error{{address argument to atomic 
builtin cannot be const-qualified ('const int *' invalid)}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3428,6 +3428,12 @@
 return ExprError();
   }
 
+  if (ValType.isConstQualified()) {
+Diag(DRE->getLocStart(), diag::err_atomic_builtin_cannot_be_const)
+<< FirstArg->getType() << FirstArg->getSourceRange();
+return ExprError();
+  }
+
   switch (ValType.getObjCLifetime()) {
   case Qualifiers::OCL_None:
   case Qualifiers::OCL_ExplicitNone:
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7079,6 +7079,8 @@
 def err_atomic_builtin_must_be_pointer_intptr : Error<
   "address argument to atomic builtin must be a pointer to integer or pointer"
   " (%0 invalid)">;
+def err_atomic_builtin_cannot_be_const : Error<
+  "address argument to atomic builtin cannot be const-qualified (%0 invalid)">;
 def err_atomic_builtin_must_be_pointer_intfltptr : Error<
   "address argument to atomic builtin must be a pointer to integer,"
   " floating-point or pointer (%0 invalid)">;


Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -248,3 +248,7 @@
 
 return buf;
 }
+
+void test21(const int *ptr) {
+  __sync_fetch_and_add(ptr, 1); // expected-error{{address argument to atomic builtin cannot be const-qualified ('const int *' invalid)}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3428,6 +3428,12 @@
 return ExprError();
   }
 
+  if (ValType.isConstQualified()) {
+Diag(DRE->getLocStart(), diag::err_atomic_builtin_cannot_be_const)
+<< FirstArg->getType() << FirstArg->getSourceRange();
+return ExprError();
+  }
+
   switch (ValType.getObjCLifetime()) {
   case Qualifiers::OCL_None:
   case Qualifiers::OCL_ExplicitNone:
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7079,6 +7079,8 @@
 def err_atomic_builtin_must_be_pointer_intptr : Error<
   "address argument to atomic builtin must be a pointer to integer or pointer"
   " (%0 invalid)">;
+def err_atomic_builtin_cannot_be_const : Error<
+  "address argument to atomic builtin cannot be const-qualified (%0 invalid)">;
 def err_atomic_builtin_must_be_pointer_intfltptr : Error<
   "address argument to atomic builtin must be a pointer to integer,"
   " floating-point or pointer (%0 invalid)">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

2018-04-23 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment.

Please always upload all patches with full context (`-U9`)


https://reviews.llvm.org/D45944



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D45944: Disallow pointers to const in __sync_fetch_and_xxx

2018-04-23 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, dblaikie.

The following code is currently accepted without a diagnostic when it should be 
prohibited:

  void f(const int *ptr) {
__sync_fetch_and_add(ptr, 1);
  }

NB: the above code is diagnosed by GCC and ICC. However, Clang attempts to 
modify the underlying object, which seems dangerous.


https://reviews.llvm.org/D45944

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Sema/builtins.c


Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -248,3 +248,7 @@
 
 return buf;
 }
+
+void test21(const int *ptr) {
+  __sync_fetch_and_add(ptr, 1); // expected-error{{address argument to atomic 
builtin cannot be const-qualified ('const int *' invalid)}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3428,6 +3428,12 @@
 return ExprError();
   }
 
+  if (ValType.isConstQualified()) {
+Diag(DRE->getLocStart(), diag::err_atomic_builtin_cannot_be_const)
+<< FirstArg->getType() << FirstArg->getSourceRange();
+return ExprError();
+  }
+
   switch (ValType.getObjCLifetime()) {
   case Qualifiers::OCL_None:
   case Qualifiers::OCL_ExplicitNone:
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7079,6 +7079,8 @@
 def err_atomic_builtin_must_be_pointer_intptr : Error<
   "address argument to atomic builtin must be a pointer to integer or pointer"
   " (%0 invalid)">;
+def err_atomic_builtin_cannot_be_const : Error<
+  "address argument to atomic builtin cannot be const-qualified (%0 invalid)">;
 def err_atomic_builtin_must_be_pointer_intfltptr : Error<
   "address argument to atomic builtin must be a pointer to integer,"
   " floating-point or pointer (%0 invalid)">;


Index: test/Sema/builtins.c
===
--- test/Sema/builtins.c
+++ test/Sema/builtins.c
@@ -248,3 +248,7 @@
 
 return buf;
 }
+
+void test21(const int *ptr) {
+  __sync_fetch_and_add(ptr, 1); // expected-error{{address argument to atomic builtin cannot be const-qualified ('const int *' invalid)}}
+}
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -3428,6 +3428,12 @@
 return ExprError();
   }
 
+  if (ValType.isConstQualified()) {
+Diag(DRE->getLocStart(), diag::err_atomic_builtin_cannot_be_const)
+<< FirstArg->getType() << FirstArg->getSourceRange();
+return ExprError();
+  }
+
   switch (ValType.getObjCLifetime()) {
   case Qualifiers::OCL_None:
   case Qualifiers::OCL_ExplicitNone:
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -7079,6 +7079,8 @@
 def err_atomic_builtin_must_be_pointer_intptr : Error<
   "address argument to atomic builtin must be a pointer to integer or pointer"
   " (%0 invalid)">;
+def err_atomic_builtin_cannot_be_const : Error<
+  "address argument to atomic builtin cannot be const-qualified (%0 invalid)">;
 def err_atomic_builtin_must_be_pointer_intfltptr : Error<
   "address argument to atomic builtin must be a pointer to integer,"
   " floating-point or pointer (%0 invalid)">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits