Memory access violation in LineString::getStartPoint()

[mloskot]

Today, when I was woring on Unit Tests I found a pretty serious bug causing memory access violation error. Here is the story:

// Factory object preparation
const int srid = 1;
geos::geom::PrecisionModel pm(1.0);
geos::geom::GeometryFactory factory(&pm, srid),

// Use factory to create empty LinearRing
LinearRingPtr ring = factory.createLinearRing();

// ring is empty, but let's see what will happen
PointPtr x = ring->getStartPoint(); <--- BUM!

The line marked with BUM! causes memory access violation error.

Here is descriptive backtrace with complete call chain (lines marked with <--- are executed during step-by-step debugging):

1.

PointPtr x = ring->getStartPoint(); <---

1.1.

Point* LineString::getStartPoint() const {
   if (isEmpty()) {
      return new Point(NULL, NULL); // <----
   }
   return getPointN(0);
}

2.

Point(NULL, NULL); <---

Parameters:

• newCoord = NULL
• factory = NULL

2.1.

Point::Point(CoordinateSequence *newCoords, const GeometryFactory *factory)
: Geometry(factory) <---
{
//...
}

3.

Geometry(factory)

factory = NULL

3.1.

Geometry::Geometry(const GeometryFactory* newFactory)
{
   factory=newFactory; <--- [1] 
   SRID=factory->getSRID(); <--- [2] BUUUUM!
   envelope=NULL;
   userData=NULL;
}

• (1) newFactory = NULL, so factory gets becomes null pointer

• (2) BUUUM! factory is a null pointer and calling member on a null pointer value causes undefined behaviour!

SRID=factory->getSRID(); 

4. Finally, GeometryFactory::getSRID() throws memory access violation error

Summary:

• the Geometry constructor is used incorrectly
• or this constructor is incorrectly designed to handle all valid cases
• This is another example of JAVISMS.

JTS version of getStartPoint is defined as follows:

public Point getStartPoint() {
   if (isEmpty()) {
      return null; // <--- [3]
   }
   return getPointN(0);
}

• (3) returning null reference is completely different semantic of returning pointer to newly allocated object, what happens in GEOS' version of getStartPoint!

GEOS' version of getStartPoint is buggy in two ways:

1. Causes memory access violation
2. There is logical bug: how can user expect to get Point object returned from *empty* LinearRing??? Such behaviour does not make sense. It's also, what is most important, incompatible behaviour with that in JTS version!

I'm quite confused and I'm really affraid of possible bugs hidding in GEOS. Please, discuss it in details and let's make this software stable!

[strk@…]

This should be fixed in CVS, tests welcome.

[(none)]

Milestone 3.0.0 deleted