[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 05:03:34 PDT 2023
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 f75f7683f59a81dba1c58a8bb9706b2c12d9a261 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/3] [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 2132e8b0a8cfa29..11085c85cd88b42 100644
--- a/clang/lib/AST/Interp/Interp.h
+++ b/clang/lib/AST/Interp/Interp.h
@@ -1151,7 +1151,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 65d710077fd1cbb..8815f7f8d9ed88e 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.
@@ -349,6 +349,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 974ca72702f7dd0..637915328576af1 100644
--- a/clang/test/AST/Interp/c.c
+++ b/clang/test/AST/Interp/c.c
@@ -47,3 +47,15 @@ _Static_assert(&a != 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 dac9bf3cf619c4218e04afb7f28068d9285c0fd8 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/3] 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 637915328576af1..e980cf9d963224e 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}} \
@@ -49,13 +51,13 @@ _Static_assert(&a != 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 3479d5c6aa34d3d25f87e45dddec8e31401525cc 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/3] 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 8815f7f8d9ed88e..8babb2922b11451 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(); }
@@ -321,7 +330,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 {
@@ -360,6 +372,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));
@@ -370,6 +383,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];
}
@@ -431,12 +445,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);
}
More information about the cfe-commits
mailing list