Segmentation fault in distance() method.

[yabo]

I have a segmentation fault in Geometry::distance between these geometries (obviously the first one is the problem):

g1: 01060000000100000001030000000100000000000000
g2: 010100000000000000000000000000000000000000

g1 is a MULTIPOLYGON((EMPTY)) (this is what st_astext pretends).

The segmentation fault is produced here :

GeometryLocation (this=0x605bc0, newComponent=0x6048a0, newSegIndex=0, newPt=...) at GeometryLocation.cpp:37
pt = newPt;

This is because newPt is 0x0 (NULL). The derefencing of the NULL pointer occured here :

geos::operation::distance::ConnectedElementLocationFilter::filter_ro (this=0x7fffffffdcc0, geom=0x6048a0) at ConnectedElementLocationFilter.cpp:56
locations->push_back(new GeometryLocation(geom, 0, *(geom->getCoordinate())));

The faulty code is the *(geom->getCoordinate()). Indeed, g1 is loaded as an empty polygon. Polygon::getCoordinate() returns shell->getCoordinate() which in turns returns NULL if the shell is empty.

Note that this is not coherent with GeometryCollection::getCoordinate() which returns 'new Coordinate()' if empty.

Actually I find the GeometryCollection? solution quite ugly. More than that, it leaks memory :( (and there is a FIXME from strk by the way).

I don't know what the best fix could be. Return 0 in distance ? Throw an exception ?

This is all with geos-trunk and geos-3.2.1.

[yabo]

minimal test case.

[mloskot]

I added the attached test case to the unit tests r3213 FYI, I can't reproduce the problem with Visual C++ 10.0

[strk]

I added the actual ->distance call with r3287 and still can't reproduce the crash. yabo, can you ?

[yabo]

Sorry, didn't see this.

Unfortunately yes I can still reproduce the bug.

I just ran the test.cc with g++ 4.6 and geos 3.2.2 :

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7af7307 in geos::algorithm::CGAlgorithms::distancePointLine(geos::geom::Coordinate const&, geos::geom::Coordinate const&, geos::geom::Coordinate const&) () from /usr/lib/libgeos-3.2.2.so
(gdb) bt
#0  0x00007ffff7af7307 in geos::algorithm::CGAlgorithms::distancePointLine(geos::geom::Coordinate const&, geos::geom::Coordinate const&, geos::geom::Coordinate const&) () from /usr/lib/libgeos-3.2.2.so
#1  0x00007ffff7b6f3a9 in geos::operation::distance::DistanceOp::computeMinDistance(geos::geom::LineString const*, geos::geom::Point const*, std::vector<geos::operation::distance::GeometryLocation*, std::allocator<geos::operation::distance::GeometryLocation*> >&) () from /usr/lib/libgeos-3.2.2.so
#2  0x00007ffff7b6f5d6 in geos::operation::distance::DistanceOp::computeMinDistanceLinesPoints(std::vector<geos::geom::LineString const*, std::allocator<geos::geom::LineString const*> > const&, std::vector<geos::geom::Point const*, std::allocator<geos::geom::Point const*> > const&, std::vector<geos::operation::distance::GeometryLocation*, std::allocator<geos::operation::distance::GeometryLocation*> >&) () from /usr/lib/libgeos-3.2.2.so
#3  0x00007ffff7b6fe38 in geos::operation::distance::DistanceOp::computeFacetDistance() () from /usr/lib/libgeos-3.2.2.so
#4  0x00007ffff7b7028d in geos::operation::distance::DistanceOp::distance() () from /usr/lib/libgeos-3.2.2.so
#5  0x00007ffff7b703a0 in geos::operation::distance::DistanceOp::distance(geos::geom::Geometry const*, geos::geom::Geometry const*) () from /usr/lib/libgeos-3.2.2.so
#6  0x00007ffff7b0be09 in geos::geom::Geometry::distance(geos::geom::Geometry const*) const () from /usr/lib/libgeos-3.2.2.so
#7  0x00000000004012ab in main ()
(gdb) 

[yabo]

Oh, it doesn't SEGV with geos-svn r3275, so fixed I guess.

[strk]

Let's keep a note for 3.2.3 then... Feel like providing a patch for that one ?

[yabo]

If it can wait for next Wednesday I'll give it a try.

[strk]

Sure, no rush

[yabo]

I've worked out a patch, but what is the expected behaviour when computing the distance with an empty geometry ? Should it return 0 ? Throw an exception ? Something else ?

[yabo]

Oh I found this in trunk :

DistanceOp?.cpp:145

 if ( geom[0] == 0 || geom[1] == 0 )
  throw IllegalArgumentException("null geometries are not supported");

So I guess it answers the question. :)

[yabo]

And the next line which is :

 if ( geom[0]->isEmpty() || geom[1]->isEmpty() ) return 0.0;

[yabo]

So here are two patches. The first one is multiple compilation fix to make the 3.2.2 tag compile (missing includes, 'size_t' was used instead of 'std::size_t' and a fix for g++ 4.6 which now refuses non-initialised const). The second one fixes the segmentation fault by backporting the fix from trunk.

[yabo]

Compilation fix.

[yabo]

Segmentation fault fix.

[strk]

Fixed with r3445.