dag: normalize modRoot to fix -trimpath leak when subPackages=["."]#129
Merged
dag: normalize modRoot to fix -trimpath leak when subPackages=["."]#129
Conversation
When modRoot = "./foo" and the main package is at the module root,
moduleRoot = "${mainSrc}/${modRoot}" produces …-main-src/./foo.
linkbinary.go passes that as srcdir; compile.go builds the -trimpath
rewrite key from it; the compiler records the kernel-canonicalized
clean cwd; objabi's prefix match fails and mainSrc lands in pclntab.
subPackages = ["./cmd/…"] never hit this — that branch goes through
filepath.Join, which normalizes.
- Hoist cleanModRoot to top-level let; use it in moduleRoot.
- linkbinary.go: filepath.Clean(m.ModuleRoot) for the sp.Path == "."
branch (defense in depth — covers any other unnormalized source).
- Add mainSrc to disallowedReferences so future leaks fail at build
time instead of silently bloating the closure.
- Regression fixture (modroot-dotslash) + flake check that builds it.
aldoborrero
force-pushed
the
aldo/fix-trimpath-modroot-leak
branch
from
1e45c4d to
d73c581
Compare
Benchmark Regression Check
Baseline: |
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
2 participants
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.
Summary
When
modRoot = "./foo"and the main package is at the module root (subPackages = ["."], the default),moduleRoot = "${mainSrc}/${modRoot}"produces…-main-src/./foo.linkbinary.gopasses that assrcdirfor thesp.Path == "."branch;compile.go:88builds the-trimpathrewrite key from it;runInsets cwd to the same dirty path but the kernel canonicalizes; the compiler records the clean path;cmd/internal/objabi's prefix match misses;mainSrclands in pclntab and the binary's closure references the source tree.subPackages = ["./cmd/…"]never hit this — that branch goes throughfilepath.Join(m.ModuleRoot, clean), which normalizes.Changes
nix/dag/default.nix— hoistcleanModRootto the top-level let; use it inmoduleRootlinkbinary.go—filepath.Clean(m.ModuleRoot)for thesp.Path == "."branch (covers any other unnormalized source ofModuleRoot)nix/dag/default.nix— addmainSrctodisallowedReferencesso future leaks fail at build time instead of silently bloating the closuretests/fixtures/modroot-dotslash/+tests/nix/modroot_dotslash_test.nix— regression fixture (modRoot = "./app", default subPackages); the disallowedReferences guard fails the build if the leak recurs, and the test additionally checksnix-store -q --referencesand runs the binaryReproduction