Re: git-blame segfault
On Tue, Dec 3, 2013 at 9:45 AM, Markus Trippelsdorf wrote: > Should be fixed in gcc soon. For the curious, here is the assembler diff > (bad vs. good): Cool, Thanks. Good to know this has nothing to do with Git :-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-blame segfault
On 2013.12.02 at 16:05 +0100, Markus Trippelsdorf wrote: > On 2013.12.02 at 15:15 +0100, Antoine Pelisse wrote: > > Would that mean that gcc is doing some steps in the wrong order ? That > > is setting xecfg.hunk_func and then emptying the structure ? I've > > already had a similar bug, but that's very unfortunate. > > Yes. I think this might be the case: > > (gdb) disass > Dump of assembler code for function diff_hunks: >0x00410460 <+0>: sub$0x58,%rsp >0x00410464 <+4>: xor%eax,%eax >0x00410466 <+6>: mov%eax,%r9d >0x00410469 <+9>: add$0x20,%eax >0x0041046c <+12>:cmp$0x20,%eax >0x0041046f <+15>:movq $0x0,0x20(%rsp,%r9,1) >0x00410478 <+24>:movq $0x0,0x28(%rsp,%r9,1) >0x00410481 <+33>:movq $0x0,0x30(%rsp,%r9,1) >0x0041048a <+42>:movq $0x0,0x38(%rsp,%r9,1) >0x00410493 <+51>:jb 0x410466 >0x00410495 <+53>:lea0x20(%rsp),%r10 >0x0041049a <+58>:mov%rdx,0x20(%rsp) >0x0041049f <+63>:mov%rcx,0x48(%rsp) >0x004104a4 <+68>:add%r10,%rax >0x004104a7 <+71>:mov%r8,0x10(%rsp) >0x004104ac <+76>:mov%rsp,%rdx >0x004104af <+79>:movq $0x0,(%rax) >0x004104b6 <+86>:movq $0x0,0x8(%rax) >0x004104be <+94>:lea0x10(%rsp),%r8 >0x004104c3 <+99>:movslq 0x171882(%rip),%rax# 0x581d4c > >0x004104ca <+106>: mov%r10,%rcx >0x004104cd <+109>: movq $0x0,0x18(%rsp) >0x004104d6 <+118>: mov%rax,(%rsp) >0x004104da <+122>: callq 0x50aee0 > => 0x004104df <+127>: add$0x58,%rsp >0x004104e3 <+131>: retq > End of assembler dump. Should be fixed in gcc soon. For the curious, here is the assembler diff (bad vs. good): .type diff_hunks, @function diff_hunks: .LFB104: .cfi_startproc subq$88, %rsp .cfi_def_cfa_offset 96 xorl%eax, %eax .L31: movl%eax, %r9d addl$32, %eax cmpl$32, %eax movq$0, 32(%rsp,%r9) movq$0, 40(%rsp,%r9) movq$0, 48(%rsp,%r9) movq$0, 56(%rsp,%r9) jb .L31 leaq32(%rsp), %r10 movq%rdx, 32(%rsp) - movq%rcx, 72(%rsp) - addq%r10, %rax movq%r8, 16(%rsp) + addq%r10, %rax + leaq16(%rsp), %r8 movq%rsp, %rdx - movq$0, (%rax) movq$0, 8(%rax) - leaq16(%rsp), %r8 + movq$0, (%rax) movslq xdl_opts(%rip), %rax + movq%rcx, 72(%rsp) movq%r10, %rcx movq$0, 24(%rsp) movq%rax, (%rsp) callxdi_diff addq$88, %rsp .cfi_def_cfa_offset 8 -- Markus -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: git-blame segfault
On 2013.12.02 at 15:15 +0100, Antoine Pelisse wrote:
> On Mon, Dec 2, 2013 at 1:57 PM, Markus Trippelsdorf
> wrote:
> > When git is compiled with current gcc and "-march=native"
> > git-blame segfaults:
> >
> > For example:
> >
> > % gdb --args /var/tmp/git/git-blame gcc/tree-object-size.c
> > ...
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x in ?? ()
> > (gdb) bt
> > #0 0x in ?? ()
> > #1 0x0051240d in xdl_emit_hunk_hdr (s1=s1@entry=30, c1= > out>, s2=s2@entry=30, c2=c2@entry=6, func=func@entry=0x7fffd2d8 "",
> > funclen=0,
> > ecb=ecb@entry=0x7fffd580) at xdiff/xutils.c:460
> > #2 0x00512af7 in xdl_emit_diff (xe=0x7fffd390, xscr= > out>, ecb=0x7fffd580, xecfg=0x7fffd590) at xdiff/xemit.c:237
>
> xdl_emit_diff() should not be called, because xecfg->hunk_func should
> be non-null and called instead. xld_emit_diff() is the one that needs
> outf to be set.
>
> > #3 0x00510a5d in xdl_diff (mf1=mf1@entry=0x7fffd510,
> > mf2=mf2@entry=0x7fffd520, xpp=xpp@entry=0x7fffd570,
> > xecfg=xecfg@entry=0x7fffd590,
> > ecb=ecb@entry=0x7fffd580) at xdiff/xdiffi.c:601
>
> Here we decide that xecfg->hunk_func is empty, and ef is set to xdl_emit_diff.
>
> > #4 0x0050b005 in xdi_diff (mf1=, mf2= > out>, xpp=xpp@entry=0x7fffd570, xecfg=xecfg@entry=0x7fffd590,
> > xecb=xecb@entry=0x7fffd580)
> > at xdiff-interface.c:136
> > #5 0x004104df in diff_hunks (file_a=,
> > file_b=, ctxlen=ctxlen@entry=0,
> > hunk_func=hunk_func@entry=0x411320 ,
> > cb_data=cb_data@entry=0x7fffd830) at builtin/blame.c:105
>
> As we can see in your code below, ecb.outf is not set here, because
> it's not needed by hunk_func.
> hunk_func is set to blame_chunck_cb, passed by the function below.
>
> > #6 0x00412b54 in pass_blame_to_parent (parent=0x11da810,
> > target=0x11dab50, sb=0x7fffd6e0) at builtin/blame.c:815
>
> diff_hunks() is called with blame_chunck_cb as hunk_func.
>
> I think the best thing to do is to find out where xecfg->hunk_func
> loses the blame_chunck_cb value to NULL. You could start with a
> watchpoint.
It happens in diff_hunks (from builtin/blame.c):
(gdb) up
#5 0x004104df in diff_hunks (file_a=, file_b=, ctxlen=ctxlen@entry=0, hunk_func=hunk_func@entry=0x411320
,
cb_data=cb_data@entry=0x7fffd830) at builtin/blame.c:105
105 return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
(gdb) l
100
101 xpp.flags = xdl_opts;
102 xecfg.ctxlen = ctxlen;
103 xecfg.hunk_func = hunk_func;
104 ecb.priv = cb_data;
105 return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
106 }
107
108 /*
109 * Prepare diff_filespec and convert it using diff textconv API
(gdb) p hunk_func
$1 = (xdl_emit_hunk_consume_func_t) 0x411320
(gdb) p xecfg.hunk_func
$2 = (xdl_emit_hunk_consume_func_t) 0x0
(gdb) p xecfg
$3 = {
ctxlen = 0,
interhunkctxlen = 0,
flags = 0,
find_func = 0x0,
find_func_priv = 0x0,
hunk_func = 0x0
}
>
> Would that mean that gcc is doing some steps in the wrong order ? That
> is setting xecfg.hunk_func and then emptying the structure ? I've
> already had a similar bug, but that's very unfortunate.
Yes. I think this might be the case:
(gdb) disass
Dump of assembler code for function diff_hunks:
0x00410460 <+0>: sub$0x58,%rsp
0x00410464 <+4>: xor%eax,%eax
0x00410466 <+6>: mov%eax,%r9d
0x00410469 <+9>: add$0x20,%eax
0x0041046c <+12>:cmp$0x20,%eax
0x0041046f <+15>:movq $0x0,0x20(%rsp,%r9,1)
0x00410478 <+24>:movq $0x0,0x28(%rsp,%r9,1)
0x00410481 <+33>:movq $0x0,0x30(%rsp,%r9,1)
0x0041048a <+42>:movq $0x0,0x38(%rsp,%r9,1)
0x00410493 <+51>:jb 0x410466
0x00410495 <+53>:lea0x20(%rsp),%r10
0x0041049a <+58>:mov%rdx,0x20(%rsp)
0x0041049f <+63>:mov%rcx,0x48(%rsp)
0x004104a4 <+68>:add%r10,%rax
0x004104a7 <+71>:mov%r8,0x10(%rsp)
0x004104ac <+76>:mov%rsp,%rdx
0x004104af <+79>:movq $0x0,(%rax)
0x004104b6 <+86>:movq $0x0,0x8(%rax)
0x004104be <+94>:lea0x10(%rsp),%r8
0x004104c3 <+99>:movslq 0x171882(%rip),%rax# 0x581d4c
0x004104ca <+106>: mov%r10,%rcx
0x004104cd <+109>: movq $0x0,0x18(%rsp)
0x004104d6 <+118>: mov%rax,(%rsp)
0x004104da <+122>: callq 0x50aee0
=> 0x004104df <+127>: add$0x58,%rsp
0x004104e3 <+131>: retq
End of assembler dump.
--
Markus
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http:
Re: git-blame segfault
On Mon, Dec 2, 2013 at 1:57 PM, Markus Trippelsdorf
wrote:
> When git is compiled with current gcc and "-march=native"
> git-blame segfaults:
>
> For example:
>
> % gdb --args /var/tmp/git/git-blame gcc/tree-object-size.c
> ...
> Program received signal SIGSEGV, Segmentation fault.
> 0x in ?? ()
> (gdb) bt
> #0 0x in ?? ()
> #1 0x0051240d in xdl_emit_hunk_hdr (s1=s1@entry=30, c1= out>, s2=s2@entry=30, c2=c2@entry=6, func=func@entry=0x7fffd2d8 "",
> funclen=0,
> ecb=ecb@entry=0x7fffd580) at xdiff/xutils.c:460
> #2 0x00512af7 in xdl_emit_diff (xe=0x7fffd390, xscr= out>, ecb=0x7fffd580, xecfg=0x7fffd590) at xdiff/xemit.c:237
xdl_emit_diff() should not be called, because xecfg->hunk_func should
be non-null and called instead. xld_emit_diff() is the one that needs
outf to be set.
> #3 0x00510a5d in xdl_diff (mf1=mf1@entry=0x7fffd510,
> mf2=mf2@entry=0x7fffd520, xpp=xpp@entry=0x7fffd570,
> xecfg=xecfg@entry=0x7fffd590,
> ecb=ecb@entry=0x7fffd580) at xdiff/xdiffi.c:601
Here we decide that xecfg->hunk_func is empty, and ef is set to xdl_emit_diff.
> #4 0x0050b005 in xdi_diff (mf1=, mf2=,
> xpp=xpp@entry=0x7fffd570, xecfg=xecfg@entry=0x7fffd590,
> xecb=xecb@entry=0x7fffd580)
> at xdiff-interface.c:136
> #5 0x004104df in diff_hunks (file_a=,
> file_b=, ctxlen=ctxlen@entry=0,
> hunk_func=hunk_func@entry=0x411320 ,
> cb_data=cb_data@entry=0x7fffd830) at builtin/blame.c:105
As we can see in your code below, ecb.outf is not set here, because
it's not needed by hunk_func.
hunk_func is set to blame_chunck_cb, passed by the function below.
> #6 0x00412b54 in pass_blame_to_parent (parent=0x11da810,
> target=0x11dab50, sb=0x7fffd6e0) at builtin/blame.c:815
diff_hunks() is called with blame_chunck_cb as hunk_func.
I think the best thing to do is to find out where xecfg->hunk_func
loses the blame_chunck_cb value to NULL. You could start with a
watchpoint.
>[...]
> 460 if (ecb->outf(ecb->priv, &mb, 1) < 0)
> (gdb) l
> 455 }
> 456 buf[nb++] = '\n';
> 457
> 458 mb.ptr = buf;
> 459 mb.size = nb;
> 460 if (ecb->outf(ecb->priv, &mb, 1) < 0)
> 461 return -1;
> 462
> 463 return 0;
> 464 }
> (gdb) p *ecb
> $1 = {
> priv = 0x7fffd830,
> outf = 0x0
> }
>
> If I leave xecfg uninitialized in the following function the issue goes
> away.
Would that mean that gcc is doing some steps in the wrong order ? That
is setting xecfg.hunk_func and then emptying the structure ? I've
already had a similar bug, but that's very unfortunate.
> From builtin/blame.c:
> 94 static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,
> 95 xdl_emit_hunk_consume_func_t hunk_func, void
> *cb_data)
> 96 {
> 97 xpparam_t xpp = {0};
> 98 xdemitconf_t xecfg = {0};
> 99 xdemitcb_t ecb = {NULL};
> 100
> 101 xpp.flags = xdl_opts;
> 102 xecfg.ctxlen = ctxlen;
> 103 xecfg.hunk_func = hunk_func;
> 104 ecb.priv = cb_data;
> 105 return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
> 106 }
> 107
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
git-blame segfault
When git is compiled with current gcc and "-march=native"
git-blame segfaults:
For example:
% gdb --args /var/tmp/git/git-blame gcc/tree-object-size.c
...
Program received signal SIGSEGV, Segmentation fault.
0x in ?? ()
(gdb) bt
#0 0x in ?? ()
#1 0x0051240d in xdl_emit_hunk_hdr (s1=s1@entry=30, c1=, s2=s2@entry=30, c2=c2@entry=6, func=func@entry=0x7fffd2d8 "",
funclen=0,
ecb=ecb@entry=0x7fffd580) at xdiff/xutils.c:460
#2 0x00512af7 in xdl_emit_diff (xe=0x7fffd390, xscr=, ecb=0x7fffd580, xecfg=0x7fffd590) at xdiff/xemit.c:237
#3 0x00510a5d in xdl_diff (mf1=mf1@entry=0x7fffd510,
mf2=mf2@entry=0x7fffd520, xpp=xpp@entry=0x7fffd570,
xecfg=xecfg@entry=0x7fffd590,
ecb=ecb@entry=0x7fffd580) at xdiff/xdiffi.c:601
#4 0x0050b005 in xdi_diff (mf1=, mf2=,
xpp=xpp@entry=0x7fffd570, xecfg=xecfg@entry=0x7fffd590,
xecb=xecb@entry=0x7fffd580)
at xdiff-interface.c:136
#5 0x004104df in diff_hunks (file_a=, file_b=, ctxlen=ctxlen@entry=0, hunk_func=hunk_func@entry=0x411320
,
cb_data=cb_data@entry=0x7fffd830) at builtin/blame.c:105
#6 0x00412b54 in pass_blame_to_parent (parent=0x11da810,
target=0x11dab50, sb=0x7fffd6e0) at builtin/blame.c:815
#7 pass_blame (opt=0, origin=0x11dab50, sb=0x7fffd6e0) at
builtin/blame.c:1281
#8 assign_blame (opt=, sb=0x7fffd6e0) at
builtin/blame.c:1559
#9 cmd_blame (argc=, argv=, prefix=) at builtin/blame.c:2523
#10 0x004060b5 in run_builtin (argv=0x7fffe528, argc=2, p=0x578bd8
) at git.c:314
#11 handle_internal_command (argc=2, argv=0x7fffe528) at git.c:478
#12 0x00405772 in main (argc=2, av=) at git.c:575
(gdb) up
#1 0x0051240d in xdl_emit_hunk_hdr (s1=s1@entry=30, c1=, s2=s2@entry=30, c2=c2@entry=6, func=func@entry=0x7fffd2d8 "",
funclen=0,
ecb=ecb@entry=0x7fffd580) at xdiff/xutils.c:460
460 if (ecb->outf(ecb->priv, &mb, 1) < 0)
(gdb) l
455 }
456 buf[nb++] = '\n';
457
458 mb.ptr = buf;
459 mb.size = nb;
460 if (ecb->outf(ecb->priv, &mb, 1) < 0)
461 return -1;
462
463 return 0;
464 }
(gdb) p *ecb
$1 = {
priv = 0x7fffd830,
outf = 0x0
}
If I leave xecfg uninitialized in the following function the issue goes
away.
>From builtin/blame.c:
94 static int diff_hunks(mmfile_t *file_a, mmfile_t *file_b, long ctxlen,
95 xdl_emit_hunk_consume_func_t hunk_func, void
*cb_data)
96 {
97 xpparam_t xpp = {0};
98 xdemitconf_t xecfg = {0};
99 xdemitcb_t ecb = {NULL};
100
101 xpp.flags = xdl_opts;
102 xecfg.ctxlen = ctxlen;
103 xecfg.hunk_func = hunk_func;
104 ecb.priv = cb_data;
105 return xdi_diff(file_a, file_b, &xpp, &xecfg, &ecb);
106 }
107
I'm not sure if this a git bug or a gcc bug. In any case I've opened a
gcc bug-report here: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59363
--
Markus
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
