[GRASS-dev] [GRASS GIS] #2643: d.vect png with icon=basic/circle and long paths segfaults

GDAL · April 4, 2015, 12:02pm

#2643: d.vect png with icon=basic/circle and long paths segfaults
----------------------+-----------------------------------------------------
Reporter: pertusus | Owner: grass-dev@…
Type: defect | Status: new
Priority: major | Milestone: 7.0.1
Component: Display | Version: svn-releasebranch70
Keywords: | Platform: Linux
Cpu: x86-64 |
----------------------+-----------------------------------------------------
In some cases, d.vect segfaults. I can only reproduce a segfault when I
am in a directory with a long name (and the location name needs to be long
too, though I surmise that it is the same issue).

Here is the backtrace
{{{
gdb --args d.vect map=selection_demand icon=basic/circle
Reading symbols from /localdata/local/grass-7.0.0svn/bin/d.vect...done.
(gdb) run
Starting program: /localdata/local/grass-7.0.0svn/bin/d.vect
map=selection_demand icon=basic/circle
[Thread debugging using libthread_db enabled]

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff6cd2bef in store_xy (x=750.5, y=-nan(0x8000000000000))
at draw_line.c:22
22 png.grid[yi * png.width + xi] = png.current_color;

(gdb) bt
#0 0x00007ffff6cd2bef in store_xy (x=750.5, y=-nan(0x8000000000000)) at
draw_line.c:22
#1 0x00007ffff6cd2fe9 in draw_line (x1=<value optimized out>, y1=<value
optimized out>,
     x2=<value optimized out>, y2=<value optimized out>) at draw_line.c:60
#2 png_draw_line (x1=<value optimized out>, y1=<value optimized out>,
     x2=<value optimized out>, y2=<value optimized out>) at draw_line.c:73
#3 0x00007ffff68c8908 in path_stroke (p=0x7ffff6ed76a0,
     line=0x7ffff6cd2c00 <png_draw_line>) at path.c:106
#4 0x00007ffff7dfa42e in symbol (Symb=0x8359b0, x0=-1474999.7945148,
     y0=3456000.0026465799, fill_color=0x835920, line_color=0x835900,
     string_color=0x835900) at symbol.c:93
#5 0x0000000000406014 in draw_line (Map=0x7fffffffd1b0, type=87,
Clist=0x630400,
     color=0x7fffffffdcb0, fcolor=0x7fffffffdca0, chcat=0,
     symbol_name=0x6289d0 "basic/circle", size=5, sqrt_flag=0, id_flag=0,
     cats_color_flag=0, default_width=0, width_scale=1, zcolors=0x0,
cvarr_rgb=0x0,
     colors=0x0, cvarr_width=0x7fffffffafb0, nrec_width=0,
cvarr_size=0x7fffffffaf90,
     nrec_size=0, cvarr_rot=0x7fffffffaf70, nrec_rot=0) at lines.c:350
#6 display_lines (Map=0x7fffffffd1b0, type=87, Clist=0x630400,
color=0x7fffffffdcb0,
     fcolor=0x7fffffffdca0, chcat=0, symbol_name=0x6289d0 "basic/circle",
size=5,
     sqrt_flag=0, id_flag=0, cats_color_flag=0, default_width=0,
width_scale=1,
     zcolors=0x0, cvarr_rgb=0x0, colors=0x0, cvarr_width=0x7fffffffafb0,
nrec_width=0,
     cvarr_size=0x7fffffffaf90, nrec_size=0, cvarr_rot=0x7fffffffaf70,
nrec_rot=0)
     at lines.c:150
#7 0x0000000000408716 in display_shape (Map=0x7fffffffd1b0, type=87,
Clist=0x630400,
     window=0x7fffffffdba0, bcolor=0x7fffffffdcb0, fcolor=0x7fffffffdca0,
chcat=0,
     icon=0x6289d0 "basic/circle", size=5, size_column=0x0, sqrt_flag=0,
rot_column=0x0,
     id_flag=0, cats_colors_flag=0, rgb_column=0x0, default_width=0,
width_column=0x0,
     width_scale=1, z_style=0x0) at shape.c:207
#8 0x00000000004073ed in main (argc=0, argv=0x0) at main.c:408
}}}

I attach a reproducer. You may want to change GRASS_VERSION in
reproduce.sh and code in grass_common_setup.sh.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2643>
GRASS GIS <http://grass.osgeo.org>

GDAL · April 6, 2015, 8:16am

#2643: d.vect png with icon=basic/circle and long paths segfaults
----------------------+-----------------------------------------------------
Reporter: pertusus | Owner: grass-dev@…
Type: defect | Status: new
Priority: major | Milestone: 7.0.1
Component: Display | Version: svn-releasebranch70
Keywords: | Platform: Linux
Cpu: x86-64 |
----------------------+-----------------------------------------------------

Comment(by neteler):

I have tried to reproduce it with your scripts but no issue here (Fedora
21, 64bit).

In your report there are "<value optimized out>", can you please recompile
after "make distclean" without compiler optimization in order to obtain a
better debugger report?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2643#comment:1>
GRASS GIS <http://grass.osgeo.org>

GDAL · April 6, 2015, 9:31am

#2643: d.vect png with icon=basic/circle and long paths segfaults
----------------------+-----------------------------------------------------
Reporter: pertusus | Owner: grass-dev@…
Type: defect | Status: new
Priority: major | Milestone: 7.0.1
Component: Display | Version: svn-releasebranch70
Keywords: | Platform: Linux
Cpu: x86-64 |
----------------------+-----------------------------------------------------

Comment(by pertusus):

Replying to [comment:1 neteler]:
> I have tried to reproduce it with your scripts but no issue here (Fedora
21, 64bit).

My platform is x86_64 CentOS release 6.6.

Beware that to reproduce, you have to be in a long enough path. For
example in
{{{
/home/dumas/tmp/AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA/grass_bug_d_vect_segfault
}}}

> In your report there are "<value optimized out>", can you please
recompile after "make distclean" without compiler optimization in order to
obtain a better debugger report?

Here it is (recompiled with -O0):

{{{
(gdb) bt
#0 0x00007ffff6c9b3eb in store_xy (x=750.5, y=-nan(0x8000000000000)) at
draw_line.c:22
#1 0x00007ffff6c9b61e in draw_line (x1=750.91808904861637,
y1=35.342845063313966, x2=750.91808904861637,
     y2=35.342845063313966) at draw_line.c:60
#2 0x00007ffff6c9b6c9 in png_draw_line (x1=750.91808904861637,
y1=35.342845063313966,
     x2=750.91808904861637, y2=35.342845063313966) at draw_line.c:73
#3 0x00007ffff689022d in path_stroke (p=0x7ffff6ea1260,
line=0x7ffff6c9b647 <png_draw_line>)
     at path.c:106
#4 0x00007ffff6c9b306 in PNG_Stroke () at draw.c:43
#5 0x00007ffff688e81f in COM_Stroke () at draw.c:38
#6 0x00007ffff7df72e6 in D_stroke () at draw2.c:326
#7 0x00007ffff7df9355 in symbol (Symb=0x8369b0, x0=-1474999.7945148,
y0=3456000.0026465799,
     fill_color=0x836920, line_color=0x836900, string_color=0x836900) at
symbol.c:93
#8 0x00007ffff7df94ff in D_symbol (Symb=0x8369b0, x0=-1474999.7945148,
y0=3456000.0026465799,
     line_color=0x836900, fill_color=0x836920) at symbol.c:157
#9 0x00000000004066cf in draw_line (type=87, ltype=1, line=1,
Points=0x836960, Cats=0x836990,
     color=0x7fffffffdb50, fcolor=0x7fffffffdb40, chcat=0,
symbol_name=0x6299d0 "basic/circle", size=5,
     sqrt_flag=0, id_flag=0, cats_color_flag=0, default_width=0,
width_scale=1, zcolors=0x0,
     cvarr_rgb=0x0, colors=0x0, cvarr_width=0x7fffffffd020, nrec_width=0,
cvarr_size=0x7fffffffd000,
     nrec_size=0, cvarr_rot=0x7fffffffcfe0, nrec_rot=0, Clist=0x631400,
Symb=0x8369b0,
     line_color=0x836900, fill_color=0x836920, primary_color=0x836940,
n_points=0x7fffffffcca0,
     n_lines=0x7fffffffcc9c, n_centroids=0x7fffffffcc98,
n_boundaries=0x7fffffffcc94,
     n_faces=0x7fffffffcc90) at lines.c:350
#10 0x0000000000405f1c in display_lines (Map=0x7fffffffd210, type=87,
Clist=0x631400,
     color=0x7fffffffdb50, fcolor=0x7fffffffdb40, chcat=0,
symbol_name=0x6299d0 "basic/circle", size=5,
     sqrt_flag=0, id_flag=0, cats_color_flag=0, default_width=0,
width_scale=1, zcolors=0x0,
     cvarr_rgb=0x0, colors=0x0, cvarr_width=0x7fffffffd020, nrec_width=0,
cvarr_size=0x7fffffffd000,
     nrec_size=0, cvarr_rot=0x7fffffffcfe0, nrec_rot=0) at lines.c:150
#11 0x00000000004091b5 in display_shape (Map=0x7fffffffd210, type=87,
Clist=0x631400,
     window=0x7fffffffd190, bcolor=0x7fffffffdb50, fcolor=0x7fffffffdb40,
chcat=0,
     icon=0x6299d0 "basic/circle", size=5, size_column=0x0, sqrt_flag=0,
rot_column=0x0, id_flag=0,
     cats_colors_flag=0, rgb_column=0x0, default_width=0, width_column=0x0,
width_scale=1, z_style=0x0)
     at shape.c:207
#12 0x0000000000407d90 in main (argc=3, argv=0x7fffffffddd8) at main.c:408
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2643#comment:2>
GRASS GIS <http://grass.osgeo.org>

GDAL · April 6, 2015, 11:36am

#2643: d.vect png with icon=basic/circle and long paths segfaults
----------------------+-----------------------------------------------------
Reporter: pertusus | Owner: grass-dev@…
Type: defect | Status: new
Priority: major | Milestone: 7.0.1
Component: Display | Version: svn-releasebranch70
Keywords: | Platform: Linux
Cpu: x86-64 |
----------------------+-----------------------------------------------------

Comment(by pertusus):

I poked around a bit in gdb, and my first impression is that the error is
already in draw_line, it is never checked if x1 could be equal to x2,
which means dx = 0, so at line 59, in
{{{
y = y1 + (x - x1) * dy / dx;

}}}
y is set to nan and the segfault is a consequence of that.

I have no idea whether x1 and X2 are supposed never to be equal or not.

There are other points processed before the one that is problematic.

It really puzzles me that I can only reproduce this when paths are long.
There does not seems to be any relationship with that issue. My feeling
is that the length of paths is unrelated to that issue, but for a
mysterious reason, rounding of x1 and x2 depends on the path length and
they are exactly equal only when the path is long but by complete chance.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2643#comment:3>
GRASS GIS <http://grass.osgeo.org>

GDAL · April 7, 2015, 10:24pm

#2643: d.vect png with icon=basic/circle and long paths segfaults
----------------------+-----------------------------------------------------
Reporter: pertusus | Owner: grass-dev@…
Type: defect | Status: new
Priority: major | Milestone: 7.0.1
Component: Display | Version: svn-releasebranch70
Keywords: | Platform: Linux
Cpu: x86-64 |
----------------------+-----------------------------------------------------

Comment(by glynn):

Replying to [comment:3 pertusus]:

> I poked around a bit in gdb, and my first impression is that the error
is already in draw_line, it is never checked if x1 could be equal to x2,
which means dx = 0, so at line 59, in
{{{
y = y1 + (x - x1) * dy / dx;

}}}
> y is set to nan and the segfault is a consequence of that.
>
> I have no idea whether x1 and X2 are supposed never to be equal or not.

Line 59 is within the "else" clause of the condition
{{{
if (fabs(y1 - y2) > fabs(x1 - x2)) {
}}}

The only way for that condition to be false when fabs(x1-x2) is zero (i.e.
x1==x2) is if fabs(y1-y2) is also zero (i.e. y1==y2).

IOW, this case only occurs when x1==x2 AND y1==y2, i.e. both endpoints are
identical.

It should suffice to add

{{{
if (x1 == x2 && y1 == y2)
return;
}}}

to the start of that function.

Although it's possible that if both differences are very close to zero
(e.g. so small as to be denormals), rounding error could result in the
dependent variable being garbage, that shouldn't actually cause a problem,
as either positive or negative infinity would fail the clip-rectangle test
in store_xy(). It's only for NaN where the issue arises.

In fact, it may also suffice to invert the logic of the clip-rectangle
test, i.e. change
{{{
     if (x < png.clip_left || x >= png.clip_rite || y < png.clip_top || y
>= png.clip_bot)
         return;
}}}
to
{{{
     if (!(x >= png.clip_left && x < png.clip_rite && y >= png.clip_top &&
y < png.clip_bot))
         return;
}}}

The rationale is that any comparison involving NaN is always false (except
for !=, which is always true, even for NaN!=NaN). So a NaN value should
fail an "inside" test as well as the current "outside" test.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2643#comment:4>
GRASS GIS <http://grass.osgeo.org>

GDAL · April 8, 2015, 8:18am

#2643: d.vect png with icon=basic/circle and long paths segfaults
----------------------+-----------------------------------------------------
Reporter: pertusus | Owner: grass-dev@…
Type: defect | Status: new
Priority: major | Milestone: 7.0.1
Component: Display | Version: svn-releasebranch70
Keywords: | Platform: Linux
Cpu: x86-64 |
----------------------+-----------------------------------------------------

Comment(by neteler):

Replying to [comment:4 glynn]:
> IOW, this case only occurs when x1==x2 AND y1==y2, i.e. both endpoints
are identical.
>
> It should suffice to add
>
> {{{
> if (x1 == x2 && y1 == y2)
> return;
> }}}
>
> to the start of that function.
>
> Although it's possible that if both differences are very close to zero

What about comparing the differences against GRASS_EPSILON?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2643#comment:5>
GRASS GIS <http://grass.osgeo.org>

GDAL · April 9, 2015, 7:06pm

#2643: d.vect png with icon=basic/circle and long paths segfaults
----------------------+-----------------------------------------------------
Reporter: pertusus | Owner: grass-dev@…
Type: defect | Status: new
Priority: major | Milestone: 7.0.1
Component: Display | Version: svn-releasebranch70
Keywords: | Platform: Linux
Cpu: x86-64 |
----------------------+-----------------------------------------------------

Comment(by glynn):

Fudge factors are best avoided. And probably unnecessary here; this issue
appears to be limited to the specific case where both endpoints are
exactly equal.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2643#comment:6>
GRASS GIS <http://grass.osgeo.org>