{"thread":{"id":"20267","subject":"Fw: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","startedAt":"2009-07-28T22:23:00Z","lastAt":"2009-08-18T21:35:17Z","messageCount":11,"participants":["Josh ben Jore","Clemens Buchacher","Junio C Hamano","Johannes Schindelin","Fredrik Kuivinen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"118998","messageId":"D5F958F92101F74D8A5683C2FE14F4200F23102F@post.corp.w3data.com","threadId":"20267","inReplyTo":null,"subject":"Fw: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","fromName":"Josh ben Jore","fromEmail":"jbenjore@whitepages.com","sentAt":"2009-07-28T22:23:00Z","receivedAt":"2009-07-28T22:23:00Z","isPatch":false,"sender":{"key":"jbenjore@whitepages.com","avatar":null},"body":"Hello there,\nWith 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.\n\nThe original message follows. I hope this one goes through ok.\n\n...\n\nHello there,\nIm trying to merge some branches together but all git versions from v1.6.0.4..origin/master which is\ncurrently df73af5f667a479764d2b2195cb0cb60b0b89e3d evince this problem. In the hopes of diagnosing\nthis, I have run git under gdb and captured both a stack trace and dumped the structures being passed at each\nlevel. Ive also included the output from GIT_MERGE_VERBOSITY=5\n\njbenjore <at> jbenjore0:~/segdev$ GIT_MERGE_VERBOSITY=5 git merge origin/master\nMerging:\na7b6206 Merge branch 'bsa_02' of ssh://git.dev.pages/opt/git/repos/dev into bsa_02\nvirtual origin/master\nfound 3 common ancestor(s):\n8c39e98 25243: Update parents to point to dev\n1e994c1 24902: Update the sample das responses that are hashes at the top level to be arrays since we use\nDAS::lookup_multi in our search base now\ne4a886b Adding legacy click log processing scripts\n  Merging:\n  8c39e98 25243: Update parents to point to dev\n  1e994c1 24902: Update the sample das responses that are hashes at the top level to be arrays since we use\nDAS::lookup_multi in our search base now\n  found 1 common ancestor(s):\n  051594a Merge branch 'rel_090617' of git+ssh://bmaldonado <at> git.dev.pages:22/opt/git/repos/dev\ninto rel_090617\n  Skipped config/conf/target/dev/host/dshanks0/common/services.yml (merged same as existing)\n  Skipped config/conf/target/dev-mchui/parent (merged same as existing)\n  Skipped config/conf/target/dev/common/directory.ini.erb (merged same as existing)\n  Skipped config/conf/target/dev/common/services.yml (merged same as existing)\n  Skipped config/conf/target/dev/etc/apache2/ports.conf.erb (merged same as existing)\n  Skipped config/conf/target/dev/host/asargent0/common/services.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/asargent0/common/test.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/jhall0/wpn_rails/appserver.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/jhoover0/common/services.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/kvm-aaron/common/directory.ini (merged same as existing)\n  Skipped config/conf/target/dev/host/kvm-aaron/common/services.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/kvm-aaron/common/test.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/kvm-coomer2/common/directory.ini (merged same as existing)\n  Skipped config/conf/target/dev/host/kvm-jason/common/services.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/kvm-jkh/common/services.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/kvm-seabass/common/directory.ini (merged same as existing)\n  Skipped config/conf/target/dev/host/kvm-seabass/common/services.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/kvm-tyler/common/directory.ini (merged same as existing)\n  Skipped config/conf/target/dev/host/kvm-tyler/common/services.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/sebastianb0/common/services.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/sebastianb0/wpn_rails/appserver.yml (merged same as existing)\n  CONFLICT (rename/add): Rename\nconfig/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/dev/wpn_rails/appserver.yml\nin Temporary merge branch 1. config/conf/target/dev/wpn_rails/appse2\n  Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge branch 2 instead\n  Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as existing)\n  Skipped config/conf/target/dev/wpn_rails/cache.yml (merged same as existing)\n  Skipped config/conf/target/dev/wpn_rails/customer_support.yml (merged same as existing)\n  Skipped config/conf/target/dev/wpn_rails/debug.yml (merged same as existing)\n  Skipped config/conf/target/dev/wpn_rails/omniture.yml (merged same as existing)\n  Skipped config/conf/target/dev/zxtm/rules.yml (merged same as existing)\n  Skipped config/conf/target/dev/host/msquires1/common/credentials.ini (merged same as existing)\n  Skipped config/conf/target/dev/host/msquires1/common/directory.ini (merged same as existing)\n  Skipped config/conf/target/dev/host/msquires1/toolserver/toolserver.ini (merged same as existing)\n  Skipped config/dynconf/target/dev/dynamic_config.yml (merged same as existing)\n  Skipped config/dynconf/target/dev/mobile_useragents.yml (merged same as existing)\n  Removing zxtm/traffic_scripts/web/beta_test.ts.erb\n  There are unmerged index entries:\n  2 config/conf/target/dev/wpn_rails/appserver.yml\n  3 config/conf/target/dev/wpn_rails/appserver.yml\n  Merging:\n  virtual merged tree\n  e4a886b Adding legacy click log processing scripts\n  found 1 common ancestor(s):\n  09fb055 Merge commit 'rel_090630_prod_02'\nSegmentation fault\n\nBacktrace without variables:\n\n#0  parse_tree (item=0x0) at tree.c:255\n#1  0x00000000004766ec in init_tree_desc_from_tree (desc=0x7fffe99a01a8, tree=0x0) at merge-recursive.c:161\n#2  0x0000000000477c37 in merge_trees (o=0x7fffe99a04a0, head=0x0, merge=0x7a81c0,\ncommon=0x7ac158, result=0x7fffe99a03f8) at merge-recursive.c:186\n#3  0x0000000000479207 in merge_recursive (o=0x7fffe99a04a0, h1=0x18555c0, h2=0x795fc8, ca=0x0,\nresult=0x7fffe99a0460) at merge-recursive.c:1291\n#4  0x00000000004791ac in merge_recursive (o=0x7fffe99a04a0, h1=0x796058, h2=0x795f80,\nca=0x9e7b20, result=0x7fffe99a0538) at merge-recursive.c:1277\n#5  0x0000000000432efc in try_merge_strategy (strategy=<value optimized out>, common=0x7d5ac0,\nhead_arg=0x4b2eca \"HEAD\") at builtin-merge.c:565\n#6  0x00000000004341cd in cmd_merge (argc=<value optimized out>, argv=0x7fffe99a2050,\nprefix=<value optimized out>) at builtin-merge.c:1114\n#7  0x00000000004049a3 in handle_internal_command (argc=2, argv=0x7fffe99a2050) at git.c:246\n#8  0x0000000000404b8e in main (argc=2, argv=0x7fffe99a2050) at git.c:438\n\n\nBacktrace with variables:\n#0  parse_tree (item=0x0) at tree.c:255\n\n#1  0x00000000004766ec in init_tree_desc_from_tree (desc=0x7fffe99a01a8, tree=0x0) at merge-recursive.c:161\ndesc = {buffer = 0xf0000, entry = {sha1 = 0x7a0138 \"\\005\", path = 0xfff0ffff <Address 0xfff0ffff out of\nbounds>, mode = 4566669}, size = 8195856}\n\n#2  0x0000000000477c37 in merge_trees (o=0x7fffe99a04a0, head=0x0, merge=0x7a81c0,\ncommon=0x7ac158, result=0x7fffe99a03f8) at merge-recursive.c:186\nmerge = {object = {parsed = 0, used = 0, type = 2, flags = 0, sha1 =\n{0xee53bfdc      0x01365a0a      0x0c481bb8      0x36629e07}, buffer = 0x0, size = 0}\ncommon = {object = {parsed = 1, used = 0, type = 2, flags = 0, sha1 =\n{0xc9539f40      0x82243895      0x02e944ac      0x73ed2a75}}, buffer = 0x1d11d90, size = 1663}\n*result = 0\n\n#3  0x0000000000479207 in merge_recursive (o=0x7fffe99a04a0, h1=0x18555c0, h2=0x795fc8, ca=0x0,\nresult=0x7fffe99a0460) at merge-recursive.c:1291\no = {branch1 = 0x4c8db6 \"Temporary merge branch 1\", branch2 = 0x4c8dcf \"Temporary merge branch 2\",\nsubtree_merge = 0, buffer_output = 1, verbosity = 2, diff_rename_limit = -1, \n  merge_rename_limit = -1, call_depth = 1, obuf = {alloc = 0, len = 0, buf = 0x7360e0 \"\"},\ncurrent_file_set = {items = 0x19d0f90, nr = 14750, alloc = 14752, strdup_strings = 1}, \n  current_directory_set = {items = 0x19c8180, nr = 2256, alloc = 2272, strdup_strings = 1}}\nh1 = {object = {parsed = 1, used = 0, type = 0, flags = 0, sha1 = '\\0' <repeats 19 times>}, util = 0x4c8e01,\nindegree = 0, date = 0, parents = 0x9e2290, tree = 0x0, buffer = 0x0}\nh2 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 =\n{0xb386a8e4      0xa839c34b      0xe1b5acde      0xb64bda52}},\nutil = 0x0, indegree = 0, date = 1248282000, parents = 0x794e30, tree = 0x7a81c0, \n  buffer = 0x7b3160 \"tree dcbf53ee0a5a3601b81b480c079e62369679bbdf\\nparent\n5046433c0b601f5a24f1149d44ad7b7b7f5fc9f8\\nauthor Michael Bradshaw <mbradshaw <at> W3M166.(none)>\n1248282000 -0700\\ncommitter Michae}\n\n#4  0x00000000004791ac in merge_recursive (o=0x7fffe99a04a0, h1=0x796058, h2=0x795f80,\nca=0x9e7b20, result=0x7fffe99a0538) at merge-recursive.c:1277\no = {branch1 = 0x4c8db6 \"Temporary merge branch 1\", branch2 = 0x4c8dcf \"Temporary merge branch 2\",\nsubtree_merge = 0, buffer_output = 1, verbosity = 2, diff_rename_limit = -1, \n  merge_rename_limit = -1, call_depth = 1, obuf = {alloc = 0, len = 0, buf = 0x7360e0 \"\"},\ncurrent_file_set = {items = 0x19d0f90, nr = 14750, alloc = 14752, strdup_strings = 1}, \n  current_directory_set = {items = 0x19c8180, nr = 2256, alloc = 2272, strdup_strings = 1}}\nh1 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 = {0x6520b6a7      0x3ef5afe8      0xe4086f4a      0x90a87a8e}}, util = 0x0, indegree = 0, date =\n1248795280, parents = 0x7949b0, tree = 0x7a7fb8, \n  buffer = 0x794c80 \"tree 57fee64850eec3f8a57ffe76878f5e76cdd26ed3\\nparent\n9320f2c497012a8c3529eada30b9e697757b7556\\nparent\nbfcc7b0ef71c259251c76a5809ccce5970278539\\nauthor Josh ben Jore <jbenjore <at> whit}\nh2 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 =\n{0xf9fe8cc6      0xe64e40bf      0x08e55e41      0x305e1a47}}, util = 0x7fffe99a28c0,\nindegree = 0, date = 1248308671, parents = 0x77b6c0, tree = 0x7a7f90, \n  buffer = 0x794160 \"tree 4b46fa508134e277a219193e705f2f06a903c380\\nparent\ne4a886b34bc339a8deacb5e152da4bb6d77cacff\\nparent\n23a435ff7710bce43c1918cfd2c6fb2655ea043c\\nauthor Michael Bradshaw <mbradshaw@}\nca = {item = 0x79bf20, next = 0x9e3190}\n*result = (struct commit *) 0xff0a000000000000\n**result = \n\n#5  0x0000000000432efc in try_merge_strategy (strategy=<value optimized out>, common=0x7d5ac0,\nhead_arg=0x4b2eca \"HEAD\") at builtin-merge.c:565\ncommon = {item = 0x795fc8, next = 0x89df40}\n\n\n#6  0x00000000004341cd in cmd_merge (argc=<value optimized out>, argv=0x7fffe99a2050,\nprefix=<value optimized out>) at builtin-merge.c:1114\nargv[0] = \"origin/master\"\nargv[1] = 0\nargv[2] = 0\n\n#7  0x00000000004049a3 in handle_internal_command (argc=2, argv=0x7fffe99a2050) at git.c:246\nargv[0] = \"origin/master\"\nargv[1] = 0\nargv[2] = 0\n\n#8  0x0000000000404b8e in main (argc=2, argv=0x7fffe99a2050) at git.c:438\nargv[0] = \"origin/master\"\nargv[1] = 0\nargv[2] = 0\n\n\nRegards,\nJosh\n"},{"id":"119062","messageId":"C695A6A4.61CD9%jbenjore@whitepages.com","threadId":"20267","inReplyTo":"D5F958F92101F74D8A5683C2FE14F4200F23102F@post.corp.w3data.com","subject":"Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","fromName":"Josh ben Jore","fromEmail":"jbenjore@whitepages.com","sentAt":"2009-07-29T14:11:32Z","receivedAt":"2009-07-29T14:11:32Z","isPatch":false,"sender":{"key":"jbenjore@whitepages.com","avatar":null},"body":"I know more now. I've never cracked the hood on git before so this is all\nnew to me. The below is a transcript of my thinking but the conclusion I'm\nat /now/ is that merge_trees should return a tree to mrtree but returns NULL\ninstead.\n\nTranscript follows:\n\nThe root merge is:\n\n    git rev-parse bsa_02 # 7b62065e8aff53e4a6f08e48e7aa8902f68e8d6\n    git rev-parse master # c68cfef9bf404ee6415ee508471a5e3034d755da\n\n    # GIT_MERGE_VERBOSITY=5\n    Merging:\n    a7b6206 Merge branch 'bsa_02' of ssh://git.dev.pages/opt/git/repos/dev\ninto bsa_02\n    virtual origin/master\n\nThe merge bases for those are:\n\n    git merge-base bsa_02 master\n    # 8c39e98cdd59753f2a5c6f9fc829592173fbdbfb 25243: Update parents to\npoint to dev\n    # 1e994c1c3397c4919103faf0b0bd264be08ec279 24902: Update the sample das\nresponses that are hashes at the top level to be arrays since we use\nDAS::lookup_multi in our search base now\n    # e4a886b34bc339a8deacb5e152da4bb6d77cacff Adding legacy click log\nprocessing scripts\n\n    # GIT_MERGE_VERBOSITY=5\n    found 3 common ancestor(s):\n    8c39e98 25243: Update parents to point to dev\n    1e994c1 24902: Update the sample das responses that are hashes at the\ntop level to be arrays since we use DAS::lookup_multi in our search base now\n    e4a886b Adding legacy click log processing scripts\n\nRunning the merge with GIT_MERGE_VERBOSITY=5 shows how the recursive\nstrategy works. It sorts the merge-bases by date, merges two at a\ntime, producing a new virtual commit, merging the next item with\nresult of the last.\n\nMerging 8c39e98 and 1e994c1:\n\n      Merging:\n      8c39e98 25243: Update parents to point to dev\n      1e994c1 24902: Update the sample das responses that are hashes at the\ntop level to be arrays since we use DAS::lookup_multi in our search base now\n      found 1 common ancestor(s):\n      051594a Merge branch 'rel_090617' of git+ssh://bmaldonado <at>\ngit.dev.pages:22/opt/git/repos/dev into rel_090617\n\nIt's going to produce a conflict:\n\n      Skipped config/conf/target/dev/host/dshanks0/common/services.yml\n(merged same as existing)\n      Skipped config/conf/target/dev-mchui/parent (merged same as existing)\n      Skipped config/conf/target/dev/common/directory.ini.erb (merged same\nas existing)\n      Skipped config/conf/target/dev/common/services.yml (merged same as\nexisting)\n      Skipped config/conf/target/dev/etc/apache2/ports.conf.erb (merged same\nas existing)\n      Skipped config/conf/target/dev/host/asargent0/common/services.yml\n(merged same as existing)\n      Skipped config/conf/target/dev/host/asargent0/common/test.yml (merged\nsame as existing)\n      Skipped config/conf/target/dev/host/jhall0/wpn_rails/appserver.yml\n(merged same as existing)\n      Skipped config/conf/target/dev/host/jhoover0/common/services.yml\n(merged same as existing)\n      Skipped config/conf/target/dev/host/kvm-aaron/common/directory.ini\n(merged same as existing)\n      Skipped config/conf/target/dev/host/kvm-aaron/common/services.yml\n(merged same as existing)\n      Skipped config/conf/target/dev/host/kvm-aaron/common/test.yml (merged\nsame as existing)\n      Skipped config/conf/target/dev/host/kvm-coomer2/common/directory.ini\n(merged same as existing)\n      Skipped config/conf/target/dev/host/kvm-jason/common/services.yml\n(merged same as existing)\n      Skipped config/conf/target/dev/host/kvm-jkh/common/services.yml\n(merged same as existing)\n      Skipped config/conf/target/dev/host/kvm-seabass/common/directory.ini\n(merged same as existing)\n      Skipped config/conf/target/dev/host/kvm-seabass/common/services.yml\n(merged same as existing)\n      Skipped config/conf/target/dev/host/kvm-tyler/common/directory.ini\n(merged same as existing)\n      Skipped config/conf/target/dev/host/kvm-tyler/common/services.yml\n(merged same as existing)\n      Skipped config/conf/target/dev/host/sebastianb0/common/services.yml\n(merged same as existing)\n      Skipped\nconfig/conf/target/dev/host/sebastianb0/wpn_rails/appserver.yml (merged same\nas existing)\n      CONFLICT (rename/add): Rename\nconfig/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/de\nv/wpn_rails/appserver.yml in Temporary merge branch 1.\nconfig/conf/target/dev/wpn_rails/a\\\nppse2\n      Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary\nmerge branch 2 instead\n      Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as\nexisting)\n      Skipped config/conf/target/dev/wpn_rails/cache.yml (merged same as\nexisting)\n      Skipped config/conf/target/dev/wpn_rails/customer_support.yml (merged\nsame as existing)\n      Skipped config/conf/target/dev/wpn_rails/debug.yml (merged same as\nexisting)\n      Skipped config/conf/target/dev/wpn_rails/omniture.yml (merged same as\nexisting)\n      Skipped config/conf/target/dev/zxtm/rules.yml (merged same as\nexisting)\n      Skipped config/conf/target/dev/host/msquires1/common/credentials.ini\n(merged same as existing)\n      Skipped config/conf/target/dev/host/msquires1/common/directory.ini\n(merged same as existing)\n      Skipped\nconfig/conf/target/dev/host/msquires1/toolserver/toolserver.ini (merged same\nas existing)\n      Skipped config/dynconf/target/dev/dynamic_config.yml (merged same as\nexisting)\n      Skipped config/dynconf/target/dev/mobile_useragents.yml (merged same\nas existing)\n      Removing zxtm/traffic_scripts/web/beta_test.ts.erb\n      There are unmerged index entries:\n      2 config/conf/target/dev/wpn_rails/appserver.yml\n      3 config/conf/target/dev/wpn_rails/appserver.yml\n\nThe resultant trees are merged but return a null in mrtree.\n\n    clean = merge_trees(o, h1->tree, h2->tree,\nmerged_common_ancestors->tree,\n                        &mrtree);\n\n\nmerge_recursive.c:1294 produced a \"virtual merged tree\" and returned\nit for use by the next merge. This \"virtual merged tree\" uses the NULL\ntree as generated by merge_trees.\n\n    if (o->call_depth) {\n            *result = make_virtual_commit(mrtree, \"merged tree\");\n            commit_list_insert(h1, &(*result)->parents);\n            commit_list_insert(h2, &(*result)->parents->next);\n    }\n\nAlso, the action *result=... overwrites one of the parameters to this\nmerge. The pointer to merged_common_ancestors is being read from in\nthis loop as well as modified. Presumably, the result of this is so\nmerged_common_ancestors is continually refreshed as a new tree.\n\n  1131          for (iter = ca; iter; iter = iter->next) {\n  ...\n  1146                  merge_recursive(o, merged_common_ancestors,\niter->item,\n  1147                                  NULL, &merged_common_ancestors);\n\nThe status of merged_common_ancestors is that it is not reified and\ndoes not exist in the git database. The ->tree entry is NULL because\nmerge_trees said so.\n\n  merged_common_ancestors = {\n    object = {\n      parsed = 1,\n      used = 0,\n      type = 0,\n      flags = 0,\n      sha1 = '\\0' <repeats 19 times>\n    },\n    util = 0x4c8e01,\n    indegree = 0,\n    date = 0,\n    parents = 0x9e2290,\n    tree = 0x0,\n    buffer = 0x0\n  }\n\nThe merge between the \"virtual merged tree\" of (8c39e98 & 1e994c1)\ncurrently in merged_common_ancestors and e4a886b off of the ca/iter\nloop attempts to proceed:\n\n  Merging:\n  virtual merged tree\n  e4a886b Adding legacy click log processing scripts\n  found 1 common ancestor(s):\n  09fb055 Merge commit 'rel_090630_prod_02'\n\nIt passes an empty list of common ancestors so merge_recursive finds\nsome itself. Here, 'ca' corresponds to the NULL passed by the\nrecursive call of merge_recursive to itself.\n\n  1113          if (!ca) {\n  1114                  ca = get_merge_bases(h1, h2, 1);\n  1115                  ca = reverse_commit_list(ca);\n  1116          }\n\nThis found the sole ancestor 09fb055 and shifted it off the 'ca' list.\n\n  1122          merged_common_ancestors = pop_commit(&ca);\n\nThe sub-merging loop is now skipped because ca is empty. There was no\nmore work to do so nothing is done to ancestor 09fb055.\n\n  1131          for (iter = ca; iter; iter = iter->next) {\n\nNow \"virtual merged tree\" and e4a886b are handed off to be actually\nmerged, I guess.\n\n  1157          clean = merge_trees(o, h1->tree, h2->tree,\nmerged_common_ancestors->tree,\n  1158                              &mrtree);\n\nWe will now segfault because h1->tree is NULL. h1->tree's NULL is the\nNULL returned to mrtree by the merge_trees() of above.\n\nRegards,\nJosh\n\nOn 7/28/09 3:23 PM, \"Josh ben Jore\" <jbenjore@whitepages.com> wrote:\n\n> Hello there,\n> With apologies, I am resending the previous message after taking care to strip\n> gdb's dump of the object->sha1 pointers and replace them with the contents of\n> gdb's \"x/4 $thing->object->sha1\". I heard from one of you that gmail showed\n> only random crud. It looked ok to me. Sorry for the annoyance.\n>\n> The original message follows. I hope this one goes through ok.\n>\n> ...\n>\n> Hello there,\n> Im trying to merge some branches together but all git versions from\n> v1.6.0.4..origin/master which is\n> currently df73af5f667a479764d2b2195cb0cb60b0b89e3d evince this problem. In the\n> hopes of diagnosing\n> this, I have run git under gdb and captured both a stack trace and dumped the\n> structures being passed at each\n> level. Ive also included the output from GIT_MERGE_VERBOSITY=5\n>\n> jbenjore <at> jbenjore0:~/segdev$ GIT_MERGE_VERBOSITY=5 git merge\n> origin/master\n> Merging:\n> a7b6206 Merge branch 'bsa_02' of ssh://git.dev.pages/opt/git/repos/dev into\n> bsa_02\n> virtual origin/master\n> found 3 common ancestor(s):\n> 8c39e98 25243: Update parents to point to dev\n> 1e994c1 24902: Update the sample das responses that are hashes at the top\n> level to be arrays since we use\n> DAS::lookup_multi in our search base now\n> e4a886b Adding legacy click log processing scripts\n>   Merging:\n>   8c39e98 25243: Update parents to point to dev\n>   1e994c1 24902: Update the sample das responses that are hashes at the top\n> level to be arrays since we use\n> DAS::lookup_multi in our search base now\n>   found 1 common ancestor(s):\n>   051594a Merge branch 'rel_090617' of git+ssh://bmaldonado <at>\n> git.dev.pages:22/opt/git/repos/dev\n> into rel_090617\n>   Skipped config/conf/target/dev/host/dshanks0/common/services.yml (merged\n> same as existing)\n>   Skipped config/conf/target/dev-mchui/parent (merged same as existing)\n>   Skipped config/conf/target/dev/common/directory.ini.erb (merged same as\n> existing)\n>   Skipped config/conf/target/dev/common/services.yml (merged same as existing)\n>   Skipped config/conf/target/dev/etc/apache2/ports.conf.erb (merged same as\n> existing)\n>   Skipped config/conf/target/dev/host/asargent0/common/services.yml (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/asargent0/common/test.yml (merged same\n> as existing)\n>   Skipped config/conf/target/dev/host/jhall0/wpn_rails/appserver.yml (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/jhoover0/common/services.yml (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/kvm-aaron/common/directory.ini (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/kvm-aaron/common/services.yml (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/kvm-aaron/common/test.yml (merged same\n> as existing)\n>   Skipped config/conf/target/dev/host/kvm-coomer2/common/directory.ini (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/kvm-jason/common/services.yml (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/kvm-jkh/common/services.yml (merged same\n> as existing)\n>   Skipped config/conf/target/dev/host/kvm-seabass/common/directory.ini (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/kvm-seabass/common/services.yml (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/kvm-tyler/common/directory.ini (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/kvm-tyler/common/services.yml (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/sebastianb0/common/services.yml (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/sebastianb0/wpn_rails/appserver.yml\n> (merged same as existing)\n>   CONFLICT (rename/add): Rename\n> config/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/dev/\n> wpn_rails/appserver.yml\n> in Temporary merge branch 1. config/conf/target/dev/wpn_rails/appse2\n>   Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge\n> branch 2 instead\n>   Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as\n> existing)\n>   Skipped config/conf/target/dev/wpn_rails/cache.yml (merged same as existing)\n>   Skipped config/conf/target/dev/wpn_rails/customer_support.yml (merged same\n> as existing)\n>   Skipped config/conf/target/dev/wpn_rails/debug.yml (merged same as existing)\n>   Skipped config/conf/target/dev/wpn_rails/omniture.yml (merged same as\n> existing)\n>   Skipped config/conf/target/dev/zxtm/rules.yml (merged same as existing)\n>   Skipped config/conf/target/dev/host/msquires1/common/credentials.ini (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/msquires1/common/directory.ini (merged\n> same as existing)\n>   Skipped config/conf/target/dev/host/msquires1/toolserver/toolserver.ini\n> (merged same as existing)\n>   Skipped config/dynconf/target/dev/dynamic_config.yml (merged same as\n> existing)\n>   Skipped config/dynconf/target/dev/mobile_useragents.yml (merged same as\n> existing)\n>   Removing zxtm/traffic_scripts/web/beta_test.ts.erb\n>   There are unmerged index entries:\n>   2 config/conf/target/dev/wpn_rails/appserver.yml\n>   3 config/conf/target/dev/wpn_rails/appserver.yml\n>   Merging:\n>   virtual merged tree\n>   e4a886b Adding legacy click log processing scripts\n>   found 1 common ancestor(s):\n>   09fb055 Merge commit 'rel_090630_prod_02'\n> Segmentation fault\n>\n> Backtrace without variables:\n>\n> #0  parse_tree (item=0x0) at tree.c:255\n> #1  0x00000000004766ec in init_tree_desc_from_tree (desc=0x7fffe99a01a8,\n> tree=0x0) at merge-recursive.c:161\n> #2  0x0000000000477c37 in merge_trees (o=0x7fffe99a04a0, head=0x0,\n> merge=0x7a81c0,\n> common=0x7ac158, result=0x7fffe99a03f8) at merge-recursive.c:186\n> #3  0x0000000000479207 in merge_recursive (o=0x7fffe99a04a0, h1=0x18555c0,\n> h2=0x795fc8, ca=0x0,\n> result=0x7fffe99a0460) at merge-recursive.c:1291\n> #4  0x00000000004791ac in merge_recursive (o=0x7fffe99a04a0, h1=0x796058,\n> h2=0x795f80,\n> ca=0x9e7b20, result=0x7fffe99a0538) at merge-recursive.c:1277\n> #5  0x0000000000432efc in try_merge_strategy (strategy=<value optimized out>,\n> common=0x7d5ac0,\n> head_arg=0x4b2eca \"HEAD\") at builtin-merge.c:565\n> #6  0x00000000004341cd in cmd_merge (argc=<value optimized out>,\n> argv=0x7fffe99a2050,\n> prefix=<value optimized out>) at builtin-merge.c:1114\n> #7  0x00000000004049a3 in handle_internal_command (argc=2,\n> argv=0x7fffe99a2050) at git.c:246\n> #8  0x0000000000404b8e in main (argc=2, argv=0x7fffe99a2050) at git.c:438\n>\n>\n> Backtrace with variables:\n> #0  parse_tree (item=0x0) at tree.c:255\n>\n> #1  0x00000000004766ec in init_tree_desc_from_tree (desc=0x7fffe99a01a8,\n> tree=0x0) at merge-recursive.c:161\n> desc = {buffer = 0xf0000, entry = {sha1 = 0x7a0138 \"\\005\", path = 0xfff0ffff\n> <Address 0xfff0ffff out of\n> bounds>, mode = 4566669}, size = 8195856}\n>\n> #2  0x0000000000477c37 in merge_trees (o=0x7fffe99a04a0, head=0x0,\n> merge=0x7a81c0,\n> common=0x7ac158, result=0x7fffe99a03f8) at merge-recursive.c:186\n> merge = {object = {parsed = 0, used = 0, type = 2, flags = 0, sha1 =\n> {0xee53bfdc      0x01365a0a      0x0c481bb8      0x36629e07}, buffer = 0x0,\n> size = 0}\n> common = {object = {parsed = 1, used = 0, type = 2, flags = 0, sha1 =\n> {0xc9539f40      0x82243895      0x02e944ac      0x73ed2a75}}, buffer =\n> 0x1d11d90, size = 1663}\n> *result = 0\n>\n> #3  0x0000000000479207 in merge_recursive (o=0x7fffe99a04a0, h1=0x18555c0,\n> h2=0x795fc8, ca=0x0,\n> result=0x7fffe99a0460) at merge-recursive.c:1291\n> o = {branch1 = 0x4c8db6 \"Temporary merge branch 1\", branch2 = 0x4c8dcf\n> \"Temporary merge branch 2\",\n> subtree_merge = 0, buffer_output = 1, verbosity = 2, diff_rename_limit = -1,\n>   merge_rename_limit = -1, call_depth = 1, obuf = {alloc = 0, len = 0, buf =\n> 0x7360e0 \"\"},\n> current_file_set = {items = 0x19d0f90, nr = 14750, alloc = 14752,\n> strdup_strings = 1},\n>   current_directory_set = {items = 0x19c8180, nr = 2256, alloc = 2272,\n> strdup_strings = 1}}\n> h1 = {object = {parsed = 1, used = 0, type = 0, flags = 0, sha1 = '\\0'\n> <repeats 19 times>}, util = 0x4c8e01,\n> indegree = 0, date = 0, parents = 0x9e2290, tree = 0x0, buffer = 0x0}\n> h2 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 =\n> {0xb386a8e4      0xa839c34b      0xe1b5acde      0xb64bda52}},\n> util = 0x0, indegree = 0, date = 1248282000, parents = 0x794e30, tree =\n> 0x7a81c0,\n>   buffer = 0x7b3160 \"tree dcbf53ee0a5a3601b81b480c079e62369679bbdf\\nparent\n> 5046433c0b601f5a24f1149d44ad7b7b7f5fc9f8\\nauthor Michael Bradshaw <mbradshaw\n> <at> W3M166.(none)>\n> 1248282000 -0700\\ncommitter Michae}\n>\n> #4  0x00000000004791ac in merge_recursive (o=0x7fffe99a04a0, h1=0x796058,\n> h2=0x795f80,\n> ca=0x9e7b20, result=0x7fffe99a0538) at merge-recursive.c:1277\n> o = {branch1 = 0x4c8db6 \"Temporary merge branch 1\", branch2 = 0x4c8dcf\n> \"Temporary merge branch 2\",\n> subtree_merge = 0, buffer_output = 1, verbosity = 2, diff_rename_limit = -1,\n>   merge_rename_limit = -1, call_depth = 1, obuf = {alloc = 0, len = 0, buf =\n> 0x7360e0 \"\"},\n> current_file_set = {items = 0x19d0f90, nr = 14750, alloc = 14752,\n> strdup_strings = 1},\n>   current_directory_set = {items = 0x19c8180, nr = 2256, alloc = 2272,\n> strdup_strings = 1}}\n> h1 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 = {0x6520b6a7\n> 0x3ef5afe8      0xe4086f4a      0x90a87a8e}}, util = 0x0, indegree = 0, date =\n> 1248795280, parents = 0x7949b0, tree = 0x7a7fb8,\n>   buffer = 0x794c80 \"tree 57fee64850eec3f8a57ffe76878f5e76cdd26ed3\\nparent\n> 9320f2c497012a8c3529eada30b9e697757b7556\\nparent\n> bfcc7b0ef71c259251c76a5809ccce5970278539\\nauthor Josh ben Jore <jbenjore <at>\n> whit}\n> h2 = {object = {parsed = 1, used = 0, type = 1, flags = 0, sha1 =\n> {0xf9fe8cc6      0xe64e40bf      0x08e55e41      0x305e1a47}}, util =\n> 0x7fffe99a28c0,\n> indegree = 0, date = 1248308671, parents = 0x77b6c0, tree = 0x7a7f90,\n>   buffer = 0x794160 \"tree 4b46fa508134e277a219193e705f2f06a903c380\\nparent\n> e4a886b34bc339a8deacb5e152da4bb6d77cacff\\nparent\n> 23a435ff7710bce43c1918cfd2c6fb2655ea043c\\nauthor Michael Bradshaw <mbradshaw@}\n> ca = {item = 0x79bf20, next = 0x9e3190}\n> *result = (struct commit *) 0xff0a000000000000\n> **result =\n>\n> #5  0x0000000000432efc in try_merge_strategy (strategy=<value optimized out>,\n> common=0x7d5ac0,\n> head_arg=0x4b2eca \"HEAD\") at builtin-merge.c:565\n> common = {item = 0x795fc8, next = 0x89df40}\n>\n>\n> #6  0x00000000004341cd in cmd_merge (argc=<value optimized out>,\n> argv=0x7fffe99a2050,\n> prefix=<value optimized out>) at builtin-merge.c:1114\n> argv[0] = \"origin/master\"\n> argv[1] = 0\n> argv[2] = 0\n>\n> #7  0x00000000004049a3 in handle_internal_command (argc=2,\n> argv=0x7fffe99a2050) at git.c:246\n> argv[0] = \"origin/master\"\n> argv[1] = 0\n> argv[2] = 0\n>\n> #8  0x0000000000404b8e in main (argc=2, argv=0x7fffe99a2050) at git.c:438\n> argv[0] = \"origin/master\"\n> argv[1] = 0\n> argv[2] = 0\n>\n>\n> Regards,\n> Josh\n>\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at  http://vger.kernel.org/majordomo-info.html\n>\n"},{"id":"119075","messageId":"20090729161030.GA3848@localhost","threadId":"20267","inReplyTo":"D5F958F92101F74D8A5683C2FE14F4200F23102F@post.corp.w3data.com","subject":"Re: Fw: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","fromName":"Clemens Buchacher","fromEmail":"drizzd@aon.at","sentAt":"2009-07-29T16:10:30Z","receivedAt":"2009-07-29T16:10:30Z","isPatch":false,"sender":{"key":"drizzd@gmx.net","avatar":"https://avatars.githubusercontent.com/u/59082?v=4"},"body":"Hi,\n\nOn Tue, Jul 28, 2009 at 03:23:00PM -0700, Josh ben Jore wrote:\n[...]\n>   CONFLICT (rename/add): Rename\n> config/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/dev/wpn_rails/appserver.yml\n> in Temporary merge branch 1. config/conf/target/dev/wpn_rails/appse2\n>   Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge branch 2 instead\n>   Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as existing)\n[...]\n>   There are unmerged index entries:\n>   2 config/conf/target/dev/wpn_rails/appserver.yml\n>   3 config/conf/target/dev/wpn_rails/appserver.yml\n>   Merging:\n>   virtual merged tree\n>   e4a886b Adding legacy click log processing scripts\n>   found 1 common ancestor(s):\n>   09fb055 Merge commit 'rel_090630_prod_02'\n> Segmentation fault\n\nYeah, if process_entry leaves unmerged entries, write_tree_from_memory will\nreturn NULL. I can reproduce with the following script (same principle as\nt7405).\n\nClemens\n---\n\ndiff --git a/t/t6035-merge-recursive-ra.sh b/t/t6035-merge-recursive-ra.sh\nnew file mode 100755\nindex 0000000..15f2c7d\n--- /dev/null\n+++ b/t/t6035-merge-recursive-ra.sh\n@@ -0,0 +1,59 @@\n+#!/bin/sh\n+\n+test_description='test recursive merge with rename/add conflicts'\n+\n+. ./test-lib.sh\n+\n+#\n+# history\n+#\n+#        a --- c\n+#      /   \\ /\n+# root      X\n+#      \\   / \\\n+#        b --- d\n+#\n+\n+test_expect_success setup '\n+\n+\t: >src &&\n+\tgit add src &&\n+\ttest_tick &&\n+\tgit commit -m root &&\n+\n+\tgit checkout -b a master &&\n+\tgit mv src dst\n+\ttest_tick &&\n+\tgit commit -m a &&\n+\n+\tgit checkout -b b master &&\n+\t: >dst &&\n+\tgit add dst &&\n+\ttest_tick &&\n+\tgit commit -m b\n+\n+\tgit checkout -b c a &&\n+\tgit merge -s ours b &&\n+\n+\tgit checkout -b d b &&\n+\tgit merge -s ours a\n+'\n+\n+test_expect_success 'merging with rename/add conflict' '\n+\n+\tgit checkout -b test1 a &&\n+\ttest_must_fail git merge b &&\n+\ttest -f .git/MERGE_MSG &&\n+\tgit diff &&\n+\ttest -n \"$(git ls-files -u)\"\n+'\n+\n+test_expect_success 'merging with a rename/add conflict between merge bases' '\n+\n+\tgit reset --hard HEAD &&\n+\tgit checkout -b test2 c &&\n+\tgit merge d\n+\n+'\n+\n+test_done\n-- \n1.6.3.1\n"},{"id":"119097","messageId":"C69602DC.61D6D%jbenjore@whitepages.com","threadId":"20267","inReplyTo":"20090729161030.GA3848@localhost","subject":"Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","fromName":"Josh ben Jore","fromEmail":"jbenjore@whitepages.com","sentAt":"2009-07-29T20:45:00Z","receivedAt":"2009-07-29T20:45:00Z","isPatch":false,"sender":{"key":"jbenjore@whitepages.com","avatar":null},"body":"On 7/29/09 9:10 AM, \"Clemens Buchacher\" <drizzd@aon.at> wrote:\n> Hi,\n> \n> On Tue, Jul 28, 2009 at 03:23:00PM -0700, Josh ben Jore wrote:\n> [...]\n>>   CONFLICT (rename/add): Rename\n>> config/conf/target/dev-ubuntu/wpn_rails/appserver.yml->config/conf/target/dev\n>> /wpn_rails/appserver.yml\n>> in Temporary merge branch 1. config/conf/target/dev/wpn_rails/appse2\n>>   Adding as config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge\n>> branch 2 instead\n>>   Skipped config/conf/target/dev/wpn_rails/appserver.yml (merged same as\n>> existing)\n> [...]\n>>   There are unmerged index entries:\n>>   2 config/conf/target/dev/wpn_rails/appserver.yml\n>>   3 config/conf/target/dev/wpn_rails/appserver.yml\n>>   Merging:\n>>   virtual merged tree\n>>   e4a886b Adding legacy click log processing scripts\n>>   found 1 common ancestor(s):\n>>   09fb055 Merge commit 'rel_090630_prod_02'\n>> Segmentation fault\n> \n> Yeah, if process_entry leaves unmerged entries, write_tree_from_memory will\n> return NULL. I can reproduce with the following script (same principle as\n> t7405).\n> \n> Clemens\n> ---\n> \n> diff --git a/t/t6035-merge-recursive-ra.sh b/t/t6035-merge-recursive-ra.sh\n> new file mode 100755\n\nThank you for the unit test. It fails as expected but I didn't verify that\nit failed with the segfault as I'm experiencing. Your explanation of \"There\nare unmerged index entries:\" is interesting. I've taken that as the focal\npoint for working around the bug in my repo with the unfixed software.\n\nI've also started getting acquainted with the code in merge_tree to see if I\ncan identify the problem. I do not understand the se_stage values of 0 vs 2\nor 3. These are not defined as constants or documented in the code. I'm\nviewing them as magic for now. :-/\n\nJosh\n"},{"id":"119153","messageId":"7vy6q6k7ax.fsf@alter.siamese.dyndns.org","threadId":"20267","inReplyTo":"20090729161030.GA3848@localhost","subject":"Re: Fw: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-30T06:34:46Z","receivedAt":"2009-07-30T06:34:46Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Clemens Buchacher <drizzd@aon.at> writes:\n\n> Yeah, if process_entry leaves unmerged entries, write_tree_from_memory will\n> return NULL. I can reproduce with the following script (same principle as\n> t7405).\n\nYour script does not seem to cause write_tree_from_memory() to be called\nwith an unmerged index while it tries to create a virtual commit.\n"},{"id":"119154","messageId":"7vtz0uk5z3.fsf@alter.siamese.dyndns.org","threadId":"20267","inReplyTo":"C695A6A4.61CD9%jbenjore@whitepages.com","subject":"Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-30T07:03:28Z","receivedAt":"2009-07-30T07:03:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh ben Jore <jbenjore@whitepages.com> writes:\n\n> I know more now. \n\nThanks.  The log was a bit hard to read with linewrapping; here is what I\ncould glean out of it anyway.\n\nYou have something like this in the output\n\n    CONFLICT (rename/add):\n\n    Rename config/conf/target/dev-ubuntu/wpn_rails/appserver.yml\n    ->\n    config/conf/target/dev/wpn_rails/appserver.yml\n    in\n    Temporary merge branch 1.\n    config/conf/target/dev/wpn_rails/appse2\n\n    Adding as\n    config/conf/target/dev/wpn_rails/appserver.yml~Temporary merge branch 2\n    instead\n\nwhich almost matches this part from merge_recursive.c\n\n\t} else if (!sha_eq(dst_other.sha1, null_sha1)) {\n\t\tconst char *new_path;\n\t\tclean_merge = 0;\n\t\ttry_merge = 1;\n\t\toutput(o, 1, \"CONFLICT (rename/add): Rename %s->%s in %s. \"\n\t\t       \"%s added in %s\",\n\t\t       ren1_src, ren1_dst, branch1,\n\t\t       ren1_dst, branch2);\n\t\tnew_path = unique_path(o, ren1_dst, branch2);\n\t\toutput(o, 1, \"Adding as %s instead\", new_path);\n\t\tupdate_file(o, 0, dst_other.sha1, dst_other.mode, new_path);\n\nalthough I do not see \"%s added in %s\" part, which means we cannot see what\nren1_dst nor branch2 were.\n\nAnd the crucial bit is:\n\n>       There are unmerged index entries:\n>       2 config/conf/target/dev/wpn_rails/appserver.yml\n>       3 config/conf/target/dev/wpn_rails/appserver.yml\n\nSo the code did not want to add config/conf/target/dev/wpn_rails/appse2???\nand instead tried to add it with suffix.  That is what the \"update_file()\"\ndoes for a recursive \"virtual base\" merge --- it is supposed to resolve\neverything down to stage#0.\n\nBut it forgets to resolve the original path (in dev/ not in dev-ubuntu/\nand without the \"~Temporary\" suffix).\n\nSince I do not have an access to exact details, nor reproducible history,\nthis is shot in the dark, but I think this may fix it.\n\nThe codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that\n\"unmerged\" path, while the other branch added something else to the same\npath, and decided to add that at an alternative path, and the intent of\nthat is so that it can safely resolve the \"renamed\" side to its final\ndestination.  The added update_file() call is about finishing that\nconflict resolution the code forgets to do.\n\n merge-recursive.c |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex d415c41..868b383 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -955,6 +955,7 @@ static int process_renames(struct merge_options *o,\n \t\t\t\tnew_path = unique_path(o, ren1_dst, branch2);\n \t\t\t\toutput(o, 1, \"Adding as %s instead\", new_path);\n \t\t\t\tupdate_file(o, 0, dst_other.sha1, dst_other.mode, new_path);\n+\t\t\t\tupdate_file(o, 0, src_other.sha1, src_other.mode, ren1_dst);\n \t\t\t} else if ((item = string_list_lookup(ren1_dst, renames2Dst))) {\n \t\t\t\tren2 = item->util;\n \t\t\t\tclean_merge = 0;\n"},{"id":"119156","messageId":"C69698D6.61E1C%jbenjore@whitepages.com","threadId":"20267","inReplyTo":"7vtz0uk5z3.fsf@alter.siamese.dyndns.org","subject":"Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","fromName":"Josh ben Jore","fromEmail":"jbenjore@whitepages.com","sentAt":"2009-07-30T07:24:54Z","receivedAt":"2009-07-30T07:24:54Z","isPatch":false,"sender":{"key":"jbenjore@whitepages.com","avatar":null},"body":"On 7/30/09 12:03 AM, \"Junio C Hamano\" <gitster@pobox.com> wrote:\n\n> Josh ben Jore <jbenjore@whitepages.com> writes:\n> \n>> I know more now.\n> \n> Thanks.  The log was a bit hard to read with linewrapping; here is what I\n> could glean out of it anyway.\n\nSorry about that. I didn't realize it was wrapped. I thought I'd use my\nwork's Outlook since I was addressing a problem for work.\n\n> Since I do not have an access to exact details, nor reproducible history,\n> this is shot in the dark, but I think this may fix it.\n\nIt is a very accurate shot in the dark. It appears to have fixed it when\napplied to 0a53e9ddeaddad63ad106860237bbf53411d11a7 GIT 1.6.4. I'll be\ntrying this against v1.6.0.4 tomorrow.\n\n> The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that\n> \"unmerged\" path, while the other branch added something else to the same\n> path, and decided to add that at an alternative path, and the intent of\n> that is so that it can safely resolve the \"renamed\" side to its final\n> destination.  The added update_file() call is about finishing that\n> conflict resolution the code forgets to do.\n> \n>  merge-recursive.c |    1 +\n>  1 files changed, 1 insertions(+), 0 deletions(-)\n> \n> diff --git a/merge-recursive.c b/merge-recursive.c\n> index d415c41..868b383 100644\n> --- a/merge-recursive.c\n> +++ b/merge-recursive.c\n> @@ -955,6 +955,7 @@ static int process_renames(struct merge_options *o,\n>                                 new_path = unique_path(o, ren1_dst, branch2);\n>                                 output(o, 1, \"Adding as %s instead\",\n> new_path);\n>                                 update_file(o, 0, dst_other.sha1,\n> dst_other.mode, new_path);\n> +                               update_file(o, 0, src_other.sha1,\n> src_other.mode, ren1_dst);\n>                         } else if ((item = string_list_lookup(ren1_dst,\n> renames2Dst))) {\n>                                 ren2 = item->util;\n>                                 clean_merge = 0;\n> \n\nThanks very much,\nJosh\n"},{"id":"119158","messageId":"7vws5qhaho.fsf@alter.siamese.dyndns.org","threadId":"20267","inReplyTo":"C69698D6.61E1C%jbenjore@whitepages.com","subject":"Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-30T07:54:11Z","receivedAt":"2009-07-30T07:54:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Josh ben Jore <jbenjore@whitepages.com> writes:\n\n> It is a very accurate shot in the dark. It appears to have fixed it when\n> applied to 0a53e9ddeaddad63ad106860237bbf53411d11a7 GIT 1.6.4. I'll be\n> trying this against v1.6.0.4 tomorrow.\n\nThanks.  After re-reading the patch, I am reasonably sure that it is the\nright fix.\n\nEarlier we fixed similar issues with bf74106 (merge-recursive: never leave\nindex unmerged while recursing, 2009-05-09) and 0c44c94 (merge-recursive:\ndo not die on a conflicting submodule, 2009-04-29).\n\nThe former is a fix to 36e3b5e (merge-recursive: mark rename/delete\nconflict as unmerged, 2008-12-22) which is probably newer than 1.6.0.4\ncodebase, and unless you are using submodules, the latter is probalby also\nsafe to ignore if you are cherry-picking to the ancient 1.6.0.4 codebase.\n\n>> The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that\n>> \"unmerged\" path, while the other branch added something else to the same\n>> path, and decided to add that at an alternative path, and the intent of\n>> that is so that it can safely resolve the \"renamed\" side to its final\n>> destination.  The added update_file() call is about finishing that\n>> conflict resolution the code forgets to do.\n>> \n>>  merge-recursive.c |    1 +\n>>  1 files changed, 1 insertions(+), 0 deletions(-)\n>> \n>> diff --git a/merge-recursive.c b/merge-recursive.c\n>> index d415c41..868b383 100644\n>> --- a/merge-recursive.c\n>> +++ b/merge-recursive.c\n>> @@ -955,6 +955,7 @@ static int process_renames(struct merge_options *o,\n>>                                 new_path = unique_path(o, ren1_dst, branch2);\n>>                                 output(o, 1, \"Adding as %s instead\",\n>> new_path);\n>>                                 update_file(o, 0, dst_other.sha1,\n>> dst_other.mode, new_path);\n>> +                               update_file(o, 0, src_other.sha1,\n>> src_other.mode, ren1_dst);\n>>                         } else if ((item = string_list_lookup(ren1_dst,\n>> renames2Dst))) {\n>>                                 ren2 = item->util;\n>>                                 clean_merge = 0;\n>> \n>\n> Thanks very much,\n> Josh\n"},{"id":"119159","messageId":"alpine.DEB.1.00.0907300958430.4589@intel-tinevez-2-302","threadId":"20267","inReplyTo":"7viqhaipg0.fsf@alter.siamese.dyndns.org","subject":"Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-07-30T08:00:57Z","receivedAt":"2009-07-30T08:00:57Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 30 Jul 2009, Junio C Hamano wrote:\n\n> Junio C Hamano <gitster@pobox.com> writes:\n> \n> > The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at \n> > that \"unmerged\" path, while the other branch added something else to \n> > the same path, and decided to add that at an alternative path, and the \n> > intent of that is so that it can safely resolve the \"renamed\" side to \n> > its final destination.  The added update_file() call is about \n> > finishing that conflict resolution the code forgets to do.\n> \n> By the way, (I am CC'ing Fredrik, the author of the merge-recursive.py,\n> the original implementation, even though I haven't seen him around here\n> for a long time. Two \"thieves\" are also CC'ed), I think the way recursive\n> strategy tries to handle rename/add and rename/rename conflicts by coming\n> up with a temporary pathname is fundamentally wrong.\n> \n> If our branch (stage #2) added by renaming and their branch (stage #3)\n> added by creating the same path, why can't we simply leave these two\n> stages in unmerged state without funny \"unique_path()\" renaming?\n\nAFAIR we could not leave the two in unmerged state because the \"recursive\" \npart of the recursive merge demands that we be able (at the intermediate \nmerges) to write a tree.\n\nBut I agree that coming up with unique paths is wrong: if it is a \nrename/rename conflict, it should be\n\n\t<<<<<<<\n\t=======\n\t<file1>\n\t>>>>>>>\n\nfor one and\n\n\t<<<<<<<\n\t<file2>\n\t=======\n\t>>>>>>>\n\nfor the other rename target.\n\nCiao,\nDscho\n"},{"id":"119203","messageId":"7vd47h6614.fsf@alter.siamese.dyndns.org","threadId":"20267","inReplyTo":"7vtz0uk5z3.fsf@alter.siamese.dyndns.org","subject":"[PATCH] merge-recursive: don't segfault while handling rename clashes","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-07-31T00:38:15Z","receivedAt":"2009-07-31T00:38:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"When a branch moves A to B while the other branch created B (or moved C to\nB), the code tried to rename one of them to B~something to preserve both\nversions, and failed to register temporary resolution for the original\npath B at stage#0 during virtual ancestor computation.  This left the\nindex in unmerged state and caused a segfault.\n\nA better solution is to merge these two versions of B's in place and use\nthe (potentially conflicting) result as the intermediate merge result in\nthe virtual ancestor.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n Junio C Hamano <gitster@pobox.com> writes:\n\n > The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that\n > \"unmerged\" path, while the other branch added something else to the same\n > path, and decided to add that at an alternative path, and the intent of\n > that is so that it can safely resolve the \"renamed\" side to its final\n > destination.  The added update_file() call is about finishing that\n > conflict resolution the code forgets to do.\n > ...\n\n Yesterday's patch squashes the conflicted path down to stage#0 even for\n outermost merge, which is not quite correct.\n\n This may be a better fix.\n\n merge-recursive.c                 |   28 ++++++++++++++++--\n t/t6036-recursive-corner-cases.sh |   56 +++++++++++++++++++++++++++++++++++++\n 2 files changed, 81 insertions(+), 3 deletions(-)\n create mode 100755 t/t6036-recursive-corner-cases.sh\n\ndiff --git a/merge-recursive.c b/merge-recursive.c\nindex d415c41..10d7913 100644\n--- a/merge-recursive.c\n+++ b/merge-recursive.c\n@@ -952,9 +952,31 @@ static int process_renames(struct merge_options *o,\n \t\t\t\t       \"%s added in %s\",\n \t\t\t\t       ren1_src, ren1_dst, branch1,\n \t\t\t\t       ren1_dst, branch2);\n-\t\t\t\tnew_path = unique_path(o, ren1_dst, branch2);\n-\t\t\t\toutput(o, 1, \"Adding as %s instead\", new_path);\n-\t\t\t\tupdate_file(o, 0, dst_other.sha1, dst_other.mode, new_path);\n+\t\t\t\tif (o->call_depth) {\n+\t\t\t\t\tstruct merge_file_info mfi;\n+\t\t\t\t\tstruct diff_filespec one, a, b;\n+\n+\t\t\t\t\tone.path = a.path = b.path =\n+\t\t\t\t\t\t(char *)ren1_dst;\n+\t\t\t\t\thashcpy(one.sha1, null_sha1);\n+\t\t\t\t\tone.mode = 0;\n+\t\t\t\t\thashcpy(a.sha1, ren1->pair->two->sha1);\n+\t\t\t\t\ta.mode = ren1->pair->two->mode;\n+\t\t\t\t\thashcpy(b.sha1, dst_other.sha1);\n+\t\t\t\t\tb.mode = dst_other.mode;\n+\t\t\t\t\tmfi = merge_file(o, &one, &a, &b,\n+\t\t\t\t\t\t\t branch1,\n+\t\t\t\t\t\t\t branch2);\n+\t\t\t\t\toutput(o, 1, \"Adding merged %s\", ren1_dst);\n+\t\t\t\t\tupdate_file(o, 0,\n+\t\t\t\t\t\t    mfi.sha,\n+\t\t\t\t\t\t    mfi.mode,\n+\t\t\t\t\t\t    ren1_dst);\n+\t\t\t\t} else {\n+\t\t\t\t\tnew_path = unique_path(o, ren1_dst, branch2);\n+\t\t\t\t\toutput(o, 1, \"Adding as %s instead\", new_path);\n+\t\t\t\t\tupdate_file(o, 0, dst_other.sha1, dst_other.mode, new_path);\n+\t\t\t\t}\n \t\t\t} else if ((item = string_list_lookup(ren1_dst, renames2Dst))) {\n \t\t\t\tren2 = item->util;\n \t\t\t\tclean_merge = 0;\ndiff --git a/t/t6036-recursive-corner-cases.sh b/t/t6036-recursive-corner-cases.sh\nnew file mode 100755\nindex 0000000..a9d1613\n--- /dev/null\n+++ b/t/t6036-recursive-corner-cases.sh\n@@ -0,0 +1,56 @@\n+#!/bin/sh\n+\n+test_description='recursive merge corner cases'\n+\n+. ./test-lib.sh\n+\n+#\n+#  L1  L2\n+#   o---o\n+#  / \\ / \\\n+# o   X   ?\n+#  \\ / \\ /\n+#   o---o\n+#  R1  R2\n+#\n+\n+test_expect_success setup '\n+\tten=\"0 1 2 3 4 5 6 7 8 9\"\n+\tfor i in $ten\n+\tdo\n+\t\techo line $i in a sample file\n+\tdone >one &&\n+\tfor i in $ten\n+\tdo\n+\t\techo line $i in another sample file\n+\tdone >two &&\n+\tgit add one two &&\n+\ttest_tick && git commit -m initial &&\n+\n+\tgit branch L1 &&\n+\tgit checkout -b R1 &&\n+\tgit mv one three &&\n+\ttest_tick && git commit -m R1 &&\n+\n+\tgit checkout L1 &&\n+\tgit mv two three &&\n+\ttest_tick && git commit -m L1 &&\n+\n+\tgit checkout L1^0 &&\n+\ttest_tick && git merge -s ours R1 &&\n+\tgit tag L2 &&\n+\n+\tgit checkout R1^0 &&\n+\ttest_tick && git merge -s ours L1 &&\n+\tgit tag R2\n+'\n+\n+test_expect_success merge '\n+\tgit reset --hard &&\n+\tgit checkout L2^0 &&\n+\n+\ttest_must_fail git merge -s recursive R2^0\n+'\n+\n+test_done\n+\n-- \n1.6.4.13.ge65800\n"},{"id":"121166","messageId":"4c8ef70908181435o1f9c6d0dncd84eb57b070b105@mail.gmail.com","threadId":"20267","inReplyTo":"7viqhaipg0.fsf@alter.siamese.dyndns.org","subject":"Re: Null deref in recursive merge in df73af5f667a479764d2b2195cb0cb60b0b89e3d","fromName":"Fredrik Kuivinen","fromEmail":"frekui@gmail.com","sentAt":"2009-08-18T21:35:17Z","receivedAt":"2009-08-18T21:35:17Z","isPatch":false,"sender":{"key":"frekui@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13770967?v=4"},"body":"On Thu, Jul 30, 2009 at 9:45 AM, Junio C Hamano<gitster@pobox.com> wrote:\n> Junio C Hamano <gitster@pobox.com> writes:\n>\n>> The codepath saw that one branch renamed dev-ubuntu/ stuff to dev/ at that\n>> \"unmerged\" path, while the other branch added something else to the same\n>> path, and decided to add that at an alternative path, and the intent of\n>> that is so that it can safely resolve the \"renamed\" side to its final\n>> destination.  The added update_file() call is about finishing that\n>> conflict resolution the code forgets to do.\n>\n> By the way, (I am CC'ing Fredrik, the author of the merge-recursive.py,\n> the original implementation, even though I haven't seen him around here\n> for a long time. Two \"thieves\" are also CC'ed), I think the way recursive\n> strategy tries to handle rename/add and rename/rename conflicts by coming\n> up with a temporary pathname is fundamentally wrong.\n\nIt was a long time ago I looked at the merge stuff (or any git code in\nfact) so I\nmay have forgotten some things. However, as far as I can see your analysis is\ncorrect and the temporary pathname stuff can be avoided when the conflicts\nonly involve files (and no directories). As you wrote, for D/F\nconflicts one need\nthe temporary pathnames in order to build a tree which is used in the virtual\nancestor.\n\n[Nice analysis of when temporary pathnames are needed and when they are not\nneeded snipped.]\n\n- Fredrik\n\nPS. I am sorry for the late reply.\n"}]}