implement PR7569, warning about assignment to null, which

people seem to write when they want a deterministic trap.
Suggest instead that they use a volatile pointer or 
__builtin_trap.

llvm-svn: 107756

4 files changed, 31 insertions, 7 deletions

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 136dfb5ad06..f88a0ae6641 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -21,12 +21,6 @@ def ext_expr_not_ice : Extension<
   "expression is not integer constant expression "
   "(but is allowed as an extension)">;
 
-def ext_null_pointer_expr_not_ice : Extension<
-  "null pointer expression is not an integer constant expression "
-  "(but is allowed as an extension)">;
-
-
-
 // Semantic analysis of constant literals.
 def ext_predef_outside_function : Warning<
   "predefined identifier is only valid inside function">;
@@ -2095,6 +2089,11 @@ def err_typecheck_unary_expr : Error<
   "invalid argument type %0 to unary expression">;
 def err_typecheck_indirection_requires_pointer : Error<
   "indirection requires pointer operand (%0 invalid)">;
+def warn_indirection_through_null : Warning<
+  "indirection of non-volatile null pointer will be deleted, not trap">;
+def note_indirection_through_null : Note<
+  "consider using __builtin_trap() or qualifying pointer with 'volatile'">;
+
 def err_indirection_requires_nonfragile_object : Error<
   "indirection cannot be to an interface in non-fragile ABI (%0 invalid)">;
 def err_direct_interface_unsupported : Error<
diff --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 8b7c689ae23..342c522442d 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -5892,6 +5892,23 @@ QualType Sema::CheckAssignmentOperands(Expr *LHS, Expr *&RHS,
                                RHS, AA_Assigning))
     return QualType();
 
+  
+  // Check to see if the destination operand is a dereferenced null pointer.  If
+  // so, and if not volatile-qualified, this is undefined behavior that the
+  // optimizer will delete, so warn about it.  People sometimes try to use this
+  // to get a deterministic trap and are surprised by clang's behavior.  This
+  // only handles the pattern "*null = whatever", which is a very syntactic
+  // check.
+  if (UnaryOperator *UO = dyn_cast<UnaryOperator>(LHS->IgnoreParenCasts()))
+    if (UO->getOpcode() == UnaryOperator::Deref &&
+        UO->getSubExpr()->IgnoreParenCasts()->
+          isNullPointerConstant(Context, Expr::NPC_ValueDependentIsNotNull) &&
+        !UO->getType().isVolatileQualified()) {
+    Diag(UO->getOperatorLoc(), diag::warn_indirection_through_null)
+        << UO->getSubExpr()->getSourceRange();
+    Diag(UO->getOperatorLoc(), diag::note_indirection_through_null);
+  }
+  
   // C99 6.5.16p3: The type of an assignment expression is the type of the
   // left operand unless the left operand has qualified type, in which case
   // it is the unqualified version of the type of the left operand.
diff --git a/clang/test/Analysis/misc-ps.m b/clang/test/Analysis/misc-ps.m
index 253450cc42a..b1d47e214ef 100644
--- a/clang/test/Analysis/misc-ps.m
+++ b/clang/test/Analysis/misc-ps.m
@@ -582,7 +582,7 @@ void pr4781(unsigned long *raw1) {
 - (id) foo {
   if (self)
     return self;
-  *((int *) 0x0) = 0xDEADBEEF; // no-warning
+  *((volatile int *) 0x0) = 0xDEADBEEF; // no-warning
   return self;
 }
 @end
diff --git a/clang/test/Sema/exprs.c b/clang/test/Sema/exprs.c
index dbb6e80aa6d..b22b5220f28 100644
--- a/clang/test/Sema/exprs.c
+++ b/clang/test/Sema/exprs.c
@@ -134,3 +134,11 @@ void test18(int b) {
   test18_a(b, b); // expected-error {{too many arguments to function call, expected 1, have 2}}
   test18_a(); // expected-error {{too few arguments to function call, expected 1, have 0}}
 }
+
+// PR7569
+void test19() {
+  *(int*)0 = 0;   // expected-warning {{indirection of non-volatile null pointer}} \
+                  // expected-note {{consider using __builtin_trap}}
+  *(volatile int*)0 = 0;  // Ok.
+}
+