Index: lib/AST/ExprConstant.cpp =================================================================== --- lib/AST/ExprConstant.cpp +++ lib/AST/ExprConstant.cpp @@ -114,7 +114,8 @@ static unsigned findMostDerivedSubobject(ASTContext &Ctx, QualType Base, ArrayRef Path, - uint64_t &ArraySize, QualType &Type) { + uint64_t &ArraySize, QualType &Type, + bool &IsArray) { unsigned MostDerivedLength = 0; Type = Base; for (unsigned I = 0, N = Path.size(); I != N; ++I) { @@ -124,18 +125,22 @@ Type = CAT->getElementType(); ArraySize = CAT->getSize().getZExtValue(); MostDerivedLength = I + 1; + IsArray = true; } else if (Type->isAnyComplexType()) { const ComplexType *CT = Type->castAs(); Type = CT->getElementType(); ArraySize = 2; MostDerivedLength = I + 1; + IsArray = true; } else if (const FieldDecl *FD = getAsField(Path[I])) { Type = FD->getType(); ArraySize = 0; MostDerivedLength = I + 1; + IsArray = false; } else { // Path[I] describes a base class. ArraySize = 0; + IsArray = false; } } return MostDerivedLength; @@ -157,12 +162,17 @@ /// Is this a pointer one past the end of an object? bool IsOnePastTheEnd : 1; + /// Indicator of whether the most-derived object is an array element. + bool MostDerivedIsArrayElement : 1; + /// The length of the path to the most-derived object of which this is a /// subobject. - unsigned MostDerivedPathLength : 30; + unsigned MostDerivedPathLength : 29; - /// The size of the array of which the most-derived object is an element, or - /// 0 if the most-derived object is not an array element. + /// The size of the array of which the most-derived object is an element. + /// This will always be 0 if the most-derived object is not an array + /// element. 0 is not an indicator of whether or not the most-derived object + /// is an array, however, because 0-length arrays are allowed. uint64_t MostDerivedArraySize; /// The type of the most derived object referred to by this address. @@ -176,21 +186,26 @@ SubobjectDesignator() : Invalid(true) {} explicit SubobjectDesignator(QualType T) - : Invalid(false), IsOnePastTheEnd(false), MostDerivedPathLength(0), - MostDerivedArraySize(0), MostDerivedType(T) {} + : Invalid(false), IsOnePastTheEnd(false), + MostDerivedIsArrayElement(false), MostDerivedPathLength(0), + MostDerivedArraySize(0), MostDerivedType(T) {} SubobjectDesignator(ASTContext &Ctx, const APValue &V) - : Invalid(!V.isLValue() || !V.hasLValuePath()), IsOnePastTheEnd(false), - MostDerivedPathLength(0), MostDerivedArraySize(0) { + : Invalid(!V.isLValue() || !V.hasLValuePath()), IsOnePastTheEnd(false), + MostDerivedIsArrayElement(false), MostDerivedPathLength(0), + MostDerivedArraySize(0) { if (!Invalid) { IsOnePastTheEnd = V.isLValueOnePastTheEnd(); ArrayRef VEntries = V.getLValuePath(); Entries.insert(Entries.end(), VEntries.begin(), VEntries.end()); - if (V.getLValueBase()) + if (V.getLValueBase()) { + bool IsArray = false; MostDerivedPathLength = findMostDerivedSubobject(Ctx, getType(V.getLValueBase()), V.getLValuePath(), MostDerivedArraySize, - MostDerivedType); + MostDerivedType, IsArray); + MostDerivedIsArrayElement = IsArray; + } } } @@ -204,7 +219,7 @@ assert(!Invalid); if (IsOnePastTheEnd) return true; - if (MostDerivedArraySize && + if (MostDerivedIsArrayElement && Entries[MostDerivedPathLength - 1].ArrayIndex == MostDerivedArraySize) return true; return false; @@ -228,6 +243,7 @@ // This is a most-derived object. MostDerivedType = CAT->getElementType(); + MostDerivedIsArrayElement = true; MostDerivedArraySize = CAT->getSize().getZExtValue(); MostDerivedPathLength = Entries.size(); } @@ -242,6 +258,7 @@ // If this isn't a base class, it's a new most-derived object. if (const FieldDecl *FD = dyn_cast(D)) { MostDerivedType = FD->getType(); + MostDerivedIsArrayElement = false; MostDerivedArraySize = 0; MostDerivedPathLength = Entries.size(); } @@ -255,6 +272,7 @@ // This is technically a most-derived object, though in practice this // is unlikely to matter. MostDerivedType = EltTy; + MostDerivedIsArrayElement = true; MostDerivedArraySize = 2; MostDerivedPathLength = Entries.size(); } @@ -262,7 +280,8 @@ /// Add N to the address of this subobject. void adjustIndex(EvalInfo &Info, const Expr *E, uint64_t N) { if (Invalid) return; - if (MostDerivedPathLength == Entries.size() && MostDerivedArraySize) { + if (MostDerivedPathLength == Entries.size() && + MostDerivedIsArrayElement) { Entries.back().ArrayIndex += N; if (Entries.back().ArrayIndex > MostDerivedArraySize) { diagnosePointerArithmetic(Info, E, Entries.back().ArrayIndex); @@ -834,7 +853,7 @@ void SubobjectDesignator::diagnosePointerArithmetic(EvalInfo &Info, const Expr *E, uint64_t N) { - if (MostDerivedPathLength == Entries.size() && MostDerivedArraySize) + if (MostDerivedPathLength == Entries.size() && MostDerivedIsArrayElement) Info.CCEDiag(E, diag::note_constexpr_array_index) << static_cast(N) << /*array*/ 0 << static_cast(MostDerivedArraySize); @@ -2517,7 +2536,7 @@ if (A.Entries.size() != B.Entries.size()) return false; - bool IsArray = A.MostDerivedArraySize != 0; + bool IsArray = A.MostDerivedIsArrayElement; if (IsArray && A.MostDerivedPathLength != A.Entries.size()) // A is a subobject of the array element. return false; @@ -4416,7 +4435,8 @@ if (!EvalOK) { if (!this->Info.allowInvalidBaseExpr()) return false; - Result.setInvalid(E->getBase()); + Result.setInvalid(E); + return true; } const ValueDecl *MD = E->getMemberDecl(); @@ -6276,6 +6296,82 @@ return ignorePointerCastsAndParens(SubExpr); } +/// Checks to see if the given LValue's Designator is at the end of the LValue's +/// record layout. e.g. +/// struct { struct { int a, b; } fst, snd; } obj; +/// obj.fst // no +/// obj.snd // yes +/// obj.fst.a // no +/// obj.fst.b // no +/// obj.snd.a // no +/// obj.snd.b // yes +static bool isDesignatorAtObjectEnd(const ASTContext &Ctx, const LValue &LVal) { + assert(!LVal.Designator.Invalid); + + auto IsLastFieldDecl = [&Ctx](const FieldDecl *FD) { + if (FD->getParent()->isUnion()) + return true; + const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(FD->getParent()); + return FD->getFieldIndex() + 1 == Layout.getFieldCount(); + }; + + auto &Base = LVal.getLValueBase(); + if (auto *ME = dyn_cast_or_null(Base.dyn_cast())) { + if (auto *FD = dyn_cast(ME->getMemberDecl())) { + if (!IsLastFieldDecl(FD)) + return false; + } else if (auto *IFD = dyn_cast(ME->getMemberDecl())) { + for (auto *FD : IFD->chain()) + if (!IsLastFieldDecl(cast(FD))) + return false; + } + } + + QualType BaseType = getType(Base); + for (int I = 0, E = LVal.Designator.Entries.size(); I != E; ++I) { + if (BaseType->isArrayType()) { + // Because __builtin_object_size treats arrays as objects, we can ignore + // the index iff this is the last array in the Designator. + if (I + 1 == E) + return true; + auto *CAT = cast(Ctx.getAsArrayType(BaseType)); + uint64_t Index = LVal.Designator.Entries[I].ArrayIndex; + if (Index + 1 != CAT->getSize()) + return false; + BaseType = CAT->getElementType(); + } else if (BaseType->isAnyComplexType()) { + auto *CT = BaseType->castAs(); + uint64_t Index = LVal.Designator.Entries[I].ArrayIndex; + if (Index != 1) + return false; + BaseType = CT->getElementType(); + } else if (auto *FD = getAsField(LVal.Designator.Entries[I])) { + if (!IsLastFieldDecl(FD)) + return false; + BaseType = FD->getType(); + } else { + assert(getAsBaseClass(LVal.Designator.Entries[I]) != nullptr && + "Expecting cast to a base class"); + return false; + } + } + return true; +} + +/// Tests to see if the has a designator (that isn't necessarily valid). +static bool hasDesignator(const LValue &LVal) { + if (LVal.Designator.Invalid || !LVal.Designator.Entries.empty()) + return true; + + if (!LVal.InvalidBase) + return false; + + auto *E = LVal.Base.dyn_cast(); + (void)E; + assert(E != nullptr && isa(E)); + return true; +} + bool IntExprEvaluator::TryEvaluateBuiltinObjectSize(const CallExpr *E, unsigned Type) { // Determine the denoted object. @@ -6299,8 +6395,7 @@ // In the case where we're not dealing with a subobject, we discard the // subobject bit. - if (!Base.Designator.Invalid && Base.Designator.Entries.empty()) - Type = Type & ~1U; + bool SubobjectOnly = (Type & 1) != 0 && hasDesignator(Base); // If Type & 1 is 0, we need to be able to statically guarantee that the bytes // exist. If we can't verify the base, then we can't do that. @@ -6313,17 +6408,17 @@ // int b = __builtin_object_size(p->buff + 4, 2); // returns 0, not 40 // // This matches GCC's behavior. - if ((Type & 1) == 0 && Base.InvalidBase) + if (Base.InvalidBase && !SubobjectOnly) return Error(E); - // If Type & 1 is 0, the object in question is the complete object; reset to - // a complete object designator in that case. + // If we're not examining only the subobject, then we reset to a complete + // object designator // // If Type is 1 and we've lost track of the subobject, just find the complete // object instead. (If Type is 3, that's not correct behavior and we should // return 0 instead.) LValue End = Base; - if (((Type & 1) == 0) || (End.Designator.Invalid && Type == 1)) { + if (!SubobjectOnly || (End.Designator.Invalid && Type == 1)) { QualType T = getObjectType(End.getLValueBase()); if (T.isNull()) End.Designator.setInvalid(); @@ -6343,7 +6438,7 @@ // denoted by the pointer. But that's not quite right -- what we actually // want is the size of the immediately-enclosing array, if there is one. int64_t AmountToAdd = 1; - if (End.Designator.MostDerivedArraySize && + if (End.Designator.MostDerivedIsArrayElement && End.Designator.Entries.size() == End.Designator.MostDerivedPathLength) { // We got a pointer to an array. Step to its end. AmountToAdd = End.Designator.MostDerivedArraySize - @@ -6363,6 +6458,24 @@ return false; auto EndOffset = End.getLValueOffset(); + + // The following is a moderately common idiom in C: + // + // struct Foo { int a; char c[1]; }; + // struct Foo *F = (struct Foo *)malloc(sizeof(struct Foo) + strlen(Bar)); + // strcpy(&F->c[0], Bar); + // + // So, if we see that we're examining a 1-length (or 0-length) array at the + // end of a struct with an unknown base, we give up instead of breaking code + // that behaves this way. Note that we only do this when Type=1, because + // Type=3 is a lower bound, so answering conservatively is fine. + if (End.InvalidBase && SubobjectOnly && Type == 1 && + End.Designator.Entries.size() == End.Designator.MostDerivedPathLength && + End.Designator.MostDerivedIsArrayElement && + End.Designator.MostDerivedArraySize < 2 && + isDesignatorAtObjectEnd(Info.Ctx, End)) + return false; + if (BaseOffset > EndOffset) return Success(0, E); Index: test/CodeGen/object-size.c =================================================================== --- test/CodeGen/object-size.c +++ test/CodeGen/object-size.c @@ -127,7 +127,7 @@ strcpy(gp += 1, "Hi there"); } -// CHECK: @test17 +// CHECK-LABEL: @test17 void test17() { // CHECK: store i32 -1 gi = __builtin_object_size(gp++, 0); @@ -139,7 +139,7 @@ gi = __builtin_object_size(gp++, 3); } -// CHECK: @test18 +// CHECK-LABEL: @test18 unsigned test18(int cond) { int a[4], b[4]; // CHECK: phi i32* @@ -147,7 +147,7 @@ return __builtin_object_size(cond ? a : b, 0); } -// CHECK: @test19 +// CHECK-LABEL: @test19 void test19() { struct { int a, b; @@ -172,7 +172,7 @@ gi = __builtin_object_size(&foo.b, 3); } -// CHECK: @test20 +// CHECK-LABEL: @test20 void test20() { struct { int t[10]; } t[10]; @@ -186,7 +186,7 @@ gi = __builtin_object_size(&t[0].t[5], 3); } -// CHECK: @test21 +// CHECK-LABEL: @test21 void test21() { struct { int t; } t; @@ -209,7 +209,7 @@ gi = __builtin_object_size(&t.t + 1, 3); } -// CHECK: @test22 +// CHECK-LABEL: @test22 void test22() { struct { int t[10]; } t[10]; @@ -252,7 +252,7 @@ struct Test23Ty { int a; int t[10]; }; -// CHECK: @test23 +// CHECK-LABEL: @test23 void test23(struct Test23Ty *p) { // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) gi = __builtin_object_size(p, 0); @@ -285,7 +285,7 @@ } // PR24493 -- ICE if __builtin_object_size called with NULL and (Type & 1) != 0 -// CHECK: @test24 +// CHECK-LABEL: @test24 void test24() { // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 false) gi = __builtin_object_size((void*)0, 0); @@ -299,7 +299,7 @@ gi = __builtin_object_size((void*)0, 3); } -// CHECK: @test25 +// CHECK-LABEL: @test25 void test25() { // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* {{.*}}, i1 false) gi = __builtin_object_size((void*)0x1000, 0); @@ -324,7 +324,7 @@ gi = __builtin_object_size((void*)0 + 0x1000, 3); } -// CHECK: @test26 +// CHECK-LABEL: @test26 void test26() { struct { int v[10]; } t[10]; @@ -340,7 +340,7 @@ struct Test27IncompleteTy; -// CHECK: @test27 +// CHECK-LABEL: @test27 void test27(struct Test27IncompleteTy *t) { // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) gi = __builtin_object_size(t, 0); @@ -367,7 +367,7 @@ // The intent of this test is to ensure that __builtin_object_size treats `&foo` // and `(T*)&foo` identically, when used as the pointer argument. -// CHECK: @test28 +// CHECK-LABEL: @test28 void test28() { struct { int v[10]; } t[10]; @@ -391,3 +391,129 @@ gi = __builtin_object_size(addCasts(&t[1].v[1]), 3); #undef addCasts } + +struct DynStructVar { + char fst[16]; + char snd[]; +}; + +struct DynStruct0 { + char fst[16]; + char snd[0]; +}; + +struct DynStruct1 { + char fst[16]; + char snd[1]; +}; + +struct StaticStruct { + char fst[16]; + char snd[2]; +}; + +// CHECK-LABEL: @test29 +void test29(struct DynStructVar *dv, struct DynStruct0 *d0, + struct DynStruct1 *d1, struct StaticStruct *ss) { + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(dv->snd, 0); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(dv->snd, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(dv->snd, 2); + // CHECK: store i32 0 + gi = __builtin_object_size(dv->snd, 3); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(d0->snd, 0); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(d0->snd, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(d0->snd, 2); + // CHECK: store i32 0 + gi = __builtin_object_size(d0->snd, 3); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(d1->snd, 0); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(d1->snd, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(d1->snd, 2); + // CHECK: store i32 1 + gi = __builtin_object_size(d1->snd, 3); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(ss->snd, 0); + // CHECK: store i32 2 + gi = __builtin_object_size(ss->snd, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(ss->snd, 2); + // CHECK: store i32 2 + gi = __builtin_object_size(ss->snd, 3); +} + +// CHECK-LABEL: @test30 +void test30() { + struct { struct DynStruct1 fst, snd; } *nested; + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(nested->fst.snd, 0); + // CHECK: store i32 1 + gi = __builtin_object_size(nested->fst.snd, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(nested->fst.snd, 2); + // CHECK: store i32 1 + gi = __builtin_object_size(nested->fst.snd, 3); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(nested->snd.snd, 0); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(nested->snd.snd, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(nested->snd.snd, 2); + // CHECK: store i32 1 + gi = __builtin_object_size(nested->snd.snd, 3); + + union { struct DynStruct1 d1; char c[1]; } *u; + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(u->c, 0); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(u->c, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(u->c, 2); + // CHECK: store i32 1 + gi = __builtin_object_size(u->c, 3); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(u->d1.snd, 0); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(u->d1.snd, 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(u->d1.snd, 2); + // CHECK: store i32 1 + gi = __builtin_object_size(u->d1.snd, 3); +} + +// CHECK-LABEL: @test31 +void test31() { + // Miscellaneous 'writing off the end' detection tests + struct DynStructVar *dsv; + struct DynStruct0 *ds0; + struct DynStruct1 *ds1; + struct StaticStruct *ss; + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(ds1[9].snd, 1); + + // CHECH: store i32 2 + gi = __builtin_object_size(&ss[9].snd[0], 1); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(&ds1[9].snd[0], 1); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(&ds0[9].snd[0], 1); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(&dsv[9].snd[0], 1); +} Index: test/CodeGen/object-size.cpp =================================================================== --- test/CodeGen/object-size.cpp +++ test/CodeGen/object-size.cpp @@ -28,3 +28,37 @@ // CHECK: store i32 4 gi = __builtin_object_size((char*)(B*)&c, 0); } + +// CHECK-LABEL: define void @_Z5test2v() +void test2() { + struct A { char buf[16]; }; + struct B : A {}; + struct C { int i; B bs[1]; } *c; + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(&c->bs[0], 0); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(&c->bs[0], 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(&c->bs[0], 2); + // CHECK: store i32 16 + gi = __builtin_object_size(&c->bs[0], 3); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size((A*)&c->bs[0], 0); + // CHECK: store i32 16 + gi = __builtin_object_size((A*)&c->bs[0], 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size((A*)&c->bs[0], 2); + // CHECK: store i32 16 + gi = __builtin_object_size((A*)&c->bs[0], 3); + + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 false) + gi = __builtin_object_size(&c->bs[0].buf[0], 0); + // CHECK: store i32 16 + gi = __builtin_object_size(&c->bs[0].buf[0], 1); + // CHECK: call i64 @llvm.objectsize.i64.p0i8(i8* %{{.*}}, i1 true) + gi = __builtin_object_size(&c->bs[0].buf[0], 2); + // CHECK: store i32 16 + gi = __builtin_object_size(&c->bs[0].buf[0], 3); +}