python / cpython

The Python programming language
https://www.python.org
Other
63.88k stars 30.57k forks source link

Update test_crashers #108297

Closed vstinner closed 1 year ago

vstinner commented 1 year ago

Feature or enhancement

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

Proposal:

test_crashers is currently skipped and some scripts in Lib/test/crashers/ no longer crash.

Linked PRs

serhiy-storchaka commented 1 year ago

Victor, do you remember we measured the recursion limit for different kind of recursion, like via custom __getitem__, __getattr__, __call__, properties, partial(), etc, and you did an awesome work for reducing the stack consumption? Do you remember the issue number? I think that in many of such cases the recursion is still limited, and it would be nice to have tests for this, to avoid regression.

vstinner commented 1 year ago

It was in 2017, search for "Stack consumption" at https://vstinner.github.io/contrib-cpython-2017q1.html

I added _testcapi.stack_pointer() in issue #75049: see script https://bugs.python.org/file46992/stack_overflow-3.py

On the main branch with a debug build (pydebug with gcc -O0), I get:

test_python_call: 498 calls before crash, stack: 8368 bytes/call
test_python_getitem: 109060 calls before crash, stack: 55 bytes/call
test_python_iterator: 746 calls before crash, stack: 8171 bytes/call

=> total: 110304 calls, 16594 bytes

On the main branch with a release build (gcc -O3), I get:

$ ./python stack_overflow-3.py 
test_python_call: 498 calls before crash, stack: 737 bytes/call
test_python_getitem: 109060 calls before crash, stack: 3 bytes/call
test_python_iterator: 746 calls before crash, stack: 528 bytes/call

=> total: 110304 calls, 1268 bytes

test_python_getitem memory usage is quite good, it's almost free :-)

I'm not sure where 109,060 number comes from.

serhiy-storchaka commented 1 year ago

Thank you Victor. Other issues:

where there are more examples of stack overflowing code. I think that they could be included instead of the removed recursive_call.py.

We can also add separate test which runs maximally deep recursion (with the limit depending on platform) to ensure that there is no regression in new versions.

vstinner commented 1 year ago

In this issue, I looked at crasher scripts and some of them no longer crash. They should be removed. I will try to come up with a change to remove them and maybe update the others based on the closed PR #108299.

vstinner commented 1 year ago

Sadly, I failed track of this issue. While it may be nice to cleanup Lib/test/crashers/, it became a lower priority for me since I removed test_crashers, instead of running it. I close the issue. If someone is motivated to cleanup this directory, go ahead and propose a new issue and/or PR.