homepage

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: Avoid leaking linker flags into distutils: add PY_LDFLAGS_NODIST
Type: Stage: resolved
Components: Build, Distutils, Extension Modules Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: cstratak, dstufft, eric.araujo, hroncok, miss-islington, ned.deily, vstinner
Priority: Keywords: patch

Created on 2018年11月15日 16:14 by cstratak, last changed 2022年04月11日 14:59 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
demo.c cstratak, 2018年12月05日 14:43
setup.py cstratak, 2018年12月05日 14:43
Pull Requests
URL Status Linked Edit
PR 10900 merged cstratak, 2018年12月04日 16:53
PR 11169 closed vstinner, 2018年12月14日 20:41
PR 11264 merged vstinner, 2018年12月20日 14:07
PR 11265 merged vstinner, 2018年12月20日 14:18
PR 11297 merged ned.deily, 2018年12月24日 15:34
PR 11298 merged miss-islington, 2018年12月24日 15:37
PR 11299 merged miss-islington, 2018年12月24日 15:38
Messages (28)
msg329951 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018年11月15日 16:14
Through acb8c5234302f8057b331abaafb2cc8697daf58f the CFLAGS_NODIST variable was created, in order to place there compiler flags used by the interpreter, but not intended to be propagated to C extensions.
I saw a similar issue when working on backporting 67e997bcfdac55191033d57a16d1408aL1313 on python 3.6, where the -flto flag should be passed to CFLAGS_NODIST instead of BASECFLAGS, however even if that is fixed, the LDFLAGS will still be propagated to C extensions.
Thus in order to provide more flexibility in that regard, I propose to add the LDFLAGS_NODIST variable, which in a similar vein as CFLAGS_NODIST, will hold the LDFLAGS intended to be used only by the interpreter.
Thoughts or comments on this approach?
msg329952 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018年11月15日 16:15
Correction: The second commit is referring to https://github.com/python/cpython/pull/9908/ 
msg331119 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018年12月05日 14:43
So to better illustrate the actual issue I'll be using an example from the python documentation [0][1].
Get the demo.c and the setup.py. Compile cpython first with --with-lto and then compile the demo.c with ./python3 setup.py build.
You will notice that various link time optimization linker flags are passed to the extension.
[0] https://docs.python.org/3/extending/extending.html#keyword-parameters-for-extension-functions
[1] https://docs.python.org/3/extending/building.html#building-c-and-c-extensions-with-distutils 
msg331122 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018年12月05日 14:44
And here is the difference between compiling the extension with the current tip, comparing to applying my current draft PR:
Master branch with the linker flags propagated:
running build
running build_ext
building 'demo' extension
creating build
creating build/temp.linux-x86_64-3.8
gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/home/Harris/dev/cpython/_install/include/python3.8m -c demo.c -o build/temp.linux-x86_64-3.8/demo.o
creating build/lib.linux-x86_64-3.8
gcc -pthread -shared -flto -fuse-linker-plugin -ffat-lto-objects -flto-partition=none -g build/temp.linux-x86_64-3.8/demo.o -o build/lib.linux-x86_64-3.8/demo.cpython-38m-x86_64-linux-gnu.so
With introduction of the LDFLAGS_NODIST variable:
running build
running build_ext
building 'demo' extension
creating build
creating build/temp.linux-x86_64-3.8
gcc -pthread -Wno-unused-result -Wsign-compare -DNDEBUG -g -fwrapv -O3 -Wall -fPIC -I/home/Harris/dev/cpython/_install/include/python3.8m -c demo.c -o build/temp.linux-x86_64-3.8/demo.o
creating build/lib.linux-x86_64-3.8
gcc -pthread -shared build/temp.linux-x86_64-3.8/demo.o -o build/lib.linux-x86_64-3.8/demo.cpython-38m-x86_64-linux-gnu.so
msg331177 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018年12月05日 21:44
PR has been finalized.
msg331419 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018年12月09日 09:16
Unfortunately, it appears this won't be resolved in time for 3.7.2rc1 and 3.6.8rc1 and it would not be appropriate to merge something this potentially disruptive without prior exposure. Since 3.6.8 is planned to be the final 3.6.x bugfix release, unless some critical problem is discovered prior to their final releases it's likely this will not be fixed for 3.6.
msg331472 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018年12月10日 07:21
> Unfortunately, it appears this won't be resolved in time for 3.7.2rc1 and 3.6.8rc1
An update: the cutoff for these releases has been extended until about 30 hours from now so there is perhaps a small chance that the PR for this could still be updated, reviewed, and merged in time. If not, it can wait.
msg331540 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018年12月10日 21:45
The PR is pending another round of review.
msg331845 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年12月14日 18:07
There are multiple ways to configure and build Python, we should try most combinations:
* ./configure --enable-shared
* ./configure --with-lto
* ./configure --enable-optimizations
* make profile-opt
* make
* Maybe also: make install
Test:
* Build Python and make sure that python binary and C extensions (of the stdlib) are compiled with LTO
* python-config --cflags and python-config --ldflags don't leak LTO flags
* Build a C extension (Pillow) and check that there is no LTO flag in the command lines
I'm not how to test cross-compilation :-(
msg331852 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年12月14日 18:55
See also:
* bpo-35499: "make profile-opt" overrides CFLAGS_NODIST 
* bpo-35501: "make coverage" should use leak coverage flags to third party C extensions.
msg332150 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年12月19日 16:38
TL; DR PR 10900 passed my manual tests ;-)
I made some tests on PR 10900, commit d1655f939d0eeeca24c2a4fee635da087e0d499b, using 3 shell scripts.
(1) step1.sh:
--
set -x -e
git clean -fdx
./configure CC=clang --with-lto --prefix /opt/py38
make 2>&1|tee log
grep -E -- '-o python|-o Python/bltinmodule.o|Modules/_asynciomodule.o' log|grep -c lto
# 4 expected: -flto passed to compiler *and* linker
--
(2) step2.sh:
--
set -e -x
rm -rf /opt/py38
mkdir /opt/py38
make install
/opt/py38/bin/python3.8-config --cflags --ldflags --libs|grep -c lto ||:
# "0" expected here: no LTO
LD_LIBRARY_PATH=/opt/py38/lib /opt/py38/bin/python3.8 -c 'import sysconfig; print("lto" in sysconfig.get_config_var("LDFLAGS"))'
# "False" expected: no LTO in LDFLAGS
--
(3) step3.sh:
--
set -e -x
tar -xf ../Pillow-5.3.0.tar.gz
cd Pillow-5.3.0/
LD_LIBRARY_PATH=/opt/py38/lib /opt/py38/bin/python3.8 setup.py install 2>&1|tee log
grep -c lto log
--
Get Pillow tarball using:
wget https://files.pythonhosted.org/packages/1b/e1/1118d60e9946e4e77872b69c58bc2f28448ec02c99a2ce456cd1a272c5fd/Pillow-5.3.0.tar.gz
== master ==
master branch, ./configure CC=clang --with-lto:
(1) 4
(2) 0, True => ERR
(3) 5 => ERR
master branch, ./configure CC=clang --with-lto --enable-shared:
(1) 4
(2) 0, True => ERR
(3) 5 => ERR
== PR ==
With PR 10900, ./configure CC=clang --with-lto:
(1) 4
(2) 0, False => OK!
(3) 0 => OK!
With PR 10900, ./configure CC=clang --with-lto --enable-shared:
(1) 4
(2) 0, False => OK!
(3) 0 => OK!
msg332153 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年12月19日 17:05
PGO+LTO build with PR 10900: I see PGO options (-fprofile-instr-generate then -fprofile-instr-use=code.profclangd) and LTO option (-flto) passed to the compiler and to the linker, as expected:
$ git clean -fdx
$ ./configure CC=clang --with-lto --prefix /opt/py38 --enable-optimizations
$ sed -i -e 's/^PROFILE_TASK=.*/PROFILE_TASK=-c pass/' Makefile
$ make
...
# compile Python core
clang ... -flto ... -fprofile-instr-generate ... Modules/main.c
...
# link ./python program
clang -pthread -flto -g -fprofile-instr-generate -Xlinker -export-dynamic -o python Programs/python.o libpython3.8m.a -lpthread -ldl -lutil -lm -lm 
...
# compile stdlib C extension
clang ... -flto ... -fprofile-instr-generate ... -c /home/vstinner/prog/python/master/Modules/_heapqmodule.c ...
...
rm -f profile-gen-stamp
...
# compile Python core
clang ... -flto ... -fprofile-instr-use=code.profclangd ... -o Programs/python.o ./Programs/python.c
...
# link ./python program
clang -pthread -flto -g -Xlinker -export-dynamic -o python Programs/python.o libpython3.8m.a -lpthread -ldl -lutil -lm -lm 
...
# build struct extension
clang ... -flto ... -fprofile-instr-use=code.profclangd ... -c /home/vstinner/prog/python/master/Modules/_struct.c -o build/temp.linux-x86_64-3.8/home/vstinner/prog/python/master/Modules/_struct.o
warning: no profile data available for file "_struct.c" [-Wprofile-instr-unprofiled]
1 warning generated.
clang -pthread -shared -flto -g build/temp.linux-x86_64-3.8/home/vstinner/prog/python/master/Modules/_struct.o -L/opt/py38/lib -L/usr/local/lib -o build/lib.linux-x86_64-3.8/_struct.cpython-38m-x86_64-linux-gnu.so
...
msg332155 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年12月19日 17:19
New changeset cf10a750f4b50b6775719cfb17bee00bc3a9c60b by Victor Stinner (stratakis) in branch 'master':
bpo-35257: Avoid leaking LTO linker flags into distutils (GH-10900)
https://github.com/python/cpython/commit/cf10a750f4b50b6775719cfb17bee00bc3a9c60b
msg332227 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018年12月20日 14:12
This change fixes a regression introduced in 3.6.8rc1 with https://bugs.python.org/issue31354 
msg332228 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018年12月20日 14:13
And also 3.7.2rc1
msg332229 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年12月20日 14:20
Ned: We have to do something with this issue before 3.6 and 3.7 releases. Either revert or backport fixes (merge PR 11264 and PR 11265).
msg332230 - (view) Author: Charalampos Stratakis (cstratak) * Date: 2018年12月20日 14:30
Small correction. This regression has been on 3.7 for some time now (since it was the master branch then), but then I requested to have the buggy commit backported to 3.6 to fix the --with-lto flag there. Which unfortunately introduced the issue to 3.6 anyway. Master branch is fixed now and backports are open for 3.7 and 3.6 if those are to be accepted by the release manager.
msg332232 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年12月20日 15:02
I repeated the same tests for Python 3.7 on PR 11264.
I replaced "38" with "37" and "3.8" and 3.7" and my 3 scripts :-) I modified step1.sh for PGO+LTO with -O0:
set -x -e
git clean -fdx
./configure CC=clang --with-lto --prefix /opt/py37 --enable-optimizations
sed -i -e 's/^PROFILE_TASK=.*/PROFILE_TASK=-c pass/' Makefile
sed -i -e 's/-O3/-O0/' Makefile
make 2>&1|tee log
grep -E -- '-o python|-o Python/bltinmodule.o|Modules/_asynciomodule.o' log|grep -c lto
# 4 expected: -flto passed to compiler *and* linker
Test results:
./configure CC=clang --with-lto --prefix /opt/py38
(1) 4
(2) 0, False => OK!
(3) 0 => OK!
./configure CC=clang --with-lto --enable-shared
(1) 4
(2) 0, False => OK!
(3) 0 => OK!
./configure CC=clang --with-lto --enable-optimizations
(1) 8 => OK!
(2) 0, False => OK!
(3) 0 => OK!
msg332233 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年12月20日 15:03
New changeset 0198f52ea2328fd932622ad2299381f617a041f2 by Victor Stinner in branch '3.7':
bpo-35257: Avoid leaking LTO linker flags into distutils (GH-10900) (GH-11264)
https://github.com/python/cpython/commit/0198f52ea2328fd932622ad2299381f617a041f2
msg332234 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年12月20日 15:24
I repeated the same tests for Python 3.6 on PR 11265: all tests are ok!
I replaced "38" with "36" and "3.8" and 3.6" and my 3 scripts :-) I modified step1.sh for PGO+LTO and to compile with -O0 (just to make tests faster): similar script than in my previous message.
./configure CC=clang --with-lto --prefix /opt/py36
(1) 4 => OK!
(2) 0, False => OK!
(3) 0 => OK!
./configure CC=clang --with-lto --prefix /opt/py36 --enable-shared
(1) 4 => OK!
(2) 0, False => OK!
(3) 0 => OK!
./configure CC=clang --with-lto --prefix /opt/py36 --enable-optimizations
(1) 8 => OK!
(2) 0, False => OK!
(3) 0 => OK!
msg332257 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018年12月20日 20:39
Also cherry-picked to 3.6 in commit a21bedf9edeacce6f0de3b2df0783e2ad6aa3b18
[3.6] bpo-35257: Avoid leaking LTO linker flags into distutils (GH-10900) (GH-11265)
msg332259 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年12月20日 20:56
Just in case, I checked again 3.6 and 3.7 branches with "./configure CC=clang --with-lto --enable-shared": they still pass my manual tests ;-)
msg332466 - (view) Author: miss-islington (miss-islington) Date: 2018年12月24日 15:36
New changeset 44a3ee07e30e18d83e2730c093d8b0e930f0a06c by Miss Islington (bot) (Ned Deily) in branch 'master':
bpo-35257: fix broken BLDSHARED - needs LDFLAGS too (GH-11297)
https://github.com/python/cpython/commit/44a3ee07e30e18d83e2730c093d8b0e930f0a06c
msg332467 - (view) Author: miss-islington (miss-islington) Date: 2018年12月24日 15:36
New changeset 44a3ee07e30e18d83e2730c093d8b0e930f0a06c by Miss Islington (bot) (Ned Deily) in branch 'master':
bpo-35257: fix broken BLDSHARED - needs LDFLAGS too (GH-11297)
https://github.com/python/cpython/commit/44a3ee07e30e18d83e2730c093d8b0e930f0a06c
msg332469 - (view) Author: miss-islington (miss-islington) Date: 2018年12月24日 15:40
New changeset 3d4b4b80f290e622b05ca219ad6dabc07b49421a by Miss Islington (bot) in branch '3.7':
bpo-35257: fix broken BLDSHARED - needs LDFLAGS too (GH-11297)
https://github.com/python/cpython/commit/3d4b4b80f290e622b05ca219ad6dabc07b49421a
msg332470 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018年12月24日 15:41
New changeset 68f5dfd955ee7a16fabf90d7c370159cc0ca9b75 by Ned Deily (Miss Islington (bot)) in branch '3.6':
bpo-35257: fix broken BLDSHARED - needs LDFLAGS too (GH-11297) (GH-11299)
https://github.com/python/cpython/commit/68f5dfd955ee7a16fabf90d7c370159cc0ca9b75
msg332490 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2018年12月24日 16:32
New changeset f14087a4a96bb3f500e2d2bbc36c1124e90d5f73 by Ned Deily (Victor Stinner) in branch '3.7':
bpo-35257: Avoid leaking LTO linker flags into distutils (GH-10900) (GH-11264)
https://github.com/python/cpython/commit/f14087a4a96bb3f500e2d2bbc36c1124e90d5f73
New changeset 92f90242994652d6b7afe0a2c8a61cf2bccc8c32 by Ned Deily (Miss Islington (bot)) in branch '3.7':
bpo-35257: fix broken BLDSHARED - needs LDFLAGS too (GH-11297)
https://github.com/python/cpython/commit/92f90242994652d6b7afe0a2c8a61cf2bccc8c32
msg333145 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019年01月07日 11:54
New changeset 92f90242994652d6b7afe0a2c8a61cf2bccc8c32 by Ned Deily (Miss Islington (bot)) in branch '3.7':
bpo-35257: fix broken BLDSHARED - needs LDFLAGS too (GH-11297)
https://github.com/python/cpython/commit/92f90242994652d6b7afe0a2c8a61cf2bccc8c32
Since Ned pushed a new fix, I ran again my tests on the 3.7 branch with ./configure CC=clang --with-lto --prefix /opt/py37 --enable-shared:
(1) 4 => ok
(2) 0, False => ok
(3) 0 => ok
Good! It works (in my tests).
--
"make sharedmods" runs "LDSHARED=$BLDSHARED (...) ./python setup.py build"
"make libpython(...)" uses $(BLDSHARED) to link libpython.
Modules/makesetup uses BLDSHARED to build C extensions of the stdlib. Example from generated Makefile:
"Modules/posix$(EXT_SUFFIX): Modules/posixmodule.o; $(BLDSHARED) Modules/posixmodule.o -o Modules/posix$(EXT_SUFFIX) (...)"
distutils has a customize_compiler() function which uses LDSHARED from Makefile (can be overriden by environment variables): it doesn't use BLDSHARED.
It seems like BLDSHARED is only used to build Python itself and stdlib modules. So I agree that it's ok to add PY_LDFLAGS_NODIST flags to BLDSHARED.
History
Date User Action Args
2022年04月11日 14:59:08adminsetgithub: 79438
2019年01月07日 11:54:37vstinnersetmessages: + msg333145
2018年12月24日 16:32:58ned.deilysetmessages: + msg332490
2018年12月24日 15:41:43ned.deilysetmessages: + msg332470
2018年12月24日 15:40:51miss-islingtonsetmessages: + msg332469
2018年12月24日 15:38:43miss-islingtonsetpull_requests: + pull_request10534
2018年12月24日 15:37:24miss-islingtonsetpull_requests: + pull_request10533
2018年12月24日 15:36:54miss-islingtonsetmessages: + msg332467
2018年12月24日 15:36:18miss-islingtonsetnosy: + miss-islington
messages: + msg332466
2018年12月24日 15:34:44ned.deilysetpull_requests: + pull_request10532
2018年12月20日 20:56:27vstinnersetmessages: + msg332259
2018年12月20日 20:39:51ned.deilysetstatus: open -> closed
priority: release blocker ->
messages: + msg332257

resolution: fixed
stage: patch review -> resolved
2018年12月20日 15:24:26vstinnersetmessages: + msg332234
2018年12月20日 15:03:04vstinnersetmessages: + msg332233
2018年12月20日 15:02:36vstinnersetmessages: + msg332232
2018年12月20日 14:30:59cstrataksetmessages: + msg332230
2018年12月20日 14:20:15vstinnersetpriority: critical -> release blocker

messages: + msg332229
2018年12月20日 14:18:34vstinnersetpull_requests: + pull_request10496
2018年12月20日 14:13:07cstrataksetmessages: + msg332228
2018年12月20日 14:12:44cstrataksetmessages: + msg332227
2018年12月20日 14:07:13vstinnersetpull_requests: + pull_request10495
2018年12月19日 17:19:05vstinnersetmessages: + msg332155
2018年12月19日 17:05:29vstinnersetmessages: + msg332153
2018年12月19日 16:38:36vstinnersetmessages: + msg332150
2018年12月14日 20:41:59vstinnersetpull_requests: + pull_request10406
2018年12月14日 18:55:07vstinnersetmessages: + msg331852
2018年12月14日 18:07:58vstinnersetmessages: + msg331845
2018年12月10日 21:45:05cstrataksetmessages: + msg331540
2018年12月10日 11:15:58vstinnersettitle: Avoid leaking linker flags into distutils. -> Avoid leaking linker flags into distutils: add PY_LDFLAGS_NODIST
2018年12月10日 07:21:53ned.deilysetmessages: + msg331472
2018年12月09日 09:16:15ned.deilysetpriority: normal -> critical
nosy: + ned.deily
messages: + msg331419

2018年12月05日 21:44:57cstrataksettitle: Add LDFLAGS_NODIST for the LDFLAGS not intended for propagation to C extensions. -> Avoid leaking linker flags into distutils.
2018年12月05日 21:44:29cstrataksetnosy: + vstinner
messages: + msg331177
2018年12月05日 14:50:33hroncoksetnosy: + hroncok
2018年12月05日 14:44:59cstrataksetmessages: + msg331122
2018年12月05日 14:43:26cstrataksetfiles: + setup.py
2018年12月05日 14:43:17cstrataksetfiles: + demo.c

messages: + msg331119
2018年12月04日 16:53:04cstrataksetkeywords: + patch
stage: patch review
pull_requests: + pull_request10140
2018年11月15日 16:15:34cstrataksetmessages: + msg329952
2018年11月15日 16:14:01cstratakcreate

AltStyle によって変換されたページ (->オリジナル) /