-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
py: Fix two segfaults due to accessing uninitialized memory. #18732
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
base: master
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #18732 +/- ##
==========================================
- Coverage 98.41% 98.41% -0.01%
==========================================
Files 171 171
Lines 22324 22329 +5
==========================================
+ Hits 21971 21975 +4
- Misses 353 354 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Code size report: |
Accessing obj->subobj[0] without verifying that it contains a valid object (instead of a type, or a non-initialized object) is not a good idea. Signed-off-by: Matthias Urlichs <matthias@urlichs.de>
Test basics/subclass_native_exc_noinit.py segfaults without them. Signed-off-by: Matthias Urlichs <matthias@urlichs.de>
|
Added testcases to exercise the problem; one of them manages to reproduce the crash. I have no idea why the stackless test fails (works on my system). |
The test required `super().__new__` to exist, which the minimal build doesn't have. Signed-off-by: Matthias Urlichs <matthias@urlichs.de>
|
While it might be possible to further improve the diff coverage, IMHO that effort is spent more productively elsewhere. |
Summary
Accessing obj->subobj[0] without verifying that it contains a valid object (instead of a type, or a non-initialized object) is not a good idea.
Testing
My MoaT package has extensive MicroPython testcases. Before applying this PR the MicroPython process reliably segfaulted. This no longer happens.
Closes #17117 .