diff: check for merge bases before assigning sym->base

In symdiff_prepare(), we iterate over the set of parsed objects to pick
out any symmetric differences, including the left, right, and base
elements. We assign the results into pointers in a "struct symdiff", and
then complain if we didn't find a base, like so:

    sym->left = rev->pending.objects[lpos].name;
    sym->right = rev->pending.objects[rpos].name;
    sym->base = rev->pending.objects[basepos].name;
    if (basecount == 0)
            die(_("%s...%s: no merge base"), sym->left, sym->right);

But the least lines are backwards. If basecount is 0, then basepos will
be -1, and we will access memory outside of the pending array. This
isn't usually that big a deal, since we don't do anything besides a
single pointer-sized read before exiting anyway, but it does violate the
C standard, and of course memory-checking tools like ASan complain.

Let's put the basecount check first. Note that we haveto split it from
the other assignments, since the die() relies on sym->left and
sym->right having been assigned (this isn't strictly necessary, but is
easier to read than dereferencing the pending array again).

Reported-by: brian m. carlson <sandals@crustytoothpaste.net>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/diff.c b/builtin/diff.c
index 96ee2cbb2ab7db9f3eaf6589ba7fdab00131313a..64ac8ae0af4252c8a48eeb3427d3387b064d35b2 100644 (file)
--- a/builtin/diff.c
+++ b/builtin/diff.c
@@ -355,9 +355,9 @@ static void symdiff_prepare(struct rev_info *rev, struct symdiff *sym)
 
        sym->left = rev->pending.objects[lpos].name;
        sym->right = rev->pending.objects[rpos].name;
-       sym->base = rev->pending.objects[basepos].name;
        if (basecount == 0)
                die(_("%s...%s: no merge base"), sym->left, sym->right);
+       sym->base = rev->pending.objects[basepos].name;
        bitmap_unset(map, basepos);     /* unmark the base we want */
        sym->warn = basecount > 1;
        sym->skip = map;