Re: [PATCH] Fix segfault in merge-recursive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sat, 9 May 2009, Johannes Schindelin wrote:

Hi,

On Fri, 8 May 2009, Dave O wrote:

On Fri, 8 May 2009, Johannes Schindelin wrote:

When there is no "common" tree (for whatever reason), we must not
throw a segmentation fault.

Noticed by Dave O.

While this patch does prevent a segfault, it totally fails to recognize
any conflicts in the merge.  Reverting 36e3b5e produces an ordinary
merge conflict with some rename/delete conflicts, and others including
content related conflicts.  I'm not sure I wouldn't rather have the
segfault than the grossly incorrect automerge.

I'll continue debugging the triggering condition to see if I can
understand why the index is left dirty, leading to this NULL tree.

One thing I realized while trying to quickly fix the issue for you was
that the recognized merge base was NULL.  I.e. merge-recursive did _not_
find a merge base.

but due to too many renames, maybe it did not.

Probably that is the issue.

That's not what I witnessed, although it's possible I missed something:

./git-merge-crash: line 127: 29751 Segmentation fault      git merge F
count@bokonon:~/git-crash/crash-test$ git merge-base -a F G
8ffd08037781ab7811f9e7983b87a29ea9ea21d9
79ac36c0bd8525e087fdb278bac9cabfa655ba47
count@bokonon:~/git-crash/crash-test$ git merge-base -a 8ffd080 79ac36c
03ca38c681cd9f832fe68d30ea2d8dfa54cbaf75

What I did find, is that the tree is coming back NULL due to the early
return in write_tree_from_memory(), which in turn is due to
unmerged_cache() returning true.  If verbosity is up high enough, that
function will indicate that the paths of the unmerged entries are
exactly the ones affected in the commit I referenced earlier:

  There are unmerged index entries:
  3 data/moved-99
  3 data/moved-990
  [...]

Interestingly, I was able to remove quite a bit of the script and still
induce the crash, including the part where it causes too many renames.
It appears that all that's needed is a delete/rename conflict in a
recursive call.

The new version is here:
http://genericorp.net/~count/git-merge-crash-shorter

Once again, I don't really know what the implications of the index
operations that are happening here are, but the update_stages() call
in a recursive merge must be doing surprising.

    Dave
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]