From: Paul Ramsey Date: Wed, 1 Jul 2009 15:42:01 +0000 (+0000) Subject: Fix bad memory access in aggregates on nulls (#210), Mark Cave-Ayland. X-Git-Tag: 1.4.0rc1~3 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=ee9a3abf4b457b5258023ce64adec91df95483a5;p=postgis Fix bad memory access in aggregates on nulls (#210), Mark Cave-Ayland. git-svn-id: http://svn.osgeo.org/postgis/branches/1.4@4237 b70326c6-7e19-0410-871a-916f4a2858ee --- diff --git a/postgis/lwgeom_accum.c b/postgis/lwgeom_accum.c index cb17c7ad4..823ee1ef0 100644 --- a/postgis/lwgeom_accum.c +++ b/postgis/lwgeom_accum.c @@ -21,6 +21,7 @@ #include "lwgeom_pg.h" /* Local prototypes */ +Datum PGISDirectFunctionCall1(PGFunction func, Datum arg1); Datum pgis_geometry_accum_transfn(PG_FUNCTION_ARGS); Datum pgis_geometry_accum_finalfn(PG_FUNCTION_ARGS); Datum pgis_geometry_union_finalfn(PG_FUNCTION_ARGS); @@ -210,7 +211,9 @@ pgis_geometry_union_finalfn(PG_FUNCTION_ARGS) p = (pgis_abs*) PG_GETARG_POINTER(0); geometry_array = pgis_accum_finalfn(p, CurrentMemoryContext, fcinfo); - result = DirectFunctionCall1( pgis_union_geometry_array, geometry_array ); + result = PGISDirectFunctionCall1( pgis_union_geometry_array, geometry_array ); + if (!result) + PG_RETURN_NULL(); PG_RETURN_DATUM(result); } @@ -284,3 +287,26 @@ pgis_geometry_makeline_finalfn(PG_FUNCTION_ARGS) PG_RETURN_DATUM(result); } +/** +* A modified version of PostgreSQL's DirectFunctionCall1 which allows NULL results; this +* is required for aggregates that return NULL. +*/ +Datum +PGISDirectFunctionCall1(PGFunction func, Datum arg1) +{ + FunctionCallInfoData fcinfo; + Datum result; + + InitFunctionCallInfoData(fcinfo, NULL, 1, NULL, NULL); + + fcinfo.arg[0] = arg1; + fcinfo.argnull[0] = false; + + result = (*func) (&fcinfo); + + /* Check for null result, returning a "NULL" Datum if indicated */ + if (fcinfo.isnull) + return (Datum) 0; + + return result; +} diff --git a/postgis/lwgeom_geos.c b/postgis/lwgeom_geos.c index 4aa25fc45..1b9dcf1fb 100644 --- a/postgis/lwgeom_geos.c +++ b/postgis/lwgeom_geos.c @@ -107,10 +107,13 @@ Datum pgis_union_geometry_array(PG_FUNCTION_ARGS) GEOSGeometry * geos_result=NULL; int SRID=-1; size_t offset = 0; + bits8 *bitmap; + int bitmask; #if POSTGIS_DEBUG_LEVEL > 0 static int call=1; #endif #if POSTGIS_GEOS_VERSION >= 31 + int gotsrid = 0; int allpolys=1; #endif @@ -128,12 +131,21 @@ Datum pgis_union_geometry_array(PG_FUNCTION_ARGS) nelems = ArrayGetNItems(ARR_NDIM(array), ARR_DIMS(array)); + bitmap = ARR_NULLBITMAP(array); + POSTGIS_DEBUGF(3, "unite_garray: number of elements: %d", nelems); if ( nelems == 0 ) PG_RETURN_NULL(); /* One-element union is the element itself */ - if ( nelems == 1 ) PG_RETURN_POINTER((PG_LWGEOM *)(ARR_DATA_PTR(array))); + if ( nelems == 1 ) + { + /* If the element is a NULL then we need to handle it separately */ + if (bitmap && (*bitmap & 1) == 0) + PG_RETURN_NULL(); + else + PG_RETURN_POINTER((PG_LWGEOM *)(ARR_DATA_PTR(array))); + } /* Ok, we really need geos now ;) */ initGEOS(lwnotice, lwnotice); @@ -145,20 +157,43 @@ Datum pgis_union_geometry_array(PG_FUNCTION_ARGS) ** If they are, we can use UnionCascaded for faster results. */ offset = 0; + bitmask = 1; + gotsrid = 0; for ( i = 0; i < nelems; i++ ) { - PG_LWGEOM *pggeom = (PG_LWGEOM *)(ARR_DATA_PTR(array)+offset); - int pgtype = TYPE_GETTYPE(pggeom->type); - offset += INTALIGN(VARSIZE(pggeom)); - if ( ! i ) /* Initialize SRID */ + /* Don't do anything for NULL values */ + if ((bitmap && (*bitmap & bitmask) != 0) || !bitmap) { - SRID = pglwgeom_getSRID(pggeom); - if ( TYPE_HASZ(pggeom->type) ) is3d = 1; + PG_LWGEOM *pggeom = (PG_LWGEOM *)(ARR_DATA_PTR(array)+offset); + int pgtype = TYPE_GETTYPE(pggeom->type); + offset += INTALIGN(VARSIZE(pggeom)); + if ( ! gotsrid ) /* Initialize SRID */ + { + SRID = pglwgeom_getSRID(pggeom); + gotsrid = 1; + if ( TYPE_HASZ(pggeom->type) ) is3d = 1; + } + else + { + errorIfSRIDMismatch(SRID, pglwgeom_getSRID(pggeom)); + } + + if ( pgtype != POLYGONTYPE && pgtype != MULTIPOLYGONTYPE ) + { + allpolys = 0; + break; + } } - if ( pgtype != POLYGONTYPE && pgtype != MULTIPOLYGONTYPE ) - { - allpolys = 0; - break; + + /* Advance NULL bitmap */ + if (bitmap) + { + bitmask <<= 1; + if (bitmask == 0x100) + { + bitmap++; + bitmask = 1; + } } } @@ -176,59 +211,83 @@ Datum pgis_union_geometry_array(PG_FUNCTION_ARGS) ** First make an array of GEOS Polygons. */ offset = 0; + bitmap = ARR_NULLBITMAP(array); + bitmask = 1; for ( i = 0; i < nelems; i++ ) { - PG_LWGEOM *pggeom = (PG_LWGEOM *)(ARR_DATA_PTR(array)+offset); - int pgtype = TYPE_GETTYPE(pggeom->type); - offset += INTALIGN(VARSIZE(pggeom)); - if ( pgtype == POLYGONTYPE ) + /* Don't do anything for NULL values */ + if ((bitmap && (*bitmap & bitmask) != 0) || !bitmap) { - if ( curgeom == geoms_size ) + PG_LWGEOM *pggeom = (PG_LWGEOM *)(ARR_DATA_PTR(array)+offset); + int pgtype = TYPE_GETTYPE(pggeom->type); + offset += INTALIGN(VARSIZE(pggeom)); + if ( pgtype == POLYGONTYPE ) { - geoms_size *= 2; - geoms = repalloc( geoms, sizeof(GEOSGeom) * geoms_size ); - } - geoms[curgeom] = (GEOSGeometry *)POSTGIS2GEOS(pggeom); - curgeom++; - } - if ( pgtype == MULTIPOLYGONTYPE ) - { - int j = 0; - LWGEOM_INSPECTED *lwgeom = lwgeom_inspect(SERIALIZED_FORM(pggeom)); - for ( j = 0; j < lwgeom->ngeometries; j++ ) - { - LWPOLY *lwpoly = NULL; - int k = 0; if ( curgeom == geoms_size ) { geoms_size *= 2; geoms = repalloc( geoms, sizeof(GEOSGeom) * geoms_size ); } - /* This builds a LWPOLY on top of the serialized form */ - lwpoly = lwgeom_getpoly_inspected(lwgeom, j); - geoms[curgeom] = LWGEOM2GEOS(lwpoly_as_lwgeom(lwpoly)); - /* We delicately free the LWPOLY and POINTARRAY structs, leaving the serialized form below untouched. */ - for ( k = 0; k < lwpoly->nrings; k++ ) + geoms[curgeom] = (GEOSGeometry *)POSTGIS2GEOS(pggeom); + curgeom++; + } + if ( pgtype == MULTIPOLYGONTYPE ) + { + int j = 0; + LWGEOM_INSPECTED *lwgeom = lwgeom_inspect(SERIALIZED_FORM(pggeom)); + for ( j = 0; j < lwgeom->ngeometries; j++ ) { - lwfree(lwpoly->rings[k]); + LWPOLY *lwpoly = NULL; + int k = 0; + if ( curgeom == geoms_size ) + { + geoms_size *= 2; + geoms = repalloc( geoms, sizeof(GEOSGeom) * geoms_size ); + } + /* This builds a LWPOLY on top of the serialized form */ + lwpoly = lwgeom_getpoly_inspected(lwgeom, j); + geoms[curgeom] = LWGEOM2GEOS(lwpoly_as_lwgeom(lwpoly)); + /* We delicately free the LWPOLY and POINTARRAY structs, leaving the serialized form below untouched. */ + for ( k = 0; k < lwpoly->nrings; k++ ) + { + lwfree(lwpoly->rings[k]); + } + lwpoly_release(lwpoly); + curgeom++; } - lwpoly_release(lwpoly); - curgeom++; } } + /* Advance NULL bitmap */ + if (bitmap) + { + bitmask <<= 1; + if (bitmask == 0x100) + { + bitmap++; + bitmask = 1; + } + } } /* ** Take our GEOS Polygons and turn them into a GEOS MultiPolygon, ** then pass that into cascaded union. */ - g1 = GEOSGeom_createCollection(GEOS_MULTIPOLYGON, geoms, curgeom); - if ( g1 ) g2 = GEOSUnionCascaded(g1); - if ( g2 ) GEOSSetSRID(g2, SRID); - if ( g2 ) result = GEOS2POSTGIS(g2, is3d); - /* Clean up the mess. */ - if ( g1 ) GEOSGeom_destroy((GEOSGeometry *)g1); - if ( g2 ) GEOSGeom_destroy(g2); + if (curgeom > 0) + { + g1 = GEOSGeom_createCollection(GEOS_MULTIPOLYGON, geoms, curgeom); + if ( g1 ) g2 = GEOSUnionCascaded(g1); + if ( g2 ) GEOSSetSRID(g2, SRID); + if ( g2 ) result = GEOS2POSTGIS(g2, is3d); + /* Clean up the mess. */ + if ( g1 ) GEOSGeom_destroy((GEOSGeometry *)g1); + if ( g2 ) GEOSGeom_destroy(g2); + } + else + { + /* All we found were NULLs, so let's return NULL */ + PG_RETURN_NULL(); + } } else { @@ -237,51 +296,77 @@ Datum pgis_union_geometry_array(PG_FUNCTION_ARGS) ** Heterogeneous result, let's slog through this one union at a time. */ offset = 0; + bitmap = ARR_NULLBITMAP(array); + bitmask = 1; for (i=0; itype) ) is3d = 1; - - /* Check SRID homogeneity and initialize geos result */ - if ( ! i ) - { - geos_result = (GEOSGeometry *)POSTGIS2GEOS(geom); - SRID = pglwgeom_getSRID(geom); - POSTGIS_DEBUGF(3, "first geom is a %s", lwgeom_typename(TYPE_GETTYPE(geom->type))); - continue; - } - else + /* Don't do anything for NULL values */ + if ((bitmap && (*bitmap & bitmask) != 0) || !bitmap) { - errorIfSRIDMismatch(SRID, pglwgeom_getSRID(geom)); + PG_LWGEOM *geom = (PG_LWGEOM *)(ARR_DATA_PTR(array)+offset); + offset += INTALIGN(VARSIZE(geom)); + + pgis_geom = geom; + + POSTGIS_DEBUGF(3, "geom %d @ %p", i, geom); + + /* Check is3d flag */ + if ( TYPE_HASZ(geom->type) ) is3d = 1; + + /* Check SRID homogeneity and initialize geos result */ + if ( ! geos_result ) + { + geos_result = (GEOSGeometry *)POSTGIS2GEOS(geom); + SRID = pglwgeom_getSRID(geom); + POSTGIS_DEBUGF(3, "first geom is a %s", lwgeom_typename(TYPE_GETTYPE(geom->type))); + } + else + { + errorIfSRIDMismatch(SRID, pglwgeom_getSRID(geom)); + + g1 = POSTGIS2GEOS(pgis_geom); + + POSTGIS_DEBUGF(3, "unite_garray(%d): adding geom %d to union (%s)", + call, i, lwgeom_typename(TYPE_GETTYPE(geom->type))); + + g2 = GEOSUnion(g1, geos_result); + if ( g2 == NULL ) + { + GEOSGeom_destroy((GEOSGeometry *)g1); + GEOSGeom_destroy((GEOSGeometry *)geos_result); + elog(ERROR,"GEOS union() threw an error!"); + } + GEOSGeom_destroy((GEOSGeometry *)g1); + GEOSGeom_destroy((GEOSGeometry *)geos_result); + geos_result = g2; + } } - g1 = POSTGIS2GEOS(pgis_geom); - - POSTGIS_DEBUGF(3, "unite_garray(%d): adding geom %d to union (%s)", - call, i, lwgeom_typename(TYPE_GETTYPE(geom->type))); - - g2 = GEOSUnion(g1, geos_result); - if ( g2 == NULL ) + /* Advance NULL bitmap */ + if (bitmap) { - GEOSGeom_destroy((GEOSGeometry *)g1); - GEOSGeom_destroy((GEOSGeometry *)geos_result); - elog(ERROR,"GEOS union() threw an error!"); + bitmask <<= 1; + if (bitmask == 0x100) + { + bitmap++; + bitmask = 1; + } } - GEOSGeom_destroy((GEOSGeometry *)g1); - GEOSGeom_destroy((GEOSGeometry *)geos_result); - geos_result = g2; + } - GEOSSetSRID(geos_result, SRID); - result = GEOS2POSTGIS(geos_result, is3d); - GEOSGeom_destroy(geos_result); + /* If geos_result is set then we found at least one non-NULL geometry */ + if (geos_result) + { + GEOSSetSRID(geos_result, SRID); + result = GEOS2POSTGIS(geos_result, is3d); + GEOSGeom_destroy(geos_result); + } + else + { + /* All we found were NULLs, so let's return NULL */ + PG_RETURN_NULL(); + } #if POSTGIS_GEOS_VERSION >= 31 } @@ -289,8 +374,8 @@ Datum pgis_union_geometry_array(PG_FUNCTION_ARGS) if ( result == NULL ) { - elog(ERROR, "Union returned a NULL geometry."); - PG_RETURN_NULL(); /* never get here */ + /* Union returned a NULL geometry */ + PG_RETURN_NULL(); } PG_RETURN_POINTER(result);