{"thread":{"id":"66042","subject":"[PATCH 0/2] stash: avoid sparse-index expansion for in-cone paths","startedAt":"2026-07-20T22:31:24Z","lastAt":"2026-07-21T19:35:00Z","messageCount":6,"participants":["tnyman@openai.com","Taylor Blau","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"548697","messageId":"20260720223118.62821-4-tnyman@openai.com","threadId":"66042","inReplyTo":null,"subject":"[PATCH 0/2] stash: avoid sparse-index expansion for in-cone paths","fromName":"","fromEmail":"tnyman@openai.com","sentAt":"2026-07-20T22:31:19Z","receivedAt":"2026-07-20T22:31:24Z","isPatch":true,"body":"From: Ted Nyman <tnyman@openai.com>\n\n`git stash push -- <pathspec>` expands a sparse index before checking\nwhether the pathspec matches a tracked path. A pathspec wholly inside\nthe sparse-checkout cone cannot match part of a sparse-directory entry,\nso that expansion needlessly makes the command proportional to the full\nindex size.\n\nThe first patch fixes the pathspec helper to use the parsed, prefixed\npath consistently. The existing code can read past the end of the\nunprefixed path for a wildcard passed to `git rm` or `git reset` from a\nsubdirectory; AddressSanitizer reports a heap-buffer-overflow in that\ncase.\n\nThe second patch uses the helper in `git stash push`, following the same\napproach as bcf96cfca6 (\"rm: expand the index only when necessary\",\n2022-08-07). It adds compatibility coverage for the supported pathspec\nforms and a path-limited stash case to p2000.\n\nOn a cone-mode repository with 349,525 tracked paths and 49 sparse-index\nentries, the best of three runs was:\n\n  before: 18.87s (2.93s user + 15.62s system), 4 expansions\n  after:   0.06s (0.01s user +  0.02s system), 0 expansions\n\nA full-index control was unchanged (1.62s before, 1.65s after).\n\nThe series is based on 48bbf81c29 (\"The 5th batch\", 2026-07-19), the\ncurrent master. Focused sparse-index, stash, pathspec, rm, reset,\nSHA-256, and unit-test coverage passes. Clang, GCC, and sanitizer\nbuilds also pass.\n\nTed Nyman (2):\n  pathspec: use match for sparse-index expansion checks\n  stash: avoid sparse-index expansion for in-cone paths\n\n builtin/stash.c                          |  4 +-\n pathspec.c                               | 12 ++---\n t/perf/p2000-sparse-operations.sh        |  1 +\n t/t1092-sparse-checkout-compatibility.sh | 62 ++++++++++++++++++++++++\n 4 files changed, 71 insertions(+), 8 deletions(-)\n\n\nbase-commit: 48bbf81c29ca9a4479ec7850fe206518682cdb2f\n"},{"id":"548698","messageId":"20260720223118.62821-5-tnyman@openai.com","threadId":"66042","inReplyTo":"20260720223118.62821-4-tnyman@openai.com","subject":"[PATCH 1/2] pathspec: use match for sparse-index expansion checks","fromName":"","fromEmail":"tnyman@openai.com","sentAt":"2026-07-20T22:31:20Z","receivedAt":"2026-07-20T22:31:26Z","isPatch":true,"body":"From: Ted Nyman <tnyman@openai.com>\n\nThe pathspec parser computes `len` and `nowildcard_len` from\n`item.match`, which includes any prefix added when a command is run\nfrom a subdirectory. `item.original` can still contain the shorter,\nunprefixed argument.\n\nUsing `item.original + item.nowildcard_len` in\n`pathspec_needs_expanded_index()` can therefore read past the end of\nthe allocation. AddressSanitizer reports a heap-buffer-overflow for\nprefixed wildcard pathspecs passed to `git rm` and `git reset` with a\nsparse index.\n\nThe mismatch dates back to 4d1cfc1351 (\"reset: make --mixed\nsparse-aware\", 2021-11-29), which introduced the helper using\n`item.original`. b29ad38322 (\"pathspec.h: move\npathspec_needs_expanded_index() from reset.c to here\", 2022-08-07)\nlater moved it to `pathspec.c` and preserved the affected comparisons.\n\nUse `item.match` consistently when checking whether a pathspec can\nmatch a sparse-directory entry. Add coverage for prefixed wildcard\npathspecs so both commands keep the index sparse.\n\nSigned-off-by: Ted Nyman <tnyman@openai.com>\n---\n pathspec.c                               | 12 ++++++------\n t/t1092-sparse-checkout-compatibility.sh |  7 +++++++\n 2 files changed, 13 insertions(+), 6 deletions(-)\n\ndiff --git a/pathspec.c b/pathspec.c\nindex f78b22709ccb67..281858f21f9c59 100644\n--- a/pathspec.c\n+++ b/pathspec.c\n@@ -847,9 +847,9 @@ int pathspec_needs_expanded_index(struct index_state *istate,\n \t\t\t * - not-in-cone/bar*: may need expanded index\n \t\t\t * - **.c: may need expanded index\n \t\t\t */\n-\t\t\tif (strspn(item.original + item.nowildcard_len, \"*\") ==\n+\t\t\tif (strspn(item.match + item.nowildcard_len, \"*\") ==\n \t\t\t\t    (unsigned int)(item.len - item.nowildcard_len) &&\n-\t\t\t    path_in_cone_mode_sparse_checkout(item.original, istate))\n+\t\t\t    path_in_cone_mode_sparse_checkout(item.match, istate))\n \t\t\t\tcontinue;\n \n \t\t\tfor (pos = 0; pos < istate->cache_nr; pos++) {\n@@ -865,7 +865,7 @@ int pathspec_needs_expanded_index(struct index_state *istate,\n \t\t\t\t */\n \t\t\t\tif ((unsigned int)item.nowildcard_len >\n \t\t\t\t\t    ce_namelen(ce) &&\n-\t\t\t\t    !strncmp(item.original, ce->name,\n+\t\t\t\t    !strncmp(item.match, ce->name,\n \t\t\t\t\t     ce_namelen(ce))) {\n \t\t\t\t\tres = 1;\n \t\t\t\t\tbreak;\n@@ -876,13 +876,13 @@ int pathspec_needs_expanded_index(struct index_state *istate,\n \t\t\t\t * directory and the pathspec does not match the whole\n \t\t\t\t * directory, need to expand the index.\n \t\t\t\t */\n-\t\t\t\tif (!strncmp(item.original, ce->name, item.nowildcard_len) &&\n-\t\t\t\t    wildmatch(item.original, ce->name, 0)) {\n+\t\t\t\tif (!strncmp(item.match, ce->name, item.nowildcard_len) &&\n+\t\t\t\t    wildmatch(item.match, ce->name, 0)) {\n \t\t\t\t\tres = 1;\n \t\t\t\t\tbreak;\n \t\t\t\t}\n \t\t\t}\n-\t\t} else if (!path_in_cone_mode_sparse_checkout(item.original, istate) &&\n+\t\t} else if (!path_in_cone_mode_sparse_checkout(item.match, istate) &&\n \t\t\t   !matches_skip_worktree(pathspec, i, &skip_worktree_seen))\n \t\t\tres = 1;\n \ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex 9814431cd74aff..d0b42371663f9d 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -2119,6 +2119,13 @@ test_expect_success 'sparse index is not expanded: rm' '\n \tensure_not_expanded rm -r deep\n '\n \n+test_expect_success 'sparse index is not expanded: prefixed wildcard pathspec' '\n+\tinit_repos &&\n+\n+\tensure_not_expanded -C deep rm --dry-run -- \"a*\" &&\n+\tensure_not_expanded -C deep reset base -- \"a*\"\n+'\n+\n test_expect_success 'grep with and --cached' '\n \tinit_repos &&\n \n"},{"id":"548699","messageId":"20260720223118.62821-6-tnyman@openai.com","threadId":"66042","inReplyTo":"20260720223118.62821-4-tnyman@openai.com","subject":"[PATCH 2/2] stash: avoid sparse-index expansion for in-cone paths","fromName":"","fromEmail":"tnyman@openai.com","sentAt":"2026-07-20T22:31:21Z","receivedAt":"2026-07-20T22:31:29Z","isPatch":true,"body":"From: Ted Nyman <tnyman@openai.com>\n\n`git stash push -- <pathspec>` expands a sparse index before checking\nwhether the pathspec matches any tracked paths. This is unnecessary\nwhen the pathspec is wholly inside the sparse-checkout cone and makes\na path-limited stash proportional to the size of the full index.\n\nUse `pathspec_needs_expanded_index()` to expand only when a pathspec\ncan match part of a sparse-directory entry, as `git rm` and `git\nreset` already do. Keep the full-index behavior for pathspecs that\nneed it.\n\nAdd compatibility coverage for literal, prefixed, wildcard, file,\nmultiple, staged, and missing pathspecs. Add the corresponding\npath-limited stash case to p2000.\n\nOn a cone-mode repository with 349,525 tracked paths and 49 sparse\nindex entries, the best of three runs changed from 18.87s to 0.06s.\nTrace2 reported four index expansions before this change and none\nafter it.\n\nSigned-off-by: Ted Nyman <tnyman@openai.com>\n---\n builtin/stash.c                          |  4 +-\n t/perf/p2000-sparse-operations.sh        |  1 +\n t/t1092-sparse-checkout-compatibility.sh | 55 ++++++++++++++++++++++++\n 3 files changed, 58 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/stash.c b/builtin/stash.c\nindex c4809f299a313b..72c52571f8c06c 100644\n--- a/builtin/stash.c\n+++ b/builtin/stash.c\n@@ -1702,8 +1702,8 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q\n \tif (!include_untracked && ps->nr) {\n \t\tchar *ps_matched = xcalloc(ps->nr, 1);\n \n-\t\t/* TODO: audit for interaction with sparse-index. */\n-\t\tensure_full_index(the_repository->index);\n+\t\tif (pathspec_needs_expanded_index(the_repository->index, ps))\n+\t\t\tensure_full_index(the_repository->index);\n \t\tfor (size_t i = 0; i < the_repository->index->cache_nr; i++)\n \t\t\tce_path_match(the_repository->index, the_repository->index->cache[i], ps,\n \t\t\t\t      ps_matched);\ndiff --git a/t/perf/p2000-sparse-operations.sh b/t/perf/p2000-sparse-operations.sh\nindex aadf22bc2f0bb2..548a61cd9064bc 100755\n--- a/t/perf/p2000-sparse-operations.sh\n+++ b/t/perf/p2000-sparse-operations.sh\n@@ -108,6 +108,7 @@ test_perf_on_all () {\n \n test_perf_on_all git status\n test_perf_on_all 'git stash && git stash pop'\n+test_perf_on_all \"git stash push -- $SPARSE_CONE/a && git stash pop\"\n test_perf_on_all 'echo >>new && git stash -u && git stash pop'\n test_perf_on_all git add -A\n test_perf_on_all git add .\ndiff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\nindex d0b42371663f9d..4140c4d8ef2436 100755\n--- a/t/t1092-sparse-checkout-compatibility.sh\n+++ b/t/t1092-sparse-checkout-compatibility.sh\n@@ -1598,6 +1598,61 @@ test_expect_success 'sparse-index is not expanded: stash' '\n \tensure_not_expanded stash pop\n '\n \n+test_expect_success 'sparse-index is not expanded: stash in-cone pathspec' '\n+\tinit_repos &&\n+\n+\techo unrelated >>sparse-index/deep/e &&\n+\techo literal >>sparse-index/deep/a &&\n+\tensure_not_expanded stash push -- deep/a &&\n+\ttest_grep ! literal sparse-index/deep/a &&\n+\ttest_grep unrelated sparse-index/deep/e &&\n+\tensure_not_expanded stash pop &&\n+\ttest_grep literal sparse-index/deep/a &&\n+\n+\techo prefixed >>sparse-index/deep/a &&\n+\tensure_not_expanded -C deep stash push -- a &&\n+\ttest_grep ! prefixed sparse-index/deep/a &&\n+\ttest_grep unrelated sparse-index/deep/e &&\n+\tensure_not_expanded stash pop &&\n+\ttest_grep prefixed sparse-index/deep/a &&\n+\n+\techo wildcard >>sparse-index/deep/a &&\n+\tensure_not_expanded stash push -- \"deep/a*\" &&\n+\ttest_grep ! wildcard sparse-index/deep/a &&\n+\ttest_grep unrelated sparse-index/deep/e &&\n+\tensure_not_expanded stash pop &&\n+\ttest_grep wildcard sparse-index/deep/a &&\n+\n+\techo pathspec-file >>sparse-index/deep/a &&\n+\techo deep/a >pathspec-file &&\n+\tensure_not_expanded stash push --pathspec-from-file=../pathspec-file &&\n+\ttest_grep ! pathspec-file sparse-index/deep/a &&\n+\ttest_grep unrelated sparse-index/deep/e &&\n+\tensure_not_expanded stash pop &&\n+\ttest_grep pathspec-file sparse-index/deep/a &&\n+\n+\techo multiple-a >>sparse-index/deep/a &&\n+\techo multiple-e >>sparse-index/deep/e &&\n+\tensure_not_expanded stash push -- deep/a deep/e &&\n+\ttest_grep ! multiple-a sparse-index/deep/a &&\n+\ttest_grep ! multiple-e sparse-index/deep/e &&\n+\tensure_not_expanded stash pop &&\n+\ttest_grep multiple-a sparse-index/deep/a &&\n+\ttest_grep multiple-e sparse-index/deep/e &&\n+\n+\techo staged >>sparse-index/deep/a &&\n+\tgit -C sparse-index add deep/a &&\n+\tensure_not_expanded stash push --staged -- deep/a &&\n+\ttest_grep ! staged sparse-index/deep/a &&\n+\ttest_grep unrelated sparse-index/deep/e &&\n+\tensure_not_expanded stash pop --index &&\n+\ttest_grep staged sparse-index/deep/a &&\n+\ttest_must_fail git -C sparse-index diff --cached --quiet -- deep/a &&\n+\n+\tensure_not_expanded ! stash push -- deep/does-not-exist &&\n+\ttest_grep \"did not match any file\" sparse-index-error\n+'\n+\n test_expect_success 'describe tested on all' '\n \tinit_repos &&\n \n"},{"id":"548706","messageId":"al61ERa3fS2MerHp@com-79390","threadId":"66042","inReplyTo":"20260720223118.62821-5-tnyman@openai.com","subject":"Re: [PATCH 1/2] pathspec: use match for sparse-index expansion checks","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-20T23:53:53Z","receivedAt":"2026-07-20T23:53:58Z","isPatch":true,"body":"On Mon, Jul 20, 2026 at 03:31:20PM -0700, tnyman@openai.com wrote:\n> Using `item.original + item.nowildcard_len` in\n> `pathspec_needs_expanded_index()` can therefore read past the end of\n> the allocation. AddressSanitizer reports a heap-buffer-overflow for\n> prefixed wildcard pathspecs passed to `git rm` and `git reset` with a\n> sparse index.\n>\n> The mismatch dates back to 4d1cfc1351 (\"reset: make --mixed\n> sparse-aware\", 2021-11-29), which introduced the helper using\n> `item.original`. b29ad38322 (\"pathspec.h: move\n> pathspec_needs_expanded_index() from reset.c to here\", 2022-08-07)\n> later moved it to `pathspec.c` and preserved the affected comparisons.\n\nNice find. I can reliably reproduce the ASan failure you described above\nlike so:\n\n    repo=$(mktemp -d /tmp/pathspec-asan.XXXXXX)\n    trap 'rm -rf \"$repo\"' EXIT\n\n    git init \"$repo\"\n\n    cd \"$repo\"\n\n    mkdir -p deep outside\n    : >deep/a\n    : >outside/file\n    git add .\n    git commit -q -m base\n\n    git sparse-checkout init --cone --sparse-index\n    git sparse-checkout set deep\n\n    # From deep/, match is \"deep/a*\" while original is only \"a*\".\n    git.compile -C deep reset HEAD -- 'a*'\n\n(where 'git.compile' points at my build, which in this case was compiled\nwith \"make SANITIZE=address\"), and results in\n\n    ==89470==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x602000001f36 at pc 0x000106a69430 bp 0x00016b07c530 sp 0x00016b07bce0\n    READ of size 1 at 0x602000001f36 thread T0\n        #0 0x000106a6942c in strspn+0x3f0 (libclang_rt.asan_osx_dynamic.dylib:arm64e+0x1942c)\n        #1 0x0001054a35b0 in pathspec_needs_expanded_index pathspec.c:850\n        #2 0x000104fe6c24 in read_from_tree reset.c:214\n        #3 0x000104fe5774 in cmd_reset reset.c:495\n    [...]\n\nIt made me wonder whether or not this bug was trigger-able back in\n4d1cfc1351. After checking out that version, I re-ran the same script\nand got an identical buffer overflow in the 'strspn()' call.\n\nApplying your patch and repeating the same steps results in a clean\nexit.\n\n> diff --git a/pathspec.c b/pathspec.c\n> index f78b22709ccb67..281858f21f9c59 100644\n> --- a/pathspec.c\n> +++ b/pathspec.c\n> @@ -847,9 +847,9 @@ int pathspec_needs_expanded_index(struct index_state *istate,\n>  \t\t\t * - not-in-cone/bar*: may need expanded index\n>  \t\t\t * - **.c: may need expanded index\n>  \t\t\t */\n> -\t\t\tif (strspn(item.original + item.nowildcard_len, \"*\") ==\n> +\t\t\tif (strspn(item.match + item.nowildcard_len, \"*\") ==\n\nOK. The comment above is elided from the diff context, but is useful\nIMHO during review. Here we want to make sure that the remaining\nwildcard-ed portion of the pathspec element is only \"*\", which may need\nto expand the index only if we are not inside of the existing sparse\ncheckout.\n\nBut 'item.nowildcard_len' bytes ahead of 'item.original' may (at worst)\npoint into uninitialized memory, or (at best) point at a portion of the\nstring that is not in fact a wildcard (even if the pathspec item would\nnot otherwise require us to expand the sparse checkout).\n\nSo this makes sense.\n\n>  \t\t\t\t    (unsigned int)(item.len - item.nowildcard_len) &&\n> -\t\t\t    path_in_cone_mode_sparse_checkout(item.original, istate))\n> +\t\t\t    path_in_cone_mode_sparse_checkout(item.match, istate))\n\nLikewise. Here I think we *might* actually be OK, but I haven't read\n'path_in_cone_mode_sparse_checkout()' to know whether that's (a) true,\nand (b) if so, whether it's true by accident or intention.\n\nRegardless, 'item.match' makes sense here as well for the same reason.\nLikewise with the rest of the patch.\n\n> diff --git a/t/t1092-sparse-checkout-compatibility.sh b/t/t1092-sparse-checkout-compatibility.sh\n> index 9814431cd74aff..d0b42371663f9d 100755\n> --- a/t/t1092-sparse-checkout-compatibility.sh\n> +++ b/t/t1092-sparse-checkout-compatibility.sh\n> @@ -2119,6 +2119,13 @@ test_expect_success 'sparse index is not expanded: rm' '\n>  \tensure_not_expanded rm -r deep\n>  '\n>\n> +test_expect_success 'sparse index is not expanded: prefixed wildcard pathspec' '\n> +\tinit_repos &&\n> +\n> +\tensure_not_expanded -C deep rm --dry-run -- \"a*\" &&\n> +\tensure_not_expanded -C deep reset base -- \"a*\"\n> +'\n\nLooks good, this is effectively the same thing as I ran in the\nreproduction script above.\n\nThanks,\nTaylor\n"},{"id":"548707","messageId":"al61UTM0aK9j9eiP@com-79390","threadId":"66042","inReplyTo":"20260720223118.62821-6-tnyman@openai.com","subject":"Re: [PATCH 2/2] stash: avoid sparse-index expansion for in-cone paths","fromName":"Taylor Blau","fromEmail":"ttaylorr@openai.com","sentAt":"2026-07-20T23:54:57Z","receivedAt":"2026-07-20T23:55:01Z","isPatch":true,"body":"On Mon, Jul 20, 2026 at 03:31:21PM -0700, tnyman@openai.com wrote:\n> Signed-off-by: Ted Nyman <tnyman@openai.com>\n> ---\n>  builtin/stash.c                          |  4 +-\n>  t/perf/p2000-sparse-operations.sh        |  1 +\n>  t/t1092-sparse-checkout-compatibility.sh | 55 ++++++++++++++++++++++++\n>  3 files changed, 58 insertions(+), 2 deletions(-)\n\nAll looks reasonable, and it's very nice indeed to see another one of\nthese /* TODO */ comments go away ;-).\n\nVery pleasant read, this series is\n\n    Reviewed-by: Taylor Blau <ttaylorr@openai.com>\n\n, and looks good to me.\n\nThanks,\nTaylor\n"},{"id":"548735","messageId":"xmqqik68w2zi.fsf@gitster.g","threadId":"66042","inReplyTo":"al61UTM0aK9j9eiP@com-79390","subject":"Re: [PATCH 2/2] stash: avoid sparse-index expansion for in-cone paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-07-21T19:34:57Z","receivedAt":"2026-07-21T19:35:00Z","isPatch":true,"body":"Taylor Blau <ttaylorr@openai.com> writes:\n\n> On Mon, Jul 20, 2026 at 03:31:21PM -0700, tnyman@openai.com wrote:\n>> Signed-off-by: Ted Nyman <tnyman@openai.com>\n>> ---\n>>  builtin/stash.c                          |  4 +-\n>>  t/perf/p2000-sparse-operations.sh        |  1 +\n>>  t/t1092-sparse-checkout-compatibility.sh | 55 ++++++++++++++++++++++++\n>>  3 files changed, 58 insertions(+), 2 deletions(-)\n>\n> All looks reasonable, and it's very nice indeed to see another one of\n> these /* TODO */ comments go away ;-).\n>\n> Very pleasant read, this series is\n>\n>     Reviewed-by: Taylor Blau <ttaylorr@openai.com>\n>\n> , and looks good to me.\n>\n> Thanks,\n> Taylor\n\nThanks, both of you.  Let me mark the topic for 'next'.\n"}]}