[clang-format] Don't crash on malformed preprocessor conditions

[sstwcw]

Summary

Previously the program would crash on this input:

#else
#if X

The problem was that in parsePPElse, PPBranchLevel would be
incremented when the first #else gets parsed, but PPLevelBranchCount
and PPLevelBranchIndex would not be updated together.

I found the problem when working on D135740.

[owenpan]

clang/lib/Format/UnwrappedLineParser.cpp

1216

Nit.

clang/unittests/Format/FormatTest.cpp

5196–5206

Can we have individual verifyFormat or verifyNoCrashtests for easy reading and to avoid the overhead? Some examples:

#else
a;
#if X
b;
#endif
#endif

#elif X
a;
#endif
#ifdef X
b;
#endif

[sstwcw]

clang/unittests/Format/FormatTest.cpp

5196–5206

I added some tests like the examples. But I kept the generated cases. I feel more secure that way. At first I only found the bug because of the generated cases. As for overhead, the debug build of this test took 1.6s on my laptop.

[HazardyKnusperkeks]

clang/lib/Format/UnwrappedLineParser.cpp

1217

You assert >= -1, that means this has to be == -1.

clang/unittests/Format/FormatTest.cpp

5196–5206

I added some tests like the examples. But I kept the generated cases. I feel more secure that way. At first I only found the bug because of the generated cases. As for overhead, the debug build of this test took 1.6s on my laptop.

Couldn't you just take the previously crashing ones, you find important, as test cases?
The overhead is not only compile and runtime, it's also while trying to understand what's happening here.

[owenpan]

clang/lib/Format/UnwrappedLineParser.cpp

1217

Yep, as I suggested above.

[sstwcw]

clang/unittests/Format/FormatTest.cpp

5196–5206

The overhead is not only compile and runtime, it's also while trying to understand what's happening here.

I see.

[owenpan]

clang/unittests/Format/FormatTest.cpp

5209

#elif Y instead of #else?

clang/lib/Format/UnwrappedLineParser.cpp

if (IncludeGuard == IG_Inited && MaybeIncludeGuard) {
     IncludeGuardToken = IfCondition;
   }
 }
 
 void UnwrappedLineParser::parsePPElse() {
   // If a potential include guard has an #else, it's not an include guard.
   if (IncludeGuard == IG_Defined && PPBranchLevel == 0)
     IncludeGuard = IG_Rejected;
+  // Don't crash when there is an #else without an #if.
+  assert(PPBranchLevel >= -1);
+  if (PPBranchLevel == -1)
+    conditionalCompilationStart(/*Unreachable=*/true);
   conditionalCompilationAlternative();
-  if (PPBranchLevel > -1)
-    --PPBranchLevel;
   parsePPUnknown();
   ++PPBranchLevel;
 }
 
 void UnwrappedLineParser::parsePPElIf() { parsePPElse(); }
 
 void UnwrappedLineParser::parsePPEndIf() {
   conditionalCompilationEnd();

clang/unittests/Format/FormatTest.cpp

verifyFormat("#define A     \\\n"
                "#define B     \\\n"
                "  }           \\\n"
                "  }",
                getLLVMStyleWithColumns(15));
   verifyNoCrash("#if a\na(\n#else\n#endif\n{a");
   verifyNoCrash("a={0,1\n#if a\n#else\n;\n#endif\n}");
   verifyNoCrash("#if a\na(\n#else\n#endif\n) a {a,b,c,d,f,g};");
   verifyNoCrash("#ifdef A\n a(\n #else\n #endif\n) = []() {      \n)}");
+  verifyNoCrash("#else\n"
+                "#else\n"
+                "#endif\n"
+                "#endif");
+  verifyNoCrash("#else\n"
+                "#if X\n"
+                "#endif\n"
+                "#endif");
+  verifyNoCrash("#else\n"
+                "#endif\n"
+                "#if X\n"
+                "#endif");
+  verifyNoCrash("#if X\n"
+                "#else\n"
+                "#else\n"
+                "#endif\n"
+                "#endif");
+  verifyNoCrash("#endif\n"
+                "#endif");
+  verifyNoCrash("#endif\n"
+                "#else");
 }
 
 TEST_F(FormatTest, MacrosWithoutTrailingSemicolon) {
   verifyFormat("SOME_TYPE_NAME abc;"); // Gated on the newline.
   EXPECT_EQ("class A : public QObject {\n"
             "  Q_OBJECT\n"
             "\n"
             "  A() {}\n"