Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Josh ben Jore]

Hello there,
I’m trying to merge some branches together but all git versions from v1.6.0.4..origin/master which is currently df73af5f667a479764d2b2195cb0cb60b0b89e3d evince this problem. In the hopes of diagnosing this, I have run git under gdb and captured both a stack trace and dumped the structures being passed at each level. I’ve also included the output from GIT_MERGE_VERBOSITY=5

jbenjore@jbenjore0:~/segdev$ GIT_MERGE_VERBOSITY=5 git merge origin/master
Merging:
a7b6206 Merge branch 'bsa_02' of ssh://git.dev.pages/opt/git/repos/dev into bsa_02
virtual origin/master
found 3 common ancestor(s):
8c39e98 25243: Update parents to point to dev
1e994c1 24902: Update the sample das responses that are hashes at the top level to be arrays since we use DAS::lookup_multi in our search base now
e4a886b Adding legacy click log processing scripts
  Merging:
  8c39e98 25243: Update parents to point to dev
  1e994c1 24902: Update the sample das responses that are hashes at the top level to be arrays since we use DAS::lookup_multi in our search base now
  found 1 common ancestor(s):
  051594a Merge branch 'rel_090617' of git+ssh://bmaldonado@git.dev.pages:22/opt/git/repos/dev into rel_090617
  Skipped config/conf/target/dev/host/dshanks0/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev-mchui/parent (merged same as existing)
  Skipped config/conf/target/dev/common/directory.ini.erb (merged same as existing)
  Skipped config/conf/target/dev/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/etc/apache2/ports.conf.erb (merged same as existing)
  Skipped config/conf/target/dev/host/asargent0/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/asargent0/common/test.yml (merged same as existing)
  Skipped config/conf/target/dev/host/jhall0/wpn_rails/appserver.yml (merged same as existing)
  Skipped config/conf/target/dev/host/jhoover0/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-aaron/common/directory.ini (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-aaron/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-aaron/common/test.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-coomer2/common/directory.ini (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-jason/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-jkh/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-seabass/common/directory.ini (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-seabass/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-tyler/common/directory.ini (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-tyler/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/sebastianb0/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/sebastianb0/wpn_rails/appserver.yml (merged same as existing)
  CONFLICT (rename/add): Rename config/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/dev/wpn_rails/appserver.yml in Temporary merge branch 1. config/conf/target/dev/wpn_rails/appse2
  Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge branch 2 instead
  Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as existing)
  Skipped config/conf/target/dev/wpn_rails/cache.yml (merged same as existing)
  Skipped config/conf/target/dev/wpn_rails/customer_support.yml (merged same as existing)
  Skipped config/conf/target/dev/wpn_rails/debug.yml (merged same as existing)
  Skipped config/conf/target/dev/wpn_rails/omniture.yml (merged same as existing)
  Skipped config/conf/target/dev/zxtm/rules.yml (merged same as existing)
  Skipped config/conf/target/dev/host/msquires1/common/credentials.ini (merged same as existing)
  Skipped config/conf/target/dev/host/msquires1/common/directory.ini (merged same as existing)
  Skipped config/conf/target/dev/host/msquires1/toolserver/toolserver.ini (merged same as existing)
  Skipped config/dynconf/target/dev/dynamic_config.yml (merged same as existing)
  Skipped config/dynconf/target/dev/mobile_useragents.yml (merged same as existing)
  Removing zxtm/traffic_scripts/web/beta_test.ts.erb
  There are unmerged index entries:
  2 config/conf/target/dev/wpn_rails/appserver.yml
  3 config/conf/target/dev/wpn_rails/appserver.yml
  Merging:
  virtual merged tree
  e4a886b Adding legacy click log processing scripts
  found 1 common ancestor(s):
  09fb055 Merge commit 'rel_090630_prod_02'
Segmentation fault

Backtrace without variables:

#0  parse_tree (item=0x0) at tree.c:255
#1  0x00000000004766ec in init_tree_desc_from_tree (desc=0x7fffe99a01a8, tree=0x0) at merge-recursive.c:161
#2  0x0000000000477c37 in merge_trees (o=0x7fffe99a04a0, head=0x0, merge=0x7a81c0, common=0x7ac158, result=0x7fffe99a03f8) at merge-recursive.c:186
#3  0x0000000000479207 in merge_recursive (o=0x7fffe99a04a0, h1=0x18555c0, h2=0x795fc8, ca=0x0, result=0x7fffe99a0460) at merge-recursive.c:1291
#4  0x00000000004791ac in merge_recursive (o=0x7fffe99a04a0, h1=0x796058, h2=0x795f80, ca=0x9e7b20, result=0x7fffe99a0538) at merge-recursive.c:1277
#5  0x0000000000432efc in try_merge_strategy (strategy=<value optimized out>, common=0x7d5ac0, head_arg=0x4b2eca "HEAD") at builtin-merge.c:565
#6  0x00000000004341cd in cmd_merge (argc=<value optimized out>, argv=0x7fffe99a2050, prefix=<value optimized out>) at builtin-merge.c:1114
#7  0x00000000004049a3 in handle_internal_command (argc=2, argv=0x7fffe99a2050) at git.c:246
#8  0x0000000000404b8e in main (argc=2, argv=0x7fffe99a2050) at git.c:438


Backtrace with variables:
#0  parse_tree (item=0x0) at tree.c:255

#1  0x00000000004766ec in init_tree_desc_from_tree (desc=0x7fffe99a01a8, tree=0x0) at merge-recursive.c:161
desc = {buffer = 0xf0000, entry = {sha1 = 0x7a0138 "\005", path = 0xfff0ffff <Address 0xfff0ffff out of bounds>, mode = 4566669}, size = 8195856}

#2  0x0000000000477c37 in merge_trees (o=0x7fffe99a04a0, head=0x0, merge=0x7a81c0, common=0x7ac158, result=0x7fffe99a03f8) at merge-recursive.c:186
merge = {object = {parsed = 0, used = 0, type = 2, flags = 0, sha1 = "‹øSÔøΩ\nZ6\001ÔøΩ\033H\f\a\236b6\226yÔøΩÔøΩ"}, buffer = 0x0, size = 0}
x/4 merge->object->sha1 = {0xee53bfdc      0x01365a0a      0x0c481bb8      0x36629e07}
common = {object = {parsed = 1, used = 0, type = 2, flags = 0, sha1 = "@\237SÔøΩ\2258$\202ÔøΩDÔøΩ\002u*ÔøΩst@\223`"}, buffer = 0x1d11d90, size = 1663}
x/4 common->object->sha1 = {0xc9539f40      0x82243895      0x02e944ac      0x73ed2a75}
*result = 0

#3  0x0000000000479207 in merge_recursive (o=0x7fffe99a04a0, h1=0x18555c0, h2=0x795fc8, ca=0x0, result=0x7fffe99a0460) at merge-recursive.c:1291
o = {branch1 = 0x4c8db6 "Temporary merge branch 1", branch2 = 0x4c8dcf "Temporary merge branch 2", subtree_merge = 0, buffer_output = 1, verbosity = 2, diff_rename_limit = -1, 
  merge_rename_limit = -1, call_depth = 1, obuf = {alloc = 0, len = 0, buf = 0x7360e0 ""}, current_file_set = {items = 0x19d0f90, nr = 14750, alloc = 14752, strdup_strings = 1}, 
  current_directory_set = {items = 0x19c8180, nr = 2256, alloc = 2272, strdup_strings = 1}}
h1 = {object = {parsed = 1, used = 0, type = 0, flags = 0, sha1 = '\0' <repeats 19 times>}, util = 0x4c8e01, indegree = 0, date = 0, parents = 0x9e2290, tree = 0x0, buffer = 0x0}
h2 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 = "ÔøΩ\206ÔøΩKÔøΩ9ÔøΩfi&die;ÔøΩÔøΩRÔøΩKÔøΩÔøΩ|ÔøΩÔøΩ"}, util = 0x0, indegree = 0, date = 1248282000, parents = 0x794e30, tree = 0x7a81c0, 
  buffer = 0x7b3160 "tree dcbf53ee0a5a3601b81b480c079e62369679bbdf\nparent 5046433c0b601f5a24f1149d44ad7b7b7f5fc9f8\nauthor Michael Bradshaw <mbradshaw@W3M166.(none)> 1248282000 -0700\ncommitter Michae}
x/4 h2->object->sha1 = {0xb386a8e4      0xa839c34b      0xe1b5acde      0xb64bda52}

#4  0x00000000004791ac in merge_recursive (o=0x7fffe99a04a0, h1=0x796058, h2=0x795f80, ca=0x9e7b20, result=0x7fffe99a0538) at merge-recursive.c:1277
o = {branch1 = 0x4c8db6 "Temporary merge branch 1", branch2 = 0x4c8dcf "Temporary merge branch 2", subtree_merge = 0, buffer_output = 1, verbosity = 2, diff_rename_limit = -1, 
  merge_rename_limit = -1, call_depth = 1, obuf = {alloc = 0, len = 0, buf = 0x7360e0 ""}, current_file_set = {items = 0x19d0f90, nr = 14750, alloc = 14752, strdup_strings = 1}, 
  current_directory_set = {items = 0x19c8180, nr = 2256, alloc = 2272, strdup_strings = 1}}
h1 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 = "ÔøΩÔøΩ eÔøΩÔøΩ>Jo\bÔøΩ\216zÔøΩ\220/hÔøΩÔøΩ"}, util = 0x0, indegree = 0, date = 1248795280, parents = 0x7949b0, tree = 0x7a7fb8, 
  buffer = 0x794c80 "tree 57fee64850eec3f8a57ffe76878f5e76cdd26ed3\nparent 9320f2c497012a8c3529eada30b9e697757b7556\nparent bfcc7b0ef71c259251c76a5809ccce5970278539\nauthor Josh ben Jore <jbenjore@whit}
x/4 h1->object->sha1 = {0x6520b6a7      0x3ef5afe8      0xe4086f4a      0x90a87a8e}
h2 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 = "ÔøΩ\214ÔøΩÔøΩ@NÔøΩA^ÔøΩ\bG\032^04ÔøΩUÔøΩ"}, util = 0x7fffe99a28c0, indegree = 0, date = 1248308671, parents = 0x77b6c0, tree = 0x7a7f90, 
  buffer = 0x794160 "tree 4b46fa508134e277a219193e705f2f06a903c380\nparent e4a886b34bc339a8deacb5e152da4bb6d77cacff\nparent 23a435ff7710bce43c1918cfd2c6fb2655ea043c\nauthor Michael Bradshaw <mbradshaw@}
x/4 h2->object->sha1 = {0xf9fe8cc6      0xe64e40bf      0x08e55e41      0x305e1a47}
ca = {item = 0x79bf20, next = 0x9e3190}
*result = (struct commit *) 0xff0a000000000000
**result = 

#5  0x0000000000432efc in try_merge_strategy (strategy=<value optimized out>, common=0x7d5ac0, head_arg=0x4b2eca "HEAD") at builtin-merge.c:565
common = {item = 0x795fc8, next = 0x89df40}


#6  0x00000000004341cd in cmd_merge (argc=<value optimized out>, argv=0x7fffe99a2050, prefix=<value optimized out>) at builtin-merge.c:1114
argv[0] = "origin/master"
argv[1] = 0
argv[2] = 0

#7  0x00000000004049a3 in handle_internal_command (argc=2, argv=0x7fffe99a2050) at git.c:246
argv[0] = "origin/master"
argv[1] = 0
argv[2] = 0

#8  0x0000000000404b8e in main (argc=2, argv=0x7fffe99a2050) at git.c:438
argv[0] = "origin/master"
argv[1] = 0
argv[2] = 0


Regards,
Josh

Fw: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Josh ben Jore]

Hello there,
With apologies, I am resending the previous message after taking care to strip gdb's dump of the object->sha1 pointers and replace them with the contents of gdb's "x/4 $thing->object->sha1". I heard from one of you that gmail showed only random crud. It looked ok to me. Sorry for the annoyance.

The original message follows. I hope this one goes through ok.

...

Hello there,
Im trying to merge some branches together but all git versions from v1.6.0.4..origin/master which is
currently df73af5f667a479764d2b2195cb0cb60b0b89e3d evince this problem. In the hopes of diagnosing
this, I have run git under gdb and captured both a stack trace and dumped the structures being passed at each
level. Ive also included the output from GIT_MERGE_VERBOSITY=5

jbenjore <at> jbenjore0:~/segdev$ GIT_MERGE_VERBOSITY=5 git merge origin/master
Merging:
a7b6206 Merge branch 'bsa_02' of ssh://git.dev.pages/opt/git/repos/dev into bsa_02
virtual origin/master
found 3 common ancestor(s):
8c39e98 25243: Update parents to point to dev
1e994c1 24902: Update the sample das responses that are hashes at the top level to be arrays since we use
DAS::lookup_multi in our search base now
e4a886b Adding legacy click log processing scripts
  Merging:
  8c39e98 25243: Update parents to point to dev
  1e994c1 24902: Update the sample das responses that are hashes at the top level to be arrays since we use
DAS::lookup_multi in our search base now
  found 1 common ancestor(s):
  051594a Merge branch 'rel_090617' of git+ssh://bmaldonado <at> git.dev.pages:22/opt/git/repos/dev
into rel_090617
  Skipped config/conf/target/dev/host/dshanks0/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev-mchui/parent (merged same as existing)
  Skipped config/conf/target/dev/common/directory.ini.erb (merged same as existing)
  Skipped config/conf/target/dev/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/etc/apache2/ports.conf.erb (merged same as existing)
  Skipped config/conf/target/dev/host/asargent0/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/asargent0/common/test.yml (merged same as existing)
  Skipped config/conf/target/dev/host/jhall0/wpn_rails/appserver.yml (merged same as existing)
  Skipped config/conf/target/dev/host/jhoover0/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-aaron/common/directory.ini (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-aaron/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-aaron/common/test.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-coomer2/common/directory.ini (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-jason/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-jkh/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-seabass/common/directory.ini (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-seabass/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-tyler/common/directory.ini (merged same as existing)
  Skipped config/conf/target/dev/host/kvm-tyler/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/sebastianb0/common/services.yml (merged same as existing)
  Skipped config/conf/target/dev/host/sebastianb0/wpn_rails/appserver.yml (merged same as existing)
  CONFLICT (rename/add): Rename
config/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/dev/wpn_rails/appserver.yml
in Temporary merge branch 1. config/conf/target/dev/wpn_rails/appse2
  Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge branch 2 instead
  Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as existing)
  Skipped config/conf/target/dev/wpn_rails/cache.yml (merged same as existing)
  Skipped config/conf/target/dev/wpn_rails/customer_support.yml (merged same as existing)
  Skipped config/conf/target/dev/wpn_rails/debug.yml (merged same as existing)
  Skipped config/conf/target/dev/wpn_rails/omniture.yml (merged same as existing)
  Skipped config/conf/target/dev/zxtm/rules.yml (merged same as existing)
  Skipped config/conf/target/dev/host/msquires1/common/credentials.ini (merged same as existing)
  Skipped config/conf/target/dev/host/msquires1/common/directory.ini (merged same as existing)
  Skipped config/conf/target/dev/host/msquires1/toolserver/toolserver.ini (merged same as existing)
  Skipped config/dynconf/target/dev/dynamic_config.yml (merged same as existing)
  Skipped config/dynconf/target/dev/mobile_useragents.yml (merged same as existing)
  Removing zxtm/traffic_scripts/web/beta_test.ts.erb
  There are unmerged index entries:
  2 config/conf/target/dev/wpn_rails/appserver.yml
  3 config/conf/target/dev/wpn_rails/appserver.yml
  Merging:
  virtual merged tree
  e4a886b Adding legacy click log processing scripts
  found 1 common ancestor(s):
  09fb055 Merge commit 'rel_090630_prod_02'
Segmentation fault

Backtrace without variables:

#0  parse_tree (item=0x0) at tree.c:255
#1  0x00000000004766ec in init_tree_desc_from_tree (desc=0x7fffe99a01a8, tree=0x0) at merge-recursive.c:161
#2  0x0000000000477c37 in merge_trees (o=0x7fffe99a04a0, head=0x0, merge=0x7a81c0,
common=0x7ac158, result=0x7fffe99a03f8) at merge-recursive.c:186
#3  0x0000000000479207 in merge_recursive (o=0x7fffe99a04a0, h1=0x18555c0, h2=0x795fc8, ca=0x0,
result=0x7fffe99a0460) at merge-recursive.c:1291
#4  0x00000000004791ac in merge_recursive (o=0x7fffe99a04a0, h1=0x796058, h2=0x795f80,
ca=0x9e7b20, result=0x7fffe99a0538) at merge-recursive.c:1277
#5  0x0000000000432efc in try_merge_strategy (strategy=<value optimized out>, common=0x7d5ac0,
head_arg=0x4b2eca "HEAD") at builtin-merge.c:565
#6  0x00000000004341cd in cmd_merge (argc=<value optimized out>, argv=0x7fffe99a2050,
prefix=<value optimized out>) at builtin-merge.c:1114
#7  0x00000000004049a3 in handle_internal_command (argc=2, argv=0x7fffe99a2050) at git.c:246
#8  0x0000000000404b8e in main (argc=2, argv=0x7fffe99a2050) at git.c:438


Backtrace with variables:
#0  parse_tree (item=0x0) at tree.c:255

#1  0x00000000004766ec in init_tree_desc_from_tree (desc=0x7fffe99a01a8, tree=0x0) at merge-recursive.c:161
desc = {buffer = 0xf0000, entry = {sha1 = 0x7a0138 "\005", path = 0xfff0ffff <Address 0xfff0ffff out of
bounds>, mode = 4566669}, size = 8195856}

#2  0x0000000000477c37 in merge_trees (o=0x7fffe99a04a0, head=0x0, merge=0x7a81c0,
common=0x7ac158, result=0x7fffe99a03f8) at merge-recursive.c:186
merge = {object = {parsed = 0, used = 0, type = 2, flags = 0, sha1 =
{0xee53bfdc      0x01365a0a      0x0c481bb8      0x36629e07}, buffer = 0x0, size = 0}
common = {object = {parsed = 1, used = 0, type = 2, flags = 0, sha1 =
{0xc9539f40      0x82243895      0x02e944ac      0x73ed2a75}}, buffer = 0x1d11d90, size = 1663}
*result = 0

#3  0x0000000000479207 in merge_recursive (o=0x7fffe99a04a0, h1=0x18555c0, h2=0x795fc8, ca=0x0,
result=0x7fffe99a0460) at merge-recursive.c:1291
o = {branch1 = 0x4c8db6 "Temporary merge branch 1", branch2 = 0x4c8dcf "Temporary merge branch 2",
subtree_merge = 0, buffer_output = 1, verbosity = 2, diff_rename_limit = -1, 
  merge_rename_limit = -1, call_depth = 1, obuf = {alloc = 0, len = 0, buf = 0x7360e0 ""},
current_file_set = {items = 0x19d0f90, nr = 14750, alloc = 14752, strdup_strings = 1}, 
  current_directory_set = {items = 0x19c8180, nr = 2256, alloc = 2272, strdup_strings = 1}}
h1 = {object = {parsed = 1, used = 0, type = 0, flags = 0, sha1 = '\0' <repeats 19 times>}, util = 0x4c8e01,
indegree = 0, date = 0, parents = 0x9e2290, tree = 0x0, buffer = 0x0}
h2 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 =
{0xb386a8e4      0xa839c34b      0xe1b5acde      0xb64bda52}},
util = 0x0, indegree = 0, date = 1248282000, parents = 0x794e30, tree = 0x7a81c0, 
  buffer = 0x7b3160 "tree dcbf53ee0a5a3601b81b480c079e62369679bbdf\nparent
5046433c0b601f5a24f1149d44ad7b7b7f5fc9f8\nauthor Michael Bradshaw <mbradshaw <at> W3M166.(none)>
1248282000 -0700\ncommitter Michae}

#4  0x00000000004791ac in merge_recursive (o=0x7fffe99a04a0, h1=0x796058, h2=0x795f80,
ca=0x9e7b20, result=0x7fffe99a0538) at merge-recursive.c:1277
o = {branch1 = 0x4c8db6 "Temporary merge branch 1", branch2 = 0x4c8dcf "Temporary merge branch 2",
subtree_merge = 0, buffer_output = 1, verbosity = 2, diff_rename_limit = -1, 
  merge_rename_limit = -1, call_depth = 1, obuf = {alloc = 0, len = 0, buf = 0x7360e0 ""},
current_file_set = {items = 0x19d0f90, nr = 14750, alloc = 14752, strdup_strings = 1}, 
  current_directory_set = {items = 0x19c8180, nr = 2256, alloc = 2272, strdup_strings = 1}}
h1 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 = {0x6520b6a7      0x3ef5afe8      0xe4086f4a      0x90a87a8e}}, util = 0x0, indegree = 0, date =
1248795280, parents = 0x7949b0, tree = 0x7a7fb8, 
  buffer = 0x794c80 "tree 57fee64850eec3f8a57ffe76878f5e76cdd26ed3\nparent
9320f2c497012a8c3529eada30b9e697757b7556\nparent
bfcc7b0ef71c259251c76a5809ccce5970278539\nauthor Josh ben Jore <jbenjore <at> whit}
h2 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 =
{0xf9fe8cc6      0xe64e40bf      0x08e55e41      0x305e1a47}}, util = 0x7fffe99a28c0,
indegree = 0, date = 1248308671, parents = 0x77b6c0, tree = 0x7a7f90, 
  buffer = 0x794160 "tree 4b46fa508134e277a219193e705f2f06a903c380\nparent
e4a886b34bc339a8deacb5e152da4bb6d77cacff\nparent
23a435ff7710bce43c1918cfd2c6fb2655ea043c\nauthor Michael Bradshaw <mbradshaw@}
ca = {item = 0x79bf20, next = 0x9e3190}
*result = (struct commit *) 0xff0a000000000000
**result = 

#5  0x0000000000432efc in try_merge_strategy (strategy=<value optimized out>, common=0x7d5ac0,
head_arg=0x4b2eca "HEAD") at builtin-merge.c:565
common = {item = 0x795fc8, next = 0x89df40}


#6  0x00000000004341cd in cmd_merge (argc=<value optimized out>, argv=0x7fffe99a2050,
prefix=<value optimized out>) at builtin-merge.c:1114
argv[0] = "origin/master"
argv[1] = 0
argv[2] = 0

#7  0x00000000004049a3 in handle_internal_command (argc=2, argv=0x7fffe99a2050) at git.c:246
argv[0] = "origin/master"
argv[1] = 0
argv[2] = 0

#8  0x0000000000404b8e in main (argc=2, argv=0x7fffe99a2050) at git.c:438
argv[0] = "origin/master"
argv[1] = 0
argv[2] = 0


Regards,
Josh

Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Josh ben Jore]

I know more now. I've never cracked the hood on git before so this is all
new to me. The below is a transcript of my thinking but the conclusion I'm
at /now/ is that merge_trees should return a tree to mrtree but returns NULL
instead.

Transcript follows:

The root merge is:

    git rev-parse bsa_02 # 7b62065e8aff53e4a6f08e48e7aa8902f68e8d6
    git rev-parse master # c68cfef9bf404ee6415ee508471a5e3034d755da

    # GIT_MERGE_VERBOSITY=5
    Merging:
    a7b6206 Merge branch 'bsa_02' of ssh://git.dev.pages/opt/git/repos/dev
into bsa_02
    virtual origin/master

The merge bases for those are:

    git merge-base bsa_02 master
    # 8c39e98cdd59753f2a5c6f9fc829592173fbdbfb 25243: Update parents to
point to dev
    # 1e994c1c3397c4919103faf0b0bd264be08ec279 24902: Update the sample das
responses that are hashes at the top level to be arrays since we use
DAS::lookup_multi in our search base now
    # e4a886b34bc339a8deacb5e152da4bb6d77cacff Adding legacy click log
processing scripts

    # GIT_MERGE_VERBOSITY=5
    found 3 common ancestor(s):
    8c39e98 25243: Update parents to point to dev
    1e994c1 24902: Update the sample das responses that are hashes at the
top level to be arrays since we use DAS::lookup_multi in our search base now
    e4a886b Adding legacy click log processing scripts

Running the merge with GIT_MERGE_VERBOSITY=5 shows how the recursive
strategy works. It sorts the merge-bases by date, merges two at a
time, producing a new virtual commit, merging the next item with
result of the last.

Merging 8c39e98 and 1e994c1:

      Merging:
      8c39e98 25243: Update parents to point to dev
      1e994c1 24902: Update the sample das responses that are hashes at the
top level to be arrays since we use DAS::lookup_multi in our search base now
      found 1 common ancestor(s):
      051594a Merge branch 'rel_090617' of git+ssh://bmaldonado <at>
git.dev.pages:22/opt/git/repos/dev into rel_090617

It's going to produce a conflict:

      Skipped config/conf/target/dev/host/dshanks0/common/services.yml
(merged same as existing)
      Skipped config/conf/target/dev-mchui/parent (merged same as existing)
      Skipped config/conf/target/dev/common/directory.ini.erb (merged same
as existing)
      Skipped config/conf/target/dev/common/services.yml (merged same as
existing)
      Skipped config/conf/target/dev/etc/apache2/ports.conf.erb (merged same
as existing)
      Skipped config/conf/target/dev/host/asargent0/common/services.yml
(merged same as existing)
      Skipped config/conf/target/dev/host/asargent0/common/test.yml (merged
same as existing)
      Skipped config/conf/target/dev/host/jhall0/wpn_rails/appserver.yml
(merged same as existing)
      Skipped config/conf/target/dev/host/jhoover0/common/services.yml
(merged same as existing)
      Skipped config/conf/target/dev/host/kvm-aaron/common/directory.ini
(merged same as existing)
      Skipped config/conf/target/dev/host/kvm-aaron/common/services.yml
(merged same as existing)
      Skipped config/conf/target/dev/host/kvm-aaron/common/test.yml (merged
same as existing)
      Skipped config/conf/target/dev/host/kvm-coomer2/common/directory.ini
(merged same as existing)
      Skipped config/conf/target/dev/host/kvm-jason/common/services.yml
(merged same as existing)
      Skipped config/conf/target/dev/host/kvm-jkh/common/services.yml
(merged same as existing)
      Skipped config/conf/target/dev/host/kvm-seabass/common/directory.ini
(merged same as existing)
      Skipped config/conf/target/dev/host/kvm-seabass/common/services.yml
(merged same as existing)
      Skipped config/conf/target/dev/host/kvm-tyler/common/directory.ini
(merged same as existing)
      Skipped config/conf/target/dev/host/kvm-tyler/common/services.yml
(merged same as existing)
      Skipped config/conf/target/dev/host/sebastianb0/common/services.yml
(merged same as existing)
      Skipped
config/conf/target/dev/host/sebastianb0/wpn_rails/appserver.yml (merged same
as existing)
      CONFLICT (rename/add): Rename
config/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/de
v/wpn_rails/appserver.yml in Temporary merge branch 1.
config/conf/target/dev/wpn_rails/a\
ppse2
      Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary
merge branch 2 instead
      Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as
existing)
      Skipped config/conf/target/dev/wpn_rails/cache.yml (merged same as
existing)
      Skipped config/conf/target/dev/wpn_rails/customer_support.yml (merged
same as existing)
      Skipped config/conf/target/dev/wpn_rails/debug.yml (merged same as
existing)
      Skipped config/conf/target/dev/wpn_rails/omniture.yml (merged same as
existing)
      Skipped config/conf/target/dev/zxtm/rules.yml (merged same as
existing)
      Skipped config/conf/target/dev/host/msquires1/common/credentials.ini
(merged same as existing)
      Skipped config/conf/target/dev/host/msquires1/common/directory.ini
(merged same as existing)
      Skipped
config/conf/target/dev/host/msquires1/toolserver/toolserver.ini (merged same
as existing)
      Skipped config/dynconf/target/dev/dynamic_config.yml (merged same as
existing)
      Skipped config/dynconf/target/dev/mobile_useragents.yml (merged same
as existing)
      Removing zxtm/traffic_scripts/web/beta_test.ts.erb
      There are unmerged index entries:
      2 config/conf/target/dev/wpn_rails/appserver.yml
      3 config/conf/target/dev/wpn_rails/appserver.yml

The resultant trees are merged but return a null in mrtree.

    clean = merge_trees(o, h1->tree, h2->tree,
merged_common_ancestors->tree,
                        &mrtree);


merge_recursive.c:1294 produced a "virtual merged tree" and returned
it for use by the next merge. This "virtual merged tree" uses the NULL
tree as generated by merge_trees.

    if (o->call_depth) {
            *result = make_virtual_commit(mrtree, "merged tree");
            commit_list_insert(h1, &(*result)->parents);
            commit_list_insert(h2, &(*result)->parents->next);
    }

Also, the action *result=... overwrites one of the parameters to this
merge. The pointer to merged_common_ancestors is being read from in
this loop as well as modified. Presumably, the result of this is so
merged_common_ancestors is continually refreshed as a new tree.

  1131          for (iter = ca; iter; iter = iter->next) {
  ...
  1146                  merge_recursive(o, merged_common_ancestors,
iter->item,
  1147                                  NULL, &merged_common_ancestors);

The status of merged_common_ancestors is that it is not reified and
does not exist in the git database. The ->tree entry is NULL because
merge_trees said so.

  merged_common_ancestors = {
    object = {
      parsed = 1,
      used = 0,
      type = 0,
      flags = 0,
      sha1 = '\0' <repeats 19 times>
    },
    util = 0x4c8e01,
    indegree = 0,
    date = 0,
    parents = 0x9e2290,
    tree = 0x0,
    buffer = 0x0
  }

The merge between the "virtual merged tree" of (8c39e98 & 1e994c1)
currently in merged_common_ancestors and e4a886b off of the ca/iter
loop attempts to proceed:

  Merging:
  virtual merged tree
  e4a886b Adding legacy click log processing scripts
  found 1 common ancestor(s):
  09fb055 Merge commit 'rel_090630_prod_02'

It passes an empty list of common ancestors so merge_recursive finds
some itself. Here, 'ca' corresponds to the NULL passed by the
recursive call of merge_recursive to itself.

  1113          if (!ca) {
  1114                  ca = get_merge_bases(h1, h2, 1);
  1115                  ca = reverse_commit_list(ca);
  1116          }

This found the sole ancestor 09fb055 and shifted it off the 'ca' list.

  1122          merged_common_ancestors = pop_commit(&ca);

The sub-merging loop is now skipped because ca is empty. There was no
more work to do so nothing is done to ancestor 09fb055.

  1131          for (iter = ca; iter; iter = iter->next) {

Now "virtual merged tree" and e4a886b are handed off to be actually
merged, I guess.

  1157          clean = merge_trees(o, h1->tree, h2->tree,
merged_common_ancestors->tree,
  1158                              &mrtree);

We will now segfault because h1->tree is NULL. h1->tree's NULL is the
NULL returned to mrtree by the merge_trees() of above.

Regards,
Josh

On 7/28/09 3:23 PM, "Josh ben Jore" <jbenjore@whitepages.com> wrote:

> Hello there,
> With apologies, I am resending the previous message after taking care to strip
> gdb's dump of the object->sha1 pointers and replace them with the contents of
> gdb's "x/4 $thing->object->sha1". I heard from one of you that gmail showed
> only random crud. It looked ok to me. Sorry for the annoyance.
>
> The original message follows. I hope this one goes through ok.
>
> ...
>
> Hello there,
> Im trying to merge some branches together but all git versions from
> v1.6.0.4..origin/master which is
> currently df73af5f667a479764d2b2195cb0cb60b0b89e3d evince this problem. In the
> hopes of diagnosing
> this, I have run git under gdb and captured both a stack trace and dumped the
> structures being passed at each
> level. Ive also included the output from GIT_MERGE_VERBOSITY=5
>
> jbenjore <at> jbenjore0:~/segdev$ GIT_MERGE_VERBOSITY=5 git merge
> origin/master
> Merging:
> a7b6206 Merge branch 'bsa_02' of ssh://git.dev.pages/opt/git/repos/dev into
> bsa_02
> virtual origin/master
> found 3 common ancestor(s):
> 8c39e98 25243: Update parents to point to dev
> 1e994c1 24902: Update the sample das responses that are hashes at the top
> level to be arrays since we use
> DAS::lookup_multi in our search base now
> e4a886b Adding legacy click log processing scripts
>   Merging:
>   8c39e98 25243: Update parents to point to dev
>   1e994c1 24902: Update the sample das responses that are hashes at the top
> level to be arrays since we use
> DAS::lookup_multi in our search base now
>   found 1 common ancestor(s):
>   051594a Merge branch 'rel_090617' of git+ssh://bmaldonado <at>
> git.dev.pages:22/opt/git/repos/dev
> into rel_090617
>   Skipped config/conf/target/dev/host/dshanks0/common/services.yml (merged
> same as existing)
>   Skipped config/conf/target/dev-mchui/parent (merged same as existing)
>   Skipped config/conf/target/dev/common/directory.ini.erb (merged same as
> existing)
>   Skipped config/conf/target/dev/common/services.yml (merged same as existing)
>   Skipped config/conf/target/dev/etc/apache2/ports.conf.erb (merged same as
> existing)
>   Skipped config/conf/target/dev/host/asargent0/common/services.yml (merged
> same as existing)
>   Skipped config/conf/target/dev/host/asargent0/common/test.yml (merged same
> as existing)
>   Skipped config/conf/target/dev/host/jhall0/wpn_rails/appserver.yml (merged
> same as existing)
>   Skipped config/conf/target/dev/host/jhoover0/common/services.yml (merged
> same as existing)
>   Skipped config/conf/target/dev/host/kvm-aaron/common/directory.ini (merged
> same as existing)
>   Skipped config/conf/target/dev/host/kvm-aaron/common/services.yml (merged
> same as existing)
>   Skipped config/conf/target/dev/host/kvm-aaron/common/test.yml (merged same
> as existing)
>   Skipped config/conf/target/dev/host/kvm-coomer2/common/directory.ini (merged
> same as existing)
>   Skipped config/conf/target/dev/host/kvm-jason/common/services.yml (merged
> same as existing)
>   Skipped config/conf/target/dev/host/kvm-jkh/common/services.yml (merged same
> as existing)
>   Skipped config/conf/target/dev/host/kvm-seabass/common/directory.ini (merged
> same as existing)
>   Skipped config/conf/target/dev/host/kvm-seabass/common/services.yml (merged
> same as existing)
>   Skipped config/conf/target/dev/host/kvm-tyler/common/directory.ini (merged
> same as existing)
>   Skipped config/conf/target/dev/host/kvm-tyler/common/services.yml (merged
> same as existing)
>   Skipped config/conf/target/dev/host/sebastianb0/common/services.yml (merged
> same as existing)
>   Skipped config/conf/target/dev/host/sebastianb0/wpn_rails/appserver.yml
> (merged same as existing)
>   CONFLICT (rename/add): Rename
> config/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/dev/
> wpn_rails/appserver.yml
> in Temporary merge branch 1. config/conf/target/dev/wpn_rails/appse2
>   Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge
> branch 2 instead
>   Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as
> existing)
>   Skipped config/conf/target/dev/wpn_rails/cache.yml (merged same as existing)
>   Skipped config/conf/target/dev/wpn_rails/customer_support.yml (merged same
> as existing)
>   Skipped config/conf/target/dev/wpn_rails/debug.yml (merged same as existing)
>   Skipped config/conf/target/dev/wpn_rails/omniture.yml (merged same as
> existing)
>   Skipped config/conf/target/dev/zxtm/rules.yml (merged same as existing)
>   Skipped config/conf/target/dev/host/msquires1/common/credentials.ini (merged
> same as existing)
>   Skipped config/conf/target/dev/host/msquires1/common/directory.ini (merged
> same as existing)
>   Skipped config/conf/target/dev/host/msquires1/toolserver/toolserver.ini
> (merged same as existing)
>   Skipped config/dynconf/target/dev/dynamic_config.yml (merged same as
> existing)
>   Skipped config/dynconf/target/dev/mobile_useragents.yml (merged same as
> existing)
>   Removing zxtm/traffic_scripts/web/beta_test.ts.erb
>   There are unmerged index entries:
>   2 config/conf/target/dev/wpn_rails/appserver.yml
>   3 config/conf/target/dev/wpn_rails/appserver.yml
>   Merging:
>   virtual merged tree
>   e4a886b Adding legacy click log processing scripts
>   found 1 common ancestor(s):
>   09fb055 Merge commit 'rel_090630_prod_02'
> Segmentation fault
>
> Backtrace without variables:
>
> #0  parse_tree (item=0x0) at tree.c:255
> #1  0x00000000004766ec in init_tree_desc_from_tree (desc=0x7fffe99a01a8,
> tree=0x0) at merge-recursive.c:161
> #2  0x0000000000477c37 in merge_trees (o=0x7fffe99a04a0, head=0x0,
> merge=0x7a81c0,
> common=0x7ac158, result=0x7fffe99a03f8) at merge-recursive.c:186
> #3  0x0000000000479207 in merge_recursive (o=0x7fffe99a04a0, h1=0x18555c0,
> h2=0x795fc8, ca=0x0,
> result=0x7fffe99a0460) at merge-recursive.c:1291
> #4  0x00000000004791ac in merge_recursive (o=0x7fffe99a04a0, h1=0x796058,
> h2=0x795f80,
> ca=0x9e7b20, result=0x7fffe99a0538) at merge-recursive.c:1277
> #5  0x0000000000432efc in try_merge_strategy (strategy=<value optimized out>,
> common=0x7d5ac0,
> head_arg=0x4b2eca "HEAD") at builtin-merge.c:565
> #6  0x00000000004341cd in cmd_merge (argc=<value optimized out>,
> argv=0x7fffe99a2050,
> prefix=<value optimized out>) at builtin-merge.c:1114
> #7  0x00000000004049a3 in handle_internal_command (argc=2,
> argv=0x7fffe99a2050) at git.c:246
> #8  0x0000000000404b8e in main (argc=2, argv=0x7fffe99a2050) at git.c:438
>
>
> Backtrace with variables:
> #0  parse_tree (item=0x0) at tree.c:255
>
> #1  0x00000000004766ec in init_tree_desc_from_tree (desc=0x7fffe99a01a8,
> tree=0x0) at merge-recursive.c:161
> desc = {buffer = 0xf0000, entry = {sha1 = 0x7a0138 "\005", path = 0xfff0ffff
> <Address 0xfff0ffff out of
> bounds>, mode = 4566669}, size = 8195856}
>
> #2  0x0000000000477c37 in merge_trees (o=0x7fffe99a04a0, head=0x0,
> merge=0x7a81c0,
> common=0x7ac158, result=0x7fffe99a03f8) at merge-recursive.c:186
> merge = {object = {parsed = 0, used = 0, type = 2, flags = 0, sha1 =
> {0xee53bfdc      0x01365a0a      0x0c481bb8      0x36629e07}, buffer = 0x0,
> size = 0}
> common = {object = {parsed = 1, used = 0, type = 2, flags = 0, sha1 =
> {0xc9539f40      0x82243895      0x02e944ac      0x73ed2a75}}, buffer =
> 0x1d11d90, size = 1663}
> *result = 0
>
> #3  0x0000000000479207 in merge_recursive (o=0x7fffe99a04a0, h1=0x18555c0,
> h2=0x795fc8, ca=0x0,
> result=0x7fffe99a0460) at merge-recursive.c:1291
> o = {branch1 = 0x4c8db6 "Temporary merge branch 1", branch2 = 0x4c8dcf
> "Temporary merge branch 2",
> subtree_merge = 0, buffer_output = 1, verbosity = 2, diff_rename_limit = -1,
>   merge_rename_limit = -1, call_depth = 1, obuf = {alloc = 0, len = 0, buf =
> 0x7360e0 ""},
> current_file_set = {items = 0x19d0f90, nr = 14750, alloc = 14752,
> strdup_strings = 1},
>   current_directory_set = {items = 0x19c8180, nr = 2256, alloc = 2272,
> strdup_strings = 1}}
> h1 = {object = {parsed = 1, used = 0, type = 0, flags = 0, sha1 = '\0'
> <repeats 19 times>}, util = 0x4c8e01,
> indegree = 0, date = 0, parents = 0x9e2290, tree = 0x0, buffer = 0x0}
> h2 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 =
> {0xb386a8e4      0xa839c34b      0xe1b5acde      0xb64bda52}},
> util = 0x0, indegree = 0, date = 1248282000, parents = 0x794e30, tree =
> 0x7a81c0,
>   buffer = 0x7b3160 "tree dcbf53ee0a5a3601b81b480c079e62369679bbdf\nparent
> 5046433c0b601f5a24f1149d44ad7b7b7f5fc9f8\nauthor Michael Bradshaw <mbradshaw
> <at> W3M166.(none)>
> 1248282000 -0700\ncommitter Michae}
>
> #4  0x00000000004791ac in merge_recursive (o=0x7fffe99a04a0, h1=0x796058,
> h2=0x795f80,
> ca=0x9e7b20, result=0x7fffe99a0538) at merge-recursive.c:1277
> o = {branch1 = 0x4c8db6 "Temporary merge branch 1", branch2 = 0x4c8dcf
> "Temporary merge branch 2",
> subtree_merge = 0, buffer_output = 1, verbosity = 2, diff_rename_limit = -1,
>   merge_rename_limit = -1, call_depth = 1, obuf = {alloc = 0, len = 0, buf =
> 0x7360e0 ""},
> current_file_set = {items = 0x19d0f90, nr = 14750, alloc = 14752,
> strdup_strings = 1},
>   current_directory_set = {items = 0x19c8180, nr = 2256, alloc = 2272,
> strdup_strings = 1}}
> h1 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 = {0x6520b6a7
> 0x3ef5afe8      0xe4086f4a      0x90a87a8e}}, util = 0x0, indegree = 0, date =
> 1248795280, parents = 0x7949b0, tree = 0x7a7fb8,
>   buffer = 0x794c80 "tree 57fee64850eec3f8a57ffe76878f5e76cdd26ed3\nparent
> 9320f2c497012a8c3529eada30b9e697757b7556\nparent
> bfcc7b0ef71c259251c76a5809ccce5970278539\nauthor Josh ben Jore <jbenjore <at>
> whit}
> h2 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 =
> {0xf9fe8cc6      0xe64e40bf      0x08e55e41      0x305e1a47}}, util =
> 0x7fffe99a28c0,
> indegree = 0, date = 1248308671, parents = 0x77b6c0, tree = 0x7a7f90,
>   buffer = 0x794160 "tree 4b46fa508134e277a219193e705f2f06a903c380\nparent
> e4a886b34bc339a8deacb5e152da4bb6d77cacff\nparent
> 23a435ff7710bce43c1918cfd2c6fb2655ea043c\nauthor Michael Bradshaw <mbradshaw@}
> ca = {item = 0x79bf20, next = 0x9e3190}
> *result = (struct commit *) 0xff0a000000000000
> **result =
>
> #5  0x0000000000432efc in try_merge_strategy (strategy=<value optimized out>,
> common=0x7d5ac0,
> head_arg=0x4b2eca "HEAD") at builtin-merge.c:565
> common = {item = 0x795fc8, next = 0x89df40}
>
>
> #6  0x00000000004341cd in cmd_merge (argc=<value optimized out>,
> argv=0x7fffe99a2050,
> prefix=<value optimized out>) at builtin-merge.c:1114
> argv[0] = "origin/master"
> argv[1] = 0
> argv[2] = 0
>
> #7  0x00000000004049a3 in handle_internal_command (argc=2,
> argv=0x7fffe99a2050) at git.c:246
> argv[0] = "origin/master"
> argv[1] = 0
> argv[2] = 0
>
> #8  0x0000000000404b8e in main (argc=2, argv=0x7fffe99a2050) at git.c:438
> argv[0] = "origin/master"
> argv[1] = 0
> argv[2] = 0
>
>
> Regards,
> Josh
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

Re: Fw: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Clemens Buchacher]

Hi,

On Tue, Jul 28, 2009 at 03:23:00PM -0700, Josh ben Jore wrote:
[...]
>   CONFLICT (rename/add): Rename
> config/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/dev/wpn_rails/appserver.yml
> in Temporary merge branch 1. config/conf/target/dev/wpn_rails/appse2
>   Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge branch 2 instead
>   Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as existing)
[...]
>   There are unmerged index entries:
>   2 config/conf/target/dev/wpn_rails/appserver.yml
>   3 config/conf/target/dev/wpn_rails/appserver.yml
>   Merging:
>   virtual merged tree
>   e4a886b Adding legacy click log processing scripts
>   found 1 common ancestor(s):
>   09fb055 Merge commit 'rel_090630_prod_02'
> Segmentation fault

Yeah, if process_entry leaves unmerged entries, write_tree_from_memory will
return NULL. I can reproduce with the following script (same principle as
t7405).

Clemens
---

diff --git a/t/t6035-merge-recursive-ra.sh b/t/t6035-merge-recursive-ra.sh
new file mode 100755
index 0000000..15f2c7d
--- /dev/null
+++ b/t/t6035-merge-recursive-ra.sh
@@ -0,0 +1,59 @@
+#!/bin/sh
+
+test_description='test recursive merge with rename/add conflicts'
+
+. ./test-lib.sh
+
+#
+# history
+#
+#        a --- c
+#      /   \ /
+# root      X
+#      \   / \
+#        b --- d
+#
+
+test_expect_success setup '
+
+	: >src &&
+	git add src &&
+	test_tick &&
+	git commit -m root &&
+
+	git checkout -b a master &&
+	git mv src dst
+	test_tick &&
+	git commit -m a &&
+
+	git checkout -b b master &&
+	: >dst &&
+	git add dst &&
+	test_tick &&
+	git commit -m b
+
+	git checkout -b c a &&
+	git merge -s ours b &&
+
+	git checkout -b d b &&
+	git merge -s ours a
+'
+
+test_expect_success 'merging with rename/add conflict' '
+
+	git checkout -b test1 a &&
+	test_must_fail git merge b &&
+	test -f .git/MERGE_MSG &&
+	git diff &&
+	test -n "$(git ls-files -u)"
+'
+
+test_expect_success 'merging with a rename/add conflict between merge bases' '
+
+	git reset --hard HEAD &&
+	git checkout -b test2 c &&
+	git merge d
+
+'
+
+test_done
-- 
1.6.3.1

Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Josh ben Jore]

On 7/29/09 9:10 AM, "Clemens Buchacher" <drizzd@aon.at> wrote:
> Hi,
> 
> On Tue, Jul 28, 2009 at 03:23:00PM -0700, Josh ben Jore wrote:
> [...]
>>   CONFLICT (rename/add): Rename
>> config/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/dev
>> /wpn_rails/appserver.yml
>> in Temporary merge branch 1. config/conf/target/dev/wpn_rails/appse2
>>   Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge
>> branch 2 instead
>>   Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as
>> existing)
> [...]
>>   There are unmerged index entries:
>>   2 config/conf/target/dev/wpn_rails/appserver.yml
>>   3 config/conf/target/dev/wpn_rails/appserver.yml
>>   Merging:
>>   virtual merged tree
>>   e4a886b Adding legacy click log processing scripts
>>   found 1 common ancestor(s):
>>   09fb055 Merge commit 'rel_090630_prod_02'
>> Segmentation fault
> 
> Yeah, if process_entry leaves unmerged entries, write_tree_from_memory will
> return NULL. I can reproduce with the following script (same principle as
> t7405).
> 
> Clemens
> ---
> 
> diff --git a/t/t6035-merge-recursive-ra.sh b/t/t6035-merge-recursive-ra.sh
> new file mode 100755

Thank you for the unit test. It fails as expected but I didn't verify that
it failed with the segfault as I'm experiencing. Your explanation of "There
are unmerged index entries:" is interesting. I've taken that as the focal
point for working around the bug in my repo with the unfixed software.

I've also started getting acquainted with the code in merge_tree to see if I
can identify the problem. I do not understand the se_stage values of 0 vs 2
or 3. These are not defined as constants or documented in the code. I'm
viewing them as magic for now. :-/

Josh

Re: Fw: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Junio C Hamano]

Clemens Buchacher <drizzd@aon.at> writes:

> Yeah, if process_entry leaves unmerged entries, write_tree_from_memory will
> return NULL. I can reproduce with the following script (same principle as
> t7405).

Your script does not seem to cause write_tree_from_memory() to be called
with an unmerged index while it tries to create a virtual commit.

Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Junio C Hamano]

Josh ben Jore <jbenjore@whitepages.com> writes:

> I know more now. 

Thanks.  The log was a bit hard to read with linewrapping; here is what I
could glean out of it anyway.

You have something like this in the output

    CONFLICT (rename/add):

    Rename config/conf/target/dev-ubuntu/wpn_rails/appserver.yml
    ->
    config/conf/target/dev/wpn_rails/appserver.yml
    in
    Temporary merge branch 1.
    config/conf/target/dev/wpn_rails/appse2

    Adding as
    config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge branch 2
    instead

which almost matches this part from merge_recursive.c

	} else if (!sha_eq(dst_other.sha1, null_sha1)) {
		const char *new_path;
		clean_merge = 0;
		try_merge = 1;
		output(o, 1, "CONFLICT (rename/add): Rename %s->%s in %s. "
		       "%s added in %s",
		       ren1_src, ren1_dst, branch1,
		       ren1_dst, branch2);
		new_path = unique_path(o, ren1_dst, branch2);
		output(o, 1, "Adding as %s instead", new_path);
		update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);

although I do not see "%s added in %s" part, which means we cannot see what
ren1_dst nor branch2 were.

And the crucial bit is:

>       There are unmerged index entries:
>       2 config/conf/target/dev/wpn_rails/appserver.yml
>       3 config/conf/target/dev/wpn_rails/appserver.yml

So the code did not want to add config/conf/target/dev/wpn_rails/appse2???
and instead tried to add it with suffix.  That is what the "update_file()"
does for a recursive "virtual base" merge --- it is supposed to resolve
everything down to stage#0.

But it forgets to resolve the original path (in dev/ not in dev-ubuntu/
and without the "~Temporary" suffix).

Since I do not have an access to exact details, nor reproducible history,
this is shot in the dark, but I think this may fix it.

The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that
"unmerged" path, while the other branch added something else to the same
path, and decided to add that at an alternative path, and the intent of
that is so that it can safely resolve the "renamed" side to its final
destination.  The added update_file() call is about finishing that
conflict resolution the code forgets to do.

 merge-recursive.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/merge-recursive.c b/merge-recursive.c
index d415c41..868b383 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -955,6 +955,7 @@ static int process_renames(struct merge_options *o,
 				new_path = unique_path(o, ren1_dst, branch2);
 				output(o, 1, "Adding as %s instead", new_path);
 				update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);
+				update_file(o, 0, src_other.sha1, src_other.mode, ren1_dst);
 			} else if ((item = string_list_lookup(ren1_dst, renames2Dst))) {
 				ren2 = item->util;
 				clean_merge = 0;

Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Josh ben Jore]

On 7/30/09 12:03 AM, "Junio C Hamano" <gitster@pobox.com> wrote:

> Josh ben Jore <jbenjore@whitepages.com> writes:
> 
>> I know more now.
> 
> Thanks.  The log was a bit hard to read with linewrapping; here is what I
> could glean out of it anyway.

Sorry about that. I didn't realize it was wrapped. I thought I'd use my
work's Outlook since I was addressing a problem for work.

> Since I do not have an access to exact details, nor reproducible history,
> this is shot in the dark, but I think this may fix it.

It is a very accurate shot in the dark. It appears to have fixed it when
applied to 0a53e9ddeaddad63ad106860237bbf53411d11a7 GIT 1.6.4. I'll be
trying this against v1.6.0.4 tomorrow.

> The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that
> "unmerged" path, while the other branch added something else to the same
> path, and decided to add that at an alternative path, and the intent of
> that is so that it can safely resolve the "renamed" side to its final
> destination.  The added update_file() call is about finishing that
> conflict resolution the code forgets to do.
> 
>  merge-recursive.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/merge-recursive.c b/merge-recursive.c
> index d415c41..868b383 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -955,6 +955,7 @@ static int process_renames(struct merge_options *o,
>                                 new_path = unique_path(o, ren1_dst, branch2);
>                                 output(o, 1, "Adding as %s instead",
> new_path);
>                                 update_file(o, 0, dst_other.sha1,
> dst_other.mode, new_path);
> +                               update_file(o, 0, src_other.sha1,
> src_other.mode, ren1_dst);
>                         } else if ((item = string_list_lookup(ren1_dst,
> renames2Dst))) {
>                                 ren2 = item->util;
>                                 clean_merge = 0;
> 

Thanks very much,
Josh

Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Junio C Hamano]

Josh ben Jore <jbenjore@whitepages.com> writes:

> It is a very accurate shot in the dark. It appears to have fixed it when
> applied to 0a53e9ddeaddad63ad106860237bbf53411d11a7 GIT 1.6.4. I'll be
> trying this against v1.6.0.4 tomorrow.

Thanks.  After re-reading the patch, I am reasonably sure that it is the
right fix.

Earlier we fixed similar issues with bf74106 (merge-recursive: never leave
index unmerged while recursing, 2009-05-09) and 0c44c94 (merge-recursive:
do not die on a conflicting submodule, 2009-04-29).

The former is a fix to 36e3b5e (merge-recursive: mark rename/delete
conflict as unmerged, 2008-12-22) which is probably newer than 1.6.0.4
codebase, and unless you are using submodules, the latter is probalby also
safe to ignore if you are cherry-picking to the ancient 1.6.0.4 codebase.

>> The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that
>> "unmerged" path, while the other branch added something else to the same
>> path, and decided to add that at an alternative path, and the intent of
>> that is so that it can safely resolve the "renamed" side to its final
>> destination.  The added update_file() call is about finishing that
>> conflict resolution the code forgets to do.
>> 
>>  merge-recursive.c |    1 +
>>  1 files changed, 1 insertions(+), 0 deletions(-)
>> 
>> diff --git a/merge-recursive.c b/merge-recursive.c
>> index d415c41..868b383 100644
>> --- a/merge-recursive.c
>> +++ b/merge-recursive.c
>> @@ -955,6 +955,7 @@ static int process_renames(struct merge_options *o,
>>                                 new_path = unique_path(o, ren1_dst, branch2);
>>                                 output(o, 1, "Adding as %s instead",
>> new_path);
>>                                 update_file(o, 0, dst_other.sha1,
>> dst_other.mode, new_path);
>> +                               update_file(o, 0, src_other.sha1,
>> src_other.mode, ren1_dst);
>>                         } else if ((item = string_list_lookup(ren1_dst,
>> renames2Dst))) {
>>                                 ren2 = item->util;
>>                                 clean_merge = 0;
>> 
>
> Thanks very much,
> Josh

Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Johannes Schindelin]

Hi,

On Thu, 30 Jul 2009, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at 
> > that "unmerged" path, while the other branch added something else to 
> > the same path, and decided to add that at an alternative path, and the 
> > intent of that is so that it can safely resolve the "renamed" side to 
> > its final destination.  The added update_file() call is about 
> > finishing that conflict resolution the code forgets to do.
> 
> By the way, (I am CC'ing Fredrik, the author of the merge-recursive.py,
> the original implementation, even though I haven't seen him around here
> for a long time. Two "thieves" are also CC'ed), I think the way recursive
> strategy tries to handle rename/add and rename/rename conflicts by coming
> up with a temporary pathname is fundamentally wrong.
> 
> If our branch (stage #2) added by renaming and their branch (stage #3)
> added by creating the same path, why can't we simply leave these two
> stages in unmerged state without funny "unique_path()" renaming?

AFAIR we could not leave the two in unmerged state because the "recursive" 
part of the recursive merge demands that we be able (at the intermediate 
merges) to write a tree.

But I agree that coming up with unique paths is wrong: if it is a 
rename/rename conflict, it should be

	<<<<<<<
	=======
	<file1>
	>>>>>>>

for one and

	<<<<<<<
	<file2>
	=======
	>>>>>>>

for the other rename target.

Ciao,
Dscho

[PATCH] merge-recursive: don't segfault while handling rename clashes

[Junio C Hamano]

When a branch moves A to B while the other branch created B (or moved C to
B), the code tried to rename one of them to B~something to preserve both
versions, and failed to register temporary resolution for the original
path B at stage#0 during virtual ancestor computation.  This left the
index in unmerged state and caused a segfault.

A better solution is to merge these two versions of B's in place and use
the (potentially conflicting) result as the intermediate merge result in
the virtual ancestor.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 Junio C Hamano <gitster@pobox.com> writes:

 > The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that
 > "unmerged" path, while the other branch added something else to the same
 > path, and decided to add that at an alternative path, and the intent of
 > that is so that it can safely resolve the "renamed" side to its final
 > destination.  The added update_file() call is about finishing that
 > conflict resolution the code forgets to do.
 > ...

 Yesterday's patch squashes the conflicted path down to stage#0 even for
 outermost merge, which is not quite correct.

 This may be a better fix.

 merge-recursive.c                 |   28 ++++++++++++++++--
 t/t6036-recursive-corner-cases.sh |   56 +++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+), 3 deletions(-)
 create mode 100755 t/t6036-recursive-corner-cases.sh

diff --git a/merge-recursive.c b/merge-recursive.c
index d415c41..10d7913 100644
--- a/merge-recursive.c
+++ b/merge-recursive.c
@@ -952,9 +952,31 @@ static int process_renames(struct merge_options *o,
 				       "%s added in %s",
 				       ren1_src, ren1_dst, branch1,
 				       ren1_dst, branch2);
-				new_path = unique_path(o, ren1_dst, branch2);
-				output(o, 1, "Adding as %s instead", new_path);
-				update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);
+				if (o->call_depth) {
+					struct merge_file_info mfi;
+					struct diff_filespec one, a, b;
+
+					one.path = a.path = b.path =
+						(char *)ren1_dst;
+					hashcpy(one.sha1, null_sha1);
+					one.mode = 0;
+					hashcpy(a.sha1, ren1->pair->two->sha1);
+					a.mode = ren1->pair->two->mode;
+					hashcpy(b.sha1, dst_other.sha1);
+					b.mode = dst_other.mode;
+					mfi = merge_file(o, &one, &a, &b,
+							 branch1,
+							 branch2);
+					output(o, 1, "Adding merged %s", ren1_dst);
+					update_file(o, 0,
+						    mfi.sha,
+						    mfi.mode,
+						    ren1_dst);
+				} else {
+					new_path = unique_path(o, ren1_dst, branch2);
+					output(o, 1, "Adding as %s instead", new_path);
+					update_file(o, 0, dst_other.sha1, dst_other.mode, new_path);
+				}
 			} else if ((item = string_list_lookup(ren1_dst, renames2Dst))) {
 				ren2 = item->util;
 				clean_merge = 0;
diff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh
new file mode 100755
index 0000000..a9d1613
--- /dev/null
+++ b/t/t6036-recursive-corner-cases.sh
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+test_description='recursive merge corner cases'
+
+. ./test-lib.sh
+
+#
+#  L1  L2
+#   o---o
+#  / \ / \
+# o   X   ?
+#  \ / \ /
+#   o---o
+#  R1  R2
+#
+
+test_expect_success setup '
+	ten="0 1 2 3 4 5 6 7 8 9"
+	for i in $ten
+	do
+		echo line $i in a sample file
+	done >one &&
+	for i in $ten
+	do
+		echo line $i in another sample file
+	done >two &&
+	git add one two &&
+	test_tick && git commit -m initial &&
+
+	git branch L1 &&
+	git checkout -b R1 &&
+	git mv one three &&
+	test_tick && git commit -m R1 &&
+
+	git checkout L1 &&
+	git mv two three &&
+	test_tick && git commit -m L1 &&
+
+	git checkout L1^0 &&
+	test_tick && git merge -s ours R1 &&
+	git tag L2 &&
+
+	git checkout R1^0 &&
+	test_tick && git merge -s ours L1 &&
+	git tag R2
+'
+
+test_expect_success merge '
+	git reset --hard &&
+	git checkout L2^0 &&
+
+	test_must_fail git merge -s recursive R2^0
+'
+
+test_done
+
-- 
1.6.4.13.ge65800

Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d

[Fredrik Kuivinen]

On Thu, Jul 30, 2009 at 9:45 AM, Junio C Hamano<gitster@pobox.com> wrote:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that
>> "unmerged" path, while the other branch added something else to the same
>> path, and decided to add that at an alternative path, and the intent of
>> that is so that it can safely resolve the "renamed" side to its final
>> destination.  The added update_file() call is about finishing that
>> conflict resolution the code forgets to do.
>
> By the way, (I am CC'ing Fredrik, the author of the merge-recursive.py,
> the original implementation, even though I haven't seen him around here
> for a long time. Two "thieves" are also CC'ed), I think the way recursive
> strategy tries to handle rename/add and rename/rename conflicts by coming
> up with a temporary pathname is fundamentally wrong.

It was a long time ago I looked at the merge stuff (or any git code in
fact) so I
may have forgotten some things. However, as far as I can see your analysis is
correct and the temporary pathname stuff can be avoided when the conflicts
only involve files (and no directories). As you wrote, for D/F
conflicts one need
the temporary pathnames in order to build a tree which is used in the virtual
ancestor.

[Nice analysis of when temporary pathnames are needed and when they are not
needed snipped.]

- Fredrik

PS. I am sorry for the late reply.