10000 bpo-42212: smelly.py also checks the dynamic library (GH-23423) · python/cpython@ac7d016 · GitHub
[go: up one dir, main page]
Skip to content

Commit ac7d016

Browse files
vstinnervstinner
authored
bpo-42212: smelly.py also checks the dynamic library (GH-23423)
The smelly.py script now also checks the Python dynamic library and extension modules, not only the Python static library. Make also the script more verbose: explain what it does. The GitHub Action job now builds Python with the libpython dynamic library.
1 parent 14d81dc commit ac7d016

File tree

3 files changed

+124
-36
lines changed

3 files changed

+124
-36
lines changed

‎.github/workflows/build.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ jobs:
6060
run: sudo ./.github/workflows/posix-deps-apt.sh
6161
- name: Build CPython
6262
run: |
63-
./configure --with-pydebug
63+
# Build Python with the libpython dynamic library
64+
./configure --with-pydebug --enable-shared
6465
make -j4 regen-all
6566
- name: Check for changes
6667
run: |

‎Misc/NEWS.d/next/Tools-Demos/2020-11-20-15-11-05.bpo-42212.sjzgOf.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
The smelly.py script now also checks the Python dynamic library and extension
2+
modules, not only the Python static library. Make also the script more verbose:
3+
explain what it does.

‎Tools/scripts/smelly.py

Lines changed: 119 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,47 @@
11
#!/usr/bin/env python
22
# Script checking that all symbols exported by libpython start with Py or _Py
33

4+
import os.path
45
import subprocess
56
import sys
67
import sysconfig
78

89

9-
def get_exported_symbols():
10-
LIBRARY = sysconfig.get_config_var('LIBRARY')
11-
if not LIBRARY:
12-
raise Exception("failed to get LIBRARY")
10+
ALLOWED_PREFIXES = ('Py', '_Py')
11+
if sys.platform == 'darwin':
12+
ALLOWED_PREFIXES += ('__Py',)
13+
14+
IGNORED_EXTENSION = "_ctypes_test"
15+
# Ignore constructor and destructor functions
16+
IGNORED_SYMBOLS = {'_init', '_fini'}
17+
18+
19+
def is_local_symbol_type(symtype):
20+
# Ignore local symbols.
21+
22+
# If lowercase, the symbol is usually local; if uppercase, the symbol
23+
# is global (external). There are however a few lowercase symbols that
24+
# are shown for special global symbols ("u", "v" and "w").
25+
if symtype.islower() and symtype not in "uvw":
26+
return True
27+
28+
# Ignore the initialized data section (d and D) and the BSS data
29+
# section. For example, ignore "__bss_start (type: B)"
30+
# and "_edata (type: D)".
31+
if symtype in "bBdD":
32+
return True
33+
34+
return False
1335

14-
args = ('nm', '-p', LIBRARY)
36+
37+
def get_exported_symbols(library, dynamic=False):
38+
print(f"Check that {library} only exports symbols starting with Py or _Py")
39+
40+
# Only look at dynamic symbols
41+
args = ['nm', '--no-sort']
42+
if dynamic:
43+
args.append('--dynamic')
44+
args.append(library)
1545
print("+ %s" % ' '.join(args))
1646
proc = subprocess.run(args, stdout=subprocess.PIPE, universal_newlines=True)
1747
if proc.returncode:
@@ -25,12 +55,9 @@ def get_exported_symbols():
2555

2656

2757
def get_smelly_symbols(stdout):
28-
symbols = []
29-
ignored_symtypes = set()
30-
31-
allowed_prefixes = ('Py', '_Py')
32-
if sys.platform == 'darwin':
33-
allowed_prefixes += ('__Py',)
58+
smelly_symbols = []
59+
python_symbols = []
60+
local_symbols = []
3461

3562
for line in stdout.splitlines():
3663
# Split line '0000000000001b80 D PyTextIOWrapper_Type'
@@ -42,41 +69,98 @@ def get_smelly_symbols(stdout):
4269
continue
4370

4471
symtype = parts[1].strip()
45-
# Ignore private symbols.
46-
#
47-
# If lowercase, the symbol is usually local; if uppercase, the symbol
48-
# is global (external). There are however a few lowercase symbols that
49-
# are shown for special global symbols ("u", "v" and "w").
50-
if symtype.islower() and symtype not in "uvw":
51-
ignored_symtypes.add(symtype)
72+
symbol = parts[-1]
73+
result = '%s (type: %s)' % (symbol, symtype)
74+
75+
if symbol.startswith(ALLOWED_PREFIXES):
76+
python_symbols.append(result)
5277
continue
5378

54-
symbol = parts[-1]
55-
if symbol.startswith(allowed_prefixes):
79+
if is_local_symbol_type(symtype):
80+
local_symbols.append(result)
81+
elif symbol in IGNORED_SYMBOLS:
82+
local_symbols.append(result)
83+
else:
84+
smelly_symbols.append(result)
85+
86+
if local_symbols:
87+
print(f"Ignore {len(local_symbols)} local symbols")
88+
return smelly_symbols, python_symbols
89+
90+
91+
def check_library(library, dynamic=False):
92+
nm_output = get_exported_symbols(library, dynamic)
93+
smelly_symbols, python_symbols = get_smelly_symbols(nm_output)
94+
95+
if not smelly_symbols:
96+
print(f"OK: no smelly symbol found ({len(python_symbols)} Python symbols)")
97+
return 0
98+
99+
print()
100+
smelly_symbols.sort()
101+
for symbol in smelly_symbols:
102+
print("Smelly symbol: %s" % symbol)
103+
104+
print()
105+
print("ERROR: Found %s smelly symbols!" % len(smelly_symbols))
106+
return len(smelly_symbols)
107+
108+
109+
def check_extensions():
110+
print(__file__)
111+
srcdir = os.path.dirname(os.path.dirname(os.path.dirname(__file__)))
112+
filename = os.path.join(srcdir, "pybuilddir.txt")
113+
try:
114+
with open(filename, encoding="utf-8") as fp:
115+
pybuilddir = fp.readline()
116+
except FileNotFoundError:
117+
print(f"Cannot check extensions because {filename} does not exist")
118+
return True
119+
120+
print(f"Check extension modules from {pybuilddir} directory")
121+
builddir = os.path.join(srcdir, pybuilddir)
122+
nsymbol = 0
123+
for name in os.listdir(builddir):
124+
if not name.endswith(".so"):
125+
continue
126+
if IGNORED_EXTENSION in name:
127+
print()
128+
print(f"Ignore extension: {name}")
56129
continue
57-
symbol = '%s (type: %s)' % (symbol, symtype)
58-
symbols.append(symbol)
59130

60-
if ignored_symtypes:
61-
print("Ignored symbol types: %s" % ', '.join(sorted(ignored_symtypes)))
62131
print()
63-
return symbols
132+
filename = os.path.join(builddir, name)
133+
nsymbol += check_library(filename, dynamic=True)
134+
135+
return nsymbol
64136

65137

66138
def main():
67-
nm_output = get_exported_symbols()
68-
symbols = get_smelly_symbols(nm_output)
139+
# static library
140+
LIBRARY = sysconfig.get_config_var('LIBRARY')
141+
if not LIBRARY:
142+
raise Exception("failed to get LIBRARY variable from sysconfig")
143+
nsymbol = check_library(LIBRARY)
144+
145+
# dynamic library
146+
LDLIBRARY = sysconfig.get_config_var('LDLIBRARY')
147+
if not LDLIBRARY:
148+
raise Exception("failed to get LDLIBRARY variable from sysconfig")
149+
if LDLIBRARY != LIBRARY:
150+
print()
151+
nsymbol += check_library(LDLIBRARY, dynamic=True)
69152

70-
if not symbols:
71-
print("OK: no smelly symbol found")
72-
sys.exit(0)
153+
# Check extension modules like _ssl.cpython-310d-x86_64-linux-gnu.so
154+
nsymbol += check_extensions()
155+
156+
if nsymbol:
157+
print()
158+
print(f"ERROR: Found {nsymbol} smelly symbols in total!")
159+
sys.exit(1)
73160

74-
symbols.sort()
75-
for symbol in symbols:
76-
print("Smelly symbol: %s" % symbol)
77161
print()
78-
print("ERROR: Found %s smelly symbols!" % len(symbols))
79-
sys.exit(1)
162+
print(f"OK: all exported symbols of all libraries "
163+
f"are prefixed with {' or '.join(map(repr, ALLOWED_PREFIXES))}")
80164

81165

82166
if __name__ == "__main__":

0 commit comments

Comments
 (0)
0