[clang] Extension: allow recursive macros (PR #65851)

via cfe-commits cfe-commits at lists.llvm.org
Mon Sep 11 01:03:50 PDT 2023


https://github.com/kelbon updated https://github.com/llvm/llvm-project/pull/65851:

>From 2f807b312baef8c6038c2452b84232acb6d6d2c2 Mon Sep 17 00:00:00 2001
From: Kelbon Nik <kelbonage at gmail.com>
Date: Sat, 9 Sep 2023 17:51:15 +0400
Subject: [PATCH 1/3] add define2 pp directive

---
 clang/include/clang/Basic/TokenKinds.def |  1 +
 clang/include/clang/Lex/MacroInfo.h      | 19 +++++++++----------
 clang/include/clang/Lex/Preprocessor.h   |  2 +-
 clang/lib/Basic/IdentifierTable.cpp      |  2 ++
 clang/lib/Format/WhitespaceManager.cpp   |  2 +-
 clang/lib/Lex/MacroInfo.cpp              |  3 ++-
 clang/lib/Lex/PPDirectives.cpp           | 16 +++++++++++-----
 7 files changed, 27 insertions(+), 18 deletions(-)

diff --git a/clang/include/clang/Basic/TokenKinds.def b/clang/include/clang/Basic/TokenKinds.def
index 45ebc200b168986..f059d809823ab42 100644
--- a/clang/include/clang/Basic/TokenKinds.def
+++ b/clang/include/clang/Basic/TokenKinds.def
@@ -115,6 +115,7 @@ PPKEYWORD(__include_macros)
 
 // C99 6.10.3 - Macro Replacement.
 PPKEYWORD(define)
+PPKEYWORD(define2)
 PPKEYWORD(undef)
 
 // C99 6.10.4 - Line Control.
diff --git a/clang/include/clang/Lex/MacroInfo.h b/clang/include/clang/Lex/MacroInfo.h
index 00c1c3866bbd9ca..4f0c8e987610e50 100644
--- a/clang/include/clang/Lex/MacroInfo.h
+++ b/clang/include/clang/Lex/MacroInfo.h
@@ -102,6 +102,10 @@ class MacroInfo {
   /// like \#define A A.
   bool IsDisabled : 1;
 
+  // True if 'define2' used,
+  // ignores 'IsDisabled' and enables expansion anyway
+  bool AllowRecurse : 1;
+
   /// True if this macro is either defined in the main file and has
   /// been used, or if it is not defined in the main file.
   ///
@@ -278,18 +282,13 @@ class MacroInfo {
   /// Return true if this macro is enabled.
   ///
   /// In other words, that we are not currently in an expansion of this macro.
-  bool isEnabled() const { return !IsDisabled; }
-
-  void EnableMacro() {
-    assert(IsDisabled && "Cannot enable an already-enabled macro!");
-    IsDisabled = false;
-  }
+  bool isEnabled() const { return AllowRecurse || !IsDisabled; }
+  void setAllowRecursive(bool Allow) { AllowRecurse = Allow; }
+  bool isAllowRecurse() const { return AllowRecurse; }
 
-  void DisableMacro() {
-    assert(!IsDisabled && "Cannot disable an already-disabled macro!");
-    IsDisabled = true;
-  }
+  void EnableMacro() { IsDisabled = false; }
 
+  void DisableMacro() { IsDisabled = true; }
   /// Determine whether this macro was used for a header guard.
   bool isUsedForHeaderGuard() const { return UsedForHeaderGuard; }
 
diff --git a/clang/include/clang/Lex/Preprocessor.h b/clang/include/clang/Lex/Preprocessor.h
index 9efe439bc5f2192..de121ce82fd1d7b 100644
--- a/clang/include/clang/Lex/Preprocessor.h
+++ b/clang/include/clang/Lex/Preprocessor.h
@@ -2754,7 +2754,7 @@ class Preprocessor {
   void replayPreambleConditionalStack();
 
   // Macro handling.
-  void HandleDefineDirective(Token &Tok, bool ImmediatelyAfterHeaderGuard);
+  void HandleDefineDirective(Token &Tok, bool ImmediatelyAfterHeaderGuard, bool AllowRecurse);
   void HandleUndefDirective();
 
   // Conditional Inclusion.
diff --git a/clang/lib/Basic/IdentifierTable.cpp b/clang/lib/Basic/IdentifierTable.cpp
index afb30268f2973ce..4de3565c8c6d9a8 100644
--- a/clang/lib/Basic/IdentifierTable.cpp
+++ b/clang/lib/Basic/IdentifierTable.cpp
@@ -433,6 +433,8 @@ tok::PPKeywordKind IdentifierInfo::getPPKeywordID() const {
   unsigned Len = getLength();
   if (Len < 2) return tok::pp_not_keyword;
   const char *Name = getNameStart();
+  if (std::string_view(Name, Len) == "define2")
+    return tok::pp_define2;
   switch (HASH(Len, Name[0], Name[2])) {
   default: return tok::pp_not_keyword;
   CASE( 2, 'i', '\0', if);
diff --git a/clang/lib/Format/WhitespaceManager.cpp b/clang/lib/Format/WhitespaceManager.cpp
index b7bd8d27dc976b1..d8ab76d6761553e 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -743,7 +743,7 @@ void WhitespaceManager::alignConsecutiveMacros() {
     if (!Current || Current->isNot(tok::identifier))
       return false;
 
-    if (!Current->Previous || Current->Previous->isNot(tok::pp_define))
+    if (!Current->Previous || !Current->Previous->isOneOf(tok::pp_define, tok::pp_define2))
       return false;
 
     // For a macro function, 0 spaces are required between the
diff --git a/clang/lib/Lex/MacroInfo.cpp b/clang/lib/Lex/MacroInfo.cpp
index 39bb0f44eff25ba..9c3619c7c909304 100644
--- a/clang/lib/Lex/MacroInfo.cpp
+++ b/clang/lib/Lex/MacroInfo.cpp
@@ -50,7 +50,7 @@ static_assert(MacroInfoSizeChecker<sizeof(void *)>::AsExpected,
 MacroInfo::MacroInfo(SourceLocation DefLoc)
     : Location(DefLoc), IsDefinitionLengthCached(false), IsFunctionLike(false),
       IsC99Varargs(false), IsGNUVarargs(false), IsBuiltinMacro(false),
-      HasCommaPasting(false), IsDisabled(false), IsUsed(false),
+      HasCommaPasting(false), IsDisabled(false), AllowRecurse(false), IsUsed(false),
       IsAllowRedefinitionsWithoutWarning(false), IsWarnIfUnused(false),
       UsedForHeaderGuard(false) {}
 
@@ -157,6 +157,7 @@ LLVM_DUMP_METHOD void MacroInfo::dump() const {
   if (IsBuiltinMacro) Out << " builtin";
   if (IsDisabled) Out << " disabled";
   if (IsUsed) Out << " used";
+  if (AllowRecurse) Out << " allow_recurse";
   if (IsAllowRedefinitionsWithoutWarning)
     Out << " allow_redefinitions_without_warning";
   if (IsWarnIfUnused) Out << " warn_if_unused";
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index a4db8e7a84c07d5..4605d331eba6801 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -1111,7 +1111,11 @@ void Preprocessor::HandleSkippedDirectiveWhileUsingPCH(Token &Result,
   if (const IdentifierInfo *II = Result.getIdentifierInfo()) {
     if (II->getPPKeywordID() == tok::pp_define) {
       return HandleDefineDirective(Result,
-                                   /*ImmediatelyAfterHeaderGuard=*/false);
+                                   /*ImmediatelyAfterHeaderGuard=*/false, /*AllowRecurse=*/false);
+    }
+    if (II->getPPKeywordID() == tok::pp_define2) {
+      return HandleDefineDirective(Result,
+                                   /*ImmediatelyAfterHeaderGuard=*/false, /*AllowRecurse=*/true);
     }
     if (SkippingUntilPCHThroughHeader &&
         II->getPPKeywordID() == tok::pp_include) {
@@ -1250,7 +1254,9 @@ void Preprocessor::HandleDirective(Token &Result) {
 
     // C99 6.10.3 - Macro Replacement.
     case tok::pp_define:
-      return HandleDefineDirective(Result, ImmediatelyAfterTopLevelIfndef);
+      return HandleDefineDirective(Result, ImmediatelyAfterTopLevelIfndef, false);
+    case tok::pp_define2:
+      return HandleDefineDirective(Result, ImmediatelyAfterTopLevelIfndef, true);
     case tok::pp_undef:
       return HandleUndefDirective();
 
@@ -3036,10 +3042,10 @@ static bool isObjCProtectedMacro(const IdentifierInfo *II) {
          II->isStr("__unsafe_unretained") || II->isStr("__autoreleasing");
 }
 
-/// HandleDefineDirective - Implements \#define.  This consumes the entire macro
+/// HandleDefineDirective - Implements \#define and define2. This consumes the entire macro
 /// line then lets the caller lex the next real token.
 void Preprocessor::HandleDefineDirective(
-    Token &DefineTok, const bool ImmediatelyAfterHeaderGuard) {
+    Token &DefineTok, const bool ImmediatelyAfterHeaderGuard, bool AllowRecurse) {
   ++NumDefined;
 
   Token MacroNameTok;
@@ -3064,7 +3070,7 @@ void Preprocessor::HandleDefineDirective(
       MacroNameTok, ImmediatelyAfterHeaderGuard);
 
   if (!MI) return;
-
+  MI->setAllowRecursive(AllowRecurse);
   if (MacroShadowsKeyword &&
       !isConfigurationPattern(MacroNameTok, MI, getLangOpts())) {
     Diag(MacroNameTok, diag::warn_pp_macro_hides_keyword);

>From 9705feebf64466c2d095d52c99fedcdc800b194c Mon Sep 17 00:00:00 2001
From: Kelbon Nik <kelbonage at gmail.com>
Date: Sun, 10 Sep 2023 16:00:45 +0400
Subject: [PATCH 2/3] add recursion depth limit and tests

---
 .../include/clang/Basic/DiagnosticLexKinds.td |  2 ++
 clang/include/clang/Lex/MacroInfo.h           | 33 ++++++++++++-------
 clang/lib/Lex/MacroInfo.cpp                   |  4 +--
 clang/lib/Lex/TokenLexer.cpp                  |  3 +-
 .../test/Preprocessor/macro_vaopt_expand.cpp  | 20 +++++++++++
 clang/test/Preprocessor/recursive_macro.cpp   | 13 ++++++++
 6 files changed, 60 insertions(+), 15 deletions(-)
 create mode 100644 clang/test/Preprocessor/recursive_macro.cpp

diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 940cca67368492f..f940fa01a3d2f38 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -440,6 +440,8 @@ def err_pp_missing_lparen_in_vaopt_use : Error<
 def err_pp_vaopt_nested_use : Error<
   "__VA_OPT__ cannot be nested within its own replacement tokens">;
 
+def err_pp_macro_recursion_depth_limit_exceeded : Error<
+  "macro recursion depth limit exceeded">;
 def err_vaopt_paste_at_start : Error<
   "'##' cannot appear at start of __VA_OPT__ argument">;
 
diff --git a/clang/include/clang/Lex/MacroInfo.h b/clang/include/clang/Lex/MacroInfo.h
index 4f0c8e987610e50..267292083058645 100644
--- a/clang/include/clang/Lex/MacroInfo.h
+++ b/clang/include/clang/Lex/MacroInfo.h
@@ -65,6 +65,12 @@ class MacroInfo {
 
   /// Length in characters of the macro definition.
   mutable unsigned DefinitionLength;
+
+  enum : uint16_t { recursion_depth_limit = 16'000 };
+  /// recursion depth,
+  /// > 0 if we have started an expansion of this macro already.
+  /// for 'define' max is 1, for 'define2' max is depth limit
+  uint16_t Depth = 0;
   mutable bool IsDefinitionLengthCached : 1;
 
   /// True if this macro is function-like, false if it is object-like.
@@ -96,14 +102,7 @@ class MacroInfo {
   //===--------------------------------------------------------------------===//
   // State that changes as the macro is used.
 
-  /// True if we have started an expansion of this macro already.
-  ///
-  /// This disables recursive expansion, which would be quite bad for things
-  /// like \#define A A.
-  bool IsDisabled : 1;
-
-  // True if 'define2' used,
-  // ignores 'IsDisabled' and enables expansion anyway
+  // True if 'define2' used, enables expansion anyway
   bool AllowRecurse : 1;
 
   /// True if this macro is either defined in the main file and has
@@ -282,13 +281,23 @@ class MacroInfo {
   /// Return true if this macro is enabled.
   ///
   /// In other words, that we are not currently in an expansion of this macro.
-  bool isEnabled() const { return AllowRecurse || !IsDisabled; }
+  bool isEnabled() const {
+    // macro disabled if depth exceeds and stops infinite recursion
+    if (AllowRecurse)
+      return Depth < recursion_depth_limit;
+    return Depth == 0;
+  }
   void setAllowRecursive(bool Allow) { AllowRecurse = Allow; }
   bool isAllowRecurse() const { return AllowRecurse; }
 
-  void EnableMacro() { IsDisabled = false; }
-
-  void DisableMacro() { IsDisabled = true; }
+  void EnableMacro() {
+    assert(Depth != 0 && "Cannot enable not disabled macro");
+    --Depth;
+  }
+  // returns false if max recursion depth exceeded
+  [[nodiscard]] bool TryDisableMacro() {
+    return ++Depth < recursion_depth_limit;
+  }
   /// Determine whether this macro was used for a header guard.
   bool isUsedForHeaderGuard() const { return UsedForHeaderGuard; }
 
diff --git a/clang/lib/Lex/MacroInfo.cpp b/clang/lib/Lex/MacroInfo.cpp
index 9c3619c7c909304..110e3232b66dc14 100644
--- a/clang/lib/Lex/MacroInfo.cpp
+++ b/clang/lib/Lex/MacroInfo.cpp
@@ -50,7 +50,7 @@ static_assert(MacroInfoSizeChecker<sizeof(void *)>::AsExpected,
 MacroInfo::MacroInfo(SourceLocation DefLoc)
     : Location(DefLoc), IsDefinitionLengthCached(false), IsFunctionLike(false),
       IsC99Varargs(false), IsGNUVarargs(false), IsBuiltinMacro(false),
-      HasCommaPasting(false), IsDisabled(false), AllowRecurse(false), IsUsed(false),
+      HasCommaPasting(false), AllowRecurse(false), IsUsed(false),
       IsAllowRedefinitionsWithoutWarning(false), IsWarnIfUnused(false),
       UsedForHeaderGuard(false) {}
 
@@ -155,7 +155,7 @@ LLVM_DUMP_METHOD void MacroInfo::dump() const {
   // FIXME: Dump locations.
   Out << "MacroInfo " << this;
   if (IsBuiltinMacro) Out << " builtin";
-  if (IsDisabled) Out << " disabled";
+  if (!isEnabled()) Out << " disabled";
   if (IsUsed) Out << " used";
   if (AllowRecurse) Out << " allow_recurse";
   if (IsAllowRedefinitionsWithoutWarning)
diff --git a/clang/lib/Lex/TokenLexer.cpp b/clang/lib/Lex/TokenLexer.cpp
index 856d5682727fe3d..c51aec4fc17110d 100644
--- a/clang/lib/Lex/TokenLexer.cpp
+++ b/clang/lib/Lex/TokenLexer.cpp
@@ -88,7 +88,8 @@ void TokenLexer::Init(Token &Tok, SourceLocation ELEnd, MacroInfo *MI,
   // Mark the macro as currently disabled, so that it is not recursively
   // expanded.  The macro must be disabled only after argument pre-expansion of
   // function-like macro arguments occurs.
-  Macro->DisableMacro();
+  if (!Macro->TryDisableMacro())
+    PP.Diag(Tok, diag::err_pp_macro_recursion_depth_limit_exceeded);
 }
 
 /// Create a TokenLexer for the specified token stream.  This does not
diff --git a/clang/test/Preprocessor/macro_vaopt_expand.cpp b/clang/test/Preprocessor/macro_vaopt_expand.cpp
index 5eb0facb83f7364..9766b081516fd09 100644
--- a/clang/test/Preprocessor/macro_vaopt_expand.cpp
+++ b/clang/test/Preprocessor/macro_vaopt_expand.cpp
@@ -148,3 +148,23 @@
 
 #undef F
 #undef G
+
+#define merge_all_expand2(a, b) a ## b
+#define merge_all_expand(a, b) merge_all_expand2(a, b)
+#define2 concat_all(head, ...) merge_all_expand(head, __VA_OPT__(concat_all(__VA_ARGS__)))
+29: concat_all(aa, bb, cc)
+30: [concat_all()]
+// CHECK: 29: aabbcc
+// CHECK: 30: []
+
+#undef merge_all_expand
+#undef merge_all_expand2
+#undef concat_all
+
+#define2 reverse(head, ...) __VA_OPT__(reverse(__VA_ARGS__) , ) head
+
+31: reverse(1, 2, 3)
+
+// CHECK: 31: 3, 2, 1
+
+#undef reverse
diff --git a/clang/test/Preprocessor/recursive_macro.cpp b/clang/test/Preprocessor/recursive_macro.cpp
new file mode 100644
index 000000000000000..0fee436bf4d9ce0
--- /dev/null
+++ b/clang/test/Preprocessor/recursive_macro.cpp
@@ -0,0 +1,13 @@
+// RUN: %clang_cc1 %s -Eonly -std=c++11 -pedantic -verify
+
+#define2 A A
+
+A //expected-error {{macro recursion depth limit exceeded}}
+
+#undef A
+
+#define2 boom(x) boom(x)
+
+boom(5)  //expected-error {{macro recursion depth limit exceeded}}
+
+#undef boom

>From 224a84f6922bf862cc4c04e79e5ab2de996ed273 Mon Sep 17 00:00:00 2001
From: Kelbon Nik <kelbonage at gmail.com>
Date: Mon, 11 Sep 2023 12:03:36 +0400
Subject: [PATCH 3/3] rewrite Lexer recursion to loop

---
 clang/lib/Lex/Lexer.cpp                       | 32 +++++++++----------
 clang/lib/Lex/PPMacroExpansion.cpp            |  2 ++
 .../test/Preprocessor/macro_vaopt_expand.cpp  |  4 +--
 3 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/clang/lib/Lex/Lexer.cpp b/clang/lib/Lex/Lexer.cpp
index 37c3e4175d4736e..178bcc08a72e208 100644
--- a/clang/lib/Lex/Lexer.cpp
+++ b/clang/lib/Lex/Lexer.cpp
@@ -847,26 +847,26 @@ bool Lexer::isAtEndOfMacroExpansion(SourceLocation loc,
                                     const SourceManager &SM,
                                     const LangOptions &LangOpts,
                                     SourceLocation *MacroEnd) {
-  assert(loc.isValid() && loc.isMacroID() && "Expected a valid macro loc");
+  for(SourceLocation expansionLoc; true;loc = expansionLoc) {
+    assert(loc.isValid() && loc.isMacroID() && "Expected a valid macro loc");
 
-  SourceLocation spellLoc = SM.getSpellingLoc(loc);
-  unsigned tokLen = MeasureTokenLength(spellLoc, SM, LangOpts);
-  if (tokLen == 0)
-    return false;
+    SourceLocation spellLoc = SM.getSpellingLoc(loc);
+    unsigned tokLen = MeasureTokenLength(spellLoc, SM, LangOpts);
+    if (tokLen == 0)
+      return false;
 
-  SourceLocation afterLoc = loc.getLocWithOffset(tokLen);
-  SourceLocation expansionLoc;
-  if (!SM.isAtEndOfImmediateMacroExpansion(afterLoc, &expansionLoc))
-    return false;
+    SourceLocation afterLoc = loc.getLocWithOffset(tokLen);
+    if (!SM.isAtEndOfImmediateMacroExpansion(afterLoc, &expansionLoc))
+      return false;
 
-  if (expansionLoc.isFileID()) {
-    // No other macro expansions.
-    if (MacroEnd)
-      *MacroEnd = expansionLoc;
-    return true;
+    if (expansionLoc.isFileID()) {
+      // No other macro expansions.
+      if (MacroEnd)
+        *MacroEnd = expansionLoc;
+      return true;
+    }
   }
-
-  return isAtEndOfMacroExpansion(expansionLoc, SM, LangOpts, MacroEnd);
+  llvm_unreachable("");
 }
 
 static CharSourceRange makeRangeFromFileLocs(CharSourceRange Range,
diff --git a/clang/lib/Lex/PPMacroExpansion.cpp b/clang/lib/Lex/PPMacroExpansion.cpp
index 775cbfafa999602..a0093fca0ab0afc 100644
--- a/clang/lib/Lex/PPMacroExpansion.cpp
+++ b/clang/lib/Lex/PPMacroExpansion.cpp
@@ -614,6 +614,8 @@ bool Preprocessor::HandleMacroExpandedIdentifier(Token &Identifier,
     if (IdentifierInfo *NewII = Identifier.getIdentifierInfo()) {
       if (MacroInfo *NewMI = getMacroInfo(NewII))
         if (!NewMI->isEnabled() || NewMI == MI) {
+          if (NewMI->isAllowRecurse() && NewMI == MI)
+            Diag(Identifier, diag::err_pp_macro_recursion_depth_limit_exceeded);
           Identifier.setFlag(Token::DisableExpand);
           // Don't warn for "#define X X" like "#define bool bool" from
           // stdbool.h.
diff --git a/clang/test/Preprocessor/macro_vaopt_expand.cpp b/clang/test/Preprocessor/macro_vaopt_expand.cpp
index 9766b081516fd09..17be324fdd71392 100644
--- a/clang/test/Preprocessor/macro_vaopt_expand.cpp
+++ b/clang/test/Preprocessor/macro_vaopt_expand.cpp
@@ -163,8 +163,8 @@
 
 #define2 reverse(head, ...) __VA_OPT__(reverse(__VA_ARGS__) , ) head
 
-31: reverse(1, 2, 3)
+31: reverse(1,2,3)
 
-// CHECK: 31: 3, 2, 1
+// CHECK: 31: 3,2,1
 
 #undef reverse



More information about the cfe-commits mailing list