Fix undefined behaviour in zigzag with negative inputs (2.4 branch)

(References #3882)

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

diff --git a/NEWS b/NEWS
index eb9402faaa6fcf68ffce960acb1c28f720bdf569..ca710c727502535fc0efdf8a9d8a4d25419727ba 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -12,6 +12,7 @@ YYYY/MM/DD
   - #3875, Fix undefined behaviour in shift operation
   - #3864, Performance improvements for b-tree geometry sorts
   - #3874, lw_dist2d_pt_arc division by zero
+  - #3882, undefined behaviour in zigzag with negative inputs
 
 
 PostGIS 2.4.0

diff --git a/liblwgeom/cunit/cu_varint.c b/liblwgeom/cunit/cu_varint.c
index 9312bf4fd72e1ab0521f6a0c653054baf8c113f7..069f0c380d9002f300d9e26ba5ba5c66dfa41526 100644 (file)
--- 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 d84cf91d5e5a20ae485607bd6057b3df1e5c517f..78af2a4656a0b2a1ba9bb9c18d4e58e7816a4554 100644 (file)
--- 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 1325f9de1283f77e0bad3db3445a1459dd3ac188..62eb99298c82bc018a459f4e7daec31b1af6e732 100644 (file)
--- 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);