Skip to content
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

Compilation warnings in getpath.c with gcc on Ubuntu / -D_FORTIFY_SOURCE=2 #76556

Closed
pitrou opened this issue Dec 19, 2017 · 10 comments
Closed
Labels
3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@pitrou
Copy link
Member

pitrou commented Dec 19, 2017

BPO 32375
Nosy @pitrou, @vstinner, @serhiy-storchaka

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:

assignee = None
closed_at = <Date 2019-09-26.01:12:36.866>
created_at = <Date 2017-12-19.15:05:24.282>
labels = ['interpreter-core', 'type-bug', '3.9']
title = 'Compilation warnings in getpath.c with gcc on Ubuntu / -D_FORTIFY_SOURCE=2'
updated_at = <Date 2019-09-26.01:12:36.865>
user = 'https://github.com/pitrou'

bugs.python.org fields:

activity = <Date 2019-09-26.01:12:36.865>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-09-26.01:12:36.866>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2017-12-19.15:05:24.282>
creator = 'pitrou'
dependencies = []
files = []
hgrepos = []
issue_num = 32375
keywords = []
message_count = 10.0
messages = ['308653', '308697', '308708', '308721', '308755', '308756', '308757', '308758', '316349', '353252']
nosy_count = 3.0
nosy_names = ['pitrou', 'vstinner', 'serhiy.storchaka']
pr_nums = []
priority = 'low'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue32375'
versions = ['Python 3.9']

@pitrou
Copy link
Member Author

pitrou commented Dec 19, 2017

On Ubuntu 16.04:

$ gcc --version
gcc (Ubuntu 5.4.0-6ubuntu1~16.04.5) 5.4.0 20160609
Copyright (C) 2015 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ make
[...]
In function ‘wcsncpy’,
    inlined from ‘calculate_zip_path’ at ./Modules/getpath.c:797:5:
/usr/include/x86_64-linux-gnu/bits/wchar2.h:200:9: warning: call to ‘__wcsncpy_chk_warn’ declared with attribute warning: wcsncpy called with length bigger than size of destination buffer
  return __wcsncpy_chk_warn (__dest, __src, __n,
         ^
In function ‘wcsncpy’,
    inlined from ‘calculate_zip_path’ at ./Modules/getpath.c:806:9:
/usr/include/x86_64-linux-gnu/bits/wchar2.h:200:9: warning: call to ‘__wcsncpy_chk_warn’ declared with attribute warning: wcsncpy called with length bigger than size of destination buffer
  return __wcsncpy_chk_warn (__dest, __src, __n,
         ^
In function ‘wcsncpy’,
    inlined from ‘calculate_argv0_path’ at ./Modules/getpath.c:683:5:
/usr/include/x86_64-linux-gnu/bits/wchar2.h:200:9: warning: call to ‘__wcsncpy_chk_warn’ declared with attribute warning: wcsncpy called with length bigger than size of destination buffer
  return __wcsncpy_chk_warn (__dest, __src, __n,
         ^
In function ‘wcsncpy’,
    inlined from ‘calculate_argv0_path’ at ./Modules/getpath.c:736:13:
/usr/include/x86_64-linux-gnu/bits/wchar2.h:200:9: warning: call to ‘__wcsncpy_chk_warn’ declared with attribute warning: wcsncpy called with length bigger than size of destination buffer
  return __wcsncpy_chk_warn (__dest, __src, __n,
         ^

@pitrou pitrou added 3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Dec 19, 2017
@vstinner
Copy link
Member

In function ‘wcsncpy’,
inlined from ‘calculate_zip_path’ at ./Modules/getpath.c:797:5:
/usr/include/x86_64-linux-gnu/bits/wchar2.h:200:9: warning: call to ‘__wcsncpy_chk_warn’ declared with attribute warning: wcsncpy called with length bigger than size of destination buffer
return __wcsncpy_chk_warn (__dest, __src, __n,
^

Line 797:

wcsncpy(calculate->zip_path, prefix, MAXPATHLEN);

calculate type is "PyCalculatePath *" which is defined as:

typedef struct {
...
wchar_t zip_path[MAXPATHLEN+1]; /* ".../lib/pythonXY.zip" */
...
} PyCalculatePath;

Earlier, all bytes are set to 0:

memset(&calculate, 0, sizeof(calculate));

So I don't see how wcsncpy() can overflow.

By the way, I'm unable to reproduce the warning on Fedora 27 with GCC 7.2.1. Are you using -D_FORTIFY_SOURCE=1? Are you compiling Python in release mode? Can you try to find the command line compiling getpath.c?

@pitrou
Copy link
Member Author

pitrou commented Dec 20, 2017

Here is the command line:

gcc -pthread -c -Wno-unused-result -Wsign-compare -g -Og -Wall -Wstrict-prototypes -std=c99 -Wextra -Wno-unused-result -Wno-unused-parameter -Wno-missing-field-initializers -Werror=implicit-function-declaration -I. -I./Include -DPy_BUILD_CORE -DPYTHONPATH='":"' \
-DPREFIX='"/usr/local"' \
-DEXEC_PREFIX='"/usr/local"' \
-DVERSION='"3.7"' \
-DVPATH='""' \
-o Modules/getpath.o ./Modules/getpath.c

@serhiy-storchaka
Copy link
Member

The same on Ubuntu 17.10 with gcc 7.2.0.

@vstinner
Copy link
Member

Ah, I missed the warning because I forge -O0 when I build Python.

https://wiki.ubuntu.com/ToolChain/CompilerFlags

Ubuntu adds -D_FORTIFY_SOURCE=2 flag by default.

The warnings can be seen with "-D_FORTIFY_SOURCE=2 -Og", but not with "-D_FORTIFY_SOURCE=2 -O3". Moreover, "-D_FORTIFY_SOURCE=2 -O0" complains that _FORTIFY_SOURCE requires to optimize the code.

It looks more like a false alarm because -D_FORTIFY_SOURCE=2 is incompatible with -Og.

Maybe we should force -D_FORTIFY_SOURCE=0 when we build Python in debug mode?

@pitrou
Copy link
Member Author

pitrou commented Dec 20, 2017

I don't think so. It's good to have fortify enabled, especially in debug mode :-)

If the warnings are harmless and there isn't an easy way to suppress them, then I'm ok to close this issue.

@vstinner vstinner changed the title Compilation warnings with gcc Compilation warnings in getpath.c with gcc on Ubuntu / -D_FORTIFY_SOURCE=2 Dec 20, 2017
@vstinner
Copy link
Member

getpath.c uses many buffers of MAXPATHLEN+1 wide characters. Example:

wchar_t argv0_path[MAXPATHLEN+1];

These buffers are initialized to zero to make sure that the last character is always a NULL character.

To keep the final NULL character, string copies use:

wcsncpy(dest, src, MAXPATHLEN);

This code is wrong: it truncates the string if it's longer than MAXPATHLEN characters.

I modified the code to move global buffers closer to where there are used, and to dynamically allocate strings on the heap, rather using fixed sizes. But I didn't finish to "cleanup" Modules/getpath.c and PC/getpathp.c. The code still uses the buffer of fixed size and truncate strings.

The real fix would be to avoid these fixed-size buffers, and only use dynamically allocated strings.

I modified the code to allow to report errors. Previously, it wasn't possible exception using Py_FatalError() which is not a nice way to report errors, especially when Python is embedded in an application.

@vstinner
Copy link
Member

See bpo-32030 for my huge refactoring work on the Python initialization code.

@serhiy-storchaka
Copy link
Member

Warnings are emitted when compile with gcc-5, gcc-6 and gcc-7, but not when compile with gcc-4.8 or gcc-8.

Versions:

gcc-4.8 (Ubuntu 4.8.5-4ubuntu8) 4.8.5
gcc-5 (Ubuntu 5.5.0-12ubuntu1) 5.5.0 20171010
gcc-6 (Ubuntu 6.4.0-17ubuntu1) 6.4.0 20180424
gcc-7 (Ubuntu 7.3.0-16ubuntu3) 7.3.0
gcc-8 (Ubuntu 8-20180414-1ubuntu2) 8.0.1 20180414 (experimental) [trunk revision 259383]

@vstinner
Copy link
Member

I just recompiled the master branch of Python twice using these flags:

  • -D_FORTIFY_SOURCE=2 -Og
  • -D_FORTIFY_SOURCE=2 -O3

I got a few warnings, the same that I get without FORTIFY SOURCE.

Using -D_FORTIFY_SOURCE=2 -O3, I get one warning, but it's no longer from getpath.c but in socketmodule.c. So I created a new issue: bpo-38282 "socketmodule.c: _FORTIFY_SOURCE=2 warning in AF_ALG case of getsockaddrarg()".

I close this issue. It has been fixed in the master branch.

@vstinner vstinner added 3.9 only security fixes and removed 3.7 (EOL) end of life labels Sep 26, 2019
@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.9 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants