Skip to content

Commit

Permalink
[clang] Diagnose functions with too many parameters (#104833)
Browse files Browse the repository at this point in the history
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
#35741 (comment),
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
  • Loading branch information
Endilll authored Aug 21, 2024
1 parent 76c0798 commit 281d178
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 1 deletion.
3 changes: 3 additions & 0 deletions clang/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
------------------------

Expand Down
7 changes: 6 additions & 1 deletion clang/include/clang/AST/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions clang/include/clang/Basic/DiagnosticParseKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -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 "
Expand Down
11 changes: 11 additions & 0 deletions clang/lib/Parse/ParseDecl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
29 changes: 29 additions & 0 deletions clang/test/Parser/function-parameter-limit.cpp
Original file line number Diff line number Diff line change
@@ -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);

0 comments on commit 281d178

Please sign in to comment.