Check for resource exhaustion when recursively parsing declarators

[aaron.ballman]

Summary

With sufficiently tortured code, it's possible to cause a stack overflow when parsing declarators. Thus, we now check for resource exhaustion when recursively parsing declarators so that we can at least warn the user we're about to crash before we actually crash.

This fixes https://github.com/llvm/llvm-project/issues/51642.

[arsenm]

I think I've seen this one before

[aaron.ballman]

Two things to note:

1. I'm not convinced that adding a test case is the right thing to do here. On the one hand, we want to verify that we now emit the diagnostic before crashing. But on the other hand, this test is extremely expensive to run (because it exhausts the stack) and its behavior is highly specific to the test platform its being run on. If someone asked me to remove the test coverage here, I would not be sad.
2. The runWithSufficientStackSpace() implementation is a copy of what's already in Sema. I investigated lifting this interface to something more common, but the issue is with generating a diagnostic (there's no common diagnostic engine between Sema and Parser that I could find). Given the simplicity of the implementation, I figured it's reasonably defensible to duplicate the code.

[aaron.ballman]

Rebased.

I removed the test coverage because it turned out to not be testing anything. If we exhaust resources, then we don't run the -verify to test the diagnostic behavior, and we're relying on the crash being the test condition, but any crash will suffice.

[erichkeane]

I don't see how you can test this behavior without figuring out how to get a 'perfect' number to warn but not crash... The only way to validate that I would expect would be to do some bizarre flag that has us just 'don't run' in the case where the warning is emitted, but that would be strange/only used for the test.

Note that I think you might have an improper comment on the Parser::runWithSufficientStackSpace, else we perhaps wish to combine the Sema.cpp and Parser.cpp implementations in some way.

clang/include/clang/Parse/Parser.h

802 ↗

(On Diff #428969)

Hmm... The Parser version of this might be used for Template Instantiation? Should we remove the SEMA one and move all uses to the Parser then?

[erichkeane]

clang/lib/Parse/Parser.cpp

2618 ↗

(On Diff #428969)

Another concern here: Do we properly initialize "Actions" in the case where we don't have a semantic Action? That is, if we are just preprocessing?

[aaron.ballman]

clang/include/clang/Parse/Parser.h

802 ↗

(On Diff #428969)

Nope, but I could be less silly and just... use the one from Sema... instead of moving it to Parser. Derp!

[aaron.ballman]

Simplify the implementation by using the facilities Sema already exposes. The behavior remains the same:

F:\source\llvm-project>llvm\out\build\x64-Debug\bin\clang.exe -cc1 -fsyntax-only "C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c"
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.c:8:1: warning: stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely [-Wstack-exhausted]
int PTR6 q3_var = 0;
^
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace, preprocessed source, and associated run script.
Stack dump:

[erichkeane]

Another concern here: Do we properly initialize "Actions" in the case where we don't have a semantic Action? That is, if we are just preprocessing?

Doh! Forgot we don't do 'Parser' in preprocessor mode. LGTM!

clang/docs/ReleaseNotes.rst

 - Added the ``-Wgnu-line-marker`` diagnostic flag (grouped under the ``-Wgnu``
   flag) which is a portability warning about use of GNU linemarker preprocessor
   directives. Fixes `Issue 55067 <https://github.com/llvm/llvm-project/issues/55067>`_.
 - Using ``#elifdef`` and ``#elifndef`` that are incompatible with C/C++
   standards before C2x/C++2b are now warned via ``-pedantic``. Additionally,
   on such language mode, ``-Wpre-c2x-compat`` and ``-Wpre-c++2b-compat``
   diagnostic flags report a compatibility issue.
   Fixes `Issue 55306 <https://github.com/llvm/llvm-project/issues/55306>`_.
-
+- Clang now checks for stack resource exhaustion when recursively parsing
+  declarators in order to give a diagnostic before we run out of stack space.
+  This fixes `Issue 51642 <https://github.com/llvm/llvm-project/issues/51642>`_.
 
 Non-comprehensive list of changes in this release
 -------------------------------------------------
 
 - Improve __builtin_dump_struct:
 
   - Support bitfields in struct and union.
   - Improve the dump format, dump both bitwidth(if its a bitfield) and field

clang/lib/Parse/ParseDecl.cpp

if (isInvalid) {
       assert(PrevSpec && "Method did not return previous specifier!");
       Diag(Tok, DiagID) << PrevSpec;
     }
     EndLoc = ConsumeToken();
   }
 }
 
 /// ParseDeclarator - Parse and verify a newly-initialized declarator.
-///
 void Parser::ParseDeclarator(Declarator &D) {
   /// This implements the 'declarator' production in the C grammar, then checks
   /// for well-formedness and issues diagnostics.
+  Actions.runWithSufficientStackSpace(D.getBeginLoc(), [&] {
     ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
+  });
 }
 
 static bool isPtrOperatorToken(tok::TokenKind Kind, const LangOptions &Lang,
                                DeclaratorContext TheContext) {
   if (Kind == tok::star || Kind == tok::caret)
     return true;
 
   // OpenCL 2.0 and later define this keyword.
if (SS.isNotEmpty()) {
 
       SourceLocation StarLoc = ConsumeToken();
       D.SetRangeEnd(StarLoc);
       DeclSpec DS(AttrFactory);
       ParseTypeQualifierListOpt(DS);
       D.ExtendWithDeclSpec(DS);
 
       // Recurse to parse whatever is left.
+      Actions.runWithSufficientStackSpace(D.getBeginLoc(), [&] {
         ParseDeclaratorInternal(D, DirectDeclParser);
+      });
 
       // Sema will have to catch (syntactically invalid) pointers into global
       // scope. It has to catch pointers into namespace scope anyway.
       D.AddTypeInfo(DeclaratorChunk::getMemberPointer(
                         SS, DS.getTypeQualifiers(), StarLoc, DS.getEndLoc()),
                     std::move(DS.getAttributes()),
                     /* Don't replace range end. */ SourceLocation());
       return;
if (Kind == tok::star || Kind == tok::caret) {
     unsigned Reqs = AR_CXX11AttributesParsed | AR_DeclspecAttributesParsed |
                     ((D.getContext() != DeclaratorContext::CXXNew)
                          ? AR_GNUAttributesParsed
                          : AR_GNUAttributesParsedAndRejected);
     ParseTypeQualifierListOpt(DS, Reqs, true, !D.mayOmitIdentifier());
     D.ExtendWithDeclSpec(DS);
 
     // Recursively parse the declarator.
-    ParseDeclaratorInternal(D, DirectDeclParser);
+    Actions.runWithSufficientStackSpace(
+        D.getBeginLoc(), [&] { ParseDeclaratorInternal(D, DirectDeclParser); });
     if (Kind == tok::star)
       // Remember that we parsed a pointer type, and remember the type-quals.
       D.AddTypeInfo(DeclaratorChunk::getPointer(
                         DS.getTypeQualifiers(), Loc, DS.getConstSpecLoc(),
                         DS.getVolatileSpecLoc(), DS.getRestrictSpecLoc(),
                         DS.getAtomicSpecLoc(), DS.getUnalignedSpecLoc()),
                     std::move(DS.getAttributes()), SourceLocation());
     else
if (DS.getTypeQualifiers() != DeclSpec::TQ_unspecified) {
              diag::err_invalid_reference_qualifier_application) << "volatile";
       // 'restrict' is permitted as an extension.
       if (DS.getTypeQualifiers() & DeclSpec::TQ_atomic)
         Diag(DS.getAtomicSpecLoc(),
              diag::err_invalid_reference_qualifier_application) << "_Atomic";
     }
 
     // Recursively parse the declarator.
-    ParseDeclaratorInternal(D, DirectDeclParser);
+    Actions.runWithSufficientStackSpace(
+        D.getBeginLoc(), [&] { ParseDeclaratorInternal(D, DirectDeclParser); });
 
     if (D.getNumTypeObjects() > 0) {
       // C++ [dcl.ref]p4: There shall be no references to references.
       DeclaratorChunk& InnerChunk = D.getTypeObject(D.getNumTypeObjects() - 1);
       if (InnerChunk.Kind == DeclaratorChunk::Reference) {
         if (const IdentifierInfo *II = D.getIdentifier())
           Diag(InnerChunk.Loc, diag::err_illegal_decl_reference_to_reference)
            << II;