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
Use of Py_ARRAY_LENGTH on pointer in posixmodule.c, win32_wchdir #70033
Comments
I found this while writing up a separate bug (CPython doesn't use static analysis!). In modules/posixmodule.c, win32_wchdir uses Py_ARRAY_LENGTH on a wchar_t*: wchar_t _new_path[MAX_PATH], *new_path = _new_path;
int result;
wchar_t env[4] = L"=x:"; if(!SetCurrentDirectoryW(path))
return FALSE;
result = GetCurrentDirectoryW(Py_ARRAY_LENGTH(new_path), new_path); ...instead of using Py_ARRAY_LENGTH(_new_path), the programmer wrote Py_ARRAY_LENGTH(new_path), doesn't work on pointers: /* Get the number of elements in a visible array This does not work on pointers, or arrays declared as [], or function Written by Rusty Russell, public domain, http://ccodearchive.net/
*/
#define Py_ARRAY_LENGTH(array) \
(sizeof(array) / sizeof((array)[0])) The same issue occurs two lines later: if (result > Py_ARRAY_LENGTH(new_path)) { Compiling with /analyze found this quite easily: c:\pythondev\repo\modules\posixmodule.c(1354): warning C6384: Dividing sizeof a pointer by another value. |
Oh, it's a regression introduced by: changeset: 87516:46aecfc5e374 |
New changeset 39ce98d9b6b7 by Victor Stinner in branch '3.5': New changeset e373ec3c2969 by Victor Stinner in branch 'default': |
Are you aware of the Coverity program? Last time I heard about Coverity, CPython had 0 bug found by Coverity ;-) |
The sad part is that Py_ARRAY_LENGTH() is written for static analysis, but it only works with some compilaters, and for example it doesn't work with Visual Studio, the only compiler on Windows officially supported. |
The other occurrence is not fixed. |
New changeset aba6caf7b9f0 by Victor Stinner in branch '3.5': |
Oops, right. Good catch. This is what happens when I skip the code review step :-/ Is it better with the second change? |
Yup, see bpo-25847.
Sadly, yeah. MSVC, when compiling as C++, should actually catch this, but CPython precludes that. |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: