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

[Douglas Gregor]

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(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>:
> 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
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
> 
> <bugfix10925.patch>_______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20110923/bafd6c2a/attachment.html>