git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
	Phillip Wood via GitGitGadget <gitgitgadget@gmail.com>
Cc: Derrick Stolee <dstolee@microsoft.com>,
	git@vger.kernel.org,
	Philippe Blain <levraiphilippeblain@gmail.com>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Elijah Newren <newren@gmail.com>,
	Phillip Wood <phillip.wood@dunelm.org.uk>
Subject: Re: [PATCH 00/11] rebase: reset_head() related fixes and improvements
Date: Sat, 2 Oct 2021 13:27:28 +0100	[thread overview]
Message-ID: <ddf5e9c2-3211-cec2-cd18-2a083e279ccb@gmail.com> (raw)
In-Reply-To: <xmqq4ka02fc4.fsf@gitster.g>

Hi Junio

On 02/10/2021 05:58, Junio C Hamano wrote:
> Junio C Hamano <gitster@pobox.com> writes:
> 
>> When merged with other topics in flight in 'seen', this seems to
>> fail the t1092 test (most likely, ds/add-rm-with-sparse-index is
>> what this interacts badly with).

Oh dear, thanks for letting me know.

> There are Two CI runs at GitHub:
> 
>   - https://github.com/git/git/actions/runs/1297117791 (d3a1c4e)
>   - https://github.com/git/git/actions/runs/1297232973 (a1108c2)
> 
> The difference between the former (which passes all the tests) and
> he latter (which fails) are only two topics:
> 
>      $ git log --first-parent --oneline d3a1c4e..a1108c2
>      a1108c2b1b Merge branch 'hn/reftable' into seen
>      e575f29006 Merge branch 'pw/fix-some-issues-in-reset-head' into seen
> 
> I think the following is the same failure I saw earlier
> 
>     https://github.com/git/git/runs/3773780195?check_suite_focus=true#step:6:5033
> 
> that the write-tree codepath hits assertion failure by detecting a
> corruption in the cache-tree data structure.

The test that fails (t1092-sparse-checkout-compatibility.sh:'merge, 
cherry-pick, and rebase') was introduced by c0b99303db ("t1092: add 
cherry-pick, rebase tests", 2021-09-08) and is in v2.33.0. It does not 
test the "apply" backend of rebase, changing it to do so makes it fail 
on the current master as that backend already uses reset_head() for the 
initial checkout so it is an existing bug that is exposed by the changes 
in this series. It seems to be a use-after-free (see below) I'll try and 
investigate but I've got no idea what is going on at the moment - the 
index is not my area of expertise.

Best Wishes

Phillip

==74345==ERROR: AddressSanitizer: heap-use-after-free on address 
0x606000001b20 at pc 0x557cbe82d3a2 bp 0x7ffdfee08090 sp 0x7ffdfee08080
READ of size 4 at 0x606000001b20 thread T0
     #0 0x557cbe82d3a1 in verify_one /home/phil/src/git/cache-tree.c:863
     #1 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #2 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #3 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #4 0x557cbe830a2b in cache_tree_verify 
/home/phil/src/git/cache-tree.c:910
     #5 0x557cbea53741 in write_locked_index 
/home/phil/src/git/read-cache.c:3250
     #6 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
     #7 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
     #8 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
     #9 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
     #10 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
     #11 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
     #12 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
     #13 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)
     #14 0x557cbe5bcb8d in _start (/home/phil/src/git/git+0x1b9b8d)

0x606000001b20 is located 0 bytes inside of 56-byte region 
[0x606000001b20,0x606000001b58)
freed by thread T0 here:
     #0 0x7fdd4bacff19 in __interceptor_free 
/build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:127
     #1 0x557cbe82af60 in cache_tree_free /home/phil/src/git/cache-tree.c:35
     #2 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
     #3 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
     #4 0x557cbe82aee5 in cache_tree_free /home/phil/src/git/cache-tree.c:31
     #5 0x557cbeb2557a in ensure_full_index 
/home/phil/src/git/sparse-index.c:310
     #6 0x557cbea45c4a in index_name_stage_pos 
/home/phil/src/git/read-cache.c:588
     #7 0x557cbe82ce37 in verify_one /home/phil/src/git/cache-tree.c:850
     #8 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #9 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #10 0x557cbe82ca9d in verify_one /home/phil/src/git/cache-tree.c:840
     #11 0x557cbe830a2b in cache_tree_verify 
/home/phil/src/git/cache-tree.c:910
     #12 0x557cbea53741 in write_locked_index 
/home/phil/src/git/read-cache.c:3250
     #13 0x557cbeab7fdd in reset_head /home/phil/src/git/reset.c:87
     #14 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
     #15 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
     #16 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
     #17 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
     #18 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
     #19 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
     #20 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

previously allocated by thread T0 here:
     #0 0x7fdd4bad0459 in __interceptor_calloc 
/build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:154
     #1 0x557cbebc1807 in xcalloc /home/phil/src/git/wrapper.c:140
     #2 0x557cbe82b7d8 in cache_tree /home/phil/src/git/cache-tree.c:17
     #3 0x557cbe82b7d8 in prime_cache_tree_rec 
/home/phil/src/git/cache-tree.c:763
     #4 0x557cbe82b837 in prime_cache_tree_rec 
/home/phil/src/git/cache-tree.c:764
     #5 0x557cbe82b837 in prime_cache_tree_rec 
/home/phil/src/git/cache-tree.c:764
     #6 0x557cbe8304e1 in prime_cache_tree 
/home/phil/src/git/cache-tree.c:779
     #7 0x557cbeab7fa7 in reset_head /home/phil/src/git/reset.c:85
     #8 0x557cbe72147f in cmd_rebase builtin/rebase.c:2074
     #9 0x557cbe5bd151 in run_builtin /home/phil/src/git/git.c:461
     #10 0x557cbe5bd151 in handle_builtin /home/phil/src/git/git.c:714
     #11 0x557cbe5c0503 in run_argv /home/phil/src/git/git.c:781
     #12 0x557cbe5c0503 in cmd_main /home/phil/src/git/git.c:912
     #13 0x557cbe5bad28 in main /home/phil/src/git/common-main.c:52
     #14 0x7fdd4b82eb24 in __libc_start_main (/usr/lib/libc.so.6+0x27b24)

SUMMARY: AddressSanitizer: heap-use-after-free 
/home/phil/src/git/cache-tree.c:863 in verify_one
Shadow bytes around the buggy address:
   0x0c0c7fff8310: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
   0x0c0c7fff8320: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fa
   0x0c0c7fff8330: fa fa fa fa 00 00 00 00 00 00 00 02 fa fa fa fa
   0x0c0c7fff8340: fd fd fd fd fd fd fd fa fa fa fa fa 00 00 00 00
   0x0c0c7fff8350: 00 00 00 02 fa fa fa fa fd fd fd fd fd fd fd fa
=>0x0c0c7fff8360: fa fa fa fa[fd]fd fd fd fd fd fd fa fa fa fa fa
   0x0c0c7fff8370: 00 00 00 00 00 00 00 02 fa fa fa fa fd fd fd fd
   0x0c0c7fff8380: fd fd fd fa fa fa fa fa 00 00 00 00 00 00 00 02
   0x0c0c7fff8390: fa fa fa fa fd fd fd fd fd fd fd fa fa fa fa fa
   0x0c0c7fff83a0: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
   0x0c0c7fff83b0: fd fd fd fa fa fa fa fa 00 00 00 00 00 00 00 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
   Addressable:           00
   Partially addressable: 01 02 03 04 05 06 07
   Heap left redzone:       fa
   Freed heap region:       fd
   Stack left redzone:      f1
   Stack mid redzone:       f2
   Stack right redzone:     f3
   Stack after return:      f5
   Stack use after scope:   f8
   Global redzone:          f9
   Global init order:       f6
   Poisoned by user:        f7
   Container overflow:      fc
   Array cookie:            ac
   Intra object redzone:    bb
   ASan internal:           fe
   Left alloca redzone:     ca
   Right alloca redzone:    cb
   Shadow gap:              cc
==74345==ABORTING


> e575f29006 (i.e. without the reftable topic) fails t1092.  If you
> revert e575f29006^2 (i.e. the "do not fork 'git checkout'") from
> that merge, all tests pass including t1092.
> 
> The reftable topic is queued near the tip of 'seen' not necessarily
> because it _breaks_ CI (I do not think it does), but it needed a
> handful of fixup commits on top.  The topic needs rerolling with the
> fixes squashed in.
> 
> Thanks.
> 


  reply	other threads:[~2021-10-02 12:27 UTC|newest]

Thread overview: 85+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-01 10:04 [PATCH 00/11] rebase: reset_head() related fixes and improvements Phillip Wood via GitGitGadget
2021-10-01 10:04 ` [PATCH 01/11] rebase: factor out checkout for up to date branch Phillip Wood via GitGitGadget
2021-10-01 10:04 ` [PATCH 02/11] reset_head(): fix checkout Phillip Wood via GitGitGadget
2021-10-01 20:26   ` Junio C Hamano
2021-10-04  9:58     ` Phillip Wood
2021-10-04 16:13       ` Junio C Hamano
2021-10-01 22:47   ` Eric Sunshine
2021-10-01 10:04 ` [PATCH 03/11] reset_head(): don't run checkout hook if there is an error Phillip Wood via GitGitGadget
2021-10-01 20:52   ` Junio C Hamano
2021-10-04 10:00     ` Phillip Wood
2021-10-12  8:48       ` Ævar Arnfjörð Bjarmason
2021-10-01 10:04 ` [PATCH 04/11] reset_head(): remove action parameter Phillip Wood via GitGitGadget
2021-10-01 20:58   ` Junio C Hamano
2021-10-04 10:00     ` Phillip Wood
2021-10-01 10:04 ` [PATCH 05/11] reset_head(): factor out ref updates Phillip Wood via GitGitGadget
2021-10-01 21:00   ` Junio C Hamano
2021-10-04 10:03     ` Phillip Wood
2021-10-01 10:04 ` [PATCH 06/11] reset_head(): make default_reflog_action optional Phillip Wood via GitGitGadget
2021-10-01 21:03   ` Junio C Hamano
2021-10-01 21:08   ` Junio C Hamano
2021-10-04 10:03     ` Phillip Wood
2021-10-01 10:04 ` [PATCH 07/11] rebase: cleanup reset_head() calls Phillip Wood via GitGitGadget
2021-10-01 10:04 ` [PATCH 08/11] reset_head(): take struct rebase_head_opts Phillip Wood via GitGitGadget
2021-10-01 21:11   ` Junio C Hamano
2021-10-04 10:09     ` Phillip Wood
2021-10-01 10:05 ` [PATCH 09/11] rebase --apply: fix reflog Phillip Wood via GitGitGadget
2021-10-01 21:12   ` Junio C Hamano
2021-10-01 10:05 ` [PATCH 10/11] rebase --apply: set ORIG_HEAD correctly Phillip Wood via GitGitGadget
2021-10-01 21:18   ` Junio C Hamano
2021-10-01 10:05 ` [PATCH 11/11] rebase -m: don't fork git checkout Phillip Wood via GitGitGadget
2021-10-02  0:38 ` [PATCH 00/11] rebase: reset_head() related fixes and improvements Junio C Hamano
2021-10-02  4:58   ` Junio C Hamano
2021-10-02 12:27     ` Phillip Wood [this message]
2021-10-02 13:12       ` Phillip Wood
2021-10-02 13:38       ` René Scharfe
2021-10-06 14:03         ` Phillip Wood
2021-12-08 14:57 ` [PATCH v2 00/14] " Phillip Wood via GitGitGadget
2021-12-08 14:57   ` [PATCH v2 01/14] rebase: factor out checkout for up to date branch Phillip Wood via GitGitGadget
2021-12-09 21:04     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 02/14] t5403: refactor rebase post-checkout hook tests Phillip Wood via GitGitGadget
2021-12-09 18:24     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 03/14] rebase: pass correct arguments to post-checkout hook Phillip Wood via GitGitGadget
2021-12-09 18:53     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 04/14] rebase: do not remove untracked files on checkout Phillip Wood via GitGitGadget
2021-12-09 19:09     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 05/14] rebase --apply: don't run post-checkout hook if there is an error Phillip Wood via GitGitGadget
2021-12-08 14:57   ` [PATCH v2 06/14] reset_head(): remove action parameter Phillip Wood via GitGitGadget
2021-12-08 14:57   ` [PATCH v2 07/14] create_autostash(): remove unneeded parameter Phillip Wood via GitGitGadget
2021-12-09 19:17     ` Junio C Hamano
2022-01-25 11:06       ` Phillip Wood
2021-12-08 14:57   ` [PATCH v2 08/14] reset_head(): factor out ref updates Phillip Wood via GitGitGadget
2021-12-08 14:57   ` [PATCH v2 09/14] reset_head(): make default_reflog_action optional Phillip Wood via GitGitGadget
2021-12-09 19:23     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 10/14] rebase: cleanup reset_head() calls Phillip Wood via GitGitGadget
2021-12-09 19:26     ` Junio C Hamano
2022-01-25 11:07       ` Phillip Wood
2021-12-08 14:57   ` [PATCH v2 11/14] reset_head(): take struct rebase_head_opts Phillip Wood via GitGitGadget
2021-12-09 19:31     ` Junio C Hamano
2021-12-08 14:57   ` [PATCH v2 12/14] rebase --apply: fix reflog Phillip Wood via GitGitGadget
2021-12-09 20:49     ` Junio C Hamano
2021-12-08 14:58   ` [PATCH v2 13/14] rebase --apply: set ORIG_HEAD correctly Phillip Wood via GitGitGadget
2021-12-11 10:59     ` Elijah Newren
2021-12-08 14:58   ` [PATCH v2 14/14] rebase -m: don't fork git checkout Phillip Wood via GitGitGadget
2021-12-09 21:04   ` [PATCH v2 00/14] rebase: reset_head() related fixes and improvements Junio C Hamano
2022-01-26 10:53     ` Phillip Wood
2022-01-27 17:37       ` Junio C Hamano
2021-12-11 11:05   ` Elijah Newren
2022-01-26 13:05   ` [PATCH v3 " Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 01/14] rebase: factor out checkout for up to date branch Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 02/14] t5403: refactor rebase post-checkout hook tests Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 03/14] rebase: pass correct arguments to post-checkout hook Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 04/14] rebase: do not remove untracked files on checkout Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 05/14] rebase --apply: don't run post-checkout hook if there is an error Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 06/14] reset_head(): remove action parameter Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 07/14] reset_head(): factor out ref updates Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 08/14] reset_head(): make default_reflog_action optional Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 09/14] create_autostash(): remove unneeded parameter Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 10/14] rebase: cleanup reset_head() calls Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 11/14] reset_head(): take struct rebase_head_opts Phillip Wood via GitGitGadget
2022-01-26 13:35       ` Ævar Arnfjörð Bjarmason
2022-01-26 14:52         ` Phillip Wood
2022-01-26 13:05     ` [PATCH v3 12/14] rebase --apply: fix reflog Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 13/14] rebase --apply: set ORIG_HEAD correctly Phillip Wood via GitGitGadget
2022-01-26 13:05     ` [PATCH v3 14/14] rebase -m: don't fork git checkout Phillip Wood via GitGitGadget
2022-02-01 17:03     ` [PATCH v3 00/14] rebase: reset_head() related fixes and improvements Elijah Newren

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ddf5e9c2-3211-cec2-cd18-2a083e279ccb@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=levraiphilippeblain@gmail.com \
    --cc=newren@gmail.com \
    --cc=phillip.wood@dunelm.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).