avoid computing zero offsets from NULL pointer

The Undefined Behavior Sanitizer in clang-11 seems to have learned a new
trick: it complains about computing offsets from a NULL pointer, even if
that offset is 0. This causes numerous test failures. For example, from
t1090:

  unpack-trees.c:1355:41: runtime error: applying zero offset to null pointer
  ...
  not ok 6 - in partial clone, sparse checkout only fetches needed blobs

The code in question looks like this:

  struct cache_entry **cache_end = cache + nr;
  ...
  while (cache != cache_end)

and we sometimes pass in a NULL and 0 for "cache" and "nr". This is
conceptually fine, as "cache_end" would be equal to "cache" in this
case, and we wouldn't enter the loop at all. But computing even a zero
offset violates the C standard. And given the fact that UBSan is
noticing this behavior, this might be a potential problem spot if the
compiler starts making unexpected assumptions based on undefined
behavior.

So let's just avoid it, which is pretty easy. In some cases we can just
switch to iterating with a numeric index (as we do in sequencer.c here).
In other cases (like the cache_end one) the use of an end pointer is
more natural; we can keep that by just explicitly checking for the
NULL/0 case when assigning the end pointer.

Note that there are two ways you can write this latter case, checking
for the pointer:

  cache_end = cache ? cache + nr : cache;

or the size:

  cache_end = nr ? cache + nr : cache;

For the case of a NULL/0 ptr/len combo, they are equivalent. But writing
it the second way (as this patch does) has the property that if somebody
were to incorrectly pass a NULL pointer with a non-zero length, we'd
continue to notice and segfault, rather than silently pretending the
length was zero.

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/sequencer.c b/sequencer.c
index b9dbf1adb078188a0163ad8ae29cce92c0587b88..4d31ec3296b328fd3db1f6e8a0d55bcc4bc8bf93 100644 (file)
--- a/sequencer.c
+++ b/sequencer.c
@@ -588,7 +588,7 @@ static int do_recursive_merge(struct repository *r,
        struct merge_options o;
        struct tree *next_tree, *base_tree, *head_tree;
        int clean;
-       char **xopt;
+       int i;
        struct lock_file index_lock = LOCK_INIT;
 
        if (repo_hold_locked_index(r, &index_lock, LOCK_REPORT_ON_ERROR) < 0)
@@ -608,8 +608,8 @@ static int do_recursive_merge(struct repository *r,
        next_tree = next ? get_commit_tree(next) : empty_tree(r);
        base_tree = base ? get_commit_tree(base) : empty_tree(r);
 
-       for (xopt = opts->xopts; xopt != opts->xopts + opts->xopts_nr; xopt++)
-               parse_merge_opt(&o, *xopt);
+       for (i = 0; i < opts->xopts_nr; i++)
+               parse_merge_opt(&o, opts->xopts[i]);
 
        clean = merge_trees(&o,
                            head_tree,

diff --git a/unpack-trees.c b/unpack-trees.c
index 2399b6818be6ddb9a6f4103cec667b2e2a9c24d0..b4a9d97f25b49f1a29dd6862803192684c02c50a 100644 (file)
--- a/unpack-trees.c
+++ b/unpack-trees.c
@@ -1348,7 +1348,7 @@ static int clear_ce_flags_1(struct index_state *istate,
                            enum pattern_match_result default_match,
                            int progress_nr)
 {
-       struct cache_entry **cache_end = cache + nr;
+       struct cache_entry **cache_end = nr ? cache + nr : cache;
 
        /*
         * Process all entries that have the given prefix and meet

diff --git a/xdiff-interface.c b/xdiff-interface.c
index 8509f9ea223a1282a367874c3e3a3ef0c351a30f..99661d43c61ad63486089ccc8cf87a77ffe5ebe8 100644 (file)
--- a/xdiff-interface.c
+++ b/xdiff-interface.c
@@ -84,8 +84,8 @@ static void trim_common_tail(mmfile_t *a, mmfile_t *b)
 {
        const int blk = 1024;
        long trimmed = 0, recovered = 0;
-       char *ap = a->ptr + a->size;
-       char *bp = b->ptr + b->size;
+       char *ap = a->size ? a->ptr + a->size : a->ptr;
+       char *bp = b->size ? b->ptr + b->size : b->ptr;
        long smaller = (a->size < b->size) ? a->size : b->size;
 
        while (blk + trimmed <= smaller && !memcmp(ap - blk, bp - blk, blk)) {