Opened 14 years ago
Closed 13 years ago
#367 closed defect (fixed)
Segmentation fault in distance() method.
Reported by: | yabo | Owned by: | strk |
---|---|---|---|
Priority: | blocker | Milestone: | 3.2.3 |
Component: | Core | Version: | main |
Severity: | Critical | Keywords: | |
Cc: |
Description
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.
Attachments (3)
Change History (16)
by , 14 years ago
comment:1 by , 13 years ago
I added the attached test case to the unit tests r3213 FYI, I can't reproduce the problem with Visual C++ 10.0
comment:2 by , 13 years ago
Milestone: | → 3.3.0 |
---|---|
Status: | new → assigned |
I added the actual ->distance call with r3287 and still can't reproduce the crash. yabo, can you ?
comment:3 by , 13 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
comment:4 by , 13 years ago
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)
comment:6 by , 13 years ago
Milestone: | 3.3.0 → 3.2.3 |
---|---|
Resolution: | fixed |
Status: | closed → reopened |
Let's keep a note for 3.2.3 then... Feel like providing a patch for that one ?
comment:9 by , 13 years ago
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 ?
comment:10 by , 13 years ago
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. :)
comment:11 by , 13 years ago
And the next line which is :
if ( geom[0]->isEmpty() || geom[1]->isEmpty() ) return 0.0;
comment:12 by , 13 years ago
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.
minimal test case.