Re: Error in `git': free(): invalid pointer (was Re: [PATCH] sequencer: improve config handling)

[Kaartic Sivaraam <kaartic.sivaraam@gmail.com>]

On Thu, 2017-12-21 at 15:06 +0100, Johannes Schindelin wrote:
> Hi Kaartic,
> I fear that the strace does not cover the `git-rebase` process nor the
> `git-rebase--helper` process (which must have been part of your run, as
> the commit affected only that part of the rebase operation).
> 

Yep. It striked me only later that the entry point of "git rebase" is
essentially a script.


> If you have valgrind installed, you may want to give it a try. Since
> git-rebase is (still, much to my dislike) a Unix shell script, you will
> have to work quite hard to get it to valgrind correctly.
> 

You're right about that it's quite hard to get to the right point of
the issue when scripts are involved.


> This is how I typically do it: 
<snip>
> Then, I look for that call (I imagine it is the `exec git rebase--helper
> ${force_rebase:+--no-ff} --continue` line), then copy-edit it and guard it
> by an environment variable:
> 
> 	test -z "$DDDD" || {
> 		valgrind git rebase--helper ${force_rebase:+--no-ff} \
> 			--continue
> 		exit
> 	}
> 	exec git rebase--helper ${force_rebase:+--no-ff} --continue
> 
> After that, you can run the same command, and trigger the new code path by
> that environment variable:
> 
> 	DDDD=1 /mnt/Source/Git/git rebase -i HEAD~10
> 
> That way, you can keep the rest of your Git calls be unaffected by the
> patch.
> 

Thanks for detailing the way to get the right information for this
issue. That was intriguing.


> BTW from your invocation, I imagine that you wanted to actually run your
> Git just-built-from-source, in-place. But for that, you would need to pass
> the --exec-path option, too, like so:
> 
> 	DDDD=1 /mnt/Source/Git/git --exec-path=/mnt/Source/Git \
> 		rebase -i HEAD~10
> 
> That way, you could edit the git-rebase--interactive file that is *not*
> installed, and avoid affecting other Git operations (you would also not
> need to guard the new code path behind a conditional).
> 

Makes sense. (I had actually used incorrect directories in my previous
mail as I did part of it manually (big mistake!) so the trace itself
might not have much sense in some parts, sorry)

Speaking of trace, (thanks to Dscho!) the one I got using 'valgrind'
(with `--leak-check=full` option) can be found below. I've kept only
relevant parts of it to avoid unwanted noise of `set -x`. Let me know
if that's needed too.

...

+ git diff-files --quiet --ignore-submodules
+ git diff-index --cached --quiet --ignore-submodules HEAD --
+ test 0 = 1
+ test -z 1
+ valgrind --leak-check=full git rebase--helper --continue
==10384== Memcheck, a memory error detector
==10384== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==10384== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info
==10384== Command: git rebase--helper --continue
==10384== 
==10384== Invalid free() / delete / delete[] / realloc()
==10384==    at 0x4C2CDDB: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10384==    by 0x24E3F8: read_populate_opts (sequencer.c:1964)
==10384==    by 0x24E3F8: sequencer_continue (sequencer.c:2753)
==10384==    by 0x1735DC: cmd_rebase__helper (rebase--helper.c:52)
==10384==    by 0x11B847: run_builtin (git.c:346)
==10384==    by 0x11B847: handle_builtin (git.c:554)
==10384==    by 0x11BB05: run_argv (git.c:606)
==10384==    by 0x11BB05: cmd_main (git.c:683)
==10384==    by 0x11AC0B: main (common-main.c:43)
==10384==  Address 0x2a898a is in a r-x mapped file /mnt/Source/Git/git-next/git segment
==10384== 
Successfully rebased and updated refs/heads/public.
==10384== 
==10384== HEAP SUMMARY:
==10384==     in use at exit: 680,882 bytes in 332 blocks
==10384==   total heap usage: 717 allocs, 386 frees, 1,709,293 bytes allocated
==10384== 
==10384== 2,053 (2,048 direct, 5 indirect) bytes in 1 blocks are definitely lost in loss record 75 of 83
==10384==    at 0x4C2BADF: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10384==    by 0x4C2DE5F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==10384==    by 0x27E0FE: xrealloc (wrapper.c:138)
==10384==    by 0x2072A3: add_object_array_with_path (object.c:319)
==10384==    by 0x23D745: add_pending_object_with_path (revision.c:163)
==10384==    by 0x24030E: add_pending_object_with_mode (revision.c:170)
==10384==    by 0x24030E: add_pending_object (revision.c:176)
==10384==    by 0x24030E: add_head_to_pending (revision.c:188)
==10384==    by 0x280EFA: has_uncommitted_changes.part.17 (wt-status.c:2288)
==10384==    by 0x24DF88: commit_staged_changes (sequencer.c:2705)
==10384==    by 0x24DF88: sequencer_continue (sequencer.c:2749)
==10384==    by 0x1735DC: cmd_rebase__helper (rebase--helper.c:52)
==10384==    by 0x11B847: run_builtin (git.c:346)
==10384==    by 0x11B847: handle_builtin (git.c:554)
==10384==    by 0x11BB05: run_argv (git.c:606)
==10384==    by 0x11BB05: cmd_main (git.c:683)
==10384==    by 0x11AC0B: main (common-main.c:43)
==10384== 
==10384== LEAK SUMMARY:
==10384==    definitely lost: 2,048 bytes in 1 blocks
==10384==    indirectly lost: 5 bytes in 1 blocks
==10384==      possibly lost: 0 bytes in 0 blocks
==10384==    still reachable: 678,829 bytes in 330 blocks
==10384==         suppressed: 0 bytes in 0 blocks
==10384== Reachable blocks (those to which a pointer was found) are not shown.
==10384== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==10384== 
==10384== For counts of detected and suppressed errors, rerun with: -v
==10384== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 0 from 0)
+ exit


I think I didn't mention I've set `commit.gpgsign` to `true` for that
repo, did I?

Let me know if anything else is needed.


HTH,
Kaartic