[postgis-tickets] r17480 - Fix undefined behaviour in implicit conversions

Raul raul at rmr.ninja
Mon Jun 10 03:55:10 PDT 2019


Author: algunenano
Date: 2019-06-10 03:55:10 -0700 (Mon, 10 Jun 2019)
New Revision: 17480

Modified:
   trunk/.travis.yml
   trunk/NEWS
   trunk/ci/travis/run_usan_clang.sh
   trunk/liblwgeom/liblwgeom_topo.h
   trunk/liblwgeom/liblwgeom_topo_internal.h
   trunk/liblwgeom/lwgeom_topo.c
   trunk/raster/rt_core/rt_band.c
   trunk/raster/rt_core/rt_serialize.c
   trunk/raster/rt_core/rt_statistics.c
   trunk/raster/rt_core/rt_wkb.c
   trunk/raster/rt_pg/rtpg_internal.c
   trunk/raster/rt_pg/rtpg_mapalgebra.c
   trunk/raster/test/regress/rt_mapalgebra.sql
   trunk/topology/postgis_topology.c
Log:
Fix undefined behaviour in implicit conversions

Cleans up errors in raster and topology and adds an
extra sanitizer build in Travis

Closes #4383
Closes https://github.com/postgis/postgis/pull/398



Modified: trunk/.travis.yml
===================================================================
--- trunk/.travis.yml	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/.travis.yml	2019-06-10 10:55:10 UTC (rev 17480)
@@ -10,7 +10,7 @@
   - tag=pg11-geos37-gdal24-proj52 mode=debug
   - tag=pg11-geos37-gdal24-proj52 mode=coverage
   - tag=pg11-geos37-gdal24-proj52 mode=usan_gcc
-  - tag=pg11-geos37-gdal24-proj52 mode=usan_clang
+  - tag=pg11-clang-geos37-gdal24-proj52 mode=usan_clang
   - tag=pg11-geos37-gdal24-proj52 mode=nowagyu
   - tag=pg11-geos37-gdal24-proj52 mode=tests
   - tag=pg10-geos36-gdal23-proj49 mode=tests

Modified: trunk/NEWS
===================================================================
--- trunk/NEWS	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/NEWS	2019-06-10 10:55:10 UTC (rev 17480)
@@ -156,6 +156,7 @@
   - #4413, Raster tile size follows GeoTIFF block size on raster2pgsql -t auto
            (Darafei Praliaskouski)
   - #4422, Modernize Python 2 code to get ready for Python 3 (Christian Clauss)
+  - #4383, Fix undefined behaviour in implicit conversions (Raúl Marín)
 
 
 PostGIS 2.5.0

Modified: trunk/ci/travis/run_usan_clang.sh
===================================================================
--- trunk/ci/travis/run_usan_clang.sh	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/ci/travis/run_usan_clang.sh	2019-06-10 10:55:10 UTC (rev 17480)
@@ -2,12 +2,18 @@
 set -e
 
 # Enable undefined behaviour sanitizer using traps
-CFLAGS_STD="-g3 -O0 -mtune=generic -fno-omit-frame-pointer -fsanitize=undefined -fsanitize-undefined-trap-on-error"
+CFLAGS_USAN="-g3 -O0 -mtune=generic -fno-omit-frame-pointer -fsanitize=undefined,implicit-conversion -fsanitize-undefined-trap-on-error -fno-sanitize-recover=implicit-conversion"
 LDFLAGS_STD="-Wl,-Bsymbolic-functions -Wl,-z,relro"
 
-/usr/local/pgsql/bin/pg_ctl -c -l /tmp/logfile start
-./autogen.sh
+# Sanitizer options to continue avoid stopping the runs on leaks (expected on postgres binaries)
+export ASAN_OPTIONS=halt_on_error=false,leak_check_at_exit=false,exitcode=0
+export MSAN_OPTIONS=halt_on_error=false,leak_check_at_exit=false,exitcode=0
 
+#Run postgres preloading sanitizer libs
+LD_PRELOAD=/usr/lib/clang/7/lib/linux/libclang_rt.asan-x86_64.so /usr/local/pgsql/bin/pg_ctl -c -l /tmp/logfile start
+
+
 # Build with Clang and usan flags
-./configure CC=clang CFLAGS="${CFLAGS_STD}" LDFLAGS="${LDFLAGS_STD}"
+./autogen.sh
+./configure CC=clang CFLAGS="${CFLAGS_USAN}" LDFLAGS="${LDFLAGS_STD}"
 bash ./ci/travis/logbt -- make -j check RUNTESTFLAGS=--verbose

Modified: trunk/liblwgeom/liblwgeom_topo.h
===================================================================
--- trunk/liblwgeom/liblwgeom_topo.h	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/liblwgeom/liblwgeom_topo.h	2019-06-10 10:55:10 UTC (rev 17480)
@@ -222,7 +222,7 @@
 					   double dist,
 					   uint64_t *numelems,
 					   int fields,
-					   int limit);
+					   int64_t limit);
 
   /**
    * Insert nodes
@@ -282,7 +282,7 @@
 					   double dist,
 					   uint64_t *numelems,
 					   int fields,
-					   int limit);
+					   int64_t limit);
 
   /**
    * Get next available edge identifier

Modified: trunk/liblwgeom/liblwgeom_topo_internal.h
===================================================================
--- trunk/liblwgeom/liblwgeom_topo_internal.h	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/liblwgeom/liblwgeom_topo_internal.h	2019-06-10 10:55:10 UTC (rev 17480)
@@ -54,7 +54,7 @@
 					     double dist,
 					     uint64_t *numelems,
 					     int fields,
-					     uint64_t limit);
+					     int64_t limit);
 
 LWT_ISO_NODE *lwt_be_getNodeById(LWT_TOPOLOGY *topo, const LWT_ELEMID *ids, uint64_t *numelems, int fields);
 
@@ -70,7 +70,7 @@
 					     double dist,
 					     uint64_t *numelems,
 					     int fields,
-					     uint64_t limit);
+					     int64_t limit);
 int lwt_be_insertEdges(LWT_TOPOLOGY *topo, LWT_ISO_EDGE *edge, uint64_t numelems);
 int
 lwt_be_updateEdges(LWT_TOPOLOGY* topo, const LWT_ISO_EDGE* sel_edge, int sel_fields, const LWT_ISO_EDGE* upd_edge, int upd_fields, const LWT_ISO_EDGE* exc_edge, int exc_fields);

Modified: trunk/liblwgeom/lwgeom_topo.c
===================================================================
--- trunk/liblwgeom/lwgeom_topo.c	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/liblwgeom/lwgeom_topo.c	2019-06-10 10:55:10 UTC (rev 17480)
@@ -163,7 +163,7 @@
 			       double dist,
 			       uint64_t *numelems,
 			       int fields,
-			       uint64_t limit)
+			       int64_t limit)
 {
   CBT5(topo, getNodeWithinDistance2D, pt, dist, numelems, fields, limit);
 }
@@ -252,7 +252,7 @@
 			       double dist,
 			       uint64_t *numelems,
 			       int fields,
-			       uint64_t limit)
+			       int64_t limit)
 {
   CBT5(topo, getEdgeWithinDistance2D, pt, dist, numelems, fields, limit);
 }

Modified: trunk/raster/rt_core/rt_band.c
===================================================================
--- trunk/raster/rt_core/rt_band.c	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/raster/rt_core/rt_band.c	2019-06-10 10:55:10 UTC (rev 17480)
@@ -978,7 +978,7 @@
 	int *converted
 ) {
 	rt_pixtype pixtype = PT_END;
-	unsigned char* data = NULL;
+	uint8_t *data = NULL;
 	uint32_t offset = 0;
 
 	int32_t checkvalint = 0;
@@ -1044,7 +1044,7 @@
 			break;
 		}
 		case PT_8BSI: {
-			data[offset] = rt_util_clamp_to_8BSI(val);
+			data[offset] = (uint8_t)rt_util_clamp_to_8BSI(val);
 			checkvalint = (int8_t) data[offset];
 			break;
 		}
@@ -1300,7 +1300,7 @@
 			}
 #endif
 		case PT_8BSI: {
-			int8_t val = data[offset];
+			int8_t val = (int8_t)data[offset];
 			*value = val;
 			break;
 		}
@@ -1491,10 +1491,10 @@
 	*npixels = NULL;
 
 	/* maximum extent */
-	max_extent[0] = x - distance[0]; /* min X */
-	max_extent[1] = y - distance[1]; /* min Y */
-	max_extent[2] = x + distance[0]; /* max X */
-	max_extent[3] = y + distance[1]; /* max Y */
+	max_extent[0] = x - (int)distance[0]; /* min X */
+	max_extent[1] = y - (int)distance[1]; /* min Y */
+	max_extent[2] = x + (int)distance[0]; /* max X */
+	max_extent[3] = y + (int)distance[1]; /* max Y */
 	RASTER_DEBUGF(4, "Maximum Extent: (%d, %d, %d, %d)",
 		max_extent[0], max_extent[1], max_extent[2], max_extent[3]);
 
@@ -1504,10 +1504,10 @@
 		_d[0]++;
 		_d[1]++;
 
-		extent[0] = x - _d[0]; /* min x */
-		extent[1] = y - _d[1]; /* min y */
-		extent[2] = x + _d[0]; /* max x */
-		extent[3] = y + _d[1]; /* max y */
+		extent[0] = x - (int)_d[0]; /* min x */
+		extent[1] = y - (int)_d[1]; /* min y */
+		extent[2] = x + (int)_d[0]; /* max x */
+		extent[3] = y + (int)_d[1]; /* max y */
 
 		RASTER_DEBUGF(4, "Processing distances: %d x %d", _d[0], _d[1]);
 		RASTER_DEBUGF(4, "Extent: (%d, %d, %d, %d)",

Modified: trunk/raster/rt_core/rt_serialize.c
===================================================================
--- trunk/raster/rt_core/rt_serialize.c	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/raster/rt_core/rt_serialize.c	2019-06-10 10:55:10 UTC (rev 17480)
@@ -261,7 +261,7 @@
 read_int16(const uint8_t** from, uint8_t littleEndian) {
     assert(NULL != from);
 
-    return read_uint16(from, littleEndian);
+    return (int16_t)read_uint16(from, littleEndian);
 }
 
 /* unused up to now
@@ -333,7 +333,7 @@
 read_int32(const uint8_t** from, uint8_t littleEndian) {
     assert(NULL != from);
 
-    return read_uint32(from, littleEndian);
+    return (int32_t)read_uint32(from, littleEndian);
 }
 
 /* unused up to now
@@ -625,7 +625,7 @@
 			}
 			case PT_8BSI: {
 				int8_t v = band->nodataval;
-				*ptr = v;
+				*ptr = (uint8_t)v;
 				ptr += 1;
 				break;
 			}

Modified: trunk/raster/rt_core/rt_statistics.c
===================================================================
--- trunk/raster/rt_core/rt_statistics.c	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/raster/rt_core/rt_statistics.c	2019-06-10 10:55:10 UTC (rev 17480)
@@ -113,7 +113,7 @@
 	uint64_t *cK, double *cM, double *cQ
 ) {
 	uint32_t x = 0;
-	uint32_t y = 0;
+	int64_t y = 0;
 	uint32_t z = 0;
 	uint32_t offset = 0;
 	uint32_t diff = 0;
@@ -928,7 +928,7 @@
 		}
 
 		RASTER_DEBUGF(5, "deleting index: %d => %f", i, qle->value);
-		qll->index[i].index = -1;
+		qll->index[i].index = UINT32_MAX;
 		qll->index[i].element = NULL;
 	}
 }
@@ -973,7 +973,7 @@
 
 	RASTER_DEBUG(5, "resetting index");
 	for (i = 0; i < qll->index_max; i++) {
-		qll->index[i].index = -1;
+		qll->index[i].index = UINT32_MAX;
 		qll->index[i].element = NULL;
 	}
 }
@@ -1032,7 +1032,7 @@
 	uint32_t j = 0;
 	uint32_t k = 0;
 	uint32_t x = 0;
-	uint32_t y = 0;
+	int64_t y = 0;
 	uint32_t z = 0;
 	uint32_t idx = 0;
 	uint32_t offset = 0;

Modified: trunk/raster/rt_core/rt_wkb.c
===================================================================
--- trunk/raster/rt_core/rt_wkb.c	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/raster/rt_core/rt_wkb.c	2019-06-10 10:55:10 UTC (rev 17480)
@@ -588,7 +588,7 @@
 			}
 			case PT_8BSI: {
 				int8_t v = band->nodataval;
-				*ptr = v;
+				*ptr = (uint8_t)v;
 				ptr += 1;
 				break;
 			}

Modified: trunk/raster/rt_pg/rtpg_internal.c
===================================================================
--- trunk/raster/rt_pg/rtpg_internal.c	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/raster/rt_pg/rtpg_internal.c	2019-06-10 10:55:10 UTC (rev 17480)
@@ -60,9 +60,9 @@
 	const char *tmp = str;
 	char *result;
 	int found = 0;
-	int length, reslen;
-	int oldlen = strlen(oldstr);
-	int newlen = strlen(newstr);
+	int64_t length, reslen;
+	size_t oldlen = strlen(oldstr);
+	size_t newlen = strlen(newstr);
 	int limit = (count != NULL && *count > 0) ? *count : -1;
 
 	tmp = str;

Modified: trunk/raster/rt_pg/rtpg_mapalgebra.c
===================================================================
--- trunk/raster/rt_pg/rtpg_mapalgebra.c	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/raster/rt_pg/rtpg_mapalgebra.c	2019-06-10 10:55:10 UTC (rev 17480)
@@ -457,10 +457,10 @@
 	i++;
 
 	for (z = 0; z < arg->rasters; z++) {
-		_pos[i] = arg->src_pixel[z][0] + 1;
+		_pos[i] = (Datum)arg->src_pixel[z][0] + 1;
 		i++;
 
-		_pos[i] = arg->src_pixel[z][1] + 1;
+		_pos[i] = (Datum)arg->src_pixel[z][1] + 1;
 		i++;
 	}
 
@@ -7068,8 +7068,8 @@
 					_pixel[i] = 0;
 
 					/* row/column */
-					_x = x - (int) _rastoffset[i][0];
-					_y = y - (int) _rastoffset[i][1];
+					_x = (int)x - (int)_rastoffset[i][0];
+					_y = (int)y - (int)_rastoffset[i][1];
 
 					/* store _x and _y in 1-based */
 					_pos[i][0] = _x + 1;

Modified: trunk/raster/test/regress/rt_mapalgebra.sql
===================================================================
--- trunk/raster/test/regress/rt_mapalgebra.sql	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/raster/test/regress/rt_mapalgebra.sql	2019-06-10 10:55:10 UTC (rev 17480)
@@ -115,8 +115,6 @@
 DO $$ DECLARE r record;
 BEGIN
 -- NOTE: added OFFSET 0 to CTE clauses to force PostgreSQL 12+ to materialize like old versions did
--- this ONLY works for PostgreSQL version 9.1 or higher
-IF array_to_string(regexp_matches(split_part(version(), ' ', 2), E'([0-9]+)\.([0-9]+)'), '')::int > 90 THEN
 	WITH foo AS (
 		SELECT
 			t1.rid,
@@ -195,111 +193,6 @@
 	INTO r
 	FROM foo;
 	RAISE NOTICE 'record = %', r;
-
-ELSE
-
-	WITH foo AS (
-		SELECT
-			t1.rid,
-			ST_Union(t2.rast) AS rast
-		FROM raster_nmapalgebra_in t1
-		JOIN raster_nmapalgebra_in t2
-			ON ST_Intersects(t1.rast, t2.rast)
-			AND t2.rid BETWEEN 10 AND 18
-		WHERE t1.rid = 10
-		GROUP BY t1.rid
-	), bar AS (
-		SELECT
-			t1.rid,
-			ST_MapAlgebra(
-				ARRAY[ROW(t2.rast, 1)]::rastbandarg[],
-				'raster_nmapalgebra_test(double precision[], int[], text[])'::regprocedure,
-				'32BUI',
-				'CUSTOM', t1.rast,
-				1, 1
-			) AS rast
-		FROM raster_nmapalgebra_in t1
-		JOIN foo t2
-			ON t1.rid = t2.rid OFFSET 0
-	)
-	SELECT
-		rid,
-		(ST_Metadata(rast)),
-		(ST_BandMetadata(rast, 1)),
-		ST_Value(rast, 1, 1, 1)
-	INTO r
-	FROM bar;
-	RAISE NOTICE 'record = %', r;
-
-	WITH foo AS (
-		SELECT
-			t1.rid,
-			ST_Union(t2.rast) AS rast
-		FROM raster_nmapalgebra_in t1
-		JOIN raster_nmapalgebra_in t2
-			ON ST_Intersects(t1.rast, t2.rast)
-			AND t2.rid BETWEEN 10 AND 18
-		WHERE t1.rid = 14
-		GROUP BY t1.rid
-	), bar AS (
-		SELECT
-			t1.rid,
-			ST_MapAlgebra(
-				ARRAY[ROW(t2.rast, 1)]::rastbandarg[],
-				'raster_nmapalgebra_test(double precision[], int[], text[])'::regprocedure,
-				'32BUI',
-				'CUSTOM', t1.rast,
-				1, 1
-			) AS rast
-		FROM raster_nmapalgebra_in t1
-		JOIN foo t2
-			ON t1.rid = t2.rid OFFSET 0
-	)
-	SELECT
-		rid,
-		(ST_Metadata(rast)),
-		(ST_BandMetadata(rast, 1)),
-		ST_Value(rast, 1, 1, 1)
-	INTO r
-	FROM bar;
-	RAISE NOTICE 'record = %', r;
-
-	WITH foo AS (
-		SELECT
-			t1.rid,
-			ST_Union(t2.rast) AS rast
-		FROM raster_nmapalgebra_in t1
-		JOIN raster_nmapalgebra_in t2
-			ON ST_Intersects(t1.rast, t2.rast)
-			AND t2.rid BETWEEN 10 AND 18
-		WHERE t1.rid = 17
-		GROUP BY t1.rid
-	), bar AS (
-		SELECT
-			t1.rid,
-			ST_MapAlgebra(
-				ARRAY[ROW(t2.rast, 1)]::rastbandarg[],
-				'raster_nmapalgebra_test(double precision[], int[], text[])'::regprocedure,
-				'32BUI',
-				'CUSTOM', t1.rast,
-				1, 1,
-				'1000'
-			) AS rast
-		FROM raster_nmapalgebra_in t1
-		JOIN foo t2
-			ON t1.rid = t2.rid OFFSET 0
-	)
-	SELECT
-		rid,
-		(ST_Metadata(rast)),
-		(ST_BandMetadata(rast, 1)),
-		ST_Value(rast, 1, 1, 1)
-	INTO r
-	FROM bar;
-	RAISE NOTICE 'record = %', r;
-
-END IF;
-
 END $$;
 
 INSERT INTO raster_nmapalgebra_in

Modified: trunk/topology/postgis_topology.c
===================================================================
--- trunk/topology/postgis_topology.c	2019-06-09 11:31:34 UTC (rev 17479)
+++ trunk/topology/postgis_topology.c	2019-06-10 10:55:10 UTC (rev 17480)
@@ -221,6 +221,7 @@
   topo = palloc(sizeof(LWT_BE_TOPOLOGY));
   topo->be_data = (LWT_BE_DATA *)be; /* const cast.. */
   topo->name = pstrdup(name);
+  topo->hasZ = 0;
 
   dat = SPI_getbinval(SPI_tuptable->vals[0], SPI_tuptable->tupdesc, 1, &isnull);
   if ( isnull )
@@ -1366,11 +1367,11 @@
 			   double dist,
 			   uint64_t *numelems,
 			   int fields,
-			   int limit)
+			   int64_t limit)
 {
   LWT_ISO_EDGE *edges;
   int spi_result;
-  int elems_requested = limit;
+  int64_t elems_requested = limit;
   size_t hexewkb_size;
   char *hexewkb;
   MemoryContext oldcontext = CurrentMemoryContext;
@@ -1406,7 +1407,7 @@
   }
   else if ( elems_requested > 0 )
   {
-    appendStringInfo(sql, " LIMIT %d", elems_requested);
+	  appendStringInfo(sql, " LIMIT " INT64_FORMAT, elems_requested);
   }
   POSTGIS_DEBUGF(1, "cb_getEdgeWithinDistance2D: query is: %s", sql->data);
   spi_result = SPI_execute(sql->data, !topo->be_data->data_changed, limit >= 0 ? limit : 0);
@@ -1422,7 +1423,7 @@
 
   POSTGIS_DEBUGF(1,
 		 "cb_getEdgeWithinDistance2D: edge query "
-		 "(limited by %d) returned " UINT64_FORMAT " rows",
+		 "(limited by " INT64_FORMAT ") returned " UINT64_FORMAT " rows",
 		 elems_requested,
 		 SPI_processed);
   *numelems = SPI_processed;
@@ -1466,7 +1467,7 @@
 			   double dist,
 			   uint64_t *numelems,
 			   int fields,
-			   int limit)
+			   int64_t limit)
 {
   MemoryContext oldcontext = CurrentMemoryContext;
   LWT_ISO_NODE *nodes;
@@ -1475,7 +1476,7 @@
   char *hexewkb;
   StringInfoData sqldata;
   StringInfo sql = &sqldata;
-  int elems_requested = limit;
+  int64_t elems_requested = limit;
   uint64_t i;
 
   initStringInfo(sql);
@@ -1514,7 +1515,7 @@
   }
   else if ( elems_requested > 0 )
   {
-    appendStringInfo(sql, " LIMIT %d", elems_requested);
+	  appendStringInfo(sql, " LIMIT " INT64_FORMAT, elems_requested);
   }
   spi_result = SPI_execute(sql->data, !topo->be_data->data_changed, limit >= 0 ? limit : 0);
   MemoryContextSwitchTo( oldcontext ); /* switch back */
@@ -1530,7 +1531,7 @@
 
   POSTGIS_DEBUGF(1,
 		 "cb_getNodeWithinDistance2D: node query "
-		 "(limited by %d) returned " UINT64_FORMAT " rows",
+		 "(limited by " INT64_FORMAT ") returned " UINT64_FORMAT " rows",
 		 elems_requested,
 		 SPI_processed);
   if ( ! SPI_processed )



More information about the postgis-tickets mailing list