george.burgess.iv created this revision.
george.burgess.iv added a reviewer: rsmith.
george.burgess.iv added subscribers: cfe-commits, mzolotukhin.
In C, a common idiom is:
```
struct Foo { int a; char cs[1] };
struct Foo *F = (struct Foo *)malloc(sizeof(Foo) + strlen(SomeString));
strcpy(F->cs, SomeString);
```
Currently, __builtin_object_size does not allow for this, which breaks some
existing code. This patch makes us answer conservatively in Clang if the
following conditions are met:
- Type is 1 or 3
- The Base is invalid/can't be determined
- The subobject we're referencing is the last subobject in the struct
- The subobject we're referencing is an array with 0 or 1 elements (for 0
elements, both `char foo[]` and `char foo[0]` syntaxes are supported)
http://reviews.llvm.org/D12821
Files:
lib/AST/ExprConstant.cpp
test/CodeGen/object-size.c
Index: test/CodeGen/object-size.c
===================================================================
--- test/CodeGen/object-size.c
+++ test/CodeGen/object-size.c
@@ -391,3 +391,63 @@
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: @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 0
+ 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);
+}
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<APValue::LValuePathEntry> 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<ComplexType>();
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.
+ 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<PathEntry> 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<FieldDecl>(D)) {
MostDerivedType = FD->getType();
+ MostDerivedIsArrayElement = false;
MostDerivedArraySize = 0;
MostDerivedPathLength = Entries.size();
}
@@ -255,14 +272,16 @@
// This is technically a most-derived object, though in practice this
// is unlikely to matter.
MostDerivedType = EltTy;
+ MostDerivedIsArrayElement = true;
MostDerivedArraySize = 2;
MostDerivedPathLength = Entries.size();
}
void diagnosePointerArithmetic(EvalInfo &Info, const Expr *E, uint64_t N);
/// 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);
@@ -495,7 +514,10 @@
EM_PotentialConstantExpressionUnevaluated,
/// Evaluate as a constant expression. Continue evaluating if we find a
- /// MemberExpr with a base that can't be evaluated.
+ /// MemberExpr with a base that can't be evaluated. In this case, the Base
+ /// of an invalid LValue will be the the MemberExpr that contained the
+ /// Base that we couldn't evaluate, and all Offsets will be from said
+ /// MemberExpr.
EM_DesignatorFold,
} EvalMode;
@@ -834,7 +856,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<int>(N) << /*array*/ 0
<< static_cast<unsigned>(MostDerivedArraySize);
@@ -2517,7 +2539,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 +4438,7 @@
if (!EvalOK) {
if (!this->Info.allowInvalidBaseExpr())
return false;
- Result.setInvalid(E->getBase());
+ Result.setInvalid(E);
}
const ValueDecl *MD = E->getMemberDecl();
@@ -4432,6 +4454,11 @@
} else
return this->Error(E);
+ // Because we set the Base to be the MemberExpr instead of E->getBase(), the
+ // Offset should be from the MemberExpr instead of the MemberExpr's base.
+ if (Result.InvalidBase)
+ Result.Offset = CharUnits::Zero();
+
if (MD->getType()->isReferenceType()) {
APValue RefValue;
if (!handleLValueToRValueConversion(this->Info, E, MD->getType(), Result,
@@ -6342,7 +6369,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 -
@@ -6362,6 +6389,31 @@
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.
+ if (End.InvalidBase && (Type & 1) != 0 &&
+ End.Designator.MostDerivedIsArrayElement &&
+ End.Designator.MostDerivedArraySize < 2) {
+ // EM_FoldDesignator requires that all invalid bases be MemberExprs
+ assert(isa<MemberExpr>(End.getLValueBase().get<const Expr*>()));
+ QualType BaseType = End.getLValueBase().get<const Expr*>()->getType();
+ CharUnits SizeOfBase;
+ if (!HandleSizeof(Info, E->getLocStart(), BaseType, SizeOfBase))
+ return false;
+
+ bool AtEndOfObject = SizeOfBase == EndOffset;
+ if (AtEndOfObject)
+ return false;
+ }
+
if (BaseOffset > EndOffset)
return Success(0, E);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits