Skip to content

Commit 6195e22

Browse files
committed
Revert "[clang][Interp] Create full type info for dummy pointers"
This reverts commit eef5798. This breaks two tests on an arm builder: https://lab.llvm.org/buildbot/#/builders/245/builds/23496
1 parent adb2547 commit 6195e22

File tree

7 files changed

+57
-36
lines changed

7 files changed

+57
-36
lines changed

clang/lib/AST/Interp/Descriptor.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,14 @@ Descriptor::Descriptor(const DeclTy &D)
309309
assert(Source && "Missing source");
310310
}
311311

312+
/// Dummy array.
313+
Descriptor::Descriptor(const DeclTy &D, UnknownSize)
314+
: Source(D), ElemSize(1), Size(UnknownSizeMark), MDSize(0),
315+
AllocSize(MDSize), ElemRecord(nullptr), IsConst(true), IsMutable(false),
316+
IsTemporary(false), IsArray(true), IsDummy(true) {
317+
assert(Source && "Missing source");
318+
}
319+
312320
QualType Descriptor::getType() const {
313321
if (auto *E = asExpr())
314322
return E->getType();

clang/lib/AST/Interp/Descriptor.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ struct Descriptor final {
125125
/// Flag indicating if the block is an array.
126126
const bool IsArray = false;
127127
/// Flag indicating if this is a dummy descriptor.
128-
bool IsDummy = false;
128+
const bool IsDummy = false;
129129

130130
/// Storage management methods.
131131
const BlockCtorFn CtorFn = nullptr;
@@ -159,8 +159,8 @@ struct Descriptor final {
159159
/// Allocates a dummy descriptor.
160160
Descriptor(const DeclTy &D);
161161

162-
/// Make this descriptor a dummy descriptor.
163-
void makeDummy() { IsDummy = true; }
162+
/// Allocates a dummy array descriptor.
163+
Descriptor(const DeclTy &D, UnknownSize);
164164

165165
QualType getType() const;
166166
QualType getElemQualType() const;

clang/lib/AST/Interp/Interp.h

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -823,9 +823,9 @@ inline bool CmpHelperEQ<Pointer>(InterpState &S, CodePtr OpPC, CompareFn Fn) {
823823
// element in the same array are NOT equal. They have the same Base value,
824824
// but a different Offset. This is a pretty rare case, so we fix this here
825825
// by comparing pointers to the first elements.
826-
if (!LHS.isZero() && LHS.isArrayRoot())
826+
if (!LHS.isZero() && !LHS.isDummy() && LHS.isArrayRoot())
827827
VL = LHS.atIndex(0).getByteOffset();
828-
if (!RHS.isZero() && RHS.isArrayRoot())
828+
if (!RHS.isZero() && !RHS.isDummy() && RHS.isArrayRoot())
829829
VR = RHS.atIndex(0).getByteOffset();
830830

831831
S.Stk.push<BoolT>(BoolT::from(Fn(Compare(VL, VR))));
@@ -1241,16 +1241,14 @@ inline bool GetPtrField(InterpState &S, CodePtr OpPC, uint32_t Off) {
12411241
!CheckNull(S, OpPC, Ptr, CSK_Field))
12421242
return false;
12431243

1244-
if (!CheckExtern(S, OpPC, Ptr))
1245-
return false;
1246-
if (!CheckRange(S, OpPC, Ptr, CSK_Field))
1247-
return false;
1248-
if (!CheckSubobject(S, OpPC, Ptr, CSK_Field))
1249-
return false;
1250-
1251-
if (Ptr.isBlockPointer() && Off > Ptr.block()->getSize())
1252-
return false;
1253-
1244+
if (CheckDummy(S, OpPC, Ptr)) {
1245+
if (!CheckExtern(S, OpPC, Ptr))
1246+
return false;
1247+
if (!CheckRange(S, OpPC, Ptr, CSK_Field))
1248+
return false;
1249+
if (!CheckSubobject(S, OpPC, Ptr, CSK_Field))
1250+
return false;
1251+
}
12541252
S.Stk.push<Pointer>(Ptr.atField(Off));
12551253
return true;
12561254
}
@@ -1994,6 +1992,11 @@ inline bool ArrayElemPtr(InterpState &S, CodePtr OpPC) {
19941992
if (!Ptr.isZero()) {
19951993
if (!CheckArray(S, OpPC, Ptr))
19961994
return false;
1995+
1996+
if (Ptr.isDummy()) {
1997+
S.Stk.push<Pointer>(Ptr);
1998+
return true;
1999+
}
19972000
}
19982001

19992002
if (!OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr))
@@ -2010,6 +2013,11 @@ inline bool ArrayElemPtrPop(InterpState &S, CodePtr OpPC) {
20102013
if (!Ptr.isZero()) {
20112014
if (!CheckArray(S, OpPC, Ptr))
20122015
return false;
2016+
2017+
if (Ptr.isDummy()) {
2018+
S.Stk.push<Pointer>(Ptr);
2019+
return true;
2020+
}
20132021
}
20142022

20152023
if (!OffsetHelper<T, ArithOp::Add>(S, OpPC, Offset, Ptr))
@@ -2045,12 +2053,12 @@ inline bool ArrayElemPop(InterpState &S, CodePtr OpPC, uint32_t Index) {
20452053
inline bool ArrayDecay(InterpState &S, CodePtr OpPC) {
20462054
const Pointer &Ptr = S.Stk.pop<Pointer>();
20472055

2048-
if (Ptr.isZero()) {
2056+
if (Ptr.isZero() || Ptr.isDummy()) {
20492057
S.Stk.push<Pointer>(Ptr);
20502058
return true;
20512059
}
20522060

2053-
if (!Ptr.isUnknownSizeArray() || Ptr.isDummy()) {
2061+
if (!Ptr.isUnknownSizeArray()) {
20542062
S.Stk.push<Pointer>(Ptr.atIndex(0));
20552063
return true;
20562064
}

clang/lib/AST/Interp/Pointer.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,10 +139,9 @@ APValue Pointer::toAPValue() const {
139139
else
140140
llvm_unreachable("Invalid allocation type");
141141

142-
if (isDummy() || isUnknownSizeArray() || Desc->asExpr()) {
142+
if (isDummy() || isUnknownSizeArray() || Desc->asExpr())
143143
return APValue(Base, CharUnits::Zero(), Path,
144144
/*IsOnePastEnd=*/false, /*IsNullPtr=*/false);
145-
}
146145

147146
// TODO: compute the offset into the object.
148147
CharUnits Offset = CharUnits::Zero();

clang/lib/AST/Interp/Program.cpp

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -148,20 +148,17 @@ std::optional<unsigned> Program::getOrCreateDummy(const ValueDecl *VD) {
148148
if (auto It = DummyVariables.find(VD); It != DummyVariables.end())
149149
return It->second;
150150

151+
// Create dummy descriptor.
152+
// We create desriptors of 'array of unknown size' if the type is an array
153+
// type _and_ the size isn't known (it's not a ConstantArrayType). If the size
154+
// is known however, we create a regular dummy pointer.
151155
Descriptor *Desc;
152-
if (std::optional<PrimType> T = Ctx.classify(VD->getType()))
153-
Desc = createDescriptor(VD, *T, std::nullopt, true, false);
156+
if (const auto *AT = VD->getType()->getAsArrayTypeUnsafe();
157+
AT && !isa<ConstantArrayType>(AT))
158+
Desc = allocateDescriptor(VD, Descriptor::UnknownSize{});
154159
else
155-
Desc = createDescriptor(VD, VD->getType().getTypePtr(), std::nullopt, true,
156-
false);
157-
if (!Desc)
158160
Desc = allocateDescriptor(VD);
159161

160-
assert(Desc);
161-
Desc->makeDummy();
162-
163-
assert(Desc->isDummy());
164-
165162
// Allocate a block for storage.
166163
unsigned I = Globals.size();
167164

@@ -317,7 +314,8 @@ Record *Program::getOrCreateRecord(const RecordDecl *RD) {
317314
for (const FieldDecl *FD : RD->fields()) {
318315
// Note that we DO create fields and descriptors
319316
// for unnamed bitfields here, even though we later ignore
320-
// them everywhere. That's so the FieldDecl's getFieldIndex() matches.
317+
// them everywhere. That's because so the FieldDecl's
318+
// getFieldIndex() matches.
321319

322320
// Reserve space for the field's descriptor and the offset.
323321
BaseSize += align(sizeof(InlineDescriptor));
@@ -350,7 +348,6 @@ Descriptor *Program::createDescriptor(const DeclTy &D, const Type *Ty,
350348
Descriptor::MetadataSize MDSize,
351349
bool IsConst, bool IsTemporary,
352350
bool IsMutable, const Expr *Init) {
353-
354351
// Classes and structures.
355352
if (const auto *RT = Ty->getAs<RecordType>()) {
356353
if (const auto *Record = getOrCreateRecord(RT->getDecl()))

clang/test/AST/Interp/builtin-align-cxx.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,19 @@
22
// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 %s -fsyntax-only -verify=expected,both -fexperimental-new-constant-interpreter
33
// RUN: %clang_cc1 -triple=x86_64-unknown-unknown -std=c++11 %s -fsyntax-only -verify=ref,both
44

5+
6+
/// This is just a copy of the one from test/SemaCXX/ with some of the
7+
/// diagnostic output adapted.
8+
/// Also, align32array has an initializer now, which means it's not just
9+
/// a dummy pointer for us and we do actually have type information for it.
10+
/// In the future, we need to retain type information for dummy pointers as
11+
/// well, so here is a test that will break once we do that:
12+
namespace {
13+
_Alignas(32) char heh[4];
14+
static_assert(!__builtin_is_aligned(&heh[1], 4), ""); // expected-error {{failed}}
15+
}
16+
17+
518
// Check that we don't crash when using dependent types in __builtin_align:
619
template <typename a, a b>
720
void *c(void *d) { // both-note{{candidate template ignored}}
@@ -164,7 +177,7 @@ static_assert(wrap_align_up(static_cast<bool>(1), const_value(1 << 21)), ""); //
164177
// both-note@-1{{in instantiation of function template specialization 'wrap_align_up<bool>' requested here}}
165178

166179
// Check constant evaluation for pointers:
167-
_Alignas(32) char align32array[128];
180+
_Alignas(32) char align32array[128] = {};
168181
static_assert(&align32array[0] == &align32array[0], "");
169182
// __builtin_align_up/down can be constant evaluated as a no-op for values
170183
// that are known to have greater alignment:

clang/test/AST/Interp/c.c

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -257,7 +257,3 @@ int Y __attribute__((annotate(
257257
42,
258258
(struct TestStruct) { .a = 1, .b = 2 }
259259
)));
260-
261-
/// This tests that we have full type info, even for values we cannot read.
262-
int dummyarray[5];
263-
_Static_assert(&dummyarray[0] < &dummyarray[1], ""); // pedantic-warning {{GNU extension}}

0 commit comments

Comments
 (0)