Correctness issue, particularly for Julia 1.12 - use after free bug (#39)

Co-authored-by: Anshul Singhvi <anshulsinghvi@gmail.com>

Author: mattwigway

Project.toml

@@ -8,10 +8,11 @@ GeoInterface = "cf35fbd7-0cd7-5166-be24-54bfbe79505f"
 LibSpatialIndex_jll = "00e98e2a-4326-5239-88cb-15dcbe1c18d0"
 
 [compat]
-Aqua = "0.7"
+Aqua = "0.8"
 GeoInterface = "1"
 LibSpatialIndex_jll = "1.9"
 julia = "1.6"
+Test = "1"
 
 [extras]
 Aqua = "4c88cf16-eb10-579e-8560-4a9242c79595"

src/LibSpatialIndex.jl

@@ -179,8 +179,13 @@ module LibSpatialIndex
             minvalues::Vector{Float64},
             maxvalues::Vector{Float64}
         )
-        C.Index_InsertData(rtree.index, Int64(id), pointer(minvalues),
-            pointer(maxvalues), UInt32(length(minvalues)), Ptr{UInt8}(0), Cint(0)
+        length(minvalues) == rtree.ndim || throw(DimensionMismatch("Minimum values must have same length as RTree dimensions"))
+        length(maxvalues) == rtree.ndim || throw(DimensionMismatch("Maximum values must have same length as RTree dimensions"))
+
+        # pass C_NULL as data - it should not be dereferenced, but making it null hopefully means if it is
+        # it will be easier to debug
+        C.Index_InsertData(rtree.index, Int64(id), minvalues,
+            maxvalues, UInt32(rtree.ndim), C_NULL, Cint(0)
         )
     end
     function insert!(rtree::RTree, id::Integer, extent::GI.Extent)
@@ -213,10 +218,13 @@ module LibSpatialIndex
             minvalues::Vector{Float64},
             maxvalues::Vector{Float64}
         )
+        length(minvalues) == rtree.ndim || throw(DimensionMismatch("Minimum values must have same length as RTree dimensions"))
+        length(maxvalues) == rtree.ndim || throw(DimensionMismatch("Maximum values must have same length as RTree dimensions"))
+        
         items = Ref{Ptr{Int64}}()
         nresults = Ref{UInt64}()
-        result = C.Index_Intersects_id(rtree.index, pointer(minvalues),
-            pointer(maxvalues), UInt32(length(minvalues)), items, nresults
+        result = C.Index_Intersects_id(rtree.index, minvalues,
+            maxvalues, UInt32(rtree.ndim), items, nresults
         )
         _checkresult(result, "Index_Intersects_id: Failed to evaluate")
         unsafe_wrap(Array, items[], nresults[])
@@ -260,10 +268,13 @@ module LibSpatialIndex
             maxvalues::Vector{Float64},
             k::Integer
         )
+        length(minvalues) == rtree.ndim || throw(DimensionMismatch("Minimum values must have same length as RTree dimensions"))
+        length(maxvalues) == rtree.ndim || throw(DimensionMismatch("Maximum values must have same length as RTree dimensions"))
+
         items = Ref{Ptr{Int64}}()
         nresults = Ref{UInt64}(k)
-        result = C.Index_NearestNeighbors_id(rtree.index, pointer(minvalues),
-            pointer(maxvalues), UInt32(length(minvalues)), items, nresults)
+        result = C.Index_NearestNeighbors_id(rtree.index, minvalues,
+            maxvalues, UInt32(rtree.ndim), items, nresults)
         _checkresult(result, "Index_NearestNeighbors_id: Failed to evaluate")
         unsafe_wrap(Array, items[], nresults[])
     end
@@ -300,5 +311,4 @@ module LibSpatialIndex
     end
 
     _not_point_or_ext_error() = throw(ArgumentError("object is not a point, and does not have an extent"))
-
-end # module
+end # module

test/runtests.jl

@@ -4,72 +4,96 @@ import GeoInterface as GI
 import LibSpatialIndex as SI
 import Aqua
 
-@testset "Simple Tutorial" begin
-    # based on https://github.com/libspatialindex/libspatialindex/wiki/Simple-Tutorial
-    println("Testing LibSpatialIndex v$(SI.version())")
-    props = SI.C.IndexProperty_Create()
-    SI.C.IndexProperty_SetIndexType(props, SI.C.RT_RTree)
-    SI.C.IndexProperty_SetIndexStorage(props, SI.C.RT_Memory)
-    idx = SI.C.Index_Create(props)
-    SI.C.IndexProperty_Destroy(props)
-    @test Bool(SI.C.Index_IsValid(idx))
+@testset "LibSpatialIndex" begin
+    @testset "Simple Tutorial" begin
+        # based on https://github.com/libspatialindex/libspatialindex/wiki/Simple-Tutorial
+        println("Testing LibSpatialIndex v$(SI.version())")
+        props = SI.C.IndexProperty_Create()
+        SI.C.IndexProperty_SetIndexType(props, SI.C.RT_RTree)
+        SI.C.IndexProperty_SetIndexStorage(props, SI.C.RT_Memory)
+        idx = SI.C.Index_Create(props)
+        SI.C.IndexProperty_Destroy(props)
+        @test Bool(SI.C.Index_IsValid(idx))
 
-    # load()
-    min = [0.5, 0.5]
-    max = [0.5, 0.5]
-    SI.C.Index_InsertData(idx, 1, min, max, 2, Ptr{UInt8}(C_NULL), 0)
+        # load()
+        min = [0.5, 0.5]
+        max = [0.5, 0.5]
+        SI.C.Index_InsertData(idx, 1, min, max, 2, Ptr{UInt8}(C_NULL), 0)
 
-    # query()
-    min = [0.0, 0.0]
-    max = [1.0, 1.0]
-    ndims = UInt32(2)
-    nresults = Ref{UInt64}()
-    SI.C.Index_Intersects_count(idx, min, max, ndims, nresults)
-    @test nresults[] == 1
+        # query()
+        min = [0.0, 0.0]
+        max = [1.0, 1.0]
+        ndims = UInt32(2)
+        nresults = Ref{UInt64}()
+        SI.C.Index_Intersects_count(idx, min, max, ndims, nresults)
+        @test nresults[] == 1
 
-    # bounds()
-    pmins = zeros(2)
-    pmaxs = zeros(2)
-    ndims = Ref{UInt32}()
-    SI.C.Index_GetBounds(idx, pointer_from_objref(pmins), pointer_from_objref(pmaxs), ndims);
-    @test ndims[] == 2
-    @test isapprox(pmins, [0.5, 0.5])
-    @test isapprox(pmaxs, [0.5, 0.5])
-end
+        # bounds()
+        pmins_p = Ref{Ptr{Float64}}()
+        pmaxs_p = Ref{Ptr{Float64}}()
+        ndims = Ref{UInt32}()
 
-@testset "Simple Operations" begin
-    rtree = SI.RTree(2)
-    result = SI.insert!(rtree, 1, [0.,0.], [1.,1.])
-    @test result == SI.C.RT_None
-    result = SI.insert!(rtree, 2, [0.,0.], [2.,2.])
-    @test result == SI.C.RT_None
-    @test SI.intersects(rtree, [0.,0.],[1.,1.]) == [1,2]
-    @test SI.intersects(rtree, [0.,0.]) == [1,2]
-    @test SI.intersects(rtree, [2.,2.]) == [2]
-    @test SI.intersects(rtree, [1.5,1.5],[2.,2.]) == [2]
-    @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 1)) == [2]
-    @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 2)) == [1,2]
-    @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 3)) == [1,2]
-    @test sort(SI.knn(rtree, [2.,2.], 1)) == [2]
-    @test sort(SI.knn(rtree, [2.,2.], 2)) == [1,2]
-end
+        SI.C.Index_GetBounds(idx, pmins_p, pmaxs_p, ndims);
 
-@testset "GeoInterface/Extents Operations" begin
-    rtree = SI.RTree(2)
-    result = SI.insert!(rtree, 1, GI.Extent(X=(0.0, 1.0), Y=(0.0, 1.0)))
-    @test result == SI.C.RT_None
+        @test ndims[] == 2
+        pmins = unsafe_wrap(Vector{Float64}, pmins_p[], ndims[], own=true)
+        pmaxs = unsafe_wrap(Vector{Float64}, pmaxs_p[], ndims[], own=true)
 
-    polygon = GI.Polygon([GI.LinearRing([(0.0, 0.0), (0.5, 0.0), (2.0, 0.5), (0.0, 2.0), (0.0, 0.0)])])
-    result = SI.insert!(rtree, 2, polygon)
+        @test isapprox(pmins, [0.5, 0.5])
+        @test isapprox(pmaxs, [0.5, 0.5])
+    end
 
-    @test result == SI.C.RT_None
-    @test SI.intersects(rtree, GI.LineString([(0.0, 0.0), (1.0, 1.0)])) == [1, 2]
-    @test SI.intersects(rtree, GI.Point(0.0, 0.0)) == [1, 2]
-    @test SI.intersects(rtree, (X=2.0, Y=2.0)) == [2]
-    @test sort(SI.knn(rtree, GI.Extent(X=(2.0, 2.0), Y=(2.0, 2.0)), 1)) == [2]
-    @test sort(SI.knn(rtree, (2.0, 2.0), 1)) == [2]
-end
+    @testset "Simple Operations" begin
+        rtree = SI.RTree(2)
+        result = SI.insert!(rtree, 1, [0.,0.], [1.,1.])
+        @test result == SI.C.RT_None
+        result = SI.insert!(rtree, 2, [0.,0.], [2.,2.])
+        @test result == SI.C.RT_None
+        @test SI.intersects(rtree, [0.,0.],[1.,1.]) == [1,2]
+        @test SI.intersects(rtree, [0.,0.]) == [1,2]
+        @test SI.intersects(rtree, [2.,2.]) == [2]
+        @test SI.intersects(rtree, [1.5,1.5],[2.,2.]) == [2]
+        @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 1)) == [2]
+        @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 2)) == [1,2]
+        @test sort(SI.knn(rtree, [2.,2.],[2.,2.], 3)) == [1,2]
+        @test sort(SI.knn(rtree, [2.,2.], 1)) == [2]
+        @test sort(SI.knn(rtree, [2.,2.], 2)) == [1,2]
+    end
+
+    @testset "GeoInterface/Extents Operations" begin
+        rtree = SI.RTree(2)
+        result = SI.insert!(rtree, 1, GI.Extent(X=(0.0, 1.0), Y=(0.0, 1.0)))
+        @test result == SI.C.RT_None
+
+        polygon = GI.Polygon([GI.LinearRing([(0.0, 0.0), (0.5, 0.0), (2.0, 0.5), (0.0, 2.0), (0.0, 0.0)])])
+        result = SI.insert!(rtree, 2, polygon)
+
+        @test result == SI.C.RT_None
+        @test SI.intersects(rtree, GI.LineString([(0.0, 0.0), (1.0, 1.0)])) == [1, 2]
+        @test SI.intersects(rtree, GI.Point(0.0, 0.0)) == [1, 2]
+        @test SI.intersects(rtree, (X=2.0, Y=2.0)) == [2]
+        @test sort(SI.knn(rtree, GI.Extent(X=(2.0, 2.0), Y=(2.0, 2.0)), 1)) == [2]
+        @test sort(SI.knn(rtree, (2.0, 2.0), 1)) == [2]
+    end
+
+    @testset "Bounds checks" begin
+        # confirm that bounds checks on the Julia side are working
+        tree = SI.RTree(2)
+        @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0], [0.0, 0.1])
+        @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0, 1.0], [0.1])
+        # should throw even if they're the same length if they don't match dimensions
+        @test_throws DimensionMismatch SI.insert!(tree, 0, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1])
+
+        @test_throws DimensionMismatch SI.intersects(tree, [0.0], [0.0, 0.1])
+        @test_throws DimensionMismatch SI.intersects(tree, [0.0, 1.0], [0.1])
+        @test_throws DimensionMismatch SI.intersects(tree, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1])
+
+        @test_throws DimensionMismatch SI.knn(tree, [0.0], [0.0, 0.1], 1)
+        @test_throws DimensionMismatch SI.knn(tree, [0.0, 1.0], [0.1], 1)
+        @test_throws DimensionMismatch SI.knn(tree, [0.0, 0.1, 1.1], [0.0, 0.1, 1.1], 1)
+    end
 
-@testset "Aqua" begin
-    Aqua.test_all(LibSpatialIndex)
+    @testset "Aqua" begin
+        Aqua.test_all(LibSpatialIndex)
+    end
 end