{"thread":{"id":"35449","subject":"git-blame segfault","startedAt":"2013-12-02T12:57:48Z","lastAt":"2013-12-03T09:04:41Z","messageCount":5,"participants":["Markus Trippelsdorf","Antoine Pelisse"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"231366","messageId":"20131202125748.GA275@x4","threadId":"35449","inReplyTo":null,"subject":"git-blame segfault","fromName":"Markus Trippelsdorf","fromEmail":"markus@trippelsdorf.de","sentAt":"2013-12-02T12:57:48Z","receivedAt":"2013-12-02T12:57:48Z","isPatch":false,"sender":{"key":"markus@trippelsdorf.de","avatar":null},"body":"When git is compiled with current gcc and \"-march=native\" \ngit-blame segfaults:\n\nFor example:\n\n % gdb --args /var/tmp/git/git-blame gcc/tree-object-size.c\n...\nProgram received signal SIGSEGV, Segmentation fault.\n0x0000000000000000 in ?? ()\n(gdb) bt\n#0  0x0000000000000000 in ?? ()\n#1  0x000000000051240d in xdl_emit_hunk_hdr (s1=s1@entry=30, c1=<optimized out>, s2=s2@entry=30, c2=c2@entry=6, func=func@entry=0x7fffffffd2d8 \"\", funclen=0, \n    ecb=ecb@entry=0x7fffffffd580) at xdiff/xutils.c:460\n#2  0x0000000000512af7 in xdl_emit_diff (xe=0x7fffffffd390, xscr=<optimized out>, ecb=0x7fffffffd580, xecfg=0x7fffffffd590) at xdiff/xemit.c:237\n#3  0x0000000000510a5d in xdl_diff (mf1=mf1@entry=0x7fffffffd510, mf2=mf2@entry=0x7fffffffd520, xpp=xpp@entry=0x7fffffffd570, xecfg=xecfg@entry=0x7fffffffd590, \n    ecb=ecb@entry=0x7fffffffd580) at xdiff/xdiffi.c:601\n#4  0x000000000050b005 in xdi_diff (mf1=<optimized out>, mf2=<optimized out>, xpp=xpp@entry=0x7fffffffd570, xecfg=xecfg@entry=0x7fffffffd590, xecb=xecb@entry=0x7fffffffd580)\n    at xdiff-interface.c:136\n#5  0x00000000004104df in diff_hunks (file_a=<optimized out>, file_b=<optimized out>, ctxlen=ctxlen@entry=0, hunk_func=hunk_func@entry=0x411320 <blame_chunk_cb>, \n    cb_data=cb_data@entry=0x7fffffffd830) at builtin/blame.c:105\n#6  0x0000000000412b54 in pass_blame_to_parent (parent=0x11da810, target=0x11dab50, sb=0x7fffffffd6e0) at builtin/blame.c:815\n#7  pass_blame (opt=0, origin=0x11dab50, sb=0x7fffffffd6e0) at builtin/blame.c:1281\n#8  assign_blame (opt=<optimized out>, sb=0x7fffffffd6e0) at builtin/blame.c:1559\n#9  cmd_blame (argc=<optimized out>, argv=<optimized out>, prefix=<optimized out>) at builtin/blame.c:2523\n#10 0x00000000004060b5 in run_builtin (argv=0x7fffffffe528, argc=2, p=0x578bd8 <commands.22612+120>) at git.c:314\n#11 handle_internal_command (argc=2, argv=0x7fffffffe528) at git.c:478\n#12 0x0000000000405772 in main (argc=2, av=<optimized out>) at git.c:575\n(gdb) up\n#1  0x000000000051240d in xdl_emit_hunk_hdr (s1=s1@entry=30, c1=<optimized out>, s2=s2@entry=30, c2=c2@entry=6, func=func@entry=0x7fffffffd2d8 \"\", funclen=0, \n    ecb=ecb@entry=0x7fffffffd580) at xdiff/xutils.c:460\n460             if (ecb->outf(ecb->priv, &mb, 1) < 0)\n(gdb) l\n455             }\n456             buf[nb++] = '\\n';\n457\n458             mb.ptr = buf;\n459             mb.size = nb;\n460             if (ecb->outf(ecb->priv, &mb, 1) < 0)\n461                     return -1;\n462\n463             return 0;\n464     }\n(gdb) p *ecb\n$1 = {\n  priv = 0x7fffffffd830, \n  outf = 0x0\n}\n\nIf I leave xecfg uninitialized in the following function the issue goes\naway.\n\n>From builtin/blame.c:\n  94 static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,\n  95                       xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)\n  96 {\n  97         xpparam_t xpp = {0};\n  98         xdemitconf_t xecfg = {0};\n  99         xdemitcb_t ecb = {NULL};\n 100\n 101         xpp.flags = xdl_opts;\n 102         xecfg.ctxlen = ctxlen;\n 103         xecfg.hunk_func = hunk_func;\n 104         ecb.priv = cb_data;\n 105         return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);\n 106 }\n 107\n\nI'm not sure if this a git bug or a gcc bug. In any case I've opened a\ngcc bug-report here: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59363\n\n\n-- \nMarkus\n"},{"id":"231368","messageId":"CALWbr2w8sRRPJdjnpEwiGYe+T4KnvmRtV2n3yTesz8869q_=zA@mail.gmail.com","threadId":"35449","inReplyTo":"20131202125748.GA275@x4","subject":"Re: git-blame segfault","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-12-02T14:15:38Z","receivedAt":"2013-12-02T14:15:38Z","isPatch":false,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Mon, Dec 2, 2013 at 1:57 PM, Markus Trippelsdorf\n<markus@trippelsdorf.de> wrote:\n> When git is compiled with current gcc and \"-march=native\"\n> git-blame segfaults:\n>\n> For example:\n>\n>  % gdb --args /var/tmp/git/git-blame gcc/tree-object-size.c\n> ...\n> Program received signal SIGSEGV, Segmentation fault.\n> 0x0000000000000000 in ?? ()\n> (gdb) bt\n> #0  0x0000000000000000 in ?? ()\n> #1  0x000000000051240d in xdl_emit_hunk_hdr (s1=s1@entry=30, c1=<optimized out>, s2=s2@entry=30, c2=c2@entry=6, func=func@entry=0x7fffffffd2d8 \"\", funclen=0,\n>     ecb=ecb@entry=0x7fffffffd580) at xdiff/xutils.c:460\n> #2  0x0000000000512af7 in xdl_emit_diff (xe=0x7fffffffd390, xscr=<optimized out>, ecb=0x7fffffffd580, xecfg=0x7fffffffd590) at xdiff/xemit.c:237\n\nxdl_emit_diff() should not be called, because xecfg->hunk_func should\nbe non-null and called instead. xld_emit_diff() is the one that needs\noutf to be set.\n\n> #3  0x0000000000510a5d in xdl_diff (mf1=mf1@entry=0x7fffffffd510, mf2=mf2@entry=0x7fffffffd520, xpp=xpp@entry=0x7fffffffd570, xecfg=xecfg@entry=0x7fffffffd590,\n>     ecb=ecb@entry=0x7fffffffd580) at xdiff/xdiffi.c:601\n\nHere we decide that xecfg->hunk_func is empty, and ef is set to xdl_emit_diff.\n\n> #4  0x000000000050b005 in xdi_diff (mf1=<optimized out>, mf2=<optimized out>, xpp=xpp@entry=0x7fffffffd570, xecfg=xecfg@entry=0x7fffffffd590, xecb=xecb@entry=0x7fffffffd580)\n>     at xdiff-interface.c:136\n> #5  0x00000000004104df in diff_hunks (file_a=<optimized out>, file_b=<optimized out>, ctxlen=ctxlen@entry=0, hunk_func=hunk_func@entry=0x411320 <blame_chunk_cb>,\n>     cb_data=cb_data@entry=0x7fffffffd830) at builtin/blame.c:105\n\nAs we can see in your code below, ecb.outf is not set here, because\nit's not needed by hunk_func.\nhunk_func is set to blame_chunck_cb, passed by the function below.\n\n> #6  0x0000000000412b54 in pass_blame_to_parent (parent=0x11da810, target=0x11dab50, sb=0x7fffffffd6e0) at builtin/blame.c:815\n\ndiff_hunks() is called with blame_chunck_cb as hunk_func.\n\nI think the best thing to do is to find out where xecfg->hunk_func\nloses the blame_chunck_cb value to NULL. You could start with a\nwatchpoint.\n\n>[...]\n> 460             if (ecb->outf(ecb->priv, &mb, 1) < 0)\n> (gdb) l\n> 455             }\n> 456             buf[nb++] = '\\n';\n> 457\n> 458             mb.ptr = buf;\n> 459             mb.size = nb;\n> 460             if (ecb->outf(ecb->priv, &mb, 1) < 0)\n> 461                     return -1;\n> 462\n> 463             return 0;\n> 464     }\n> (gdb) p *ecb\n> $1 = {\n>   priv = 0x7fffffffd830,\n>   outf = 0x0\n> }\n>\n> If I leave xecfg uninitialized in the following function the issue goes\n> away.\n\nWould that mean that gcc is doing some steps in the wrong order ? That\nis setting xecfg.hunk_func and then emptying the structure ? I've\nalready had a similar bug, but that's very unfortunate.\n\n> From builtin/blame.c:\n>   94 static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,\n>   95                       xdl_emit_hunk_consume_func_t hunk_func, void *cb_data)\n>   96 {\n>   97         xpparam_t xpp = {0};\n>   98         xdemitconf_t xecfg = {0};\n>   99         xdemitcb_t ecb = {NULL};\n>  100\n>  101         xpp.flags = xdl_opts;\n>  102         xecfg.ctxlen = ctxlen;\n>  103         xecfg.hunk_func = hunk_func;\n>  104         ecb.priv = cb_data;\n>  105         return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);\n>  106 }\n>  107\n"},{"id":"231370","messageId":"20131202150541.GB275@x4","threadId":"35449","inReplyTo":"CALWbr2w8sRRPJdjnpEwiGYe+T4KnvmRtV2n3yTesz8869q_=zA@mail.gmail.com","subject":"Re: git-blame segfault","fromName":"Markus Trippelsdorf","fromEmail":"markus@trippelsdorf.de","sentAt":"2013-12-02T15:05:41Z","receivedAt":"2013-12-02T15:05:41Z","isPatch":false,"sender":{"key":"markus@trippelsdorf.de","avatar":null},"body":"On 2013.12.02 at 15:15 +0100, Antoine Pelisse wrote:\n> On Mon, Dec 2, 2013 at 1:57 PM, Markus Trippelsdorf\n> <markus@trippelsdorf.de> wrote:\n> > When git is compiled with current gcc and \"-march=native\"\n> > git-blame segfaults:\n> >\n> > For example:\n> >\n> >  % gdb --args /var/tmp/git/git-blame gcc/tree-object-size.c\n> > ...\n> > Program received signal SIGSEGV, Segmentation fault.\n> > 0x0000000000000000 in ?? ()\n> > (gdb) bt\n> > #0  0x0000000000000000 in ?? ()\n> > #1  0x000000000051240d in xdl_emit_hunk_hdr (s1=s1@entry=30, c1=<optimized out>, s2=s2@entry=30, c2=c2@entry=6, func=func@entry=0x7fffffffd2d8 \"\", funclen=0,\n> >     ecb=ecb@entry=0x7fffffffd580) at xdiff/xutils.c:460\n> > #2  0x0000000000512af7 in xdl_emit_diff (xe=0x7fffffffd390, xscr=<optimized out>, ecb=0x7fffffffd580, xecfg=0x7fffffffd590) at xdiff/xemit.c:237\n> \n> xdl_emit_diff() should not be called, because xecfg->hunk_func should\n> be non-null and called instead. xld_emit_diff() is the one that needs\n> outf to be set.\n> \n> > #3  0x0000000000510a5d in xdl_diff (mf1=mf1@entry=0x7fffffffd510, mf2=mf2@entry=0x7fffffffd520, xpp=xpp@entry=0x7fffffffd570, xecfg=xecfg@entry=0x7fffffffd590,\n> >     ecb=ecb@entry=0x7fffffffd580) at xdiff/xdiffi.c:601\n> \n> Here we decide that xecfg->hunk_func is empty, and ef is set to xdl_emit_diff.\n> \n> > #4  0x000000000050b005 in xdi_diff (mf1=<optimized out>, mf2=<optimized out>, xpp=xpp@entry=0x7fffffffd570, xecfg=xecfg@entry=0x7fffffffd590, xecb=xecb@entry=0x7fffffffd580)\n> >     at xdiff-interface.c:136\n> > #5  0x00000000004104df in diff_hunks (file_a=<optimized out>, file_b=<optimized out>, ctxlen=ctxlen@entry=0, hunk_func=hunk_func@entry=0x411320 <blame_chunk_cb>,\n> >     cb_data=cb_data@entry=0x7fffffffd830) at builtin/blame.c:105\n> \n> As we can see in your code below, ecb.outf is not set here, because\n> it's not needed by hunk_func.\n> hunk_func is set to blame_chunck_cb, passed by the function below.\n> \n> > #6  0x0000000000412b54 in pass_blame_to_parent (parent=0x11da810, target=0x11dab50, sb=0x7fffffffd6e0) at builtin/blame.c:815\n> \n> diff_hunks() is called with blame_chunck_cb as hunk_func.\n> \n> I think the best thing to do is to find out where xecfg->hunk_func\n> loses the blame_chunck_cb value to NULL. You could start with a\n> watchpoint.\n\nIt happens in diff_hunks (from builtin/blame.c):\n\n(gdb) up\n#5  0x00000000004104df in diff_hunks (file_a=<optimized out>, file_b=<optimized out>, ctxlen=ctxlen@entry=0, hunk_func=hunk_func@entry=0x411320 <blame_chunk_cb>, \n    cb_data=cb_data@entry=0x7fffffffd830) at builtin/blame.c:105\n105             return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);\n(gdb) l\n100\n101             xpp.flags = xdl_opts;\n102             xecfg.ctxlen = ctxlen;\n103             xecfg.hunk_func = hunk_func;\n104             ecb.priv = cb_data;\n105             return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);\n106     }\n107\n108     /*\n109      * Prepare diff_filespec and convert it using diff textconv API\n(gdb) p hunk_func\n$1 = (xdl_emit_hunk_consume_func_t) 0x411320 <blame_chunk_cb>\n(gdb) p xecfg.hunk_func\n$2 = (xdl_emit_hunk_consume_func_t) 0x0\n(gdb) p xecfg\n$3 = {\n  ctxlen = 0, \n  interhunkctxlen = 0, \n  flags = 0, \n  find_func = 0x0, \n  find_func_priv = 0x0, \n  hunk_func = 0x0\n}\n> \n> Would that mean that gcc is doing some steps in the wrong order ? That\n> is setting xecfg.hunk_func and then emptying the structure ? I've\n> already had a similar bug, but that's very unfortunate.\n\nYes. I think this might be the case:\n\n(gdb) disass\nDump of assembler code for function diff_hunks:\n   0x0000000000410460 <+0>:     sub    $0x58,%rsp\n   0x0000000000410464 <+4>:     xor    %eax,%eax\n   0x0000000000410466 <+6>:     mov    %eax,%r9d\n   0x0000000000410469 <+9>:     add    $0x20,%eax\n   0x000000000041046c <+12>:    cmp    $0x20,%eax\n   0x000000000041046f <+15>:    movq   $0x0,0x20(%rsp,%r9,1)\n   0x0000000000410478 <+24>:    movq   $0x0,0x28(%rsp,%r9,1)\n   0x0000000000410481 <+33>:    movq   $0x0,0x30(%rsp,%r9,1)\n   0x000000000041048a <+42>:    movq   $0x0,0x38(%rsp,%r9,1)\n   0x0000000000410493 <+51>:    jb     0x410466 <diff_hunks+6>\n   0x0000000000410495 <+53>:    lea    0x20(%rsp),%r10\n   0x000000000041049a <+58>:    mov    %rdx,0x20(%rsp)\n   0x000000000041049f <+63>:    mov    %rcx,0x48(%rsp)\n   0x00000000004104a4 <+68>:    add    %r10,%rax\n   0x00000000004104a7 <+71>:    mov    %r8,0x10(%rsp)\n   0x00000000004104ac <+76>:    mov    %rsp,%rdx\n   0x00000000004104af <+79>:    movq   $0x0,(%rax)\n   0x00000000004104b6 <+86>:    movq   $0x0,0x8(%rax)\n   0x00000000004104be <+94>:    lea    0x10(%rsp),%r8\n   0x00000000004104c3 <+99>:    movslq 0x171882(%rip),%rax        # 0x581d4c <xdl_opts>\n   0x00000000004104ca <+106>:   mov    %r10,%rcx\n   0x00000000004104cd <+109>:   movq   $0x0,0x18(%rsp)\n   0x00000000004104d6 <+118>:   mov    %rax,(%rsp)\n   0x00000000004104da <+122>:   callq  0x50aee0 <xdi_diff>\n=> 0x00000000004104df <+127>:   add    $0x58,%rsp\n   0x00000000004104e3 <+131>:   retq   \nEnd of assembler dump.\n\n\n-- \nMarkus\n"},{"id":"231413","messageId":"20131203084540.GA276@x4","threadId":"35449","inReplyTo":"20131202150541.GB275@x4","subject":"Re: git-blame segfault","fromName":"Markus Trippelsdorf","fromEmail":"markus@trippelsdorf.de","sentAt":"2013-12-03T08:45:40Z","receivedAt":"2013-12-03T08:45:40Z","isPatch":false,"sender":{"key":"markus@trippelsdorf.de","avatar":null},"body":"On 2013.12.02 at 16:05 +0100, Markus Trippelsdorf wrote:\n> On 2013.12.02 at 15:15 +0100, Antoine Pelisse wrote:\n> > Would that mean that gcc is doing some steps in the wrong order ? That\n> > is setting xecfg.hunk_func and then emptying the structure ? I've\n> > already had a similar bug, but that's very unfortunate.\n> \n> Yes. I think this might be the case:\n> \n> (gdb) disass\n> Dump of assembler code for function diff_hunks:\n>    0x0000000000410460 <+0>:     sub    $0x58,%rsp\n>    0x0000000000410464 <+4>:     xor    %eax,%eax\n>    0x0000000000410466 <+6>:     mov    %eax,%r9d\n>    0x0000000000410469 <+9>:     add    $0x20,%eax\n>    0x000000000041046c <+12>:    cmp    $0x20,%eax\n>    0x000000000041046f <+15>:    movq   $0x0,0x20(%rsp,%r9,1)\n>    0x0000000000410478 <+24>:    movq   $0x0,0x28(%rsp,%r9,1)\n>    0x0000000000410481 <+33>:    movq   $0x0,0x30(%rsp,%r9,1)\n>    0x000000000041048a <+42>:    movq   $0x0,0x38(%rsp,%r9,1)\n>    0x0000000000410493 <+51>:    jb     0x410466 <diff_hunks+6>\n>    0x0000000000410495 <+53>:    lea    0x20(%rsp),%r10\n>    0x000000000041049a <+58>:    mov    %rdx,0x20(%rsp)\n>    0x000000000041049f <+63>:    mov    %rcx,0x48(%rsp)\n>    0x00000000004104a4 <+68>:    add    %r10,%rax\n>    0x00000000004104a7 <+71>:    mov    %r8,0x10(%rsp)\n>    0x00000000004104ac <+76>:    mov    %rsp,%rdx\n>    0x00000000004104af <+79>:    movq   $0x0,(%rax)\n>    0x00000000004104b6 <+86>:    movq   $0x0,0x8(%rax)\n>    0x00000000004104be <+94>:    lea    0x10(%rsp),%r8\n>    0x00000000004104c3 <+99>:    movslq 0x171882(%rip),%rax        # 0x581d4c <xdl_opts>\n>    0x00000000004104ca <+106>:   mov    %r10,%rcx\n>    0x00000000004104cd <+109>:   movq   $0x0,0x18(%rsp)\n>    0x00000000004104d6 <+118>:   mov    %rax,(%rsp)\n>    0x00000000004104da <+122>:   callq  0x50aee0 <xdi_diff>\n> => 0x00000000004104df <+127>:   add    $0x58,%rsp\n>    0x00000000004104e3 <+131>:   retq   \n> End of assembler dump.\n\nShould be fixed in gcc soon. For the curious, here is the assembler diff\n(bad vs. good):\n\n        .type   diff_hunks, @function\n diff_hunks:\n .LFB104:\n        .cfi_startproc\n        subq    $88, %rsp\n        .cfi_def_cfa_offset 96\n        xorl    %eax, %eax\n .L31:\n        movl    %eax, %r9d\n        addl    $32, %eax\n        cmpl    $32, %eax\n        movq    $0, 32(%rsp,%r9)\n        movq    $0, 40(%rsp,%r9)\n        movq    $0, 48(%rsp,%r9)\n        movq    $0, 56(%rsp,%r9)\n        jb      .L31\n        leaq    32(%rsp), %r10\n        movq    %rdx, 32(%rsp)\n-       movq    %rcx, 72(%rsp)\n-       addq    %r10, %rax\n        movq    %r8, 16(%rsp)\n+       addq    %r10, %rax\n+       leaq    16(%rsp), %r8\n        movq    %rsp, %rdx\n-       movq    $0, (%rax)\n        movq    $0, 8(%rax)\n-       leaq    16(%rsp), %r8\n+       movq    $0, (%rax)\n        movslq  xdl_opts(%rip), %rax\n+       movq    %rcx, 72(%rsp)\n        movq    %r10, %rcx\n        movq    $0, 24(%rsp)\n        movq    %rax, (%rsp)\n        call    xdi_diff\n        addq    $88, %rsp\n        .cfi_def_cfa_offset 8\n\n-- \nMarkus\n"},{"id":"231414","messageId":"CALWbr2yqf2Kd34pFOp5EmSxsj0rN7nV4o6NFYz68CZ-zudULQw@mail.gmail.com","threadId":"35449","inReplyTo":"20131203084540.GA276@x4","subject":"Re: git-blame segfault","fromName":"Antoine Pelisse","fromEmail":"apelisse@gmail.com","sentAt":"2013-12-03T09:04:41Z","receivedAt":"2013-12-03T09:04:41Z","isPatch":false,"sender":{"key":"apelisse@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1929644?v=4"},"body":"On Tue, Dec 3, 2013 at 9:45 AM, Markus Trippelsdorf\n<markus@trippelsdorf.de> wrote:\n> Should be fixed in gcc soon. For the curious, here is the assembler diff\n> (bad vs. good):\n\nCool, Thanks. Good to know this has nothing to do with Git :-)\n"}]}