[clang] 281d178 - [clang] Diagnose functions with too many parameters (#104833)

via cfe-commits cfe-commits at lists.llvm.org
Wed Aug 21 06:38:28 PDT 2024


Author: Vlad Serebrennikov
Date: 2024-08-21T17:38:24+04:00
New Revision: 281d17840c35a1d80303bb6170c253fe2411f95f

URL: https://github.com/llvm/llvm-project/commit/281d17840c35a1d80303bb6170c253fe2411f95f
DIFF: https://github.com/llvm/llvm-project/commit/281d17840c35a1d80303bb6170c253fe2411f95f.diff

LOG: [clang] Diagnose functions with too many parameters (#104833)

This patch adds a parser check when a function declaration or function
type declaration (in a function pointer declaration, for example) has
too many parameters for `FunctionTypeBits::NumParams` to hold. At the
moment of writing it's a 16-bit-wide bit-field, limiting the number of
parameters at 65536.

The check is added in the parser loop that goes over comma-separated
list of function parameters. This is not the solution Aaron suggested in
https://github.com/llvm/llvm-project/issues/35741#issuecomment-1638086571,
because it was found out that it's quite hard to recover from this
particular error in `GetFullTypeForDeclarator()`. Multiple options were
tried, but all of them led to crashes down the line.

I used LLVM Compile Time Tracker to ensure this does not introduce a
performance regression. I believe changes are in the noise:
https://llvm-compile-time-tracker.com/compare.php?from=de5ea2d122c31e1551654ff506c33df299f351b8&to=424818620766cedb2770e076ee359afeb0cc14ec&stat=instructions:u

Fixes #35741

Added: 
    clang/test/Parser/function-parameter-limit.cpp

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/AST/Type.h
    clang/include/clang/Basic/DiagnosticParseKinds.td
    clang/lib/Parse/ParseDecl.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 5aedfc654e8dbb..8f98167dff31ef 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -310,6 +310,9 @@ Miscellaneous Clang Crashes Fixed
 - Fixed a crash caused by long chains of ``sizeof`` and other similar operators
   that can be followed by a non-parenthesized expression. (#GH45061)
 
+- Fixed a crash when function has more than 65536 parameters.
+  Now a diagnostic is emitted. (#GH35741)
+
 OpenACC Specific Changes
 ------------------------
 

diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 27618604192c51..575f3c17a3f691 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1929,6 +1929,11 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
     unsigned Kind : NumOfBuiltinTypeBits;
   };
 
+public:
+  static constexpr int FunctionTypeNumParamsWidth = 16;
+  static constexpr int FunctionTypeNumParamsLimit = (1 << 16) - 1;
+
+protected:
   /// FunctionTypeBitfields store various bits belonging to FunctionProtoType.
   /// Only common bits are stored here. Additional uncommon bits are stored
   /// in a trailing object after FunctionProtoType.
@@ -1966,7 +1971,7 @@ class alignas(TypeAlignment) Type : public ExtQualsTypeCommonBase {
     /// According to [implimits] 8 bits should be enough here but this is
     /// somewhat easy to exceed with metaprogramming and so we would like to
     /// keep NumParams as wide as reasonably possible.
-    unsigned NumParams : 16;
+    unsigned NumParams : FunctionTypeNumParamsWidth;
 
     /// The type of exception specification this function has.
     LLVM_PREFERRED_TYPE(ExceptionSpecificationType)

diff  --git a/clang/include/clang/Basic/DiagnosticParseKinds.td b/clang/include/clang/Basic/DiagnosticParseKinds.td
index 62a97b36737e72..464f08637332d4 100644
--- a/clang/include/clang/Basic/DiagnosticParseKinds.td
+++ b/clang/include/clang/Basic/DiagnosticParseKinds.td
@@ -481,6 +481,9 @@ def ext_decomp_decl_empty : ExtWarn<
   "ISO C++17 does not allow a decomposition group to be empty">,
   InGroup<DiagGroup<"empty-decomposition">>;
 
+def err_function_parameter_limit_exceeded : Error<
+  "too many function parameters; subsequent parameters will be ignored">;
+
 // C++26 structured bindings
 def ext_decl_attrs_on_binding : ExtWarn<
   "an attribute specifier sequence attached to a structured binding declaration "

diff  --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp
index a8a9d3f3f5b088..ed5d6ce90aa7d1 100644
--- a/clang/lib/Parse/ParseDecl.cpp
+++ b/clang/lib/Parse/ParseDecl.cpp
@@ -8025,6 +8025,17 @@ void Parser::ParseParameterDeclarationClause(
         // Consume the keyword.
         ConsumeToken();
       }
+
+      // We can only store so many parameters
+      // Skip until the the end of the parameter list, ignoring
+      // parameters that would overflow.
+      if (ParamInfo.size() == Type::FunctionTypeNumParamsLimit) {
+        Diag(ParmDeclarator.getBeginLoc(),
+             diag::err_function_parameter_limit_exceeded);
+        SkipUntil(tok::r_paren, SkipUntilFlags::StopBeforeMatch);
+        break;
+      }
+
       // Inform the actions module about the parameter declarator, so it gets
       // added to the current scope.
       Decl *Param =

diff  --git a/clang/test/Parser/function-parameter-limit.cpp b/clang/test/Parser/function-parameter-limit.cpp
new file mode 100644
index 00000000000000..29f5dde294715c
--- /dev/null
+++ b/clang/test/Parser/function-parameter-limit.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -verify %s
+
+#define P_10(x) x##0, x##1, x##2, x##3, x##4, x##5, x##6, x##7, x##8, x##9,
+#define P_100(x) P_10(x##0) P_10(x##1) P_10(x##2) P_10(x##3) P_10(x##4) \
+                 P_10(x##5) P_10(x##6) P_10(x##7) P_10(x##8) P_10(x##9)
+#define P_1000(x) P_100(x##0) P_100(x##1) P_100(x##2) P_100(x##3) P_100(x##4) \
+                  P_100(x##5) P_100(x##6) P_100(x##7) P_100(x##8) P_100(x##9)
+#define P_10000(x) P_1000(x##0) P_1000(x##1) P_1000(x##2) P_1000(x##3) P_1000(x##4) \
+                   P_1000(x##5) P_1000(x##6) P_1000(x##7) P_1000(x##8) P_1000(x##9)
+
+void func (
+  P_10000(int p)
+  P_10000(int q)
+  P_10000(int r)
+  P_10000(int s)
+  P_10000(int t)
+  P_10000(int u)
+  P_10000(int v) // expected-error {{too many function parameters; subsequent parameters will be ignored}}
+  int w);
+
+extern double(*func2)(
+  P_10000(int p)
+  P_10000(int q)
+  P_10000(int r)
+  P_10000(int s)
+  P_10000(int t)
+  P_10000(int u)
+  P_10000(int v) // expected-error {{too many function parameters; subsequent parameters will be ignored}}
+  int w);


        


More information about the cfe-commits mailing list