[GRASS-dev] [GRASS GIS] #3690: r.stats.quantile crashing with data linked through r.external

#3690: r.stats.quantile crashing with data linked through r.external
------------------------------------------+--------------------------------
Reporter: neteler | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.4.3
Component: Raster | Version: svn-
                                          | releasebranch74
Keywords: r.external, r.stats.quantile | CPU: Unspecified
Platform: Unspecified |
------------------------------------------+--------------------------------
We discovered that r.stats.quantile crashes with data linked through
r.external (when importing the same data sets with r.in.gdal it works
fine).

It seems that a corruption occurs with GDAL?

{{{
GRASS 7.4.3svn (utm30N): > r.external input="covermap.tif" band=1
output=tmp15409185587112 --overwrite -o
Reading band 1 of 1...
r.external komplett. Link to raster map <tmp15409185587112> created.

GRASS 7.4.3svn (utm30N): > r.external input="basemap.tif" band=1
output=tmp15409185587113 --overwrite -oÜ
Reading band 1 of 1...
r.external komplett. Link to raster map <tmp15409185587113> created.

GRASS 7.4.3svn (utm30N): > r.stats.quantile -p base="tmp15409185587112"
cover="tmp15409185587113" bins="2000"
Computing histograms
  100%
Speicherzugriffsfehler (Speicherabzug geschrieben)
}}}

... i.e. Core dump.

Debugging:
{{{
GRASS 7.4.3svn (utm30N): > gdb r.stats.quantile
GNU gdb (Ubuntu 7.11.1-0ubuntu1~16.5) 7.11.1
...
(gdb) r -p base="tmp15409185587112" cover="tmp15409185587113" bins="2000"
Starting program: /home/user/src/grass-7.4.svn/dist.x86_64-pc-linux-
gnu/bin/r.stats.quantile -p base="tmp15409185587112"
cover="tmp15409185587113" bins="2000"
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
Computing histograms
  100%
   27%
Program received signal SIGSEGV, Segmentation fault.
0x0000000000401fa0 in get_slot_counts (basefile=0, coverfile=1) at
main.c:161
161 bc->slots[i]++;

(gdb) bt
#0 0x0000000000401fa0 in get_slot_counts (basefile=0, coverfile=1) at
main.c:161
#1 0x0000000000403c89 in main (argc=5, argv=0x7fffffffd878) at main.c:642

(gdb) bt full
#0 0x0000000000401fa0 in get_slot_counts (basefile=0, coverfile=1) at
main.c:161
         basebuf = 0x6c5760
         coverbuf = 0x6c6eb0
         bc = 0x6f3ab0
         row = 467
         col = 1305
         i = 0
         allnull = 0
#1 0x0000000000403c89 in main (argc=5, argv=0x7fffffffd878) at main.c:642
         module = 0x7ffff7ba0ea8 <state+40>
         opt = {quant = 0x640ec0, perc = 0x641000, slots = 0x641140,
basemap = 0x7ffff7ba0f08 <state+136>, covermap = 0x640d80, output =
0x641280, file = 0x641440, fs = 0x641640}
         flag = {r = 0x7ffff7ba0ed8 <state+88>, p = 0x6426a0, t = 0x6426d0}
         basemap = 0x61edb0 "tmp15409185587112"
         covermap = 0x61ee80 "tmp15409185587113"
         outputs = 0x0
         fs = 0x7fffffffd72f "@"
         reclass = 0
         print = 1
         cover_fd = 1
         base_fd = 0
         range = {min = 1, max = 1061, first_time = 0}
         fprange = {min = 7.0767250061034996, max = 9.9984693527221999,
first_time = 0}
         i = 1
(gdb)
}}}

Looking at memory allocation and G_free() usage, I see that not all is
freed:

{{{
cd r.stats.quantile/
grep 'G_calloc\|G_free' main.c | grep bc
         bc->slots = G_calloc(bc->num_slots, sizeof(unsigned int));
         bc->bins = G_calloc(num_quants, sizeof(struct bin));
         bc->slot_bins = G_calloc(bc->num_slots, sizeof(unsigned char));
         G_free(bc->slots);
         bc->values = G_calloc(num_values, sizeof(DCELL));
         G_free(bc->slot_bins);

}}}

but I am not sure if that is related.

We can make the two GeoTIFF files offlist available.

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

#3690: r.stats.quantile crashing with data linked through r.external
--------------------------+------------------------------------------
  Reporter: neteler | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.3
Component: Raster | Version: svn-releasebranch74
Resolution: | Keywords: r.external, r.stats.quantile
       CPU: Unspecified | Platform: Unspecified
--------------------------+------------------------------------------

Comment (by mmetz):

Replying to [ticket:3690 neteler]:
> We discovered that r.stats.quantile crashes with data linked through
r.external (when importing the same data sets with r.in.gdal it works
fine).
>
> It seems that a corruption occurs with GDAL?
>
>
> {{{
> GRASS 7.4.3svn (utm30N): > r.external input="covermap.tif" band=1
output=tmp15409185587112 --overwrite -o
> Reading band 1 of 1...
> r.external komplett. Link to raster map <tmp15409185587112> created.
>
> GRASS 7.4.3svn (utm30N): > r.external input="basemap.tif" band=1
output=tmp15409185587113 --overwrite -oÜ
> Reading band 1 of 1...
> r.external komplett. Link to raster map <tmp15409185587113> created.
>
> GRASS 7.4.3svn (utm30N): > r.stats.quantile -p base="tmp15409185587112"
cover="tmp15409185587113" bins="2000"
> Computing histograms
> 100%
> Speicherzugriffsfehler (Speicherabzug geschrieben)
> }}}
>
> ... i.e. Core dump.

The above commands do not work for me because basemap and covermap are
swapped and the base map must be CELL type.

These commands

{{{
r.external input=covermap.tif band=1 output=covermap_ext
r.external input=basemap.tif band=1 output=basemap_ext
r.stats.quantile -p base=basemap_ext cover=covermap_ext bins=2000
}}}

work for me without segfault and valgrind does not find any memory
violations.

I am using GDAL 2.3.2.

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

#3690: r.stats.quantile crashing with data linked through r.external
--------------------------+------------------------------------------
  Reporter: neteler | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.3
Component: Raster | Version: svn-releasebranch74
Resolution: | Keywords: r.external, r.stats.quantile
       CPU: Unspecified | Platform: Unspecified
--------------------------+------------------------------------------

Comment (by neteler):

Replying to [comment:1 mmetz]:
> The above commands do not work for me because basemap and covermap are
swapped

ops, maybe I flipped that while anonymising the data using the output from
the QGIS Processing log file.

> and the base map must be CELL type.

Would it be possible to add a test for that?

> These commands
>
> {{{
> r.external input=covermap.tif band=1 output=covermap_ext
> r.external input=basemap.tif band=1 output=basemap_ext
> r.stats.quantile -p base=basemap_ext cover=covermap_ext bins=2000
> }}}
>
> work for me without segfault and valgrind does not find any memory
violations.
>
> I am using GDAL 2.3.2.

The machine causing problems is Linux Mint (Ubuntu). The versions I don't
have right now.

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

#3690: r.stats.quantile crashing with data linked through r.external
--------------------------+------------------------------------------
  Reporter: neteler | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.4.3
Component: Raster | Version: svn-releasebranch74
Resolution: | Keywords: r.external, r.stats.quantile
       CPU: Unspecified | Platform: Unspecified
--------------------------+------------------------------------------

Comment (by mmetz):

Replying to [comment:2 neteler]:
> Replying to [comment:1 mmetz]:
> > The above commands do not work for me because basemap and covermap are
swapped
>
> ops, maybe I flipped that while anonymising the data using the output
from the QGIS Processing log file.
>
> > and the base map must be CELL type.
>
> Would it be possible to add a test for that?

This test exists already:

{{{
ERROR: The base map must be an integer (CELL) map
}}}

> > I am using GDAL 2.3.2.
>
> The machine causing problems is Linux Mint (Ubuntu). The versions I
don't have right now.

Can you please test again on this machine, directly in GRASS, and ideally
with both GRASS 7.4 and 7.6?

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

#3690: r.stats.quantile crashing with data linked through r.external
--------------------------+------------------------------------------
  Reporter: neteler | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: normal | Milestone: 7.4.3
Component: Raster | Version: svn-releasebranch74
Resolution: fixed | Keywords: r.external, r.stats.quantile
       CPU: Unspecified | Platform: Unspecified
--------------------------+------------------------------------------
Changes (by mmetz):

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

Comment:

In [changeset:"73639" 73639]:
{{{
#!CommitTicketReference repository="" revision="73639"
r.external: do not rely on statistics stored in metadata, they can be
approximations (fixes #3690)
}}}

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

#3690: r.stats.quantile crashing with data linked through r.external
--------------------------+------------------------------------------
  Reporter: neteler | Owner: grass-dev@…
      Type: defect | Status: closed
  Priority: normal | Milestone: 7.4.3
Component: Raster | Version: svn-releasebranch74
Resolution: fixed | Keywords: r.external, r.stats.quantile
       CPU: Unspecified | Platform: Unspecified
--------------------------+------------------------------------------

Comment (by mmetz):

Replying to [comment:4 mmetz]:
> In [changeset:"73639" 73639]:
> {{{
> #!CommitTicketReference repository="" revision="73639"
> r.external: do not rely on statistics stored in metadata, they can be
approximations (fixes #3690)
> }}}

These approximate statistics stored in metadata are a nightmare! The
stored maximum value in basemap is too low:

{{{
gdalinfo -mm basemap.tif
   Min=1.000 Max=1061.000 Computed Min/Max=1.000,1063.000
}}}

However, metadata say

{{{
TIFFTAG_SOFTWARE=GRASS GIS 7.4.3svn with GDAL 2.2.2
}}}

in which case min and max should not be set at all. That means some other
software has edited the metadata in basemap.tif, inserting approximate and
thus wrong values.

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