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

[Douglas Gregor]

On Sep 26, 2011, at 2:47 AM, Stepan Dyatkovskiy wrote:

> Hello,
> OK. That was fixed in attached patch.

Committed as r140528, thanks!

	- Doug

> 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
>> 
> 
> <bugfix10925.patch>