diffcore-rename: avoid processing duplicate destinations
The rename code cannot handle an input where we have duplicate destinations (i.e., more than one diff_filepair in the queue with the same string in its pair->two->path). We end up allocating only one slot in the rename_dst mapping. If we fill in the diff_filepair for that slot, when we re-queue the results, we may queue that filepair multiple times. When the diff is finally flushed, the filepair is processed and free()d multiple times, leading to heap corruption. This situation should only happen when a tree diff sees duplicates in one of the trees (see the added test for a detailed example). Rather than handle it, the sanest thing is just to turn off rename detection altogether for the diff. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Jeff King committed
Feb 26, 2015 at 20:42 UTC
4d6be03b95c3db21db1bb8fee01128c1b13f70e7
2 files changed
+85
-2
diffcore-rename.c
+6
-2
@@ -467,8 +467,12 @@ void diffcore_rename(struct diff_options *options)
467
else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
468
is_empty_blob_sha1(p->two->sha1))
469
continue;
470
- else
471
- add_rename_dst(p->two);
470
+ else if (add_rename_dst(p->two) < 0) {
471
+ warning("skipping rename detection, detected"
472
+ " duplicate destination '%s'",
473
+ p->two->path);
474
+ goto cleanup;
475
+ }
476
}
477
else if (!DIFF_OPT_TST(options, RENAME_EMPTY) &&
478
is_empty_blob_sha1(p->one->sha1))
t/t4058-diff-duplicates.sh
new
+79
@@ -0,0 +1,79 @@
1
+#!/bin/sh
2
+
3
+test_description='test tree diff when trees have duplicate entries'
4
+. ./test-lib.sh
5
+
6
+# make_tree_entry <mode> <mode> <sha1>
7
+#
8
+# We have to rely on perl here because not all printfs understand
9
+# hex escapes (only octal), and xxd is not portable.
10
+make_tree_entry () {
11
+ printf '%s %s\0' "$1" "$2" &&
12
+ perl -e 'print chr(hex($_)) for ($ARGV[0] =~ /../g)' "$3"
13
+}
14
+
15
+# Like git-mktree, but without all of the pesky sanity checking.
16
+# Arguments come in groups of three, each group specifying a single
17
+# tree entry (see make_tree_entry above).
18
+make_tree () {
19
+ while test $# -gt 2; do
20
+ make_tree_entry "$1" "$2" "$3"
21
+ shift; shift; shift
22
+ done |
23
+ git hash-object -w -t tree --stdin
24
+}
25
+
26
+# this is kind of a convoluted setup, but matches
27
+# a real-world case. Each tree contains four entries
28
+# for the given path, one with one sha1, and three with
29
+# the other. The first tree has them split across
30
+# two subtrees (which are themselves duplicate entries in
31
+# the root tree), and the second has them all in a single subtree.
32
+test_expect_success 'create trees with duplicate entries' '
33
+ blob_one=$(echo one | git hash-object -w --stdin) &&
34
+ blob_two=$(echo two | git hash-object -w --stdin) &&
35
+ inner_one_a=$(make_tree \
36
+ 100644 inner $blob_one
37
+ ) &&
38
+ inner_one_b=$(make_tree \
39
+ 100644 inner $blob_two \
40
+ 100644 inner $blob_two \
41
+ 100644 inner $blob_two
42
+ ) &&
43
+ outer_one=$(make_tree \
44
+ 040000 outer $inner_one_a \
45
+ 040000 outer $inner_one_b
46
+ ) &&
47
+ inner_two=$(make_tree \
48
+ 100644 inner $blob_one \
49
+ 100644 inner $blob_two \
50
+ 100644 inner $blob_two \
51
+ 100644 inner $blob_two
52
+ ) &&
53
+ outer_two=$(make_tree \
54
+ 040000 outer $inner_two
55
+ ) &&
56
+ git tag one $outer_one &&
57
+ git tag two $outer_two
58
+'
59
+
60
+test_expect_success 'diff-tree between trees' '
61
+ {
62
+ printf ":000000 100644 $_z40 $blob_two A\touter/inner\n" &&
63
+ printf ":000000 100644 $_z40 $blob_two A\touter/inner\n" &&
64
+ printf ":000000 100644 $_z40 $blob_two A\touter/inner\n" &&
65
+ printf ":100644 000000 $blob_two $_z40 D\touter/inner\n" &&
66
+ printf ":100644 000000 $blob_two $_z40 D\touter/inner\n" &&
67
+ printf ":100644 000000 $blob_two $_z40 D\touter/inner\n"
68
+ } >expect &&
69
+ git diff-tree -r --no-abbrev one two >actual &&
70
+ test_cmp expect actual
71
+'
72
+
73
+test_expect_success 'diff-tree with renames' '
74
+ # same expectation as above, since we disable rename detection
75
+ git diff-tree -M -r --no-abbrev one two >actual &&
76
+ test_cmp expect actual
77
+'
78
+
79
+test_done