tidwall / tg

Geometry library for C - Fast point-in-polygon
MIT License
570 stars 15 forks source link

Segfault when running `tg_geom_geojson()` on the output of `tg_geom_new_geometrycollection()` #7

Closed asg017 closed 2 months ago

asg017 commented 2 months ago

Hey @tidwall ! First off thanks a ton for all your work on this library, I'm building a SQLite extension around it, and it's been an absolute blast!

I found a segfault when using the output of tg_geom_new_geometrycollection(), specifically when building a collection around polygons. Here's a minimal repro:

#include "g.h"
#include <stdio.h>

int main() {
  size_t size;
  struct tg_geom *g1 = tg_parse_wkt("POLYGON ((30 10, 40 40, 20 40, 10 20, 30 10))");
  struct tg_geom *g2 = tg_parse_wkt("POLYGON ((300 100, 400 400, 200 400, 100 200, 300 100))");
  struct tg_geom *collection = tg_geom_new_geometrycollection((const struct tg_geom*const[]) {g1, g2}, 2);
  printf("# geometries: %d\n", tg_geom_num_geometries(collection));

  size = tg_geom_geojson(g1, 0, 0);
  printf("GeoJSON size of g1: %zu\n", size);
  size = tg_geom_geojson(g2, 0, 0);
  printf("GeoJSON size of g2: %zu\n", size);
  size = tg_geom_geojson(collection, 0, 0);
  printf("GeoJSON size of collection: %zu\n", size);
}

Output:

# geometries: 2
GeoJSON size of g1: 76
GeoJSON size of g2: 86
Segmentation fault: 11

If I run with a sanitizer, I get this:

AddressSanitizer:DEADLYSIGNAL
=================================================================
==73562==ERROR: AddressSanitizer: SEGV on unknown address 0x000000010005 (pc 0x0001023bbc38 bp 0x00016da44e90 sp 0x00016da44e40 T0)
==73562==The signal is caused by a READ memory access.
    #0 0x1023bbc38 in tg_poly_exterior+0xb0 (a.out:arm64+0x100003c38)
    #1 0x102429440 in write_poly_points_geojson+0x40 (a.out:arm64+0x100071440)
    #2 0x1024210cc in write_geom_polygon_geojson+0x394 (a.out:arm64+0x1000690cc)
    #3 0x1023e5828 in write_geom_geojson+0xbec (a.out:arm64+0x10002d828)
    #4 0x102422428 in write_geom_geometrycollection_geojson+0x1ac (a.out:arm64+0x10006a428)
    #5 0x1023e58b8 in write_geom_geojson+0xc7c (a.out:arm64+0x10002d8b8)
    #6 0x1023e4ad0 in tg_geom_geojson+0x298 (a.out:arm64+0x10002cad0)
    #7 0x1023ba6ac in main+0x208 (a.out:arm64+0x1000026ac)
    #8 0x1a5ccbf24  (<unknown module>)
    #9 0xc071fffffffffffc  (<unknown module>)

==73562==Register values:
 x[0] = 0x0000000000010001   x[1] = 0x0000000000000000   x[2] = 0x0000000000000000   x[3] = 0x000000016da45c48  
 x[4] = 0x0000000000000000   x[5] = 0x000000016da46b30   x[6] = 0x4059000000000000   x[7] = 0x0000000000000001  
 x[8] = 0x0000000000010001   x[9] = 0x0000000000010005  x[10] = 0x00000001024db000  x[11] = 0xfc00000000000003  
x[12] = 0x0000000000001700  x[13] = 0x000000000000007f  x[14] = 0x0000000000000001  x[15] = 0x000000702db68a00  
x[16] = 0x00000001a6052370  x[17] = 0x000000016da45000  x[18] = 0x0000000000000000  x[19] = 0x000000016da45760  
x[20] = 0x0000000102508000  x[21] = 0x0000000102509910  x[22] = 0x000000016da46d80  x[23] = 0x00000001a5d463c6  
x[24] = 0x000000016da46d00  x[25] = 0x0000000000000001  x[26] = 0x0000000000000000  x[27] = 0x0000000000000000  
x[28] = 0x0000000000000000     fp = 0x000000016da44e90     lr = 0x0000000102429444     sp = 0x000000016da44e40  
AddressSanitizer can not provide additional info.
SUMMARY: AddressSanitizer: SEGV (a.out:arm64+0x100003c38) in tg_poly_exterior+0xb0
==73562==ABORTING
zsh: abort      ./a.out
sqlite-tg

Let me know if you need anything else to repro! Happy to provide compiler info/flags as needed. I also know that I may be using tg_geom_new_geometrycollection() incorrectly, the lifetime requirements are a bit tough to understand, so please let me know if I'm using it wrong.

tidwall commented 2 months ago

Hi Alex! I'm glad to hear you're having fun with the library. You are definitely using the library correctly. I was able to reproduce the bug on my side and I just pushed a fix.

As for lifetimes, the thing to remember is that any geometry created from _new_ or _parse_ should eventually be freed at some point using tg_geom_free. Geometries that are passed into a new or parse function is simply borrowed using a reference counter. Ownership of those input geometries is never taken by those functions.

asg017 commented 2 months ago

Thank you Josh, really appreciate the fast response! Can confirm that commit fixes my issue.