Undefined behavior in OverlayMixedPoints.cpp

[noloader]

Hi Everyone,

I think OverlayMixedPoints.cpp may have a touch of undefined behavior. The Undefined Behavior Sanitizer (UBsan) is triggering on line OverlayMixedPoints.cpp:280.

Here are the steps I used to tickle the UB.

$ git clone https://salsa.debian.org/debian-gis-team/geos.git

---

$ mkdir build
$ cd build

$ CFLAGS="-fsanitize=undefined -fno-sanitize-recover" CXXFLAGS="-fsanitize=undefined -fno-sanitize-recover" cmake ../
$ make -j 8

---

$ make test
...

9% tests passed, 2 tests failed out of 466

Total Test time (real) =  19.48 sec

The following tests FAILED:
        103 - unit-capi-GEOSMakeValid (Failed)
        277 - unit-operation-valid-MakeValid (Failed)
Errors while running CTest

---

$ grep -IR 'runtime error:' ../
../build/Testing/Temporary/LastTest.log:capi::GEOSMakeValid: .../geos/src/operation/overlayng/OverlayMixedPoints.cpp:280:53: runtime error: reference binding to null pointer of type 'const struct PrecisionModel'
../build/Testing/Temporary/LastTest.log:geos::operation::valid::MakeValid: .../geos/src/operation/overlayng/OverlayMixedPoints.cpp:280:53: runtime error: reference binding to null pointer of type 'const struct PrecisionModel'

[noloader]

I think you can clear the issue with the following. The self tests pass, and there are no UBsan findings.

diff --git a/src/operation/overlayng/OverlayMixedPoints.cpp b/src/operation/overlayng/OverlayMixedPoints.cpp
index b0a20a91..100a7229 100644
--- a/src/operation/overlayng/OverlayMixedPoints.cpp
+++ b/src/operation/overlayng/OverlayMixedPoints.cpp
@@ -277,7 +277,7 @@ OverlayMixedPoints::extractCoordinates(const Geometry* points, const PrecisionMo
     auto coords = detail::make_unique<CoordinateSequence>(0u, points->hasZ(), points->hasM());
     coords->reserve(points->getNumPoints());
 
-    CoordinateExtractingFilter filter(*coords, *p_pm);
+    CoordinateExtractingFilter filter(*coords, p_pm ? *p_pm : PrecisionModel());
     points->apply_ro(&filter);
     return coords;
 }

[pramsey]

Does this one work as well?

diff --git a/src/operation/overlayng/OverlayMixedPoints.cpp b/src/operation/overlayng/OverlayMixedPoints.cpp
index b0a20a91a..1285186a2 100644
--- a/src/operation/overlayng/OverlayMixedPoints.cpp
+++ b/src/operation/overlayng/OverlayMixedPoints.cpp
@@ -76,7 +76,7 @@ private:
 /*public*/
 OverlayMixedPoints::OverlayMixedPoints(int p_opCode, const Geometry* geom0, const Geometry* geom1, const PrecisionModel* p_pm)
     : opCode(p_opCode)
-    , pm(p_pm)
+    , pm(p_pm ? p_pm : geom0->getPrecisionModel())
     , geometryFactory(geom0->getFactory())
     , resultDim(OverlayUtil::resultDimension(opCode, geom0->getDimension(), geom1->getDimension()))
 {

[noloader]

@pramsey,

Does this one work as well?

Yes, that tested Ok. No runtime errors, and all self tests passed.

[pramsey]

Wonderful! Thank you very much for the helpful report!