Tracking down a segfault in delta_base_cache

Hi git, I've been seeing git segfault over the past few days. I'm on Mac OS X 10.12, 64-bit, compiling with clang (Apple LLVM version 8.0.0 (clang-800.0.40)).

I first noticed it during a checkout, then also during `log -u`. I'm still debugging, but wanted to give a heads-up in case anyone else is seeing this.

~/D/S/A/HLT $ git-log -u -n1000 >/dev/null fish: 'git-log' terminated by signal SIGSEGV (Address boundary error)

~/D/S/A/HLT $ git fsck Checking object directories: 100% (256/256), done. fish: 'git fsck' terminated by signal SIGSEGV (Address boundary error)

~/D/S/A/HLT $ git --version git version 2.10.0.129.g35f6318

Running git-fsck from 2.9.2 validates the repository data.

Bisect says:

8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c is the first bad commit
commit 8261e1f139db3f8aa6f9fd7d98c876cbeb0f927c
Author: Jeff King <peff@peff.net>
Date:   Mon Aug 22 18:00:07 2016 -0400

    delta_base_cache: use hashmap.h

Backtrace for the `log -u` case is below. I'll follow up with my progress. -Jonathon

$ lldb /Users/jmah/Documents/Streams/git/git-log -- -u
(lldb) target create "/Users/jmah/Documents/Streams/git/git-log"
Current executable set to '/Users/jmah/Documents/Streams/git/git-log' (x86_64).
(lldb) settings set -- target.run-args  "-u"
(lldb) process launch -o /dev/null
Process 92815 launched: '/Users/jmah/Documents/Streams/git/git-log' (x86_64)
Process 92815 stopped
* thread #1: tid = 0x1c30677, 0x00000001001bba80 git-log`release_delta_base_cache(ent=0xffffffffffffffd0) + 16 at sha1_file.c:2171, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10)
    frame #0: 0x00000001001bba80 git-log`release_delta_base_cache(ent=0xffffffffffffffd0) + 16 at sha1_file.c:2171
   2168	
   2169	static inline void release_delta_base_cache(struct delta_base_cache_entry *ent)
   2170	{
-> 2171		free(ent->data);
   2172		detach_delta_base_cache_entry(ent);
   2173	}
   2174	
(lldb) bt
warning: could not load any Objective-C class information. This will significantly reduce the quality of type information available.
* thread #1: tid = 0x1c30677, 0x00000001001bba80 git-log`release_delta_base_cache(ent=0xffffffffffffffd0) + 16 at sha1_file.c:2171, queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x10)
  * frame #0: 0x00000001001bba80 git-log`release_delta_base_cache(ent=0xffffffffffffffd0) + 16 at sha1_file.c:2171
    frame #1: 0x00000001001bcadf git-log`add_delta_base_cache(p=0x00000001006062f0, base_offset=1792781, base=0x000000015749a000, base_size=1617761, type=OBJ_BLOB) + 143 at sha1_file.c:2199
    frame #2: 0x00000001001bc0d6 git-log`unpack_entry(p=0x00000001006062f0, obj_offset=1792781, final_type=0x00007fff5fbfe7fc, final_size=0x000000010185a5a0) + 1590 at sha1_file.c:2345
    frame #3: 0x00000001001c2209 git-log`cache_or_unpack_entry(p=0x00000001006062f0, base_offset=2692554, base_size=0x000000010185a5a0, type=0x00007fff5fbfe7fc) + 73 at sha1_file.c:2162
    frame #4: 0x00000001001bed8d git-log`read_packed_sha1(sha1="?c?????}\x0e'\x81҄MH;yP?, type=0x00007fff5fbfe7fc, size=0x000000010185a5a0) + 93 at sha1_file.c:2765
    frame #5: 0x00000001001bcc17 git-log`read_object(sha1="?c?????}\x0e'\x81҄MH;yP?, type=0x00007fff5fbfe7fc, size=0x000000010185a5a0) + 119 at sha1_file.c:2813
    frame #6: 0x00000001001be013 git-log`read_sha1_file_extended(sha1="?c?????}\x0e'\x81҄MH;yP?, type=0x00007fff5fbfe7fc, size=0x000000010185a5a0, flag=1) + 67 at sha1_file.c:2841
    frame #7: 0x00000001001073ba git-log`read_sha1_file(sha1="?c?????}\x0e'\x81҄MH;yP?, type=0x00007fff5fbfe7fc, size=0x000000010185a5a0) + 42 at cache.h:1056
    frame #8: 0x0000000100106ce6 git-log`diff_populate_filespec(s=0x000000010185a570, flags=2) + 1334 at diff.c:2845
    frame #9: 0x0000000100106670 git-log`diff_filespec_is_binary(one=0x000000010185a570) + 160 at diff.c:2248
    frame #10: 0x00000001001124bc git-log`builtin_diff(name_a="Applications/IDE/PlugIns/IDEPlugIns/IDEPlugIns.xcodeproj/project.pbxproj", name_b="Applications/IDE/PlugIns/IDEPlugIns/IDEPlugIns.xcodeproj/project.pbxproj", one=0x000000010185a570, two=0x0000000101878310, xfrm_msg="index e063d6f..288f95f 100644\n", must_show_header=0, o=0x00007fff5fbff4b8, complete_rewrite=0) + 1852 at diff.c:2383
    frame #11: 0x00000001001116ce git-log`run_diff_cmd(pgm=0x0000000000000000, name="Applications/IDE/PlugIns/IDEPlugIns/IDEPlugIns.xcodeproj/project.pbxproj", other=0x0000000000000000, attr_path="Applications/IDE/PlugIns/IDEPlugIns/IDEPlugIns.xcodeproj/project.pbxproj", one=0x000000010185a570, two=0x0000000101878310, msg=0x00007fff5fbfed18, o=0x00007fff5fbff4b8, p=0x000000010186c130) + 734 at diff.c:3134
    frame #12: 0x0000000100111350 git-log`run_diff(p=0x000000010186c130, o=0x00007fff5fbff4b8) + 720 at diff.c:3222
    frame #13: 0x000000010010d75d git-log`diff_flush_patch(p=0x000000010186c130, o=0x00007fff5fbff4b8) + 157 at diff.c:4202
    frame #14: 0x000000010010b9bc git-log`diff_flush(options=0x00007fff5fbff4b8) + 1148 at diff.c:4722
    frame #15: 0x000000010014418b git-log`log_tree_diff_flush(opt=0x00007fff5fbfefc0) + 507 at log-tree.c:781
    frame #16: 0x00000001001445fe git-log`log_tree_diff(opt=0x00007fff5fbfefc0, commit=0x0000000153506540, log=0x00007fff5fbfeed8) + 606 at log-tree.c:848
    frame #17: 0x000000010014428e git-log`log_tree_commit(opt=0x00007fff5fbfefc0, commit=0x0000000153506540) + 238 at log-tree.c:877
    frame #18: 0x0000000100064b89 git-log`cmd_log_walk(rev=0x00007fff5fbfefc0) + 185 at log.c:360
    frame #19: 0x0000000100066405 git-log`cmd_log(argc=2, argv=0x00007fff5fbff9d0, prefix=0x0000000000000000) + 309 at log.c:682
    frame #20: 0x000000010000274f git-log`run_builtin(p=0x0000000100264970, argc=2, argv=0x00007fff5fbff9d0) + 431 at git.c:352
    frame #21: 0x0000000100001a9a git-log`handle_builtin(argc=2, argv=0x00007fff5fbff9d0) + 138 at git.c:539
    frame #22: 0x00000001000017e4 git-log`cmd_main(argc=2, argv=0x00007fff5fbff9d0) + 116 at git.c:635
    frame #23: 0x00000001000c9eb4 git-log`main(argc=2, argv=0x00007fff5fbff9d0) + 68 at common-main.c:40
    frame #24: 0x00007fffd87ff255 libdyld.dylib`start + 1

On Wed, Sep 14, 2016 at 05:42:29PM -0700, Jonathon Mah wrote:

Have you tried with the patch in:

  http://public-inbox.org/git/20160912164616.vg33kldazuthff3d@sigill.intra.peff.net/

?

The problems I saw with valgrind weren't here, but would explain this. We free() the previous node, then walk forward from its "next" pointer. On my Linux box, that happens to work, but we could be feeding total junk to the list pointer, which would meant ent->data is junk, and free() notices.

-Peff

All the examples I've tried work when I use that. Thanks!

Jonathon Mah <me@jonathonmah.com> writes:

Peff, thanks for a quick suggestion and Jonathon, thanks for a quick confirmation.

On Thu, Sep 15, 2016 at 10:34:43AM -0700, Junio C Hamano wrote:

Better still would have been for me not to introduce the segfault in the first place. ;)

-Peff