From 281d17840c35a1d80303bb6170c253fe2411f95f Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov Date: Wed, 21 Aug 2024 17:38:24 +0400 Subject: [PATCH] [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 --- clang/docs/ReleaseNotes.rst | 3 ++ clang/include/clang/AST/Type.h | 7 ++++- .../clang/Basic/DiagnosticParseKinds.td | 3 ++ clang/lib/Parse/ParseDecl.cpp | 11 +++++++ .../test/Parser/function-parameter-limit.cpp | 29 +++++++++++++++++++ 5 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 clang/test/Parser/function-parameter-limit.cpp 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>; +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);