[cfe-commits] [clang] bugfux #10925 Segmentation fault on valid code

[Stepan Dyatkovskiy]

Hello,
OK. That was fixed in attached patch.

Douglas Gregor wrote:
> Hello Stepan,
>
> On Sep 21, 2011, at 1:09 AM, Stepan Dyatkovskiy wrote:
>
>> Hi,
>> Please find the fixed patch.
>
> Index: test/Parser/2011-09-20-UsingTypenameIdentifiers.cpp
> ===================================================================
> --- test/Parser/2011-09-20-UsingTypenameIdentifiers.cpp(revision 0)
> +++ test/Parser/2011-09-20-UsingTypenameIdentifiers.cpp(revision 0)
> @@ -0,0 +1,16 @@
>
> Please fold this test into an existing test for 'using typename'.
>
> Index: lib/Parse/ParseDeclCXX.cpp
> ===================================================================
> --- lib/Parse/ParseDeclCXX.cpp(revision 140239)
> +++ lib/Parse/ParseDeclCXX.cpp(working copy)
> @@ -433,12 +433,14 @@
>                                      Decl **OwnedType) {
>    CXXScopeSpec SS;
>    SourceLocation TypenameLoc;
> +  SourceLocation TypenameEndLoc;
>    bool IsTypeName;
>    // Ignore optional 'typename'.
>    // FIXME: This is wrong; we should parse this as a typename-specifier.
>    if (Tok.is <http://Tok.is>(tok::kw_typename)) {
>      TypenameLoc = Tok.getLocation();
> +    TypenameEndLoc = TypenameLoc.getLocWithOffset(Tok.getLength());
>      ConsumeToken();
>      IsTypeName = true;
>    }
> @@ -545,6 +547,15 @@
>      return 0;
>    }
> +  // "typename" keyword is allowed for identifiers only,
> +  // because it may be a type definition.
> +  if (IsTypeName && Name.getKind() != UnqualifiedId::IK_Identifier) {
> +    Diag(Name.getSourceRange().getBegin(), 
> diag::err_typename_identifiers_only)
> + << FixItHint::CreateRemoval(SourceRange(TypenameLoc, TypenameEndLoc));
> +    // Proceed parsing, but reset the IsTypeName flag.
> +    IsTypeName = false;
> +  }
>
> The fix & recovery look good, but you don't actually need 
> TypenameEndLoc, since the end of a SourceRange points to the beginning 
> of the last token in the source range. There's a short explanation here:
>
> http://clang.llvm.org/docs/InternalsManual.html#SourceRange
>
> - Doug
>
>> Regards,
>> Stepan
>> 20.09.2011, 22:59, "David Blaikie" <dblaikie at gmail.com 
>> <mailto:dblaikie at gmail.com>>:
>>
>>         C++ allows "typename" keywords with identifiers only. In this
>>         case compiler interpret identifier as type definition. Clang
>>         doesn't check for this case and crash.
>>         Please find the patch attached for review.
>>
>>     Could you simplify the test case from the bug & include it in the
>>     change? (& the diagnostic message should probably use the word
>>     "typename" not "type name" (since it refers to an identifier).
>>     You could also, potentially, include a removal fixit to remove
>>     the typename token if you think that might be the likely fix
>>     (seems to be in this case))
>>     - David
>>
>>     _______________________________________________
>>     cfe-commits mailing list
>>     cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>>     http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>> <bugfix10925.patch>_______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: bugfix10925.patch
Type: text/x-patch
Size: 2179 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110926/09b13e65/attachment.bin>