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>
This commit is contained in:
parent
4c616c2ba1
commit
d20bc01a51
@ -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,
|
||||
|
@ -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
|
||||
|
@ -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)) {
|
||||
|
Loading…
Reference in New Issue
Block a user