Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Undiagnosed implementation limit of 127 nested function prototypes #19981

Closed
rnk opened this issue Apr 29, 2014 · 11 comments
Closed

Undiagnosed implementation limit of 127 nested function prototypes #19981

rnk opened this issue Apr 29, 2014 · 11 comments
Labels
bugzilla Issues migrated from bugzilla clang:frontend Language frontend issues, e.g. anything involving "Sema" crash Prefer [crash-on-valid] or [crash-on-invalid]

Comments

@rnk
Copy link
Collaborator

rnk commented Apr 29, 2014

Bugzilla Link 19607
Version unspecified
OS Windows NT
CC @majnemer,@mordante,@riccibruno,@rjmccall

Extended Description

ParmVarDecl::setScopeInfo() is defined like so:

void setScopeInfo(unsigned scopeDepth, unsigned parameterIndex) {
assert(!ParmVarDeclBits.IsObjCMethodParam);
ParmVarDeclBits.ScopeDepthOrObjCQuals = scopeDepth;
assert(ParmVarDeclBits.ScopeDepthOrObjCQuals == scopeDepth
&& "truncation!");
setParameterIndex(parameterIndex);
}

It's possible to construct a program that trips this assertion by creating a deeply nested function prototype:

void foo(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)(void (*f)()))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))))));

Assertion failed: ParmVarDeclBits.ScopeDepthOrObjCQuals == scopeDepth && "truncation!", file ..\tools\clang\include\clang/AST/Decl.h, line 1276

I wanted one more bit in VarDeclBits.TSCSpec, so I considered taking it from here. However, I discovered we don't enforce an upper bound on this quantity.

@rnk
Copy link
Collaborator Author

rnk commented Apr 29, 2014

Relatedly, we cap out at 2**15 function parameters:

$ cat t.py
print 'void foo(',
for i in range(0, 2**15):
print 'int a%d,' % i,
print 'int a%d);' % (i + 1)

$ python t.py | clang -c -x c -
Assertion failed: NumParams == params.size() && "function has too many parameters", file ..\tools\clang\lib\AST\Type.cpp, line 1604

@rnk
Copy link
Collaborator Author

rnk commented May 25, 2017

*** Bug llvm/llvm-bugzilla-archive#33162 has been marked as a duplicate of this bug. ***

@mordante
Copy link
Member

I created a patch [1] for the original submission, will look at the second part later.

[1] https://reviews.llvm.org/D63975

@riccibruno
Copy link
Contributor

Looking at FunctionTypeBitfields, It looks like the limits now for the number of function parameters is 2^16-1. It would be possible to bump this a little bit since there is some space left in FunctionTypeBitfields (but [implimits] only ask for 8 bits...)

I wonder if it would be worth spending some time going through the various limits and :

  1. Add a check and an error message instead of just ignoring the situation (would help with fuzzing too).
  2. Test these limits.
  3. Document these limits like MSVC does (eg: https://docs.microsoft.com/en-us/cpp/cpp/compiler-limits?view=vs-2019).

@mordante
Copy link
Member

mordante commented Jul 1, 2019

I wonder if it would be worth spending some time going through the various
limits and :

  1. Add a check and an error message instead of just ignoring the situation
    (would help with fuzzing too).

I think that would be better, also avoids confusing error messages like in bug 33162, where the error referred to an existing identifier.

  1. Test these limits.

I assume that should be part of the patch introducing the error message.

  1. Document these limits like MSVC does (eg:
    https://docs.microsoft.com/en-us/cpp/cpp/compiler-limits?view=vs-2019).

According to [implimits] "Every implementation shall document those limitations where known." so I think that should be done.

@rjmccall
Copy link
Contributor

rjmccall commented Jul 1, 2019

I would definitely support both documenting and enforcing these limits. I don't think we should bump them unless we actually think they're currently unacceptable.

@mordante
Copy link
Member

*** Bug llvm/llvm-bugzilla-archive#31968 has been marked as a duplicate of this bug. ***

@mordante
Copy link
Member

*** Bug #11122 has been marked as a duplicate of this bug. ***

@mordante
Copy link
Member

mentioned in issue llvm/llvm-bugzilla-archive#31968

@rnk
Copy link
Collaborator Author

rnk commented Nov 26, 2021

mentioned in issue llvm/llvm-bugzilla-archive#33162

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 9, 2021
@Endilll
Copy link
Contributor

Endilll commented Jun 25, 2023

The first test case appears to be fixed in Clang 10: https://godbolt.org/z/szcM4cfG3
I can't reproduce the second test case using assertion builds of Clang 3.4 and 3.5: https://godbolt.org/z/EnKKvbGKb

@Endilll Endilll closed this as completed Jun 25, 2023
@Endilll Endilll added the crash Prefer [crash-on-valid] or [crash-on-invalid] label Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugzilla Issues migrated from bugzilla clang:frontend Language frontend issues, e.g. anything involving "Sema" crash Prefer [crash-on-valid] or [crash-on-invalid]
Projects
None yet
Development

No branches or pull requests

5 participants