Patchwork [STABLE?] parsers: avoid signed integer overflow in calculation of leaf-node index

login
register
mail settings
Submitter Yuya Nishihara
Date April 29, 2015, 2:46 p.m.
Message ID <86a1b0c138484c57501d.1430318787@mimosa>
Download mbox | patch
Permalink /patch/8816/
State Accepted
Headers show

Comments

Yuya Nishihara - April 29, 2015, 2:46 p.m.
# HG changeset patch
# User Yuya Nishihara <yuya@tcha.org>
# Date 1430316454 -32400
#      Wed Apr 29 23:07:34 2015 +0900
# Branch stable
# Node ID 86a1b0c138484c57501d436efc803c8ad4972928
# Parent  73b0e11a9cb8fea9b4f0a4ce4267409e8f2054cd
parsers: avoid signed integer overflow in calculation of leaf-node index

If v = -INT_MAX - 1, -v would exceed INT_MAX. I don't think this would cause
problems such as issue4627, but we can't blame it as a compiler bug because
signed integer overflow is undefined in C.
Matt Mackall - April 29, 2015, 3:46 p.m.
On Wed, 2015-04-29 at 23:46 +0900, Yuya Nishihara wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya@tcha.org>
> # Date 1430316454 -32400
> #      Wed Apr 29 23:07:34 2015 +0900
> # Branch stable
> # Node ID 86a1b0c138484c57501d436efc803c8ad4972928
> # Parent  73b0e11a9cb8fea9b4f0a4ce4267409e8f2054cd
> parsers: avoid signed integer overflow in calculation of leaf-node index
> 
> If v = -INT_MAX - 1, -v would exceed INT_MAX. I don't think this would cause
> problems such as issue4627, but we can't blame it as a compiler bug because
> signed integer overflow is undefined in C.

I guess, queued for stable. I'd like to find a way to make this less
fragile for the caller though. I think it might be cleaner to just
change the comparisons to >= INT_MAX but I haven't had time to think
about it in detail. It doesn't really address the "undefined behavior in
C is REALLY undefined and not just unspecified-but-vaguely-sensible"
problem though.

Patch

diff --git a/mercurial/parsers.c b/mercurial/parsers.c
--- a/mercurial/parsers.c
+++ b/mercurial/parsers.c
@@ -1312,7 +1312,7 @@  static int nt_find(indexObject *self, co
 			const char *n;
 			Py_ssize_t i;
 
-			v = -v - 1;
+			v = -(v + 1);
 			n = index_node(self, v);
 			if (n == NULL)
 				return -2;
@@ -1368,7 +1368,7 @@  static int nt_insert(indexObject *self, 
 			return 0;
 		}
 		if (v < 0) {
-			const char *oldnode = index_node(self, -v - 1);
+			const char *oldnode = index_node(self, -(v + 1));
 			int noff;
 
 			if (!oldnode || !memcmp(oldnode, node, 20)) {