<div dir="ltr"><div>+Richard, Hans</div><div><br></div><div>This patch fixes a crash that's also present in Clang 3.8. So, I think it should find its way into 3.8.2, if possible.</div><div><br></div><div>Thank you! :)</div><br><div class="gmail_quote">---------- Forwarded message ----------<br>From: <b class="gmail_sendername">George Burgess IV via cfe-commits</b> <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span><br>Date: Mon, Jun 27, 2016 at 12:40 PM<br>Subject: r273911 - [ExprConstant] Fix PR28314 - crash while evluating objectsize.<br>To: <a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br><br><br>Author: gbiv<br>
Date: Mon Jun 27 14:40:41 2016<br>
New Revision: 273911<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=273911&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=273911&view=rev</a><br>
Log:<br>
[ExprConstant] Fix PR28314 - crash while evluating objectsize.<br>
<br>
This fixes a crash in code like:<br>
```<br>
struct A {<br>
struct B b;<br>
char c[1];<br>
}<br>
<br>
int foo(struct A* a) { return __builtin_object_size(a->c, 0); }<br>
```<br>
<br>
We wouldn't check whether the structs we were examining were invalid,<br>
and getting the layout of an invalid struct is (unsurprisingly) A Bad<br>
Thing. With this patch, we'll always return conservatively if we see an<br>
invalid struct, since I'm assuming the presence of an invalid struct<br>
means that our compilation failed (so having a conservative result isn't<br>
such a big deal).<br>
<br>
Modified:<br>
cfe/trunk/lib/AST/ExprConstant.cpp<br>
cfe/trunk/test/Sema/builtin-object-size.c<br>
<br>
Modified: cfe/trunk/lib/AST/ExprConstant.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=273911&r1=273910&r2=273911&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=273911&r1=273910&r2=273911&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)<br>
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Jun 27 14:40:41 2016<br>
@@ -6540,25 +6540,32 @@ static const Expr *ignorePointerCastsAnd<br>
///<br>
/// Please note: this function is specialized for how __builtin_object_size<br>
/// views "objects".<br>
+///<br>
+/// If this encounters an invalid RecordDecl, it will always return true.<br>
static bool isDesignatorAtObjectEnd(const ASTContext &Ctx, const LValue &LVal) {<br>
assert(!LVal.Designator.Invalid);<br>
<br>
- auto IsLastFieldDecl = [&Ctx](const FieldDecl *FD) {<br>
- if (FD->getParent()->isUnion())<br>
+ auto IsLastOrInvalidFieldDecl = [&Ctx](const FieldDecl *FD, bool &Invalid) {<br>
+ const RecordDecl *Parent = FD->getParent();<br>
+ Invalid = Parent->isInvalidDecl();<br>
+ if (Invalid || Parent->isUnion())<br>
return true;<br>
- const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(FD->getParent());<br>
+ const ASTRecordLayout &Layout = Ctx.getASTRecordLayout(Parent);<br>
return FD->getFieldIndex() + 1 == Layout.getFieldCount();<br>
};<br>
<br>
auto &Base = LVal.getLValueBase();<br>
if (auto *ME = dyn_cast_or_null<MemberExpr>(Base.dyn_cast<const Expr *>())) {<br>
if (auto *FD = dyn_cast<FieldDecl>(ME->getMemberDecl())) {<br>
- if (!IsLastFieldDecl(FD))<br>
- return false;<br>
+ bool Invalid;<br>
+ if (!IsLastOrInvalidFieldDecl(FD, Invalid))<br>
+ return Invalid;<br>
} else if (auto *IFD = dyn_cast<IndirectFieldDecl>(ME->getMemberDecl())) {<br>
- for (auto *FD : IFD->chain())<br>
- if (!IsLastFieldDecl(cast<FieldDecl>(FD)))<br>
- return false;<br>
+ for (auto *FD : IFD->chain()) {<br>
+ bool Invalid;<br>
+ if (!IsLastOrInvalidFieldDecl(cast<FieldDecl>(FD), Invalid))<br>
+ return Invalid;<br>
+ }<br>
}<br>
}<br>
<br>
@@ -6581,8 +6588,9 @@ static bool isDesignatorAtObjectEnd(cons<br>
return false;<br>
BaseType = CT->getElementType();<br>
} else if (auto *FD = getAsField(LVal.Designator.Entries[I])) {<br>
- if (!IsLastFieldDecl(FD))<br>
- return false;<br>
+ bool Invalid;<br>
+ if (!IsLastOrInvalidFieldDecl(FD, Invalid))<br>
+ return Invalid;<br>
BaseType = FD->getType();<br>
} else {<br>
assert(getAsBaseClass(LVal.Designator.Entries[I]) != nullptr &&<br>
<br>
Modified: cfe/trunk/test/Sema/builtin-object-size.c<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtin-object-size.c?rev=273911&r1=273910&r2=273911&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/builtin-object-size.c?rev=273911&r1=273910&r2=273911&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/test/Sema/builtin-object-size.c (original)<br>
+++ cfe/trunk/test/Sema/builtin-object-size.c Mon Jun 27 14:40:41 2016<br>
@@ -52,3 +52,27 @@ void f6(void)<br>
__builtin___memccpy_chk (buf, b, '\0', sizeof(b), __builtin_object_size (buf, 0));<br>
__builtin___memccpy_chk (b, buf, '\0', sizeof(buf), __builtin_object_size (b, 0)); // expected-warning {{'__builtin___memccpy_chk' will always overflow destination buffer}}<br>
}<br>
+<br>
+int pr28314(void) {<br>
+ struct {<br>
+ struct InvalidField a; // expected-error{{has incomplete type}} expected-note 3{{forward declaration of 'struct InvalidField'}}<br>
+ char b[0];<br>
+ } *p;<br>
+<br>
+ struct {<br>
+ struct InvalidField a; // expected-error{{has incomplete type}}<br>
+ char b[1];<br>
+ } *p2;<br>
+<br>
+ struct {<br>
+ struct InvalidField a; // expected-error{{has incomplete type}}<br>
+ char b[2];<br>
+ } *p3;<br>
+<br>
+ int a = 0;<br>
+ a += __builtin_object_size(&p->a, 0);<br>
+ a += __builtin_object_size(p->b, 0);<br>
+ a += __builtin_object_size(p2->b, 0);<br>
+ a += __builtin_object_size(p3->b, 0);<br>
+ return a;<br>
+}<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</div><br></div>