From: Paul Ramsey <pramsey@cleverelephant.ca>
Date: Tue, 10 Oct 2017 16:59:34 +0000 (+0000)
Subject: Fix undefined behaviour in zigzag with negative inputs
X-Git-Tag: 2.5.0alpha~423
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=e8384ab814257bdfbc7a7da5cd5e60df938580b7;p=postgis

Fix undefined behaviour in zigzag with negative inputs
(References #3882)



git-svn-id: http://svn.osgeo.org/postgis/trunk@15951 b70326c6-7e19-0410-871a-916f4a2858ee
---

diff --git a/liblwgeom/cunit/cu_varint.c b/liblwgeom/cunit/cu_varint.c
index 9312bf4fd..069f0c380 100644
--- a/liblwgeom/cunit/cu_varint.c
+++ b/liblwgeom/cunit/cu_varint.c
@@ -220,13 +220,27 @@ static void test_zigzag(void)
 	{
 		a = b = i;
 		CU_ASSERT_EQUAL(a, unzigzag64(zigzag64(a)));
-		CU_ASSERT_EQUAL(b, unzigzag32(zigzag64(b)));
+		CU_ASSERT_EQUAL(b, unzigzag32(zigzag32(b)));
 
 		a = b = -1 * i;
 		CU_ASSERT_EQUAL(a, unzigzag64(zigzag64(a)));
-		CU_ASSERT_EQUAL(b, unzigzag32(zigzag64(b)));
+		CU_ASSERT_EQUAL(b, unzigzag32(zigzag32(b)));
 	}
 
+	//8
+	CU_ASSERT_EQUAL(-INT8_MAX, unzigzag8(zigzag8(-INT8_MAX)));
+	CU_ASSERT_EQUAL(INT8_MAX, unzigzag8(zigzag8(INT8_MAX)));
+	CU_ASSERT_EQUAL(0, unzigzag8(zigzag8(0)));
+
+	//32
+	CU_ASSERT_EQUAL(-INT32_MAX, unzigzag32(zigzag32(-INT32_MAX)));
+	CU_ASSERT_EQUAL(INT32_MAX, unzigzag32(zigzag32(INT32_MAX)));
+	CU_ASSERT_EQUAL(0, unzigzag32(zigzag32(0)));
+
+	//64
+	CU_ASSERT_EQUAL(-INT64_MAX, unzigzag64(zigzag64(-INT64_MAX)));
+	CU_ASSERT_EQUAL(INT64_MAX, unzigzag64(zigzag64(INT64_MAX)));
+	CU_ASSERT_EQUAL(0, unzigzag64(zigzag64(0)));
 }
 
 
diff --git a/liblwgeom/varint.c b/liblwgeom/varint.c
index d84cf91d5..78af2a465 100644
--- a/liblwgeom/varint.c
+++ b/liblwgeom/varint.c
@@ -167,41 +167,44 @@ varint_size(const uint8_t *the_start, const uint8_t *the_end)
 
 uint64_t zigzag64(int64_t val)
 {
-	return (val << 1) ^ (val >> 63);
+	return val >= 0 ?
+		((uint64_t)val) << 1 :
+		((((uint64_t)(-1 - val)) << 1) | 0x01);
 }
 
 uint32_t zigzag32(int32_t val)
 {
-	return (val << 1) ^ (val >> 31);
+	return val >= 0 ?
+		((uint32_t)val) << 1 :
+		((((uint32_t)(-1 - val)) << 1) | 0x01);
 }
 
 uint8_t zigzag8(int8_t val)
 {
-	return (val << 1) ^ (val >> 7);
+	return val >= 0 ?
+		((uint8_t)val) << 1 :
+		((((uint8_t)(-1 - val)) << 1) | 0x01);
 }
 
 int64_t unzigzag64(uint64_t val)
 {
-        if ( val & 0x01 )
-            return -1 * (int64_t)((val+1) >> 1);
-        else
-            return (int64_t)(val >> 1);
+	return !(val & 0x01) ?
+		((int64_t)(val >> 1)) :
+		(-1 * (int64_t)((val+1) >> 1));
 }
 
 int32_t unzigzag32(uint32_t val)
 {
-        if ( val & 0x01 )
-            return -1 * (int32_t)((val+1) >> 1);
-        else
-            return (int32_t)(val >> 1);
+	return !(val & 0x01) ?
+		((int32_t)(val >> 1)) :
+		(-1 * (int32_t)((val+1) >> 1));
 }
 
 int8_t unzigzag8(uint8_t val)
 {
-        if ( val & 0x01 )
-            return -1 * (int8_t)((val+1) >> 1);
-        else
-            return (int8_t)(val >> 1);
+	return !(val & 0x01) ?
+		((int8_t)(val >> 1)) :
+		(-1 * (int8_t)((val+1) >> 1));
 }
 
 
diff --git a/liblwgeom/varint.h b/liblwgeom/varint.h
index 1325f9de1..62eb99298 100644
--- a/liblwgeom/varint.h
+++ b/liblwgeom/varint.h
@@ -42,6 +42,8 @@ uint64_t varint_u64_decode(const uint8_t *the_start, const uint8_t *the_end, siz
 
 size_t varint_size(const uint8_t *the_start, const uint8_t *the_end);
 
+/* Support from -INT{8,32,64}_MAX to INT{8,32,64}_MAX),
+ * e.g INT8_MIN is not supported in zigzag8 */
 uint64_t zigzag64(int64_t val);
 uint32_t zigzag32(int32_t val);
 uint8_t zigzag8(int8_t val);