This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: Two minor fixes for C module object
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: larry Nosy List: larry, vstinner
Priority: normal Keywords: patch

Created on 2021-04-27 11:06 by larry, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 25658 merged larry, 2021-04-27 11:11
Messages (3)
msg392055 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2021-04-27 11:06
While working on another issue, I noticed two minor nits in the C implementation of the module object.  Both are related to getting a module's name.

--

First, the C function module_dir() (module.__dir__) starts by ensuring the module dict is valid.  If the module dict is invalid, it wants to format an exception using the name of the module, which it gets from PyModule_GetName().  However, PyModule_GetName() gets the name of the module from the dict.  So getting the name in this circumstance will never succeed.

When module_dir() wants to format the error but can't get the name, it knows that PyModule_GetName() must have already raised an exception.  So it leaves that exception alone and returns an error.  The end result is that the exception raised here is kind of useless and misleading: dir(module) on a module with no __dict__ raises SystemError("nameless module").  I changed the code to actually raise the exception it wanted to raise, just without a real module name: TypeError("<module>.__dict__ is not a dictionary").  This seems more useful, and would do a better job putting the programmer who encountered this on the right track of figuring out what was going on.

--

Second, the C API function PyModule_GetNameObject() checks to see if the module has a dict.  If m->md_dict is not NULL, it calls _PyDict_GetItemIdWithError().  However, it's possible for m->md_dict to be None.  And if you call _PyDict_GetItemIdWithError(Py_None, ...) it will *crash*.

Unfortunately, this crash was due to my own bug in the other branch.  Fixing my code made the crash go away.  I assert that this is still possible at the API level.

The fix is easy: add a PyDict_Check() to PyModule_GetNameObject().

Unfortunately, I don't know how to add a unit test for this.  Having changed module_dir() above, I can't find any other interfaces callable from Python that eventually call PyModule_GetNameObject().  So I don't know how to trick the runtime into reproducing this error.
msg392375 - (view) Author: Larry Hastings (larry) * (Python committer) Date: 2021-04-30 03:14
Thanks for the review, Victor!
msg392401 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-04-30 09:53
commit 175a54b2d8f967605f1d46b5cadccdcf2b54842f
Author: larryhastings <larry@hastings.org>
Date:   Thu Apr 29 20:13:25 2021 -0700

    Two minor fixes for accessing a module's name. (#25658)
    
    While working on another issue, I noticed two minor nits in the C implementation of the module object.  Both are related to getting a module's name.
    
    First, the C function module_dir() (module.__dir__) starts by ensuring the module dict is valid.  If the module dict is invalid, it wants to format an exception using the name of the module, which it gets from PyModule_GetName().  However, PyModule_GetName() gets the name of the module from the dict.  So getting the name in this circumstance will never succeed.
    
    When module_dir() wants to format the error but can't get the name, it knows that PyModule_GetName() must have already raised an exception.  So it leaves that exception alone and returns an error.  The end result is that the exception raised here is kind of useless and misleading: dir(module) on a module with no __dict__ raises SystemError("nameless module").  I changed the code to actually raise the exception it wanted to raise, just without a real module name: TypeError("<module>.__dict__ is not a dictionary").  This seems more useful, and would do a better job putting the programmer who encountered this on the right track of figuring out what was going on.
    
    Second, the C API function PyModule_GetNameObject() checks to see if the module has a dict.  If m->md_dict is not NULL, it calls _PyDict_GetItemIdWithError().  However, it's possible for m->md_dict to be None.  And if you call _PyDict_GetItemIdWithError(Py_None, ...) it will *crash*.
    
    Unfortunately, this crash was due to my own bug in the other branch.  Fixing my code made the crash go away.  I assert that this is still possible at the API level.
    
    The fix is easy: add a PyDict_Check() to PyModule_GetNameObject().
    
    Unfortunately, I don't know how to add a unit test for this.  Having changed module_dir() above, I can't find any other interfaces callable from Python that eventually call PyModule_GetNameObject().  So I don't know how to trick the runtime into reproducing this error.
    
    Since both these changes are minor--each entails only a small edit to only one line--I didn't bother with a news item.
History
Date User Action Args
2022-04-11 14:59:44adminsetgithub: 88117
2021-04-30 09:53:44vstinnersetmessages: + msg392401
2021-04-30 03:14:44larrysetstatus: open -> closed
resolution: fixed
messages: + msg392375

stage: patch review -> resolved
2021-04-27 11:48:08vstinnersetnosy: + vstinner
2021-04-27 11:11:02larrysetkeywords: + patch
stage: needs patch -> patch review
pull_requests: + pull_request24349
2021-04-27 11:06:19larrycreate