git-stash segfaults ...

git-stash segfaults ...

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 526 bytes --]

  When you don't use it from the root of the tree: I was in a subdir of
my git tree, trying to do:

  git stash apply

  and I had:

  $ git stash apply
  error: missing object referenced by
  '0ca54c1fba7fea7dca8a1cc59bce2b2849f26d20' Segmentation fault

  going in the root of the repository (where .git is), all went fine.

Cheers,
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: git-stash segfaults ...

[Alex Riesen]

Pierre Habouzit, Mon, Aug 20, 2007 19:44:27 +0200:
>   When you don't use it from the root of the tree: I was in a subdir of
> my git tree, trying to do:
> 
>   git stash apply
> 
>   and I had:
> 
>   $ git stash apply
>   error: missing object referenced by
>   '0ca54c1fba7fea7dca8a1cc59bce2b2849f26d20' Segmentation fault
> 
>   going in the root of the repository (where .git is), all went fine.

Can't reproduce, but... There are places where the result of the
function which gives this message (deref_tag from tag.c) is not
checked for NULL and used. merge-recursive (used by git-stash) is one
of them (and sha1_name is another).

Pierre, could you please try the patch below and see if the crash is
replaced with a failure?

BTW, is there something unusual happening to that repo? Like manual
object removing, or removal of repo your repo was cloned from, using
something like "git clone -l -s", maybe?

diff --git a/merge-recursive.c b/merge-recursive.c
index 16f6a0f..464fc4e 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -1701,6 +1701,8 @@ static struct commit *get_ref(const char *ref)
 	if (get_sha1(ref, sha1))
 		die("Could not resolve ref '%s'", ref);
 	object = deref_tag(parse_object(sha1), ref, strlen(ref));
+	if (!object)
+		die("Could not resolve ref '%s' down to object", ref);
 	if (object->type == OBJ_TREE)
 		return make_virtual_commit((struct tree*)object,
 			better_branch_name(ref));

Re: git-stash segfaults ...

[Junio C Hamano]

Pierre Habouzit <madcoder@debian.org> writes:

>   When you don't use it from the root of the tree: I was in a subdir of
> my git tree, trying to do:
>
>   git stash apply
>
>   and I had:
>
>   $ git stash apply
>   error: missing object referenced by
>   '0ca54c1fba7fea7dca8a1cc59bce2b2849f26d20' Segmentation fault
>
>   going in the root of the repository (where .git is), all went fine.

I think this was fixed on Jul 25 with ceff079b and 1.5.3-rc4
should have it.

Re: git-stash segfaults ...

[Pierre Habouzit]

[-- Attachment #1: Type: text/plain, Size: 878 bytes --]

On Mon, Aug 20, 2007 at 08:44:22PM +0000, Junio C Hamano wrote:
> Pierre Habouzit <madcoder@debian.org> writes:
> 
> >   When you don't use it from the root of the tree: I was in a subdir of
> > my git tree, trying to do:
> >
> >   git stash apply
> >
> >   and I had:
> >
> >   $ git stash apply
> >   error: missing object referenced by
> >   '0ca54c1fba7fea7dca8a1cc59bce2b2849f26d20' Segmentation fault
> >
> >   going in the root of the repository (where .git is), all went fine.
> 
> I think this was fixed on Jul 25 with ceff079b and 1.5.3-rc4
> should have it.

  Oh ok, sorry for the noise then, I was using rc3 indeed, debian does
not has rc4 packaged yet :)

Cheers,
-- 
·O·  Pierre Habouzit
··O                                                madcoder@debian.org
OOO                                                http://www.madism.org

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: git-stash segfaults ...

[Jeff King]

On Mon, Aug 20, 2007 at 10:02:55PM +0200, Alex Riesen wrote:

> diff --git a/merge-recursive.c b/merge-recursive.c
> index 16f6a0f..464fc4e 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1701,6 +1701,8 @@ static struct commit *get_ref(const char *ref)
>  	if (get_sha1(ref, sha1))
>  		die("Could not resolve ref '%s'", ref);
>  	object = deref_tag(parse_object(sha1), ref, strlen(ref));
> +	if (!object)
> +		die("Could not resolve ref '%s' down to object", ref);
>  	if (object->type == OBJ_TREE)
>  		return make_virtual_commit((struct tree*)object,
>  			better_branch_name(ref));

I posted the exact same patch almost a month ago, but apparently nobody
was interested in applying it:

  http://mid.gmane.org/20070726050726.GC32617@coredump.intra.peff.net

-Peff

Re: git-stash segfaults ...

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I posted the exact same patch almost a month ago, but apparently nobody
> was interested in applying it:
>
>   http://mid.gmane.org/20070726050726.GC32617@coredump.intra.peff.net

I think you were told that your patch was a band-aid for the
symptom, and was not the real fix, in the thread you quoted.

The real fix would be to make merge-recursive operable from
subdirectory (perhaps just the matter of cd-up, but I am not
supposed to be looking at anything other than regression and
documentation fixes this late in the 1.5.3 cycle).

Re: git-stash segfaults ...

[Jeff King]

On Mon, Aug 20, 2007 at 11:54:24PM -0700, Junio C Hamano wrote:

> I think you were told that your patch was a band-aid for the
> symptom, and was not the real fix, in the thread you quoted.

To which I responded that it is still better to report an error than it
is to segfault. In this case, the error message _isn't_ fixing the main
problem, but at least it gives a better starting point for tracking
problems than a segfault.  Not to mention that it _does_ provide the
correct error message in a variety of other situations (e.g.,
merge-recursive given bogus arguments).

But really, merge-recursive isn't generally called by users, and so it
shouldn't get bogus input, so perhaps it isn't worth caring about.

-Peff