Skip to content

Commit ff48875

Browse files
tbaederrgokulbmishra
authored andcommitted
[clang][bytecode] Check pointers in GetPtrField{,Pop} (llvm#167335)
The pointer needs to point to a record. Fixes llvm#166371
1 parent afec769 commit ff48875

File tree

6 files changed

+60
-26
lines changed

6 files changed

+60
-26
lines changed

clang/lib/AST/ByteCode/Interp.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1448,6 +1448,10 @@ static bool getField(InterpState &S, CodePtr OpPC, const Pointer &Ptr,
14481448
return false;
14491449
}
14501450

1451+
// We can't get the field of something that's not a record.
1452+
if (!Ptr.getFieldDesc()->isRecord())
1453+
return false;
1454+
14511455
if ((Ptr.getByteOffset() + Off) >= Ptr.block()->getSize())
14521456
return false;
14531457

clang/lib/AST/ByteCode/Interp.h

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2317,13 +2317,11 @@ std::optional<Pointer> OffsetHelper(InterpState &S, CodePtr OpPC,
23172317
template <PrimType Name, class T = typename PrimConv<Name>::T>
23182318
bool AddOffset(InterpState &S, CodePtr OpPC) {
23192319
const T &Offset = S.Stk.pop<T>();
2320-
Pointer Ptr = S.Stk.pop<Pointer>();
2321-
if (Ptr.isBlockPointer())
2322-
Ptr = Ptr.expand();
2320+
const Pointer &Ptr = S.Stk.pop<Pointer>().expand();
23232321

23242322
if (std::optional<Pointer> Result = OffsetHelper<T, ArithOp::Add>(
23252323
S, OpPC, Offset, Ptr, /*IsPointerArith=*/true)) {
2326-
S.Stk.push<Pointer>(*Result);
2324+
S.Stk.push<Pointer>(Result->narrow());
23272325
return true;
23282326
}
23292327
return false;
@@ -2332,11 +2330,11 @@ bool AddOffset(InterpState &S, CodePtr OpPC) {
23322330
template <PrimType Name, class T = typename PrimConv<Name>::T>
23332331
bool SubOffset(InterpState &S, CodePtr OpPC) {
23342332
const T &Offset = S.Stk.pop<T>();
2335-
const Pointer &Ptr = S.Stk.pop<Pointer>();
2333+
const Pointer &Ptr = S.Stk.pop<Pointer>().expand();
23362334

23372335
if (std::optional<Pointer> Result = OffsetHelper<T, ArithOp::Sub>(
23382336
S, OpPC, Offset, Ptr, /*IsPointerArith=*/true)) {
2339-
S.Stk.push<Pointer>(*Result);
2337+
S.Stk.push<Pointer>(Result->narrow());
23402338
return true;
23412339
}
23422340
return false;
@@ -2362,7 +2360,7 @@ static inline bool IncDecPtrHelper(InterpState &S, CodePtr OpPC,
23622360
if (std::optional<Pointer> Result =
23632361
OffsetHelper<OneT, Op>(S, OpPC, One, P, /*IsPointerArith=*/true)) {
23642362
// Store the new value.
2365-
Ptr.deref<Pointer>() = *Result;
2363+
Ptr.deref<Pointer>() = Result->narrow();
23662364
return true;
23672365
}
23682366
return false;
@@ -2391,8 +2389,8 @@ static inline bool DecPtr(InterpState &S, CodePtr OpPC) {
23912389
/// 3) Pushes the difference of the indices of the two pointers on the stack.
23922390
template <PrimType Name, class T = typename PrimConv<Name>::T>
23932391
inline bool SubPtr(InterpState &S, CodePtr OpPC, bool ElemSizeIsZero) {
2394-
const Pointer &LHS = S.Stk.pop<Pointer>();
2395-
const Pointer &RHS = S.Stk.pop<Pointer>();
2392+
const Pointer &LHS = S.Stk.pop<Pointer>().expand();
2393+
const Pointer &RHS = S.Stk.pop<Pointer>().expand();
23962394

23972395
if (!Pointer::hasSameBase(LHS, RHS) && S.getLangOpts().CPlusPlus) {
23982396
S.FFDiag(S.Current->getSource(OpPC),
@@ -3083,7 +3081,7 @@ inline bool ArrayElemPtr(InterpState &S, CodePtr OpPC) {
30833081
S.Stk.push<Pointer>(Ptr.atIndex(0).narrow());
30843082
return true;
30853083
}
3086-
S.Stk.push<Pointer>(Ptr);
3084+
S.Stk.push<Pointer>(Ptr.narrow());
30873085
return true;
30883086
}
30893087

@@ -3114,7 +3112,7 @@ inline bool ArrayElemPtrPop(InterpState &S, CodePtr OpPC) {
31143112
S.Stk.push<Pointer>(Ptr.atIndex(0).narrow());
31153113
return true;
31163114
}
3117-
S.Stk.push<Pointer>(Ptr);
3115+
S.Stk.push<Pointer>(Ptr.narrow());
31183116
return true;
31193117
}
31203118

@@ -3189,7 +3187,7 @@ inline bool ArrayDecay(InterpState &S, CodePtr OpPC) {
31893187
}
31903188

31913189
if (Ptr.isRoot() || !Ptr.isUnknownSizeArray()) {
3192-
S.Stk.push<Pointer>(Ptr.atIndex(0));
3190+
S.Stk.push<Pointer>(Ptr.atIndex(0).narrow());
31933191
return true;
31943192
}
31953193

clang/lib/AST/ByteCode/InterpBuiltin.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,7 +296,7 @@ static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
296296
static bool interp__builtin_strlen(InterpState &S, CodePtr OpPC,
297297
const InterpFrame *Frame,
298298
const CallExpr *Call, unsigned ID) {
299-
const Pointer &StrPtr = S.Stk.pop<Pointer>();
299+
const Pointer &StrPtr = S.Stk.pop<Pointer>().expand();
300300

301301
if (ID == Builtin::BIstrlen || ID == Builtin::BIwcslen)
302302
diagnoseNonConstexprBuiltin(S, OpPC, ID);
@@ -1440,7 +1440,7 @@ static bool interp__builtin_operator_new(InterpState &S, CodePtr OpPC,
14401440
Allocator.allocate(Desc, NumElems.getZExtValue(), S.Ctx.getEvalID(),
14411441
DynamicAllocator::Form::Operator);
14421442
assert(B);
1443-
S.Stk.push<Pointer>(Pointer(B).atIndex(0));
1443+
S.Stk.push<Pointer>(Pointer(B).atIndex(0).narrow());
14441444
return true;
14451445
}
14461446

@@ -1764,8 +1764,8 @@ static bool interp__builtin_memcpy(InterpState &S, CodePtr OpPC,
17641764
assert(Call->getNumArgs() == 3);
17651765
const ASTContext &ASTCtx = S.getASTContext();
17661766
APSInt Size = popToAPSInt(S, Call->getArg(2));
1767-
const Pointer SrcPtr = S.Stk.pop<Pointer>();
1768-
const Pointer DestPtr = S.Stk.pop<Pointer>();
1767+
Pointer SrcPtr = S.Stk.pop<Pointer>().expand();
1768+
Pointer DestPtr = S.Stk.pop<Pointer>().expand();
17691769

17701770
assert(!Size.isSigned() && "memcpy and friends take an unsigned size");
17711771

clang/lib/AST/ByteCode/Pointer.h

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -199,17 +199,19 @@ class Pointer {
199199
return Pointer(BS.Pointee, sizeof(InlineDescriptor),
200200
Offset == 0 ? Offset : PastEndMark);
201201

202-
// Pointer is one past end - magic offset marks that.
203-
if (isOnePastEnd())
204-
return Pointer(BS.Pointee, Base, PastEndMark);
205-
206-
if (Offset != Base) {
207-
// If we're pointing to a primitive array element, there's nothing to do.
208-
if (inPrimitiveArray())
209-
return *this;
210-
// Pointer is to a composite array element - enter it.
211-
if (Offset != Base)
202+
if (inArray()) {
203+
// Pointer is one past end - magic offset marks that.
204+
if (isOnePastEnd())
205+
return Pointer(BS.Pointee, Base, PastEndMark);
206+
207+
if (Offset != Base) {
208+
// If we're pointing to a primitive array element, there's nothing to
209+
// do.
210+
if (inPrimitiveArray())
211+
return *this;
212+
// Pointer is to a composite array element - enter it.
212213
return Pointer(BS.Pointee, Offset, Offset);
214+
}
213215
}
214216

215217
// Otherwise, we're pointing to a non-array element or
@@ -219,6 +221,8 @@ class Pointer {
219221

220222
/// Expands a pointer to the containing array, undoing narrowing.
221223
[[nodiscard]] Pointer expand() const {
224+
if (!isBlockPointer())
225+
return *this;
222226
assert(isBlockPointer());
223227
Block *Pointee = BS.Pointee;
224228

clang/test/AST/ByteCode/cxx23.cpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,3 +449,27 @@ namespace VolatileWrites {
449449
static_assert(test7(12)); // all-error {{not an integral constant expression}} \
450450
// all-note {{in call to}}
451451
}
452+
453+
namespace AIEWithIndex0Narrows {
454+
template <class _Tp> struct greater {
455+
constexpr void operator()(_Tp, _Tp) {}
456+
};
457+
struct S {
458+
constexpr S() : i_() {}
459+
int i_;
460+
};
461+
462+
constexpr void sort(S *__first) {
463+
for (int __start = 0; __start >= 0; --__start) {
464+
greater<S>{}(__first[0], __first[0]);
465+
}
466+
}
467+
constexpr bool test() {
468+
S *ia = new S[2];
469+
470+
sort(ia + 1);
471+
delete[] ia;
472+
return true;
473+
}
474+
static_assert(test());
475+
}

clang/test/AST/ByteCode/invalid.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,4 +106,8 @@ namespace InvalidBitCast {
106106
return ((sockaddr_in *)&addr)->sin_addr.s_addr;
107107
}
108108

109+
110+
struct s { int a; int b[1]; };
111+
struct s myx;
112+
int *myy = ((struct s *)&myx.a)->b;
109113
}

0 commit comments

Comments
 (0)