Skip to content

gh-126554: ctypes: Correctly handle NULL dlsym values #126555

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.

Sign up for GitHub

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

Merged
merged 54 commits into from
Nov 15, 2024
Merged

gh-126554: ctypes: Correctly handle NULL dlsym values #126555

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
3c3c29d
ctypes: Correctly handle NULL dlsym values
grgalex Nov 7, 2024
f71abe8
Update Misc/NEWS.d/next/C_API/2024-11-07-20-24-58.gh-issue-126554.ri1…
grgalex Nov 8, 2024
64a7394
Update Modules/_ctypes/_ctypes.c
grgalex Nov 8, 2024
512a084
Update Modules/_ctypes/_ctypes.c
grgalex Nov 8, 2024
cd7d5d7
Add dlerror() comment, handle callproc.c
grgalex Nov 8, 2024
cf39b5b
test: Add ctypes null dlsym case
grgalex Nov 8, 2024
a72ce98
Add comment to test, minor wip-debugging prints
grgalex Nov 8, 2024
c820d16
Remove wip/debug prints
grgalex Nov 8, 2024
6751125
Update Modules/_ctypes/_ctypes.c
grgalex Nov 8, 2024
e1f3b43
Update Modules/_ctypes/callproc.c
grgalex Nov 8, 2024
8f7587d
Update Modules/_ctypes/_ctypes.c
grgalex Nov 8, 2024
2b3a88c
Explicitly fail if not AttributeError raised
grgalex Nov 8, 2024
9a1af29
Tidy up Assertion in test
grgalex Nov 8, 2024
3e02e09
Update Modules/_ctypes/_ctypes.c
grgalex Nov 9, 2024
fbdbc26
Update Modules/_ctypes/_ctypes.c
grgalex Nov 9, 2024
fdf6013
Update Modules/_ctypes/callproc.c
grgalex Nov 9, 2024
3aa29e5
Update Lib/test/test_ctypes/test_dlerror.py
grgalex Nov 11, 2024
0616214
Update Lib/test/test_ctypes/test_dlerror.py
grgalex Nov 11, 2024
6d59ab7
test: Smoothly check for existence of GCC
grgalex Nov 11, 2024
31c621d
test: Clean up libfoo.so compilation command
grgalex Nov 11, 2024
53475a4
Update Lib/test/test_ctypes/test_dlerror.py
grgalex Nov 11, 2024
7fc538e
Update Modules/_ctypes/_ctypes.c
grgalex Nov 11, 2024
e20d97f
Update Modules/_ctypes/_ctypes.c
grgalex Nov 11, 2024
95b072e
Update Modules/_ctypes/_ctypes.c
grgalex Nov 11, 2024
048385c
Update Modules/_ctypes/callproc.c
grgalex Nov 11, 2024
658098f
Update Modules/_ctypes/callproc.c
grgalex Nov 11, 2024
6f4b921
Update Modules/_ctypes/_ctypes.c
grgalex Nov 11, 2024
25b6cf9
Update Misc/NEWS.d/next/C_API/2024-11-07-20-24-58.gh-issue-126554.ri1…
grgalex Nov 11, 2024
4bac254
test: Clarify string vs list in subprocess.run
grgalex Nov 11, 2024
ee6122c
test: Use string in gcc --version
grgalex Nov 11, 2024
08a7b88
Update Lib/test/test_ctypes/test_dlerror.py
grgalex Nov 11, 2024
8c6b8ae
Write to a pipe instead of stderr
encukou Nov 12, 2024
291bf16
Reorganize dlerror() code
encukou Nov 12, 2024
1cbffcc
Use PyUnicode_DecodeLocale for the error
encukou Nov 12, 2024
5748c13
Clean up PyCFuncPtr_FromDll, uses #define USE_DLERROR
grgalex Nov 12, 2024
65e2b7e
Clean up py_dl_sym, uses #define USE_DLERROR
grgalex Nov 12, 2024
38fbaed
test: Exercise 3 code paths, one for each C function we want to test
grgalex Nov 12, 2024
cc8e366
Add empty statement after label, placate C compiler
grgalex Nov 12, 2024
4319332
Move label to before #endif (WIN32)
grgalex Nov 12, 2024
8356cf4
Import ctypes, _ctypes inside test, so we don't get ImportError in wi…
grgalex Nov 12, 2024
1e1e78b
Revert "Import ctypes, _ctypes inside test, so we don't get ImportErr…
grgalex Nov 12, 2024
4e87040
Remove unnecessary goto, fix preprocessor cmd indentation
grgalex Nov 12, 2024
6a3aea7
test: Remove shell=True from gcc check, use separate arguments
grgalex Nov 12, 2024
73918bf
Remove extra whitespace
grgalex Nov 12, 2024
0ca32d4
test: Move imports to func, avoid ImportError on Windows
grgalex Nov 12, 2024
6db5963
test: Explain why we moved imports to within test function
grgalex Nov 12, 2024
d179377
dlerror: Use surrogateescape error strategy for decoding locale
grgalex Nov 12, 2024
6933314
Update Modules/_ctypes/callproc.c
grgalex Nov 13, 2024
33cf8b7
Clear DecodeLocale error, indent #ifdef, #undef
grgalex Nov 13, 2024
94dfbaa
Decref the message
encukou Nov 13, 2024
cda7771
Update Lib/test/test_ctypes/test_dlerror.py
grgalex Nov 14, 2024
4b1bb47
test: Handle different architectures, where IFUNC not supported
grgalex Nov 14, 2024
2290aa4
Update Modules/_ctypes/callproc.c
grgalex Nov 14, 2024
ab22349
Don't break when under 80 chars
grgalex Nov 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Misc/NEWS.d/next/C_API/2024-11-07-20-24-58.gh-issue-126554.ri12eb.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Correctly handle NULL dlsym() values in ctypes.
42 changes: 34 additions & 8 deletions Modules/_ctypes/_ctypes.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,18 +967,30 @@ CDataType_in_dll_impl(PyObject *type, PyTypeObject *cls, PyObject *dll,
return NULL;
}
#else
dlerror();
address = (void *)dlsym(handle, name);
if (!address) {
#ifdef __CYGWIN__
/* dlerror() isn't very helpful on cygwin */
if (!address) {
/* dlerror() isn't very helpful on cygwin */
PyErr_Format(PyExc_ValueError,
"symbol '%s' not found",
name);
return NULL;
}
#else
PyErr_SetString(PyExc_ValueError, dlerror());
#endif
char *dlerr;
dlerr = dlerror();
if (dlerr) {
PyErr_SetString(PyExc_ValueError, dlerr);
return NULL;
}
else if (!address) {
PyErr_Format(PyExc_ValueError,
"symbol '%s' not found",
name);
return NULL;
}
#endif
#endif
ctypes_state *st = get_module_state_by_def(Py_TYPE(type));
return PyCData_AtAddress(st, type, address);
Expand Down Expand Up @@ -3774,19 +3786,33 @@ PyCFuncPtr_FromDll(PyTypeObject *type, PyObject *args, PyObject *kwds)
return NULL;
}
#else
dlerror();
address = (PPROC)dlsym(handle, name);
if (!address) {
#ifdef __CYGWIN__
/* dlerror() isn't very helpful on cygwin */
if (!address) {
/* dlerror() isn't very helpful on cygwin */
PyErr_Format(PyExc_AttributeError,
"function '%s' not found",
name);
Py_DECREF(ftuple);
return NULL;
}
#else
PyErr_SetString(PyExc_AttributeError, dlerror());
#endif
char *dlerr;
dlerr = dlerror();
if (dlerr) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the case where another thread sets dlerror at the wrong moment, I'd prefer keeping !address as the condition here. We should only look at dlerr if we actually got NULL.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I see your point.

Even though glibc's dlerror() is thread safe, it doesn't really matter to us.

So, let's flip the ifs, and first check for NULL address, then for dlerr.

In this way, and given that our #ifdefs are not granular enough (esp. in this case where we are in an #else), we won't get bitten on a system where dlerror() is not thread-safe.

Of course, we could always print a wrong message, but that is just as possible even if we don't flip the ifs.

WDYT?

PyErr_SetString(PyExc_AttributeError, dlerr);
Py_DECREF(ftuple);
return NULL;
}
else if (!address) {
PyErr_Format(PyExc_AttributeError,
"function '%s' not found",
name);
Py_DECREF(ftuple);
return NULL;
}
#endif
#endif
ctypes_state *st = get_module_state_by_def(Py_TYPE(type));
if (!_validate_paramflags(st, type, paramflags)) {
Expand Down
Loading