* [PATCH 0/2] stash: avoid sparse-index expansion for in-cone paths
@ 2026-07-20 22:31 tnyman
2026-07-20 22:31 ` [PATCH 1/2] pathspec: use match for sparse-index expansion checks tnyman
2026-07-20 22:31 ` [PATCH 2/2] stash: avoid sparse-index expansion for in-cone paths tnyman
0 siblings, 2 replies; 6+ messages in thread
From: tnyman @ 2026-07-20 22:31 UTC (permalink / raw)
To: git; +Cc: Ted Nyman, Derrick Stolee, Taylor Blau, Jeff King, Victoria Dye
From: Ted Nyman <tnyman@openai.com>
`git stash push -- <pathspec>` expands a sparse index before checking
whether the pathspec matches a tracked path. A pathspec wholly inside
the sparse-checkout cone cannot match part of a sparse-directory entry,
so that expansion needlessly makes the command proportional to the full
index size.
The first patch fixes the pathspec helper to use the parsed, prefixed
path consistently. The existing code can read past the end of the
unprefixed path for a wildcard passed to `git rm` or `git reset` from a
subdirectory; AddressSanitizer reports a heap-buffer-overflow in that
case.
The second patch uses the helper in `git stash push`, following the same
approach as bcf96cfca6 ("rm: expand the index only when necessary",
2022-08-07). It adds compatibility coverage for the supported pathspec
forms and a path-limited stash case to p2000.
On a cone-mode repository with 349,525 tracked paths and 49 sparse-index
entries, the best of three runs was:
before: 18.87s (2.93s user + 15.62s system), 4 expansions
after: 0.06s (0.01s user + 0.02s system), 0 expansions
A full-index control was unchanged (1.62s before, 1.65s after).
The series is based on 48bbf81c29 ("The 5th batch", 2026-07-19), the
current master. Focused sparse-index, stash, pathspec, rm, reset,
SHA-256, and unit-test coverage passes. Clang, GCC, and sanitizer
builds also pass.
Ted Nyman (2):
pathspec: use match for sparse-index expansion checks
stash: avoid sparse-index expansion for in-cone paths
builtin/stash.c | 4 +-
pathspec.c | 12 ++---
t/perf/p2000-sparse-operations.sh | 1 +
t/t1092-sparse-checkout-compatibility.sh | 62 ++++++++++++++++++++++++
4 files changed, 71 insertions(+), 8 deletions(-)
base-commit: 48bbf81c29ca9a4479ec7850fe206518682cdb2f
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH 1/2] pathspec: use match for sparse-index expansion checks 2026-07-20 22:31 [PATCH 0/2] stash: avoid sparse-index expansion for in-cone paths tnyman @ 2026-07-20 22:31 ` tnyman 2026-07-20 23:53 ` Taylor Blau 2026-07-20 22:31 ` [PATCH 2/2] stash: avoid sparse-index expansion for in-cone paths tnyman 1 sibling, 1 reply; 6+ messages in thread From: tnyman @ 2026-07-20 22:31 UTC (permalink / raw) To: git; +Cc: Ted Nyman, Derrick Stolee, Taylor Blau, Jeff King, Victoria Dye From: Ted Nyman <tnyman@openai.com> The pathspec parser computes `len` and `nowildcard_len` from `item.match`, which includes any prefix added when a command is run from a subdirectory. `item.original` can still contain the shorter, unprefixed argument. Using `item.original + item.nowildcard_len` in `pathspec_needs_expanded_index()` can therefore read past the end of the allocation. AddressSanitizer reports a heap-buffer-overflow for prefixed wildcard pathspecs passed to `git rm` and `git reset` with a sparse index. The mismatch dates back to 4d1cfc1351 ("reset: make --mixed sparse-aware", 2021-11-29), which introduced the helper using `item.original`. b29ad38322 ("pathspec.h: move pathspec_needs_expanded_index() from reset.c to here", 2022-08-07) later moved it to `pathspec.c` and preserved the affected comparisons. Use `item.match` consistently when checking whether a pathspec can match a sparse-directory entry. Add coverage for prefixed wildcard pathspecs so both commands keep the index sparse. Signed-off-by: Ted Nyman <tnyman@openai.com> --- pathspec.c | 12 ++++++------ t/t1092-sparse-checkout-compatibility.sh | 7 +++++++ 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/pathspec.c b/pathspec.c index f78b22709ccb67..281858f21f9c59 100644 --- a/pathspec.c +++ b/pathspec.c @@ -847,9 +847,9 @@ int pathspec_needs_expanded_index(struct index_state *istate, * - not-in-cone/bar*: may need expanded index * - **.c: may need expanded index */ - if (strspn(item.original + item.nowildcard_len, "*") == + if (strspn(item.match + item.nowildcard_len, "*") == (unsigned int)(item.len - item.nowildcard_len) && - path_in_cone_mode_sparse_checkout(item.original, istate)) + path_in_cone_mode_sparse_checkout(item.match, istate)) continue; for (pos = 0; pos < istate->cache_nr; pos++) { @@ -865,7 +865,7 @@ int pathspec_needs_expanded_index(struct index_state *istate, */ if ((unsigned int)item.nowildcard_len > ce_namelen(ce) && - !strncmp(item.original, ce->name, + !strncmp(item.match, ce->name, ce_namelen(ce))) { res = 1; break; @@ -876,13 +876,13 @@ int pathspec_needs_expanded_index(struct index_state *istate, * directory and the pathspec does not match the whole * directory, need to expand the index. */ - if (!strncmp(item.original, ce->name, item.nowildcard_len) && - wildmatch(item.original, ce->name, 0)) { + if (!strncmp(item.match, ce->name, item.nowildcard_len) && + wildmatch(item.match, ce->name, 0)) { res = 1; break; } } - } else if (!path_in_cone_mode_sparse_checkout(item.original, istate) && + } else if (!path_in_cone_mode_sparse_checkout(item.match, istate) && !matches_skip_worktree(pathspec, i, &skip_worktree_seen)) res = 1; diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index 9814431cd74aff..d0b42371663f9d 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -2119,6 +2119,13 @@ test_expect_success 'sparse index is not expanded: rm' ' ensure_not_expanded rm -r deep ' +test_expect_success 'sparse index is not expanded: prefixed wildcard pathspec' ' + init_repos && + + ensure_not_expanded -C deep rm --dry-run -- "a*" && + ensure_not_expanded -C deep reset base -- "a*" +' + test_expect_success 'grep with and --cached' ' init_repos && ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] pathspec: use match for sparse-index expansion checks 2026-07-20 22:31 ` [PATCH 1/2] pathspec: use match for sparse-index expansion checks tnyman @ 2026-07-20 23:53 ` Taylor Blau 0 siblings, 0 replies; 6+ messages in thread From: Taylor Blau @ 2026-07-20 23:53 UTC (permalink / raw) To: tnyman; +Cc: git, Derrick Stolee, Taylor Blau, Jeff King, Victoria Dye On Mon, Jul 20, 2026 at 03:31:20PM -0700, tnyman@openai.com wrote: > Using `item.original + item.nowildcard_len` in > `pathspec_needs_expanded_index()` can therefore read past the end of > the allocation. AddressSanitizer reports a heap-buffer-overflow for > prefixed wildcard pathspecs passed to `git rm` and `git reset` with a > sparse index. > > The mismatch dates back to 4d1cfc1351 ("reset: make --mixed > sparse-aware", 2021-11-29), which introduced the helper using > `item.original`. b29ad38322 ("pathspec.h: move > pathspec_needs_expanded_index() from reset.c to here", 2022-08-07) > later moved it to `pathspec.c` and preserved the affected comparisons. Nice find. I can reliably reproduce the ASan failure you described above like so: repo=$(mktemp -d /tmp/pathspec-asan.XXXXXX) trap 'rm -rf "$repo"' EXIT git init "$repo" cd "$repo" mkdir -p deep outside : >deep/a : >outside/file git add . git commit -q -m base git sparse-checkout init --cone --sparse-index git sparse-checkout set deep # From deep/, match is "deep/a*" while original is only "a*". git.compile -C deep reset HEAD -- 'a*' (where 'git.compile' points at my build, which in this case was compiled with "make SANITIZE=address"), and results in ==89470==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000001f36 at pc 0x000106a69430 bp 0x00016b07c530 sp 0x00016b07bce0 READ of size 1 at 0x602000001f36 thread T0 #0 0x000106a6942c in strspn+0x3f0 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1942c) #1 0x0001054a35b0 in pathspec_needs_expanded_index pathspec.c:850 #2 0x000104fe6c24 in read_from_tree reset.c:214 #3 0x000104fe5774 in cmd_reset reset.c:495 [...] It made me wonder whether or not this bug was trigger-able back in 4d1cfc1351. After checking out that version, I re-ran the same script and got an identical buffer overflow in the 'strspn()' call. Applying your patch and repeating the same steps results in a clean exit. > diff --git a/pathspec.c b/pathspec.c > index f78b22709ccb67..281858f21f9c59 100644 > --- a/pathspec.c > +++ b/pathspec.c > @@ -847,9 +847,9 @@ int pathspec_needs_expanded_index(struct index_state *istate, > * - not-in-cone/bar*: may need expanded index > * - **.c: may need expanded index > */ > - if (strspn(item.original + item.nowildcard_len, "*") == > + if (strspn(item.match + item.nowildcard_len, "*") == OK. The comment above is elided from the diff context, but is useful IMHO during review. Here we want to make sure that the remaining wildcard-ed portion of the pathspec element is only "*", which may need to expand the index only if we are not inside of the existing sparse checkout. But 'item.nowildcard_len' bytes ahead of 'item.original' may (at worst) point into uninitialized memory, or (at best) point at a portion of the string that is not in fact a wildcard (even if the pathspec item would not otherwise require us to expand the sparse checkout). So this makes sense. > (unsigned int)(item.len - item.nowildcard_len) && > - path_in_cone_mode_sparse_checkout(item.original, istate)) > + path_in_cone_mode_sparse_checkout(item.match, istate)) Likewise. Here I think we *might* actually be OK, but I haven't read 'path_in_cone_mode_sparse_checkout()' to know whether that's (a) true, and (b) if so, whether it's true by accident or intention. Regardless, 'item.match' makes sense here as well for the same reason. Likewise with the rest of the patch. > diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh > index 9814431cd74aff..d0b42371663f9d 100755 > --- a/t/t1092-sparse-checkout-compatibility.sh > +++ b/t/t1092-sparse-checkout-compatibility.sh > @@ -2119,6 +2119,13 @@ test_expect_success 'sparse index is not expanded: rm' ' > ensure_not_expanded rm -r deep > ' > > +test_expect_success 'sparse index is not expanded: prefixed wildcard pathspec' ' > + init_repos && > + > + ensure_not_expanded -C deep rm --dry-run -- "a*" && > + ensure_not_expanded -C deep reset base -- "a*" > +' Looks good, this is effectively the same thing as I ran in the reproduction script above. Thanks, Taylor ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 2/2] stash: avoid sparse-index expansion for in-cone paths 2026-07-20 22:31 [PATCH 0/2] stash: avoid sparse-index expansion for in-cone paths tnyman 2026-07-20 22:31 ` [PATCH 1/2] pathspec: use match for sparse-index expansion checks tnyman @ 2026-07-20 22:31 ` tnyman 2026-07-20 23:54 ` Taylor Blau 1 sibling, 1 reply; 6+ messages in thread From: tnyman @ 2026-07-20 22:31 UTC (permalink / raw) To: git; +Cc: Ted Nyman, Derrick Stolee, Taylor Blau, Jeff King, Victoria Dye From: Ted Nyman <tnyman@openai.com> `git stash push -- <pathspec>` expands a sparse index before checking whether the pathspec matches any tracked paths. This is unnecessary when the pathspec is wholly inside the sparse-checkout cone and makes a path-limited stash proportional to the size of the full index. Use `pathspec_needs_expanded_index()` to expand only when a pathspec can match part of a sparse-directory entry, as `git rm` and `git reset` already do. Keep the full-index behavior for pathspecs that need it. Add compatibility coverage for literal, prefixed, wildcard, file, multiple, staged, and missing pathspecs. Add the corresponding path-limited stash case to p2000. On a cone-mode repository with 349,525 tracked paths and 49 sparse index entries, the best of three runs changed from 18.87s to 0.06s. Trace2 reported four index expansions before this change and none after it. Signed-off-by: Ted Nyman <tnyman@openai.com> --- builtin/stash.c | 4 +- t/perf/p2000-sparse-operations.sh | 1 + t/t1092-sparse-checkout-compatibility.sh | 55 ++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 2 deletions(-) diff --git a/builtin/stash.c b/builtin/stash.c index c4809f299a313b..72c52571f8c06c 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1702,8 +1702,8 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q if (!include_untracked && ps->nr) { char *ps_matched = xcalloc(ps->nr, 1); - /* TODO: audit for interaction with sparse-index. */ - ensure_full_index(the_repository->index); + if (pathspec_needs_expanded_index(the_repository->index, ps)) + ensure_full_index(the_repository->index); for (size_t i = 0; i < the_repository->index->cache_nr; i++) ce_path_match(the_repository->index, the_repository->index->cache[i], ps, ps_matched); diff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh index aadf22bc2f0bb2..548a61cd9064bc 100755 --- a/t/perf/p2000-sparse-operations.sh +++ b/t/perf/p2000-sparse-operations.sh @@ -108,6 +108,7 @@ test_perf_on_all () { test_perf_on_all git status test_perf_on_all 'git stash && git stash pop' +test_perf_on_all "git stash push -- $SPARSE_CONE/a && git stash pop" test_perf_on_all 'echo >>new && git stash -u && git stash pop' test_perf_on_all git add -A test_perf_on_all git add . diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh index d0b42371663f9d..4140c4d8ef2436 100755 --- a/t/t1092-sparse-checkout-compatibility.sh +++ b/t/t1092-sparse-checkout-compatibility.sh @@ -1598,6 +1598,61 @@ test_expect_success 'sparse-index is not expanded: stash' ' ensure_not_expanded stash pop ' +test_expect_success 'sparse-index is not expanded: stash in-cone pathspec' ' + init_repos && + + echo unrelated >>sparse-index/deep/e && + echo literal >>sparse-index/deep/a && + ensure_not_expanded stash push -- deep/a && + test_grep ! literal sparse-index/deep/a && + test_grep unrelated sparse-index/deep/e && + ensure_not_expanded stash pop && + test_grep literal sparse-index/deep/a && + + echo prefixed >>sparse-index/deep/a && + ensure_not_expanded -C deep stash push -- a && + test_grep ! prefixed sparse-index/deep/a && + test_grep unrelated sparse-index/deep/e && + ensure_not_expanded stash pop && + test_grep prefixed sparse-index/deep/a && + + echo wildcard >>sparse-index/deep/a && + ensure_not_expanded stash push -- "deep/a*" && + test_grep ! wildcard sparse-index/deep/a && + test_grep unrelated sparse-index/deep/e && + ensure_not_expanded stash pop && + test_grep wildcard sparse-index/deep/a && + + echo pathspec-file >>sparse-index/deep/a && + echo deep/a >pathspec-file && + ensure_not_expanded stash push --pathspec-from-file=../pathspec-file && + test_grep ! pathspec-file sparse-index/deep/a && + test_grep unrelated sparse-index/deep/e && + ensure_not_expanded stash pop && + test_grep pathspec-file sparse-index/deep/a && + + echo multiple-a >>sparse-index/deep/a && + echo multiple-e >>sparse-index/deep/e && + ensure_not_expanded stash push -- deep/a deep/e && + test_grep ! multiple-a sparse-index/deep/a && + test_grep ! multiple-e sparse-index/deep/e && + ensure_not_expanded stash pop && + test_grep multiple-a sparse-index/deep/a && + test_grep multiple-e sparse-index/deep/e && + + echo staged >>sparse-index/deep/a && + git -C sparse-index add deep/a && + ensure_not_expanded stash push --staged -- deep/a && + test_grep ! staged sparse-index/deep/a && + test_grep unrelated sparse-index/deep/e && + ensure_not_expanded stash pop --index && + test_grep staged sparse-index/deep/a && + test_must_fail git -C sparse-index diff --cached --quiet -- deep/a && + + ensure_not_expanded ! stash push -- deep/does-not-exist && + test_grep "did not match any file" sparse-index-error +' + test_expect_success 'describe tested on all' ' init_repos && ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] stash: avoid sparse-index expansion for in-cone paths 2026-07-20 22:31 ` [PATCH 2/2] stash: avoid sparse-index expansion for in-cone paths tnyman @ 2026-07-20 23:54 ` Taylor Blau 2026-07-21 19:34 ` Junio C Hamano 0 siblings, 1 reply; 6+ messages in thread From: Taylor Blau @ 2026-07-20 23:54 UTC (permalink / raw) To: tnyman; +Cc: git, Derrick Stolee, Taylor Blau, Jeff King, Victoria Dye On Mon, Jul 20, 2026 at 03:31:21PM -0700, tnyman@openai.com wrote: > Signed-off-by: Ted Nyman <tnyman@openai.com> > --- > builtin/stash.c | 4 +- > t/perf/p2000-sparse-operations.sh | 1 + > t/t1092-sparse-checkout-compatibility.sh | 55 ++++++++++++++++++++++++ > 3 files changed, 58 insertions(+), 2 deletions(-) All looks reasonable, and it's very nice indeed to see another one of these /* TODO */ comments go away ;-). Very pleasant read, this series is Reviewed-by: Taylor Blau <ttaylorr@openai.com> , and looks good to me. Thanks, Taylor ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] stash: avoid sparse-index expansion for in-cone paths 2026-07-20 23:54 ` Taylor Blau @ 2026-07-21 19:34 ` Junio C Hamano 0 siblings, 0 replies; 6+ messages in thread From: Junio C Hamano @ 2026-07-21 19:34 UTC (permalink / raw) To: Taylor Blau Cc: tnyman, git, Derrick Stolee, Taylor Blau, Jeff King, Victoria Dye Taylor Blau <ttaylorr@openai.com> writes: > On Mon, Jul 20, 2026 at 03:31:21PM -0700, tnyman@openai.com wrote: >> Signed-off-by: Ted Nyman <tnyman@openai.com> >> --- >> builtin/stash.c | 4 +- >> t/perf/p2000-sparse-operations.sh | 1 + >> t/t1092-sparse-checkout-compatibility.sh | 55 ++++++++++++++++++++++++ >> 3 files changed, 58 insertions(+), 2 deletions(-) > > All looks reasonable, and it's very nice indeed to see another one of > these /* TODO */ comments go away ;-). > > Very pleasant read, this series is > > Reviewed-by: Taylor Blau <ttaylorr@openai.com> > > , and looks good to me. > > Thanks, > Taylor Thanks, both of you. Let me mark the topic for 'next'. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-21 19:35 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-20 22:31 [PATCH 0/2] stash: avoid sparse-index expansion for in-cone paths tnyman 2026-07-20 22:31 ` [PATCH 1/2] pathspec: use match for sparse-index expansion checks tnyman 2026-07-20 23:53 ` Taylor Blau 2026-07-20 22:31 ` [PATCH 2/2] stash: avoid sparse-index expansion for in-cone paths tnyman 2026-07-20 23:54 ` Taylor Blau 2026-07-21 19:34 ` Junio C Hamano
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox