bug found (Re: git-fast-export SIGSEGV on solaris + backtrace)

bug found (Re: git-fast-export SIGSEGV on solaris + backtrace)

[SungHyun Nam]

Hello,

 From the gdb trace:

(gdb) p deco
$1 = (struct object_decoration *) 0x193150
(gdb) p idnums
$3 = {name = 0x0, size = 1500, nr = 7, hash = 0x190270}

It was 'off-by-one' error.
0x190270 + (1500 * 8) = 0x193150.

And the code says it:
         for (i = 0; i < idnums.size; ++i) {
                 deco++;
                 if (deco && deco->base && deco->base->type == 1) {

The 'deco' should be post-incremented? or
Checking code should be  (i < idnums.size - 1)?

Regards,
namsh

And very minor cleanup:

diff --git a/decorate.c b/decorate.c
index 23f6b00..d8b428c 100644
--- a/decorate.c
+++ b/decorate.c
@@ -36,7 +36,7 @@ static void *insert_decoration(struct decoration *n, 
struct o>
  static void grow_decoration(struct decoration *n)
  {
         int i;
-       int old_size = n->size;
+       int old_size;
         struct object_decoration *old_hash;

         old_size = n->size;

SungHyun Nam wrote:
> Hello,
> 
> Because recent GIT test failed on t9301-fast-export.sh, I traced
> it. And I found git-fast-export killed by SIGSEGV. I include a
> gdb backtrace below.
> If you want to me to check other things, please let me know.
> 
> Regards,
> namsh
> 
> [marks] ~/srcs/git/t/trash directory[66]$ gdb ../../git-fast-export
> GNU gdb 6.0
> Copyright 2003 Free Software Foundation, Inc.
> GDB is free software, covered by the GNU General Public License, and you 
> are
> welcome to change it and/or distribute copies of it under certain 
> conditions.
> Type "show copying" to see the conditions.
> There is absolutely no warranty for GDB.  Type "show warranty" for details.
> This GDB was configured as "sparc-sun-solaris2.9"...
> (gdb) r --export-marks=tmp-marks HEAD
> Starting program: /flyvo2/home/namsh/srcs/git/git-fast-export 
> --export-marks=tmp-marks HEAD
> blob
> mark :1
> data 8
> Wohlauf
> 
> reset refs/heads/marks
> commit refs/heads/marks
> mark :2
> author A U Thor <author@example.com> 1112911993 -0700
> committer C O Mitter <committer@example.com> 1112911993 -0700
> data 8
> initial
> M 100644 :1 file
> 
> blob
> mark :3
> data 9
> die Luft
> 
> blob
> mark :4
> data 12
> geht frisch
> 
> commit refs/heads/marks
> mark :5
> author A U Thor <author@example.com> 1112912053 -0700
> committer C O Mitter <committer@example.com> 1112912053 -0700
> data 7
> second
> from :2
> M 100644 :3 file
> M 100644 :4 file2
> 
> blob
> mark :6
> data 4
> und
> 
> commit refs/heads/marks
> mark :7
> author A U Thor <author@example.com> 1112912113 -0700
> committer C O Mitter <committer@example.com> 1112912113 -0700
> data 6
> third
> from :5
> M 100644 :6 file2
> 
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00043024 in export_marks (file=0xffbff0df "tmp-marks")
>     at builtin-fast-export.c:384
> 384                     if (deco && deco->base && deco->base->type == 1) {
> (gdb) p deco
> $1 = (struct object_decoration *) 0x193150
> (gdb) p deco->base
> $2 = (struct object *) 0x2009
> (gdb) p *(struct object_decoration *) 0x193150
> $5 = {base = 0x2009, decoration = 0x0}
> (gdb) p idnums
> $3 = {name = 0x0, size = 1500, nr = 7, hash = 0x190270}
> (gdb) p *(struct object_decoration *) 0x190270
> $6 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x190280
> $9 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x190290
> $10 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x1902a0
> $11 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x1902b0
> $12 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x1902c0
> $13 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x193140
> $14 = {base = 0x0, decoration = 0x0}
> (gdb) p *(struct object_decoration *) 0x193160
> $15 = {base = 0x35323962, decoration = 0x63613534}
> (gdb)
> 

Re: bug found (Re: git-fast-export SIGSEGV on solaris + backtrace)

[Junio C Hamano]

SungHyun Nam <goweol@gmail.com> writes:

> And the code says it:
>         for (i = 0; i < idnums.size; ++i) {
>                 deco++;
>                 if (deco && deco->base && deco->base->type == 1) {
>
> The 'deco' should be post-incremented? or
> Checking code should be  (i < idnums.size - 1)?

The variable "deco" is a pointer that walks over a hashtable from its
offset 0 to its end, so it can never be NULL (well, the code increments
before it tests the variable for NULLness, so it is clear that the test is
bogus).

What was I smoking when I applied df6a7ff (builtin-fast-export: Add
importing and exporting of revision marks, 2008-06-11), I have to
wonder...

Thanks for the fix.

Re: bug found (Re: git-fast-export SIGSEGV on solaris + backtrace)

[Johannes Schindelin]

Hi,

On Thu, 3 Jul 2008, Junio C Hamano wrote:

> What was I smoking when I applied df6a7ff (builtin-fast-export: Add 
> importing and exporting of revision marks, 2008-06-11), I have to 
> wonder...

FWIW I missed it in my review, too...  Well, Junio, it seems we should 
avoid smoking together for a while.

Ciao,
Dscho