[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGa13f036949b0: [clang][Interp] Check one-past-the-end
pointers in GetPtrField (authored by tbaeder).
Changed prior to commit:
https://reviews.llvm.org/D149149?vs=516763=555834#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149149/new/
https://reviews.llvm.org/D149149
Files:
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/Interp.h
clang/test/AST/Interp/records.cpp
Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -531,6 +531,29 @@
//static_assert(b.a.f == 100, "");
}
+namespace PointerArith {
+ struct A {};
+ struct B : A { int n; };
+
+ B b = {};
+ constexpr A *a1 =
+ constexpr B *b1 = + 1;
+ constexpr B *b2 = + 0;
+
+#if 0
+ constexpr A *a2 = + 1; // expected-error {{must be initialized by a constant expression}} \
+// expected-note {{cannot access base class of pointer past the end of object}} \
+// ref-error {{must be initialized by a constant expression}} \
+// ref-note {{cannot access base class of pointer past the end of object}}
+
+#endif
+ constexpr const int *pn = &( + 1)->n; // expected-error {{must be initialized by a constant expression}} \
+ // expected-note {{cannot access field of pointer past the end of object}} \
+ // ref-error {{must be initialized by a constant expression}} \
+ // ref-note {{cannot access field of pointer past the end of object}}
+
+}
+
#if __cplusplus >= 202002L
namespace VirtualCalls {
namespace Obvious {
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -67,9 +67,9 @@
bool CheckRange(InterpState , CodePtr OpPC, const Pointer ,
CheckSubobjectKind CSK);
-/// Checks if accessing a base or derived record of the given pointer is valid.
-bool CheckBaseDerived(InterpState , CodePtr OpPC, const Pointer ,
- CheckSubobjectKind CSK);
+/// Checks if Ptr is a one-past-the-end pointer.
+bool CheckSubobject(InterpState , CodePtr OpPC, const Pointer ,
+CheckSubobjectKind CSK);
/// Checks if a pointer points to const storage.
bool CheckConst(InterpState , CodePtr OpPC, const Pointer );
@@ -1121,6 +1121,9 @@
return false;
if (!CheckRange(S, OpPC, Ptr, CSK_Field))
return false;
+ if (!CheckSubobject(S, OpPC, Ptr, CSK_Field))
+return false;
+
S.Stk.push(Ptr.atField(Off));
return true;
}
@@ -1165,7 +1168,7 @@
const Pointer = S.Stk.pop();
if (!CheckNull(S, OpPC, Ptr, CSK_Derived))
return false;
- if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Derived))
+ if (!CheckSubobject(S, OpPC, Ptr, CSK_Derived))
return false;
S.Stk.push(Ptr.atFieldSub(Off));
return true;
@@ -1175,7 +1178,7 @@
const Pointer = S.Stk.peek();
if (!CheckNull(S, OpPC, Ptr, CSK_Base))
return false;
- if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+ if (!CheckSubobject(S, OpPC, Ptr, CSK_Base))
return false;
S.Stk.push(Ptr.atField(Off));
return true;
@@ -1185,7 +1188,7 @@
const Pointer = S.Stk.pop();
if (!CheckNull(S, OpPC, Ptr, CSK_Base))
return false;
- if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+ if (!CheckSubobject(S, OpPC, Ptr, CSK_Base))
return false;
S.Stk.push(Ptr.atField(Off));
return true;
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -213,8 +213,8 @@
return false;
}
-bool CheckBaseDerived(InterpState , CodePtr OpPC, const Pointer ,
- CheckSubobjectKind CSK) {
+bool CheckSubobject(InterpState , CodePtr OpPC, const Pointer ,
+CheckSubobjectKind CSK) {
if (!Ptr.isOnePastEnd())
return true;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField
shafik added a subscriber: rsmith.
shafik added inline comments.
Comment at: clang/test/AST/Interp/records.cpp:509
constexpr A *a2 = + 1; // expected-error {{must be initialized by a
constant expression}} \
// expected-note {{cannot access base class of
pointer past the end of object}} \
@rsmith it looks like you added this diagnostic a while ago see ce1ec5e1f5620
did you mean it to apply to this case? The rules have changed a lot since then
so perhaps this was invalid but now is good.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149149/new/
https://reviews.llvm.org/D149149
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField
aaron.ballman added inline comments.
Comment at: clang/test/AST/Interp/records.cpp:500
namespace PointerArith {
struct A {};
shafik wrote:
> aaron.ballman wrote:
> > Neat! For the tests in this namespace, Clang and ICC agree, GCC and MSVC
> > agree, and users get to cry: https://godbolt.org/z/7EWWrY5z6
> Yeah, unfortunate but I am pretty sure clang is correct on both of those.
I think Clang is correct on one of those, but not both of them. I think Clang
is correct to reject `constexpr const int *pn = &( + 1)->n;` as that involves
dereferencing the one-past-the-end pointer, but I think Clang is wrong to
reject `constexpr A *a2 = + 1;` as there's no dereference there.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149149/new/
https://reviews.llvm.org/D149149
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField
shafik added inline comments.
Comment at: clang/test/AST/Interp/records.cpp:500
namespace PointerArith {
struct A {};
aaron.ballman wrote:
> Neat! For the tests in this namespace, Clang and ICC agree, GCC and MSVC
> agree, and users get to cry: https://godbolt.org/z/7EWWrY5z6
Yeah, unfortunate but I am pretty sure clang is correct on both of those.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149149/new/
https://reviews.llvm.org/D149149
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField
aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.
LGTM
Comment at: clang/test/AST/Interp/records.cpp:500
namespace PointerArith {
struct A {};
Neat! For the tests in this namespace, Clang and ICC agree, GCC and MSVC agree,
and users get to cry: https://godbolt.org/z/7EWWrY5z6
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D149149/new/
https://reviews.llvm.org/D149149
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField
tbaeder added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D149149/new/ https://reviews.llvm.org/D149149 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D149149: [clang][Interp] Check one-past-the-end pointers in GetPtrField
tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Rename CheckBaseDerived to something more general and call it in
GetPtrField() as well, so we don't crash later in Pointer::toAPValue().
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D149149
Files:
clang/lib/AST/Interp/Interp.cpp
clang/lib/AST/Interp/Interp.h
clang/test/AST/Interp/records.cpp
Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -510,6 +510,12 @@
// expected-note {{cannot access base class of
pointer past the end of object}} \
// ref-error {{must be initialized by a constant
expression}} \
// ref-note {{cannot access base class of pointer
past the end of object}}
+
+ constexpr const int *pn = &( + 1)->n; // expected-error {{must be
initialized by a constant expression}} \
+ // expected-note {{cannot access
field of pointer past the end of object}} \
+ // ref-error {{must be initialized
by a constant expression}} \
+ // ref-note {{cannot access field of
pointer past the end of object}}
+
}
#if __cplusplus >= 202002L
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -67,9 +67,9 @@
bool CheckRange(InterpState , CodePtr OpPC, const Pointer ,
CheckSubobjectKind CSK);
-/// Checks if accessing a base or derived record of the given pointer is valid.
-bool CheckBaseDerived(InterpState , CodePtr OpPC, const Pointer ,
- CheckSubobjectKind CSK);
+/// Checks if Ptr is a one-past-the-end pointer.
+bool CheckSubobject(InterpState , CodePtr OpPC, const Pointer ,
+CheckSubobjectKind CSK);
/// Checks if a pointer points to const storage.
bool CheckConst(InterpState , CodePtr OpPC, const Pointer );
@@ -1039,6 +1039,8 @@
return false;
if (!CheckRange(S, OpPC, Ptr, CSK_Field))
return false;
+ if (!CheckSubobject(S, OpPC, Ptr, CSK_Field))
+return false;
S.Stk.push(Ptr.atField(Off));
return true;
}
@@ -1083,7 +1085,7 @@
const Pointer = S.Stk.pop();
if (!CheckNull(S, OpPC, Ptr, CSK_Derived))
return false;
- if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Derived))
+ if (!CheckSubobject(S, OpPC, Ptr, CSK_Derived))
return false;
S.Stk.push(Ptr.atFieldSub(Off));
return true;
@@ -1093,7 +1095,7 @@
const Pointer = S.Stk.peek();
if (!CheckNull(S, OpPC, Ptr, CSK_Base))
return false;
- if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+ if (!CheckSubobject(S, OpPC, Ptr, CSK_Base))
return false;
S.Stk.push(Ptr.atField(Off));
return true;
@@ -1103,7 +1105,7 @@
const Pointer = S.Stk.pop();
if (!CheckNull(S, OpPC, Ptr, CSK_Base))
return false;
- if (!CheckBaseDerived(S, OpPC, Ptr, CSK_Base))
+ if (!CheckSubobject(S, OpPC, Ptr, CSK_Base))
return false;
S.Stk.push(Ptr.atField(Off));
return true;
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -211,8 +211,8 @@
return false;
}
-bool CheckBaseDerived(InterpState , CodePtr OpPC, const Pointer ,
- CheckSubobjectKind CSK) {
+bool CheckSubobject(InterpState , CodePtr OpPC, const Pointer ,
+CheckSubobjectKind CSK) {
if (!Ptr.isOnePastEnd())
return true;
Index: clang/test/AST/Interp/records.cpp
===
--- clang/test/AST/Interp/records.cpp
+++ clang/test/AST/Interp/records.cpp
@@ -510,6 +510,12 @@
// expected-note {{cannot access base class of pointer past the end of object}} \
// ref-error {{must be initialized by a constant expression}} \
// ref-note {{cannot access base class of pointer past the end of object}}
+
+ constexpr const int *pn = &( + 1)->n; // expected-error {{must be initialized by a constant expression}} \
+ // expected-note {{cannot access field of pointer past the end of object}} \
+ // ref-error {{must be initialized by a constant expression}} \
+ // ref-note {{cannot access field of pointer past the end of object}}
+
}
#if __cplusplus >= 202002L
Index: clang/lib/AST/Interp/Interp.h
