Timm =?utf-8?q?Bäder?= <tbae...@redhat.com>, Timm =?utf-8?q?Bäder?= <tbae...@redhat.com>, Timm =?utf-8?q?Bäder?= <tbae...@redhat.com> Message-ID: In-Reply-To: <llvm/llvm-project/pull/69223/cl...@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?= <tbae...@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?= <tbae...@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?= <tbae...@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?= <tbae...@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, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
