[clang] a892b00 - PR49465: Disallow constant evaluation of a call to operator delete(nullptr).

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2021-03-09T15:06:06-08:00
New Revision: a892b0015ed6af5945d06e87ca5da1ad8be7ad29

URL: https://github.com/llvm/llvm-project/commit/a892b0015ed6af5945d06e87ca5da1ad8be7ad29
DIFF: https://github.com/llvm/llvm-project/commit/a892b0015ed6af5945d06e87ca5da1ad8be7ad29.diff

LOG: PR49465: Disallow constant evaluation of a call to operator delete(nullptr).

The only time we would consider allowing this is inside a call to
std::allocator<T>::deallocate, whose contract does not permit deletion
of null pointers.

Added: 
    

Modified: 
    clang/include/clang/Basic/DiagnosticASTKinds.td
    clang/lib/AST/ExprConstant.cpp
    clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/Basic/DiagnosticASTKinds.td b/clang/include/clang/Basic/DiagnosticASTKinds.td
index f6b936f5ccd9..496d86ee2fe7 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -349,6 +349,8 @@ def note_constexpr_new_delete_mismatch : Note<
   "used to delete pointer to "
   "%select{array object of type %2|non-array object of type %2|"
   "object allocated with 'new'}0}1">;
+def note_constexpr_deallocate_null : Note<
+  "'std::allocator<...>::deallocate' used to delete a null pointer">;
 def note_constexpr_delete_subobject : Note<
   "delete of pointer%select{ to subobject|}1 '%0' "
   "%select{|that does not point to complete object}1">;

diff  --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index ae7131eae01d..4213beb915af 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -6712,9 +6712,12 @@ bool HandleOperatorDeleteCall(EvalInfo &Info, const CallExpr *E) {
   if (Pointer.Designator.Invalid)
     return false;
 
-  // Deleting a null pointer has no effect.
-  if (Pointer.isNullPointer())
+  // Deleting a null pointer would have no effect, but it's not permitted by
+  // std::allocator<T>::deallocate's contract.
+  if (Pointer.isNullPointer()) {
+    Info.CCEDiag(E->getExprLoc(), diag::note_constexpr_deallocate_null);
     return true;
+  }
 
   if (!CheckDeleteKind(Info, E, Pointer, DynAlloc::StdAllocator))
     return false;

diff  --git a/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp b/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
index 097ca00640e9..f37ebc9f6365 100644
--- a/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
+++ b/clang/test/SemaCXX/cxx2a-constexpr-dynalloc.cpp
@@ -17,7 +17,7 @@ namespace std {
       return (T*)NEW(sizeof(T) * N); // expected-note 3{{heap allocation}} expected-note {{not deallocated}}
     }
     constexpr void deallocate(void *p) {
-      DELETE(p); // expected-note 2{{'std::allocator<...>::deallocate' used to delete pointer to object allocated with 'new'}}
+      DELETE(p); // #dealloc expected-note 2{{'std::allocator<...>::deallocate' used to delete pointer to object allocated with 'new'}}
     }
   };
 }
@@ -83,6 +83,11 @@ static_assert(mismatched(2, 2));
 constexpr int *escape = std::allocator<int>().allocate(3); // expected-error {{constant expression}} expected-note {{pointer to subobject of heap-allocated}}
 constexpr int leak = (std::allocator<int>().allocate(3), 0); // expected-error {{constant expression}}
 constexpr int no_lifetime_start = (*std::allocator<int>().allocate(1) = 1); // expected-error {{constant expression}} expected-note {{assignment to object outside its lifetime}}
+constexpr int no_deallocate_nullptr = (std::allocator<int>().deallocate(nullptr), 1); // expected-error {{constant expression}} expected-note {{in call}}
+// expected-note@#dealloc {{'std::allocator<...>::deallocate' used to delete a null pointer}}
+constexpr int no_deallocate_nonalloc = (std::allocator<int>().deallocate((int*)&no_deallocate_nonalloc), 1); // expected-error {{constant expression}} expected-note {{in call}}
+// expected-note@#dealloc {{delete of pointer '&no_deallocate_nonalloc' that does not point to a heap-allocated object}}
+// expected-note at -2 {{declared here}}
 
 void *operator new(std::size_t, void *p) { return p; }
 constexpr bool no_placement_new_in_user_code() { // expected-error {{never produces a constant expression}}