From c99e001486959e6b7e3a49475bd64348504e0384 Mon Sep 17 00:00:00 2001
From: Manuel Klimek <klimek@google.com>
Date: Tue, 18 Mar 2014 11:22:45 +0000
Subject: [PATCH] Fix crasher bug.

Due to not resetting the fake rparen data on the token when iterating
over annotated lines, we would pop the last element of the paren stack.

This patch fixes the underlying root cause, and makes the code more
robust against similar problems in the future:
- reset the first token when iterating on the same annotated lines due
  to preprocessor branches
- never pop the last element from the paren stack, so we do not crash,
  but rather incorrectly format
- add assert()s so we can figure out if our assumptions are violated

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@204140 91177308-0d34-0410-b5e6-96231b3b80d8
---
 lib/Format/ContinuationIndenter.cpp | 17 +++++++++++++---
 lib/Format/TokenAnnotator.cpp       | 31 +++++++++++++++++------------
 unittests/Format/FormatTest.cpp     | 14 +++++++++++++
 3 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/lib/Format/ContinuationIndenter.cpp b/lib/Format/ContinuationIndenter.cpp
index 380532a47c..cb4e4f53b2 100644
--- a/lib/Format/ContinuationIndenter.cpp
+++ b/lib/Format/ContinuationIndenter.cpp
@@ -217,8 +217,8 @@ unsigned ContinuationIndenter::addTokenToState(LineState &State, bool Newline,
                                                unsigned ExtraSpaces) {
   const FormatToken &Current = *State.NextToken;
 
-  if (State.Stack.size() == 0 ||
-      (Current.Type == TT_ImplicitStringLiteral &&
+  assert(!State.Stack.empty());
+  if ((Current.Type == TT_ImplicitStringLiteral &&
        (Current.Previous->Tok.getIdentifierInfo() == NULL ||
         Current.Previous->Tok.getIdentifierInfo()->getPPKeywordID() ==
             tok::pp_not_keyword))) {
@@ -628,8 +628,14 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
         //   SomeFunction(a, [] {
         //                     f();  // break
         //                   });
-        for (unsigned i = 0; i != Current.MatchingParen->FakeRParens; ++i)
+        for (unsigned i = 0; i != Current.MatchingParen->FakeRParens; ++i) {
+          assert(State.Stack.size() > 1);
+          if (State.Stack.size() == 1) {
+            // Do not pop the last element.
+            break;
+          }
           State.Stack.pop_back();
+        }
         bool IsObjCBlock =
             Previous &&
             (Previous->is(tok::caret) ||
@@ -709,6 +715,11 @@ unsigned ContinuationIndenter::moveStateToNextToken(LineState &State,
     // as they will have been removed early (see above).
     for (unsigned i = 0, e = Current.FakeRParens; i != e; ++i) {
       unsigned VariablePos = State.Stack.back().VariablePos;
+      assert(State.Stack.size() > 1);
+      if (State.Stack.size() == 1) {
+        // Do not pop the last element.
+        break;
+      }
       State.Stack.pop_back();
       State.Stack.back().VariablePos = VariablePos;
     }
diff --git a/lib/Format/TokenAnnotator.cpp b/lib/Format/TokenAnnotator.cpp
index 34aef99a7b..a91c847452 100644
--- a/lib/Format/TokenAnnotator.cpp
+++ b/lib/Format/TokenAnnotator.cpp
@@ -34,6 +34,7 @@ public:
       : Style(Style), Line(Line), CurrentToken(Line.First),
         KeywordVirtualFound(false), AutoFound(false), Ident_in(Ident_in) {
     Contexts.push_back(Context(tok::unknown, 1, /*IsExpression=*/false));
+    resetTokenMetadata(CurrentToken);
   }
 
 private:
@@ -571,6 +572,22 @@ public:
   }
 
 private:
+  void resetTokenMetadata(FormatToken *Token) {
+    if (Token == nullptr) return;
+
+    // Reset token type in case we have already looked at it and then
+    // recovered from an error (e.g. failure to find the matching >).
+    if (CurrentToken->Type != TT_LambdaLSquare &&
+        CurrentToken->Type != TT_FunctionLBrace &&
+        CurrentToken->Type != TT_ImplicitStringLiteral &&
+        CurrentToken->Type != TT_TrailingReturnArrow)
+      CurrentToken->Type = TT_Unknown;
+    if (CurrentToken->Role)
+      CurrentToken->Role.reset(NULL);
+    CurrentToken->FakeLParens.clear();
+    CurrentToken->FakeRParens = 0;
+  }
+
   void next() {
     if (CurrentToken != NULL) {
       determineTokenType(*CurrentToken);
@@ -581,19 +598,7 @@ private:
     if (CurrentToken != NULL)
       CurrentToken = CurrentToken->Next;
 
-    if (CurrentToken != NULL) {
-      // Reset token type in case we have already looked at it and then
-      // recovered from an error (e.g. failure to find the matching >).
-      if (CurrentToken->Type != TT_LambdaLSquare &&
-          CurrentToken->Type != TT_FunctionLBrace &&
-          CurrentToken->Type != TT_ImplicitStringLiteral &&
-          CurrentToken->Type != TT_TrailingReturnArrow)
-        CurrentToken->Type = TT_Unknown;
-      if (CurrentToken->Role)
-        CurrentToken->Role.reset(NULL);
-      CurrentToken->FakeLParens.clear();
-      CurrentToken->FakeRParens = 0;
-    }
+    resetTokenMetadata(CurrentToken);
   }
 
   /// \brief A struct to hold information valid in a specific context, e.g.
diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp
index 5bd0822281..b0324015cd 100644
--- a/unittests/Format/FormatTest.cpp
+++ b/unittests/Format/FormatTest.cpp
@@ -8173,5 +8173,19 @@ TEST_F(FormatTest, SpacesInAngles) {
   verifyFormat("A<A<int>>();", Spaces);
 }
 
+TEST_F(FormatTest, HandleUnbalancedImplicitBracesAcrossPPBranches) {
+  std::string code = "#if A\n"
+                     "#if B\n"
+                     "a.\n"
+                     "#endif\n"
+                     "    a = 1;\n"
+                     "#else\n"
+                     "#endif\n"
+                     "#if C\n"
+                     "#else\n"
+                     "#endif\n";
+  EXPECT_EQ(code, format(code));
+}
+
 } // end namespace tooling
 } // end namespace clang
-- 
2.40.0