[GRASS-dev] [GRASS GIS] #3249: v.what.rast: segfault with map without topology

#3249: v.what.rast: segfault with map without topology
-------------------------+-------------------------
Reporter: sbl | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.2.1
Component: Default | Version: svn-trunk
Keywords: v.what.rast | CPU: Unspecified
Platform: Unspecified |
-------------------------+-------------------------
Recently the requirement for topology was removed from v.what.rast.

However, the module now segfaults if used with a vector map without
topology.

The issue can be reproduced with the workflow and data in #3248, but with

{{{
v.external -b
}}}

Related discussion here: https://lists.osgeo.org/pipermail/grass-
dev/2017-January/083830.html

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+--------------------------------------
  Reporter: sbl | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.2.1
Component: Default | Version: svn-trunk
Resolution: | Keywords: v.what.rast, v. external
       CPU: Unspecified | Platform: Unspecified
--------------------------+--------------------------------------
Changes (by martinl):

* keywords: v.what.rast => v.what.rast, v. external

Old description:

Recently the requirement for topology was removed from v.what.rast.

However, the module now segfaults if used with a vector map without
topology.

The issue can be reproduced with the workflow and data in #3248, but with

{{{
v.external -b
}}}

Related discussion here: https://lists.osgeo.org/pipermail/grass-
dev/2017-January/083830.html

New description:

Recently the requirement for topology was removed from v.what.rast.

However, the module now segfaults if used with a *linked* vector map
without topology.

The issue can be reproduced with the workflow and data in #3248, but with

{{{
v.external -b
}}}

Related discussion here: https://lists.osgeo.org/pipermail/grass-
dev/2017-January/083830.html

--

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:1&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+--------------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.2.1
Component: Default | Version: svn-trunk
Resolution: | Keywords: v.what.rast, v. external
       CPU: Unspecified | Platform: Unspecified
--------------------------+--------------------------------------
Changes (by martinl):

* status: new => assigned
* cc: grass-dev@… (added)
* owner: grass-dev@… => martinl

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:2&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+-----------------------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.2.1
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, v. external, PostGIS
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------------------
Changes (by martinl):

* keywords: v.what.rast, v. external => v.what.rast, v. external, PostGIS
* component: Default => Vector

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:3&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+-----------------------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.2.1
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, v. external, PostGIS
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------------------
Description changed by martinl:

Old description:

Recently the requirement for topology was removed from v.what.rast.

However, the module now segfaults if used with a *linked* vector map
without topology.

The issue can be reproduced with the workflow and data in #3248, but with

{{{
v.external -b
}}}

Related discussion here: https://lists.osgeo.org/pipermail/grass-
dev/2017-January/083830.html

New description:

Recently the requirement for topology was removed from v.what.rast.

However, the module now segfaults if used with a vector map without
topology.

The issue can be reproduced with the workflow and data in #3248, but with

{{{
v.external -b
}}}

Related discussion here: https://lists.osgeo.org/pipermail/grass-
dev/2017-January/083830.html

--

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:4&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+-----------------------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.2.1
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, v. external, PostGIS
       CPU: Unspecified | Platform: Unspecified
--------------------------+-----------------------------------------------

Comment (by martinl):

It's not issue only for linked maps, but also for native format

{{{
v.random out=pn n=1000 -b --o column=z
v.what.rast map=pn raster=rand column=morphology
Column <morphology> not found in the table <pn>. Creating...
Reading features from vector map...

Segmentation fault
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:5&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.2.1
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------
Changes (by martinl):

* keywords: v.what.rast, v. external, PostGIS => v.what.rast, level 1

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:6&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.2.1
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------

Comment (by martinl):

Seems to be a stack overflow somewhere since Map_info struct got ruined
before segfault:

{{{
gdb) p Map.dblnk->field[0]
$11 = {number = 6, name = 0x7600000344 <error: Cannot access memory at
address 0x7600000344>, driver = 0x60d770 "sqlite", database = 0x642100
"$GISDBASE/$LOCAT\005", table = 0x60d5f0 "pn18", key = 0x60d6b0 "cat"}
}}}

{{{
#0 _int_malloc (av=av@entry=0x7ffff6f86b00 <main_arena>,
bytes=bytes@entry=26) at malloc.c:3762
#1 0x00007ffff6c68d94 in __GI___libc_malloc (bytes=26) at malloc.c:2925
#2 0x00007ffff729e088 in G__malloc (file=0x7ffff72e166b
"lib/gis/location.c", line=82, n=26) at alloc.c:39
#3 0x00007ffff72aa7c1 in G__location_path () at location.c:82
#4 0x00007ffff72a4aaa in file_name (path=0x7fffffff9990 "\001", dir=0x0,
element=0x7fffffffae00 "vector/pn17", name=0x7ffff7bc2ed7 "dbln",
mapset=0x6095f0 "PERMANENT", base=0x0)
     at file_name.c:107
#5 0x00007ffff72a4963 in G_file_name (path=0x7fffffff9990 "\001",
element=0x7fffffffae00 "vector/pn17", name=0x7ffff7bc2ed7 "dbln",
mapset=0x6095f0 "PERMANENT") at file_name.c:41
#6 0x00007ffff72c0743 in G__open (element=0x7fffffffae00 "vector/pn17",
name=0x7ffff7bc2ed7 "dbln", mapset=0x6095f0 "PERMANENT", mode=1) at
open.c:109
#7 0x00007ffff72c0929 in G_fopen_new (element=0x7fffffffae00
"vector/pn17", name=0x7ffff7bc2ed7 "dbln") at open.c:224
#8 0x00007ffff7b7ddcc in Vect_write_dblinks (Map=0x7fffffffbf40) at
field.c:923
#9 0x00007ffff7b7e395 in Vect_set_db_updated (Map=0x7fffffffbf40) at
field.c:1023
#10 0x00000000004027bc in main (argc=4, argv=0x7fffffffd9e8) at main.c:251
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:7&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.4.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------
Changes (by martinl):

* milestone: 7.2.1 => 7.4.0

Comment:

Since changes in `v.what.rast` (r70331+r70332) haven't been ported to
relbr72, setting milestone to 7.4.0.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:8&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.4.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------

Comment (by marisn):

When ever there seems to be a problem with memory, use valgrind!

The reason of segfault is a failure to allocate memory on line 197. It
comes from call to Vect_get_num_primitives few lines earlier.
Vect_get_num_primitives uses Map.plus to get info about number of
features, but plus "Holds basic topology-related information about vector
map". No topology == no number of features in plus == allocation of 0
sized memory for cache == illegal writes further in code.

Solution:
* information about number of features must be obtained in some other way
(L194)
* probably also Vect_get_num_primitives should be modified to return -1 if
topology is missing

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:9&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.4.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------

Comment (by martinl):

In [changeset:"70366" 70366]:
{{{
#!CommitTicketReference repository="" revision="70366"
v.what.rast: require level 2 (see #3249)
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:10&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.4.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------

Comment (by martinl):

> * probably also Vect_get_num_primitives should be modified to return -1
if topology is missing

Or the routine could return number of primitives by reading all features,
this issue is related conceptually with #3244. For now I changed module
back to require level2 (r70366).

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:11&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.4.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------

Comment (by martinl):

Replying to [comment:11 martinl]:
> > * probably also Vect_get_num_primitives should be modified to return
-1 if topology is missing
>
> Or the routine could return number of primitives by reading all
features, this issue is related conceptually with #3244. For now I changed
module back to require level2 (r70366).

Anyway in this case it doesn't make sense since features would be read
twice by the module. Probably best way could be allocating cache
dynamically on level 1. I will try to provide patch in the next days.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:12&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.4.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------

Comment (by martinl):

Replying to [comment:12 martinl]:
> Anyway in this case it doesn't make sense since features would be read
twice by the module. Probably best way could be allocating cache
dynamically on level 1. I will try to provide patch in the next days.

See attachment:v_what_rast_level1.diff. If no objection I will commit into
trunk.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:13&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.4.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------
Changes (by martinl):

* Attachment "v_what_rast_level1.diff" added.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.4.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------

Comment (by martinl):

Replying to [comment:9 marisn]:
> The reason of segfault is a failure to allocate memory on line 197. It
comes from call to Vect_get_num_primitives few lines earlier.
Vect_get_num_primitives uses Map.plus to get info about number of
features, but plus "Holds basic topology-related information about vector
map". No topology == no number of features in plus == allocation of 0
sized memory for cache == illegal writes further in code.

Thanks for your time and investigation.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:14&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.4.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------

Comment (by martinl):

In [changeset:"70381" 70381]:
{{{
#!CommitTicketReference repository="" revision="70381"
v.what.rast: segfault with map without topology (see #3249)
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:15&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.4.0
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------

Comment (by martinl):

Support for level 1 implemented in trunk r70381. If no objection I will
backport to relbr72.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:16&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: assigned
  Priority: normal | Milestone: 7.2.1
Component: Vector | Version: svn-trunk
Resolution: | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------
Changes (by martinl):

* milestone: 7.4.0 => 7.2.1

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:17&gt;
GRASS GIS <https://grass.osgeo.org>

#3249: v.what.rast: segfault with map without topology
--------------------------+----------------------------------
  Reporter: sbl | Owner: martinl
      Type: defect | Status: closed
  Priority: normal | Milestone: 7.2.1
Component: Vector | Version: svn-trunk
Resolution: fixed | Keywords: v.what.rast, level 1
       CPU: Unspecified | Platform: Unspecified
--------------------------+----------------------------------
Changes (by martinl):

* status: assigned => closed
* resolution: => fixed

Comment:

In [changeset:"70425" 70425]:
{{{
#!CommitTicketReference repository="" revision="70425"
v.what.rast: segfault with map without topology (fix #3249)
              (merge r70331, r70332, r70366, r70381 from trunk)
}}}

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/3249#comment:18&gt;
GRASS GIS <https://grass.osgeo.org>