https://github.com/Endilll created https://github.com/llvm/llvm-project/pull/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 >From 424818620766cedb2770e076ee359afeb0cc14ec Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Mon, 19 Aug 2024 19:26:46 +0300 Subject: [PATCH 1/5] Add in-loop check --- clang/include/clang/AST/Type.h | 7 ++++++- clang/include/clang/Basic/DiagnosticSemaKinds.td | 3 +++ clang/lib/Parse/ParseDecl.cpp | 10 ++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) 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..94f2366d484f2d 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -8025,6 +8025,16 @@ 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 = >From 11c54d40f39005ac20f8fb212981bc443a197ed8 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Mon, 19 Aug 2024 21:30:35 +0300 Subject: [PATCH 2/5] Add a test and a release note --- clang/docs/ReleaseNotes.rst | 3 ++ .../test/Parser/function-parameter-limit.cpp | 29 +++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 clang/test/Parser/function-parameter-limit.cpp 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/test/Parser/function-parameter-limit.cpp b/clang/test/Parser/function-parameter-limit.cpp new file mode 100644 index 00000000000000..c6917c51644187 --- /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); \ No newline at end of file >From 20929235140bb2b97cbc80458c081d5572dcc089 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Mon, 19 Aug 2024 21:49:42 +0300 Subject: [PATCH 3/5] Run clang-format --- clang/lib/Parse/ParseDecl.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index 94f2366d484f2d..105f41276f67ab 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -8030,7 +8030,8 @@ void Parser::ParseParameterDeclarationClause( // 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); + Diag(ParmDeclarator.getBeginLoc(), + diag::err_function_parameter_limit_exceeded); SkipUntil(tok::r_paren, SkipUntilFlags::StopBeforeMatch); break; } >From 5775814e8dd7f4724d18f6ea2ff1477feef814c6 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Mon, 19 Aug 2024 21:56:33 +0300 Subject: [PATCH 4/5] Add a newline --- clang/test/Parser/function-parameter-limit.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/test/Parser/function-parameter-limit.cpp b/clang/test/Parser/function-parameter-limit.cpp index c6917c51644187..4493b1e0f0ffdf 100644 --- a/clang/test/Parser/function-parameter-limit.cpp +++ b/clang/test/Parser/function-parameter-limit.cpp @@ -26,4 +26,4 @@ extern double(*func2)( P_10000(int t) P_10000(int u) P_10000(int v) // expected-error {{too many function parameters; subsequent parameters are skipped}} - int w); \ No newline at end of file + int w); >From 257a7eb6e95a1310d4644990b9bedd697c8714d4 Mon Sep 17 00:00:00 2001 From: Vlad Serebrennikov <serebrennikov.vladis...@gmail.com> Date: Mon, 19 Aug 2024 22:02:40 +0300 Subject: [PATCH 5/5] Replace tabs with spaces --- clang/test/Parser/function-parameter-limit.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clang/test/Parser/function-parameter-limit.cpp b/clang/test/Parser/function-parameter-limit.cpp index 4493b1e0f0ffdf..9f66a01456b831 100644 --- a/clang/test/Parser/function-parameter-limit.cpp +++ b/clang/test/Parser/function-parameter-limit.cpp @@ -2,11 +2,11 @@ #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) + 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) + 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) + 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) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
