bpo-42034:Fixed unchecked return in Objects/typeobject.c #22695
Conversation
Unchecked return in Objects/typeobject.c and possible uninitialized variables in cls and new_mro
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
|
Isn't the point that this is the function clean-up code? It wants to be run in full whether or not everything succeeds. By returning early you leak |
|
Yes, you are right. The function of the cleanup code is to run everything be it a success or a failure. But what happens when cls remains uninitialized after the function call PyArg_UnpackTuple on Line 826 and soon after that the code tries to access cls and use it's attribute tp_mro on Line 831 (if (cls->tp_mro == new_mro))? The code would then throw an exception and code to clean up temp would not be called and this exception would then be passed up the function call till it is handled gracefully. But you are right in pointing out that if we return based on the function failure, the code to clean up temp Py_DECREF(temp) will never be called. So, now I will only call the temp cleanup no matter what and instead of returning in case of failure, the code will now execute if block only in case of success. I believe this should solve both problems. As for the bail section, it is called before a call is made to the undo section (Line 794). So any change to the undo section would not have any impact on the bail section. |
Removed return to call cleanup on temp in all the cases
|
This PR is stale because it has been open for 30 days with no activity. |
|
Closing this stale PR because the CLA is still not signed. |
The return value of a function that is potentially used to initialize a local variable is not checked. Therefore, reading the local variable may result in undefined behavior.
Our AI analyzer found that this function is called for a total of 43 times. Out of these 43 times, the return value from the function call is checked at 42 instances. This is the only instance where the code misses to check the return value for success or failure.
Once such correct reference usage found in Python/hamt.c at line 2805 .
Code Extract:
https://bugs.python.org/issue42034