Re: [PATCH v3 1/2] Fix nonnull errors reported by UBSAN with GCC 7.

["Martin Liška" <mliska@suse.cz>]

[-- Attachment #1: Type: text/plain, Size: 1680 bytes --]

On 04/06/2017 02:26 PM, René Scharfe wrote:
> Am 06.04.2017 um 11:52 schrieb Martin Liška:
>> I'm sending (v2), where I updated commit message and wrapped 2 problematic
>> places to newly introduced macros that do the check. Follow-up patch can
>> change usages of memcpy and memove.
> 
>> diff --git a/apply.c b/apply.c
>> index e6dbab26a..eacca29fa 100644
>> --- a/apply.c
>> +++ b/apply.c
>> @@ -2802,9 +2802,9 @@ static void update_image(struct apply_state *state,
>>  			img->line + applied_pos + preimage_limit,
>>  			(img->nr - (applied_pos + preimage_limit)) *
>>  			sizeof(*img->line));
>> -	memcpy(img->line + applied_pos,
>> -	       postimage->line,
>> -	       postimage->nr * sizeof(*img->line));
>> +	MEMCPY(img->line + applied_pos,
>> +		postimage->line,
>> +		postimage->nr * sizeof(*img->line));
> 
> Using the existing macro COPY_ARRAY would yield a nicer result:

Yes, it's nicer. I'm sending tested version 3.

Martin

> 
> 	COPY_ARRAY(img->line + applied_pos, postimage->line, postimage->nr);
> 
>>  	if (!state->allow_overlap)
>>  		for (i = 0; i < postimage->nr; i++)
>>  			img->line[applied_pos + i].flag |= LINE_PATCHED;
>> diff --git a/builtin/ls-files.c b/builtin/ls-files.c
>> index d449e46db..7caeeb6a6 100644
>> --- a/builtin/ls-files.c
>> +++ b/builtin/ls-files.c
>> @@ -391,7 +391,7 @@ static void prune_cache(const char *prefix, size_t prefixlen)
>>  		}
>>  		last = next;
>>  	}
>> -	memmove(active_cache, active_cache + pos,
>> +	MEMMOVE(active_cache, active_cache + pos,
>>  		(last - pos) * sizeof(struct cache_entry *));
>>  	active_nr = last - pos;
>>  }
> 
> Perhaps we should add MOVE_ARRAY to complement COPY_ARRAY.
> 
> René
> 


[-- Attachment #2: 0001-Fix-nonnull-errors-reported-by-UBSAN-with-GCC-7.patch --]
[-- Type: text/x-patch, Size: 2156 bytes --]

From 4784ff90b2c4cd0d78a25c8d8ed77f299686348c Mon Sep 17 00:00:00 2001
From: marxin <mliska@suse.cz>
Date: Wed, 5 Apr 2017 14:31:32 +0200
Subject: [PATCH 1/2] Fix nonnull errors reported by UBSAN with GCC 7.

Memory functions like memmove and memcpy should only be called
with positive sizes. That is achieved by newly introduced macro
MEMMOVE and usage of ARRAY_COPY.

Signed-off-by: Martin Liska <mliska@suse.cz>
---
 apply.c            |  4 +---
 builtin/ls-files.c |  2 +-
 git-compat-util.h  | 10 ++++++++++
 3 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/apply.c b/apply.c
index e6dbab26a..121f3f414 100644
--- a/apply.c
+++ b/apply.c
@@ -2802,9 +2802,7 @@ static void update_image(struct apply_state *state,
 			img->line + applied_pos + preimage_limit,
 			(img->nr - (applied_pos + preimage_limit)) *
 			sizeof(*img->line));
-	memcpy(img->line + applied_pos,
-	       postimage->line,
-	       postimage->nr * sizeof(*img->line));
+	COPY_ARRAY(img->line + applied_pos, postimage->line, postimage->nr);
 	if (!state->allow_overlap)
 		for (i = 0; i < postimage->nr; i++)
 			img->line[applied_pos + i].flag |= LINE_PATCHED;
diff --git a/builtin/ls-files.c b/builtin/ls-files.c
index d449e46db..7caeeb6a6 100644
--- a/builtin/ls-files.c
+++ b/builtin/ls-files.c
@@ -391,7 +391,7 @@ static void prune_cache(const char *prefix, size_t prefixlen)
 		}
 		last = next;
 	}
-	memmove(active_cache, active_cache + pos,
+	MEMMOVE(active_cache, active_cache + pos,
 		(last - pos) * sizeof(struct cache_entry *));
 	active_nr = last - pos;
 }
diff --git a/git-compat-util.h b/git-compat-util.h
index 8a4a3f85e..b75e21cff 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1002,6 +1002,16 @@ int git_qsort_s(void *base, size_t nmemb, size_t size,
 		die("BUG: qsort_s() failed");			\
 } while (0)
 
+static inline void *sane_memmove(void *dest, const void *src, size_t n)
+{
+	if (n > 0)
+		return memmove(dest, src, n);
+	else
+		return dest;
+}
+
+#define MEMMOVE(dest, src, n) sane_memmove(dest, src, n)
+
 #ifndef REG_STARTEND
 #error "Git requires REG_STARTEND support. Compile with NO_REGEX=NeedsStartEnd"
 #endif
-- 
2.12.2