Opened 6 years ago
Closed 6 years ago
#4246 closed defect (fixed)
Undefined behaviour in define_plane
Reported by: | Algunenano | Owned by: | Algunenano |
---|---|---|---|
Priority: | high | Milestone: | PostGIS 3.0.0 |
Component: | postgis | Version: | master |
Keywords: | Cc: |
Description
Reproducible with regress test:
SELECT '3dDistancetest6', ST_3DDistance(a,b) FROM ( SELECT 'LINESTRING(1 1 1 , 2 2 2)'::geometry as a, 'POLYGON((0 0 0, 2 2 2, 3 3 3, 0 0 0))'::geometry as b) as foo;
Clang sanitizer:
#0 define_plane (pa=0x55f1780347c8, pl=<optimized out>) at measures3d.c:1146 1146 if((pa->npoints-1)==3) /*Triangle is special case*/ (gdb) bt #0 define_plane (pa=0x55f1780347c8, pl=<optimized out>) at measures3d.c:1146 #1 0x00007f8574ef911b in lw_dist3d_line_poly (line=<optimized out>, poly=<optimized out>, dl=0x7ffda6b15110) at measures3d.c:672 #2 0x00007f8574ef8d2d in lw_dist3d_distribute_bruteforce (lwg1=0x1, lwg2=<optimized out>, dl=<optimized out>) at measures3d.c:549 #3 0x00007f8574ef8454 in lw_dist3d_recursive (lwg1=0x55f178034700, lwg2=0x55f178034760, dl=0x7ffda6b15110) at measures3d.c:466 #4 0x00007f8574ef86e2 in lwgeom_mindistance3d_tolerance (lw1=0x55f178034700, lw2=0x55f178034760, tolerance=0) at measures3d.c:376 #5 lwgeom_mindistance3d (lw1=0x55f178034700, lw2=0x55f178034760) at measures3d.c:355 #6 0x00007f8574e97283 in LWGEOM_mindistance3d (fcinfo=0x55f1780329e0) at lwgeom_functions_basic.c:928 #7 0x000055f176357250 in ExecInterpExpr (state=<optimized out>, econtext=<optimized out>, isnull=0x7ffda6b152df) at execExprInterp.c:678 #8 0x000055f1764235ae in ExecEvalExprSwitchContext (state=<optimized out>, econtext=0x11, isNull=0xc40ba7bf2bc10000) at ../../../../src/include/executor/executor.h:303 #9 evaluate_expr (expr=<optimized out>, result_type=701, result_typmod=-1, result_collation=0) at clauses.c:4900
The debugger is pointing to the line 1146 but when stepping trough the code it's crashing around:
1183 sumx+=(v.x/vl);
So it's probably a division by zero.
Change History (11)
comment:1 by , 6 years ago
Owner: | changed from | to
---|
comment:2 by , 6 years ago
Thank you! good catch
I am guilty. It has lived there for quite a long time.
Do you fix it or do you want me to?
comment:4 by , 6 years ago
I've found several issues when investigating this peculiar case but the main one is that the polygon used (POLYGON((0 0 0, 2 2 2, 3 3 3, 0 0 0))
) isn't valid as it's just defining a 3d line.
@nicklas What do you think is the proper way to handle this? I'm thinking about lwerror("%s: Polygon does not define a plane", __func__);
when the polygon passed does not define a plane.
It's either that or be clever or default to line to line distance, but then we'd ignoring possible holes so I don't really like it.
I'm also adding a bunch of tests around the lwgeom_mindistance3d_tolerance
since it appears to have several issues with division by zero when the geometries interect.
comment:5 by , 6 years ago
I can take a deeper look later today. But from what I can remember is that I had problems handling cases when there is no plane. If the vertex points is creating a wave for instance.
But I don't remember how I thought I solved it.
If I just left it? or if I tried to catch something?
But I think for sure that it isn't bullet proof
comment:6 by , 6 years ago
Here is the PR with the fixes if you want to have a look https://github.com/postgis/postgis/pull/338
comment:7 by , 6 years ago
Hey, I'm impressed by myself
I actually documentated the thoughts about defining the plane when I wrote those functions.
I started to recall those thoughts, but here it is in the wiki.
Disabling optimizations gives a clearer view;
vl
is 0: