llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Vlad Serebrennikov (Endilll) <details> <summary>Changes</summary> 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 --- Full diff: https://github.com/llvm/llvm-project/pull/104833.diff 5 Files Affected: - (modified) clang/docs/ReleaseNotes.rst (+3) - (modified) clang/include/clang/AST/Type.h (+6-1) - (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+3) - (modified) clang/lib/Parse/ParseDecl.cpp (+11) - (added) clang/test/Parser/function-parameter-limit.cpp (+29) ``````````diff diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index a4257ea1f48c11..e1ab1d45bed766 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -288,6 +288,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/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 8a92973236ddbd..1f97db2e13ebf9 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -9311,6 +9311,9 @@ def note_constinit_missing_here : Note< def err_dimension_expr_not_constant_integer : Error< "dimension expression does not evaluate to a constant unsigned int">; +def err_function_parameter_limit_exceeded : Error< + "too many function parameters; subsequent parameters are skipped">; + def err_typecheck_cond_incompatible_operands_null : Error< "non-pointer operand type %0 incompatible with %select{NULL|nullptr}1">; def ext_empty_struct_union : Extension< diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index a8a9d3f3f5b088..105f41276f67ab 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -8025,6 +8025,17 @@ void Parser::ParseParameterDeclarationClause( // Consume the keyword. ConsumeToken(); } + + // FunctionTypeBitfields::NumParams can only hold so much. + // 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..9f66a01456b831 --- /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 are skipped}} + 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 are skipped}} + int w); `````````` </details> https://github.com/llvm/llvm-project/pull/104833 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
