r226306 - Don't crash if a declarator in a friend decl doesn't have a name.

[Richard Smith]

On Fri, Jan 16, 2015 at 11:34 AM, Nico Weber <nicolasweber at gmx.de> wrote:
> Author: nico
> Date: Fri Jan 16 13:34:13 2015
> New Revision: 226306
>
> URL: http://llvm.org/viewvc/llvm-project?rev=226306&view=rev
> Log:
> Don't crash if a declarator in a friend decl doesn't have a name.
>
> There was already an explicit check for that for the first decl.  Move that
> to a different place so that it's called for the following decls too.  Also
> don't randomly set the BitfieldSize ExprResult to true (this sets a pointer to
> true internally).
>
> Found by SLi's bot.
>
> Modified:
>     cfe/trunk/include/clang/Parse/Parser.h
>     cfe/trunk/lib/Parse/ParseDeclCXX.cpp
>     cfe/trunk/test/CXX/class/class.friend/p1.cpp
>
> Modified: cfe/trunk/include/clang/Parse/Parser.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Parse/Parser.h?rev=226306&r1=226305&r2=226306&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Parse/Parser.h (original)
> +++ cfe/trunk/include/clang/Parse/Parser.h Fri Jan 16 13:34:13 2015
> @@ -2294,7 +2294,7 @@ private:
>                                     Decl *TagDecl);
>    ExprResult ParseCXXMemberInitializer(Decl *D, bool IsFunction,
>                                         SourceLocation &EqualLoc);
> -  void ParseCXXMemberDeclaratorBeforeInitializer(Declarator &DeclaratorInfo,
> +  bool ParseCXXMemberDeclaratorBeforeInitializer(Declarator &DeclaratorInfo,
>                                                   VirtSpecifiers &VS,
>                                                   ExprResult &BitfieldSize,
>                                                   LateParsedAttrList &LateAttrs);
>
> Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=226306&r1=226305&r2=226306&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Fri Jan 16 13:34:13 2015
> @@ -2019,7 +2019,7 @@ bool Parser::isCXX11FinalKeyword() const
>
>  /// \brief Parse a C++ member-declarator up to, but not including, the optional
>  /// brace-or-equal-initializer or pure-specifier.
> -void Parser::ParseCXXMemberDeclaratorBeforeInitializer(
> +bool Parser::ParseCXXMemberDeclaratorBeforeInitializer(
>      Declarator &DeclaratorInfo, VirtSpecifiers &VS, ExprResult &BitfieldSize,
>      LateParsedAttrList &LateParsedAttrs) {
>    // member-declarator:
> @@ -2073,6 +2073,15 @@ void Parser::ParseCXXMemberDeclaratorBef
>        }
>      }
>    }
> +
> +  // If this has neither a name nor a bit width, something has gone seriously
> +  // wrong. Skip until the semi-colon or }.
> +  if (!DeclaratorInfo.hasName() && BitfieldSize.isUnset()) {
> +    // If so, skip until the semi-colon or a }.
> +    SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
> +    return true;
> +  }
> +  return false;
>  }
>
>  /// ParseCXXClassMemberDeclaration - Parse a C++ class member declaration.
> @@ -2298,14 +2307,8 @@ void Parser::ParseCXXClassMemberDeclarat
>    bool ExpectSemi = true;
>
>    // Parse the first declarator.
> -  ParseCXXMemberDeclaratorBeforeInitializer(DeclaratorInfo, VS, BitfieldSize,
> -                                            LateParsedAttrs);
> -
> -  // If this has neither a name nor a bit width, something has gone seriously
> -  // wrong. Skip until the semi-colon or }.
> -  if (!DeclaratorInfo.hasName() && BitfieldSize.isUnset()) {
> -    // If so, skip until the semi-colon or a }.
> -    SkipUntil(tok::r_brace, StopAtSemi | StopBeforeMatch);
> +  if (ParseCXXMemberDeclaratorBeforeInitializer(
> +          DeclaratorInfo, VS, BitfieldSize, LateParsedAttrs)) {
>      TryConsumeToken(tok::semi);
>      return;
>    }
> @@ -2530,7 +2533,7 @@ void Parser::ParseCXXClassMemberDeclarat
>      // Parse the next declarator.
>      DeclaratorInfo.clear();
>      VS.clear();
> -    BitfieldSize = true;

This was previously setting BitfieldSize to ExprError() (that is, null
and invalid) and now sets it to ExprResult() (that is, null but
valid). Was that your intention?

> +    BitfieldSize = nullptr;
>      Init = true;

If you don't like the implicit conversion from bool to ExprResult (and
I don't blame you...) please fix this one too for local consistency.

>      HasInitializer = false;
>      DeclaratorInfo.setCommaLoc(CommaLoc);
> @@ -2538,8 +2541,9 @@ void Parser::ParseCXXClassMemberDeclarat
>      // GNU attributes are allowed before the second and subsequent declarator.
>      MaybeParseGNUAttributes(DeclaratorInfo);
>
> -    ParseCXXMemberDeclaratorBeforeInitializer(DeclaratorInfo, VS, BitfieldSize,
> -                                              LateParsedAttrs);
> +    if (ParseCXXMemberDeclaratorBeforeInitializer(
> +            DeclaratorInfo, VS, BitfieldSize, LateParsedAttrs))
> +      break;
>    }
>
>    if (ExpectSemi &&
>
> Modified: cfe/trunk/test/CXX/class/class.friend/p1.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CXX/class/class.friend/p1.cpp?rev=226306&r1=226305&r2=226306&view=diff
> ==============================================================================
> --- cfe/trunk/test/CXX/class/class.friend/p1.cpp (original)
> +++ cfe/trunk/test/CXX/class/class.friend/p1.cpp Fri Jan 16 13:34:13 2015
> @@ -66,6 +66,10 @@ class A {
>    class facet;
>    friend class facet;  // should not assert
>    class facet {};
> +
> +  friend int Unknown::thing(); // expected-error {{use of undeclared identifier}}
> +  friend int friendfunc(), Unknown::thing(); // expected-error {{use of undeclared identifier}}
> +  friend int friendfunc(), Unknown::thing() : 4; // expected-error {{use of undeclared identifier}}
>  };
>
>  A::UndeclaredSoFar y; // expected-error {{no type named 'UndeclaredSoFar' in 'A'}}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits