r273911 - [ExprConstant] Fix PR28314 - crash while evluating objectsize.

[Hans Wennborg via cfe-commits]

+Tom who does the "dot" releases.

I don't know if he's planning for a 3.8.2, but just in case, I suppose
it's on the radar now :-)

Thanks,
Hans

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