r208702 - No longer triggering a checked iterator assert on Windows when using std::copy while deserializing attributed statements with more than one attribute.

[David Blaikie]

On Tue, May 13, 2014 at 7:55 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
> Author: aaronballman
> Date: Tue May 13 09:55:01 2014
> New Revision: 208702
>
> URL: http://llvm.org/viewvc/llvm-project?rev=208702&view=rev
> Log:
> No longer triggering a checked iterator assert on Windows when using std::copy while deserializing attributed statements with more than one attribute.

I have to say I don't quite follow where the checked failure would be
in the original code. Could you explain it?

>
> Added:
>     cfe/trunk/test/PCH/stmt-attrs.cpp
> Modified:
>     cfe/trunk/include/clang/AST/Stmt.h
>     cfe/trunk/lib/AST/Stmt.cpp
>     cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
>
> Modified: cfe/trunk/include/clang/AST/Stmt.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=208702&r1=208701&r2=208702&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/AST/Stmt.h (original)
> +++ cfe/trunk/include/clang/AST/Stmt.h Tue May 13 09:55:01 2014
> @@ -819,21 +819,25 @@ class AttributedStmt : public Stmt {
>    Stmt *SubStmt;
>    SourceLocation AttrLoc;
>    unsigned NumAttrs;
> -  const Attr *Attrs[1];
>
>    friend class ASTStmtReader;
>
>    AttributedStmt(SourceLocation Loc, ArrayRef<const Attr*> Attrs, Stmt *SubStmt)
>      : Stmt(AttributedStmtClass), SubStmt(SubStmt), AttrLoc(Loc),
>        NumAttrs(Attrs.size()) {
> -    memcpy(this->Attrs, Attrs.data(), Attrs.size() * sizeof(Attr*));
> +    memcpy(getAttrArrayPtr(), Attrs.data(), Attrs.size() * sizeof(Attr *));
>    }
>
>    explicit AttributedStmt(EmptyShell Empty, unsigned NumAttrs)
>      : Stmt(AttributedStmtClass, Empty), NumAttrs(NumAttrs) {
> -    memset(Attrs, 0, NumAttrs * sizeof(Attr*));
> +    memset(getAttrArrayPtr(), 0, NumAttrs * sizeof(Attr *));
>    }
>
> +  Attr *const *getAttrArrayPtr() const {
> +    return reinterpret_cast<Attr *const *>(this + 1);
> +  }
> +  Attr **getAttrArrayPtr() { return reinterpret_cast<Attr **>(this + 1); }
> +
>  public:
>    static AttributedStmt *Create(const ASTContext &C, SourceLocation Loc,
>                                  ArrayRef<const Attr*> Attrs, Stmt *SubStmt);
> @@ -842,7 +846,7 @@ public:
>
>    SourceLocation getAttrLoc() const { return AttrLoc; }
>    ArrayRef<const Attr*> getAttrs() const {
> -    return ArrayRef<const Attr*>(Attrs, NumAttrs);
> +    return ArrayRef<const Attr*>(getAttrArrayPtr(), NumAttrs);
>    }
>    Stmt *getSubStmt() { return SubStmt; }
>    const Stmt *getSubStmt() const { return SubStmt; }
>
> Modified: cfe/trunk/lib/AST/Stmt.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Stmt.cpp?rev=208702&r1=208701&r2=208702&view=diff
> ==============================================================================
> --- cfe/trunk/lib/AST/Stmt.cpp (original)
> +++ cfe/trunk/lib/AST/Stmt.cpp Tue May 13 09:55:01 2014
> @@ -285,8 +285,8 @@ const char *LabelStmt::getName() const {
>  AttributedStmt *AttributedStmt::Create(const ASTContext &C, SourceLocation Loc,
>                                         ArrayRef<const Attr*> Attrs,
>                                         Stmt *SubStmt) {
> -  void *Mem = C.Allocate(sizeof(AttributedStmt) +
> -                         sizeof(Attr*) * (Attrs.size() - 1),
> +  assert(!Attrs.empty() && "Attrs should not be empty");
> +  void *Mem = C.Allocate(sizeof(AttributedStmt) + sizeof(Attr *) * Attrs.size(),
>                           llvm::alignOf<AttributedStmt>());
>    return new (Mem) AttributedStmt(Loc, Attrs, SubStmt);
>  }
> @@ -294,8 +294,7 @@ AttributedStmt *AttributedStmt::Create(c
>  AttributedStmt *AttributedStmt::CreateEmpty(const ASTContext &C,
>                                              unsigned NumAttrs) {
>    assert(NumAttrs > 0 && "NumAttrs should be greater than zero");
> -  void *Mem = C.Allocate(sizeof(AttributedStmt) +
> -                         sizeof(Attr*) * (NumAttrs - 1),
> +  void *Mem = C.Allocate(sizeof(AttributedStmt) + sizeof(Attr *) * NumAttrs,
>                           llvm::alignOf<AttributedStmt>());
>    return new (Mem) AttributedStmt(EmptyShell(), NumAttrs);
>  }
>
> Modified: cfe/trunk/lib/Serialization/ASTReaderStmt.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReaderStmt.cpp?rev=208702&r1=208701&r2=208702&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ASTReaderStmt.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReaderStmt.cpp Tue May 13 09:55:01 2014
> @@ -176,7 +176,7 @@ void ASTStmtReader::VisitAttributedStmt(
>    (void)NumAttrs;
>    assert(NumAttrs == S->NumAttrs);
>    assert(NumAttrs == Attrs.size());
> -  std::copy(Attrs.begin(), Attrs.end(), S->Attrs);
> +  std::copy(Attrs.begin(), Attrs.end(), S->getAttrArrayPtr());
>    S->SubStmt = Reader.ReadSubStmt();
>    S->AttrLoc = ReadSourceLocation(Record, Idx);
>  }
>
> Added: cfe/trunk/test/PCH/stmt-attrs.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/stmt-attrs.cpp?rev=208702&view=auto
> ==============================================================================
> --- cfe/trunk/test/PCH/stmt-attrs.cpp (added)
> +++ cfe/trunk/test/PCH/stmt-attrs.cpp Tue May 13 09:55:01 2014
> @@ -0,0 +1,23 @@
> +// RUN: %clang_cc1 -std=c++11 -emit-pch -o %t.a %s
> +// RUN: %clang_cc1 -std=c++11 -include-pch %t.a %s -ast-print -o - | FileCheck %s
> +
> +#ifndef HEADER
> +#define HEADER
> +
> +inline void test(int i) {
> +  switch (i) {
> +    case 1:
> +      // Notice that the NullStmt has two attributes.
> +      [[clang::fallthrough]][[clang::fallthrough]];
> +    case 2:
> +      break;
> +  }
> +}
> +
> +#else
> +
> +void foo(void) {
> +  test(1);
> +}
> +
> +#endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits