[PATCH 0/2] Fixes for undefined behaviour

I've been playing with Clang's undefined behaviour sanitizer, which
points out a few potential issues in Git when running the test suite
(it's a runtime analysis that is compiled in by setting suitable
CFLAGS).

These patches fix one issue that I think we need to worry about and one
that's trivial to fix.

The remaining warnings are:

refs.c:2426:17: runtime error: index -1 out of bounds for type 'char [8192]'

  Caused by a loop walking backwards over the reflog which sets its scan
  pointer to be one before the start of the buffer in order to break out
  of the loop.  It seems unlikely that the (stack allocated) buffer will
  be at address zero so I don't think any sane compiler will cause us
  problems here.

tag.c:104:40: runtime error: member access within null pointer of type
'struct commit'

  This does "&lookup_commit(sha1)->object" which ends up being okay
  because "object" is the first item in struct commit.  I'm not sure
  it's worth the churn to change this.

xdiff/xutils.c:308:7: runtime error: load of misaligned address for type
'unsigned long', which requires 8 byte alignment

  This is in the XDL_FAST_HASH code, which should only be used on
  architectures where this is likely to be reasonably fast.  The commit
  introducing this code points at an LKML thread[1] discussing a similar
  implementation in the kernel, which discusses the impact of the
  unaligned access, the conclusion being that it's faster than any
  alternative.

[1] https://lkml.org/lkml/2012/3/2/452

John Keeping (2):
  diffcore-break: don't divide by zero
  bisect: avoid signed integer overflow

 bisect.c         | 2 +-
 diffcore-break.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
1.8.2.540.gf023cfe

When the source file is empty, the calculation of the merge score
results in a division by zero.  Since the merge score is initialized to
zero, it makes sense to just leave it as it is if the source size is
zero.  This means that we still use the extent of damage metric to
decide whether to break the filepair.

Signed-off-by: John Keeping <redacted>
---
 diffcore-break.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/diffcore-break.c b/diffcore-break.c
index 44f8678..37d8d05 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c

@@ -90,7 +90,8 @@ static int should_break(struct diff_filespec *src,
 	 * merge the surviving pair together if the score is
 	 * less than the minimum, after rename/copy runs.
 	 */
-	*merge_score_p = (int)(src_removed * MAX_SCORE / src->size);
+	if (src->size)
+		*merge_score_p = (int)(src_removed * MAX_SCORE / src->size);
 	if (*merge_score_p > break_score)
 		return 1;
 

-- 
1.8.2.540.gf023cfe

Signed-off-by: John Keeping <redacted>
---
 bisect.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/bisect.c b/bisect.c
index bd1b7b5..0d33c6f 100644
--- a/bisect.c
+++ b/bisect.c

@@ -526,7 +526,7 @@ struct commit_list *filter_skipped(struct commit_list *list,
  * for this application.
  */
 static int get_prn(int count) {
-	count = count * 1103515245 + 12345;
+	count = ((unsigned) count) * 1103515245 + 12345;
 	return ((unsigned)(count/65536) % PRN_MODULO);
 }
 

-- 
1.8.2.540.gf023cfe