[nanvix] Wave 5 audit cleanup: drop dead helpers + tidy setup_local + enable inet_pton#21
Closed
esaurez wants to merge 4 commits into
Closed
[nanvix] Wave 5 audit cleanup: drop dead helpers + tidy setup_local + enable inet_pton#21esaurez wants to merge 4 commits into
esaurez wants to merge 4 commits into
Conversation
Owner
Author
|
Update: added a 4th commit for audit finding H-1.
|
712e51a to
cf8e25c
Compare
11c1f5c to
c3612dc
Compare
After Wave 5's Group-A unbundling these per-library .a path variables are no longer referenced anywhere -- the build now picks up libssl / libcrypto / libz / libsqlite3 via -l<name> against the .so siblings in $(SYSROOT)/lib/. Removing the unused definitions makes it harder to accidentally reintroduce static linking by referencing one of them in a future LIBS rewrite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Both helpers are unused: the actual cpython build invokes `make -f Makefile.nanvix` which has its own inline CONFIGURE_ENV authoritative for CC / CFLAGS / LDFLAGS / LIBS. The dead Python copies had drifted to stale pre-Wave-7 link flags (literal -lsqlite3 -lz -lbz2 -llzma in LIBS, no DT_NEEDED awareness) and would have mis-built any caller that picked them up. Keep configure_opts() (still called from build.py) and the toolchain / docker path constants (referenced elsewhere). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Previously _asyncio, _datetime, select, _socket, _posixsubprocess, fcntl, and termios sat under the 'Modules with bundled-in-cpython C deps' section header, which is misleading -- none of them bundle a vendored .a; they are thin POSIX/syscall wrappers (plus _asyncio / _datetime) whose libc / libm symbols resolve against python.elf's .dynsym at dlopen time via --export-dynamic. Move them into a dedicated 'POSIX syscall wrappers + concurrency primitives' section that mirrors upstream Setup.stdlib.in's 'Modules with some UNIX dependencies' grouping. Purely cosmetic -- no change to the generated Setup.local except entry order within the *shared* block (which makesetup does not depend on; each module name still appears at most once). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Verified against the toolchain libposix.a via i686-nanvix-nm:
$ i686-nanvix-nm $(SYSROOT)/lib/libposix.a | grep "^.* T inet_"
00000000 T inet_addr
00000000 T inet_ntoa
00000000 T inet_ntop
00000000 T inet_pton
The previous ac_cv_func_inet_pton=no override was stale -- it dates
from before libposix gained inet_pton support. Flipping it to yes
lets cpython's configure define HAVE_INET_PTON in pyconfig.h, which
in turn enables the IPv6 fast paths in socketmodule.c (lines
1226-1268) and registers the socket.inet_pton / inet_ntop builtins
(socketmodule.c:6616+). The symbol resolves at dlopen time from
python.elf's .dynsym (libposix.a is whole-archived into the main
binary, exported via --export-dynamic).
inet_aton stays at =no -- libposix does not provide it (absent from
the nm dump above).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cf8e25c to
92abdb1
Compare
c3612dc to
9c15668
Compare
This was referenced Jun 16, 2026
Owner
Author
|
Closing in favor of splitting these unrelated cleanups into four focused PRs (each independent, each based on
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Three small upstream-alignment cleanups surfaced by the PR #20 audit against upstream cpython Linux's default
.somanagement behavior. All are independent of each other and of the in-flight Wave 5 build chain -- pure dead-code / cosmetic removal.Stacks on top of #20 (
feat/wave5-pr-c-unbundle-group-a). Merge after #20 lands.1.
Makefile.nanvix: drop dead LIBSSL/LIBCRYPTO/LIBZ/LIBSQLITE3 varsAfter Wave 5's Group-A unbundling these per-library
.apath variables are no longer referenced anywhere -- the build picks uplibssl/libcrypto/libz/libsqlite3via-l<name>against the.sosiblings in$(SYSROOT)/lib/. Removing the unused definitions makes it harder to accidentally reintroduce static linking by referencing one of them in a future LIBS rewrite.2.
.nanvix/config: drop deadtoolchain_paths/configure_envhelpersBoth helpers are unused: the actual cpython build invokes
make -f Makefile.nanvixwhich has its own inlineCONFIGURE_ENVauthoritative forCC/CFLAGS/LDFLAGS/LIBS. The dead Python copies had drifted to stale pre-Wave-7 link flags (literal-lsqlite3 -lz -lbz2 -llzmainLIBS, no DT_NEEDED awareness) and would have mis-built any caller that picked them up.configure_opts()(still called frombuild.py) and the toolchain / docker path constants stay.Closes the cleanup tracked in
nanvix-todo/cpython-config-py-dead-configure-env.md.3.
.nanvix/setup_local: split POSIX-wrapper modules into their own sectionPreviously
_asyncio,_datetime,select,_socket,_posixsubprocess,fcntl, andtermiossat under the 'Modules with bundled-in-cpython C deps' section header, which is misleading -- none of them bundle a vendored.a; they are thin POSIX/syscall wrappers (plus_asyncio/_datetime) whose libc / libm symbols resolve againstpython.elf's.dynsymatdlopentime via--export-dynamic.Move them into a dedicated 'POSIX syscall wrappers + concurrency primitives' section that mirrors upstream
Modules/Setup.stdlib.in's 'Modules with some UNIX dependencies' grouping. Purely cosmetic -- no change to the generatedSetup.localexcept entry order within the*shared*block (which makesetup does not depend on; each module name still appears at most once).Validation
./z lintpasses (black + pyright clean) after each commit.Items NOT in this PR (deferred / blocked)
_decimal/pyexpat) -- upstream cpython hardcodes literal-lminconfigure.ac:3918and:3834instead of using$(LIBM). Tracked separately for a future upstream PR againstpython/cpython.ac_cv_func_inet_aton=no/inet_pton=nooverrides may be stale) -- needsnmagainstlibposix.ato confirm; tracked for follow-up.--allow-multiple-definitionon .so builds) -- blocked by B-1; once libm no longer gets bundled, the override can be scoped back topython.elfonly.