[clang] [clang][Interp] Only diagnose null field access in constant contexts (PR #69223)

Timm Baeder via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 26 06:37:02 PDT 2023


Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>,
Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>,
Timm =?utf-8?q?Bäder?= <tbaeder at redhat.com>
Message-ID:
In-Reply-To: <llvm/llvm-project/pull/69223/clang at github.com>


https://github.com/tbaederr updated https://github.com/llvm/llvm-project/pull/69223

>From a4610e034b8331b201e282ef034e83095cbe395f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Mon, 16 Oct 2023 17:51:44 +0200
Subject: [PATCH 1/4] [clang][Interp] Only diagnose null field access in
 constant contexts

---
 clang/lib/AST/Interp/Interp.h     |  2 +-
 clang/lib/AST/Interp/Pointer.h    |  4 +++-
 clang/test/AST/Interp/c.c         | 12 +++++++++++
 clang/test/AST/Interp/records.cpp | 33 +++++++++++++++++++++++++++++++
 4 files changed, 49 insertions(+), 2 deletions(-)

diff --git a/clang/lib/AST/Interp/Interp.h b/clang/lib/AST/Interp/Interp.h
index 3e1b4f32e8b6912..7dd415d6e460536 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1155,7 +1155,7 @@ inline bool GetPtrGlobal(InterpState &S, CodePtr OpPC, uint32_t I) {
 /// 2) Pushes Pointer.atField(Off) on the stack
 inline bool GetPtrField(InterpState &S, CodePtr OpPC, uint32_t Off) {
   const Pointer &Ptr = S.Stk.pop<Pointer>();
-  if (!CheckNull(S, OpPC, Ptr, CSK_Field))
+  if (S.inConstantContext() && !CheckNull(S, OpPC, Ptr, CSK_Field))
     return false;
   if (!CheckExtern(S, OpPC, Ptr))
     return false;
diff --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h
index b371b306fe7a70d..478b6c175454613 100644
--- a/clang/lib/AST/Interp/Pointer.h
+++ b/clang/lib/AST/Interp/Pointer.h
@@ -296,7 +296,7 @@ class Pointer {
   bool isUnion() const;
 
   /// Checks if the storage is extern.
-  bool isExtern() const { return Pointee->isExtern(); }
+  bool isExtern() const { return Pointee && Pointee->isExtern(); }
   /// Checks if the storage is static.
   bool isStatic() const { return Pointee->isStatic(); }
   /// Checks if the storage is temporary.
@@ -351,6 +351,8 @@ class Pointer {
 
   /// Checks if the index is one past end.
   bool isOnePastEnd() const {
+    if (!Pointee)
+      return false;
     return isElementPastEnd() || getSize() == getOffset();
   }
 
diff --git a/clang/test/AST/Interp/c.c b/clang/test/AST/Interp/c.c
index e8aa8b8599f2137..5f09899a4df6b2b 100644
--- a/clang/test/AST/Interp/c.c
+++ b/clang/test/AST/Interp/c.c
@@ -67,3 +67,15 @@ _Static_assert(&Test50 != (void*)0, ""); // ref-warning {{always true}} \
                                          // expected-warning {{always true}} \
                                          // pedantic-expected-warning {{always true}} \
                                          // pedantic-expected-warning {{is a GNU extension}}
+
+struct y {int x,y;};
+int a2[(long)&((struct y*)0)->y]; // expected-warning {{folded to constant array}} \
+                                  // pedantic-expected-warning {{folded to constant array}} \
+                                  // ref-warning {{folded to constant array}} \
+                                  // pedantic-ref-warning {{folded to constant array}}
+
+const struct y *yy = (struct y*)0;
+const long L = (long)(&(yy->y)); // expected-error {{not a compile-time constant}} \
+                                 // pedantic-expected-error {{not a compile-time constant}} \
+                                 // ref-error {{not a compile-time constant}} \
+                                 // pedantic-ref-error {{not a compile-time constant}}
diff --git a/clang/test/AST/Interp/records.cpp b/clang/test/AST/Interp/records.cpp
index e899e37915f0398..280eaf34898ceca 100644
--- a/clang/test/AST/Interp/records.cpp
+++ b/clang/test/AST/Interp/records.cpp
@@ -1102,3 +1102,36 @@ namespace DelegatingConstructors {
   static_assert(d4.a == 10, "");
   static_assert(d4.b == 12, "");
 }
+
+namespace AccessOnNullptr {
+  struct F {
+    int a;
+  };
+
+  constexpr int a() { // expected-error {{never produces a constant expression}} \
+                      // ref-error {{never produces a constant expression}}
+    F *f = nullptr;
+
+    f->a = 0; // expected-note 2{{cannot access field of null pointer}} \
+              // ref-note 2{{cannot access field of null pointer}}
+    return f->a;
+  }
+  static_assert(a() == 0, ""); // expected-error {{not an integral constant expression}} \
+                               // expected-note {{in call to 'a()'}} \
+                               // ref-error {{not an integral constant expression}} \
+                               // ref-note {{in call to 'a()'}}
+
+  constexpr int a2() { // expected-error {{never produces a constant expression}} \
+                      // ref-error {{never produces a constant expression}}
+    F *f = nullptr;
+
+
+    const int *a = &(f->a); // expected-note 2{{cannot access field of null pointer}} \
+                            // ref-note 2{{cannot access field of null pointer}}
+    return f->a;
+  }
+  static_assert(a2() == 0, ""); // expected-error {{not an integral constant expression}} \
+                               // expected-note {{in call to 'a2()'}} \
+                               // ref-error {{not an integral constant expression}} \
+                               // ref-note {{in call to 'a2()'}}
+}

>From 2427d71d609de53ef54df648ebe5952f097da62d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Mon, 16 Oct 2023 19:02:50 +0200
Subject: [PATCH 2/4] Cast pointers to intptr_t instead of long

---
 clang/test/AST/Interp/c.c | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/clang/test/AST/Interp/c.c b/clang/test/AST/Interp/c.c
index 5f09899a4df6b2b..6bfcded0a78646c 100644
--- a/clang/test/AST/Interp/c.c
+++ b/clang/test/AST/Interp/c.c
@@ -3,6 +3,8 @@
 // RUN: %clang_cc1 -verify=ref -std=c11 %s
 // RUN: %clang_cc1 -pedantic -verify=pedantic-ref -std=c11 %s
 
+typedef __INTPTR_TYPE__ intptr_t;
+
 _Static_assert(1, "");
 _Static_assert(0 != 1, "");
 _Static_assert(1.0 == 1.0, ""); // pedantic-ref-warning {{not an integer constant expression}} \
@@ -69,13 +71,13 @@ _Static_assert(&Test50 != (void*)0, ""); // ref-warning {{always true}} \
                                          // pedantic-expected-warning {{is a GNU extension}}
 
 struct y {int x,y;};
-int a2[(long)&((struct y*)0)->y]; // expected-warning {{folded to constant array}} \
-                                  // pedantic-expected-warning {{folded to constant array}} \
-                                  // ref-warning {{folded to constant array}} \
-                                  // pedantic-ref-warning {{folded to constant array}}
+int a2[(intptr_t)&((struct y*)0)->y]; // expected-warning {{folded to constant array}} \
+                                      // pedantic-expected-warning {{folded to constant array}} \
+                                      // ref-warning {{folded to constant array}} \
+                                      // pedantic-ref-warning {{folded to constant array}}
 
 const struct y *yy = (struct y*)0;
-const long L = (long)(&(yy->y)); // expected-error {{not a compile-time constant}} \
-                                 // pedantic-expected-error {{not a compile-time constant}} \
-                                 // ref-error {{not a compile-time constant}} \
-                                 // pedantic-ref-error {{not a compile-time constant}}
+const intptr_t L = (intptr_t)(&(yy->y)); // expected-error {{not a compile-time constant}} \
+                                         // pedantic-expected-error {{not a compile-time constant}} \
+                                         // ref-error {{not a compile-time constant}} \
+                                         // pedantic-ref-error {{not a compile-time constant}}

>From e5df0a1e19cc8cff7785b0f8bae3b35a10948efd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Thu, 26 Oct 2023 14:03:01 +0200
Subject: [PATCH 3/4] Add assertions

---
 clang/lib/AST/Interp/Pointer.h | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/clang/lib/AST/Interp/Pointer.h b/clang/lib/AST/Interp/Pointer.h
index 478b6c175454613..843bcad16b5d1e3 100644
--- a/clang/lib/AST/Interp/Pointer.h
+++ b/clang/lib/AST/Interp/Pointer.h
@@ -199,7 +199,10 @@ class Pointer {
   bool isField() const { return Base != 0 && Base != RootPtrMark; }
 
   /// Accessor for information about the declaration site.
-  const Descriptor *getDeclDesc() const { return Pointee->Desc; }
+  const Descriptor *getDeclDesc() const {
+    assert(Pointee);
+    return Pointee->Desc;
+  }
   SourceLocation getDeclLoc() const { return getDeclDesc()->getLocation(); }
 
   /// Returns a pointer to the object of which this pointer is a field.
@@ -298,9 +301,15 @@ class Pointer {
   /// Checks if the storage is extern.
   bool isExtern() const { return Pointee && Pointee->isExtern(); }
   /// Checks if the storage is static.
-  bool isStatic() const { return Pointee->isStatic(); }
+  bool isStatic() const {
+    assert(Pointee);
+    return Pointee->isStatic();
+  }
   /// Checks if the storage is temporary.
-  bool isTemporary() const { return Pointee->isTemporary(); }
+  bool isTemporary() const {
+    assert(Pointee);
+    return Pointee->isTemporary();
+  }
   /// Checks if the storage is a static temporary.
   bool isStaticTemporary() const { return isStatic() && isTemporary(); }
 
@@ -323,7 +332,10 @@ class Pointer {
   }
 
   /// Returns the declaration ID.
-  std::optional<unsigned> getDeclID() const { return Pointee->getDeclID(); }
+  std::optional<unsigned> getDeclID() const {
+    assert(Pointee);
+    return Pointee->getDeclID();
+  }
 
   /// Returns the byte offset from the start.
   unsigned getByteOffset() const {
@@ -362,6 +374,7 @@ class Pointer {
   /// Dereferences the pointer, if it's live.
   template <typename T> T &deref() const {
     assert(isLive() && "Invalid pointer");
+    assert(Pointee);
     if (isArrayRoot())
       return *reinterpret_cast<T *>(Pointee->rawData() + Base +
                                     sizeof(InitMapPtr));
@@ -372,6 +385,7 @@ class Pointer {
   /// Dereferences a primitive element.
   template <typename T> T &elem(unsigned I) const {
     assert(I < getNumElems());
+    assert(Pointee);
     return reinterpret_cast<T *>(Pointee->data() + sizeof(InitMapPtr))[I];
   }
 
@@ -433,12 +447,14 @@ class Pointer {
   /// Returns a descriptor at a given offset.
   InlineDescriptor *getDescriptor(unsigned Offset) const {
     assert(Offset != 0 && "Not a nested pointer");
+    assert(Pointee);
     return reinterpret_cast<InlineDescriptor *>(Pointee->rawData() + Offset) -
            1;
   }
 
   /// Returns a reference to the InitMapPtr which stores the initialization map.
   InitMapPtr &getInitMap() const {
+    assert(Pointee);
     return *reinterpret_cast<InitMapPtr *>(Pointee->rawData() + Base);
   }
 

>From e8469646c5545d80d8b90cee98c2ea7277c95509 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Timm=20B=C3=A4der?= <tbaeder at redhat.com>
Date: Thu, 26 Oct 2023 15:27:54 +0200
Subject: [PATCH 4/4] Fix checkDummy() for null pointers

---
 clang/lib/AST/Interp/Interp.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/lib/AST/Interp/Interp.cpp b/clang/lib/AST/Interp/Interp.cpp
index 31d43b6010c1878..1ebbadc375f38c8 100644
--- a/clang/lib/AST/Interp/Interp.cpp
+++ b/clang/lib/AST/Interp/Interp.cpp
@@ -216,7 +216,7 @@ bool CheckLive(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
 }
 
 bool CheckDummy(InterpState &S, CodePtr OpPC, const Pointer &Ptr) {
-  return !Ptr.isDummy();
+  return !Ptr.isZero() && !Ptr.isDummy();
 }
 
 bool CheckNull(InterpState &S, CodePtr OpPC, const Pointer &Ptr,



More information about the cfe-commits mailing list