[GRASS5] r.texture segfault

Hi,

we found a bug in r.texture which I'm unable to resolve.
To reproduce it in Spearfish, run

   g.region rast=spot.image -p
   r.rescale spot.image to=0,255 out=spot.resc
   r.texture in=spot.resc -v pref=spot.image size=10 distance=5
   Segmentation fault

I found with the debugger following

(gdb) r in=spot.resc -v pref=spot.image size=10 distance=5
Starting program: /hardmnt/thuille0/ssi/software/cvsgrass61/dist.i686-pc-linux-gnu/bin/r.texture in=spot.resc -v pref=spot.image size=10 distance=5
Reading the raster map...done
   0%
Program received signal SIGSEGV, Segmentation fault.
0x08049574 in h_measure (grays=0x87798e0, rows=10, cols=10, t_m=12, t_d=5)
    at h_measure.c:124
124 while (tone[y] != grays[row][col + d])
(gdb) bt
#0 0x08049574 in h_measure (grays=0x87798e0, rows=10, cols=10, t_m=12, t_d=5)
    at h_measure.c:124
#1 0x0804d657 in main (argc=6, argv=0xbfe19214) at main.c:309

The 'ddd' tells me that tone[y] is unavailable. Probably y get's
too large and then the crash occurs.

How to fix this?

Thanks

Markus

we found a bug in r.texture which I'm unable to resolve.

To reproduce it in Spearfish, run

   g.region rast=spot.image -p
   r.rescale spot.image to=0,255 out=spot.resc
   r.texture in=spot.resc -v pref=spot.image size=10 distance=5
   Segmentation fault

I found with the debugger following

(gdb) r in=spot.resc -v pref=spot.image size=10 distance=5
Starting program: /hardmnt/thuille0/ssi/software/cvsgrass61/dist.i686-pc-linux-gnu/bin/r.texture in=spot.resc -v pref=spot.image size=10 distance=5
Reading the raster map...done
   0%
Program received signal SIGSEGV, Segmentation fault.
0x08049574 in h_measure (grays=0x87798e0, rows=10, cols=10, t_m=12, t_d=5)
    at h_measure.c:124
124 while (tone[y] != grays[row][col + d])
(gdb) bt
#0 0x08049574 in h_measure (grays=0x87798e0, rows=10, cols=10, t_m=12, t_d=5)
    at h_measure.c:124
#1 0x0804d657 in main (argc=6, argv=0xbfe19214) at main.c:309

The 'ddd' tells me that tone[y] is unavailable. Probably y get's
too large and then the crash occurs.

How to fix this?

a full back trace should show the value of y when it segfaults:

(gdb) bt full

r.texture/h_measure.c:

#define PGM_MAXMAXVAL 255
int tone[PGM_MAXMAXVAL];

That 255 should really be 256?

int tone[255];

allocates tone[0] -> tone[254], maybe it loops up the value of y trying
to match a grey value of 255 and then kerplunk?

Hamish

> we found a bug in r.texture which I'm unable to resolve.
>
> To reproduce it in Spearfish, run
>
> g.region rast=spot.image -p
> r.rescale spot.image to=0,255 out=spot.resc
> r.texture in=spot.resc -v pref=spot.image size=10 distance=5
> Segmentation fault
>
> I found with the debugger following
>
> (gdb) r in=spot.resc -v pref=spot.image size=10 distance=5
> Starting program:
> /hardmnt/thuille0/ssi/software/cvsgrass61/dist.i686-pc-linux-gnu/bi
> n/r.texture in=spot.resc -v pref=spot.image size=10 distance=5
> Reading the raster map...done
> 0%
> Program received signal SIGSEGV, Segmentation fault.
> 0x08049574 in h_measure (grays=0x87798e0, rows=10, cols=10, t_m=12,
> t_d=5)
> at h_measure.c:124
> 124 while (tone[y] != grays[row][col + d])

..

> The 'ddd' tells me that tone[y] is unavailable. Probably y get's
> too large and then the crash occurs.

adding on line 126
  if(y>255) printf("grays[row][col + d]=%d\n", grays[row][col + d]);

we see that

> at h_measure.c:124
> 124 while (tone[y] != grays[row][col + d])

is trying to match tone[y] to -2147483648.

Apparently the module was never updated for NULL support?

Run 'r.null null=0' on spot.image first & it works ok.

by the way, 'r.colors rule=bcyr' looks a lot better than the default
for the result file.

r.texture/h_measure.c:

#define PGM_MAXMAXVAL 255
int tone[PGM_MAXMAXVAL];

That 255 should really be 256?

int tone[255];

allocates tone[0] -> tone[254], maybe it loops up the value of y
trying to match a grey value of 255 and then kerplunk?

still true?

Hamish

On Thu, Jun 30, 2005 at 10:15:15PM +1200, Hamish wrote:

> > we found a bug in r.texture which I'm unable to resolve.
> >
> > To reproduce it in Spearfish, run
> >
> > g.region rast=spot.image -p
> > r.rescale spot.image to=0,255 out=spot.resc
> > r.texture in=spot.resc -v pref=spot.image size=10 distance=5
> > Segmentation fault
> >
> > I found with the debugger following
> >
> > (gdb) r in=spot.resc -v pref=spot.image size=10 distance=5
> > Starting program:
> > /hardmnt/thuille0/ssi/software/cvsgrass61/dist.i686-pc-linux-gnu/bi
> > n/r.texture in=spot.resc -v pref=spot.image size=10 distance=5
> > Reading the raster map...done
> > 0%
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x08049574 in h_measure (grays=0x87798e0, rows=10, cols=10, t_m=12,
> > t_d=5)
> > at h_measure.c:124
> > 124 while (tone[y] != grays[row][col + d])
..
> > The 'ddd' tells me that tone[y] is unavailable. Probably y get's
> > too large and then the crash occurs.

adding on line 126
  if(y>255) printf("grays[row][col + d]=%d\n", grays[row][col + d]);

we see that

> > at h_measure.c:124
> > 124 while (tone[y] != grays[row][col + d])

is trying to match tone[y] to -2147483648.

Right, I found the same.

Apparently the module was never updated for NULL support?

I think that G_get_raster_row() should do the job.
But probably the r.rescale isn't? Means that r.reclass shows
a problem as r.rescale does a system call?

Run 'r.null null=0' on spot.image first & it works ok.

by the way, 'r.colors rule=bcyr' looks a lot better than the default
for the result file.

Please apply the needed patch.

> r.texture/h_measure.c:
>
> #define PGM_MAXMAXVAL 255
> int tone[PGM_MAXMAXVAL];
>
>
> That 255 should really be 256?
>
> int tone[255];
>
> allocates tone[0] -> tone[254], maybe it loops up the value of y
> trying to match a grey value of 255 and then kerplunk?

still true?

Hamish

I'm not sure how to fix the NULL/-2147483648 problem.

Markus

Markus Neteler wrote:

> Apparently the module was never updated for NULL support?

I think that G_get_raster_row() should do the job.

Huh? If you don't want nulls, it should probably be using
G_get_map_row() instead.

I'm not sure how to fix the NULL/-2147483648 problem.

h_measure.c, line 83:

      if (grays[row][col] > PGM_MAXMAXVAL)
  G_fatal_error ("Too many categories (max: %i). Try to rescale or reclassify the map", PGM_MAXMAXVAL);

should probably check both cases, i.e.:

      if (grays[row][col] < 0 || grays[row][col] >= PGM_MAXMAXVAL)

If PGM_MAXMAXVAL is changed to 256, then it needs to be ">=". If it
is left at 255, then the array definition:

  int tone[PGM_MAXMAXVAL];

needs to be:

  int tone[PGM_MAXMAXVAL+1];

--
Glynn Clements <glynn@gclements.plus.com>

Hi,

I have applied below changes (kept PGM_MAXMAXVAL).

Now I get

r.texture in=spot.resc2 -v pref=spot.image size=10 distance=5
Reading the raster map...done
ERROR: Too many categories (found: -2147483648, max: 255). Try to rescale
       or reclassify the map

although using the slightly modified extended test:

On Thu, Jun 30, 2005 at 10:37:34PM +0100, Glynn Clements wrote:
...

should probably check both cases, i.e.:

      if (grays[row][col] < 0 || grays[row][col] >= PGM_MAXMAXVAL)

If PGM_MAXMAXVAL is changed to 256, then it needs to be ">=". If it
is left at 255, then the array definition:

  int tone[PGM_MAXMAXVAL];

needs to be:

  int tone[PGM_MAXMAXVAL+1];

In CVS, it is now:

   if (grays[row][col] < 0 || grays[row][col] > PGM_MAXMAXVAL)
      G_fatal_error ("Too many categories (found: %i, max: %i). Try to rescale or reclassify the map", grays[row][col], PGM_MAXMAXVAL);

Interesting (for me).

Markus

Markus Neteler wrote:

I have applied below changes (kept PGM_MAXMAXVAL).

Now I get

r.texture in=spot.resc2 -v pref=spot.image size=10 distance=5
Reading the raster map...done
ERROR: Too many categories (found: -2147483648, max: 255). Try to rescale
       or reclassify the map

although using the slightly modified extended test:

The test only validates that the data is usable. It doesn't attempt to
"fix" data which isn't usable.

For r.texture, "usable" means that all cells contain values between 0
and 255 inclusive. No nulls, no negative values, no values >255.

If your data isn't in that range, you need to modify your data.
r.texture can't reasonably do this itself; it can't decide how to
replace invalid cell values (e.g. for a null cell, zero is no more
appropriate as a replacement than any other value).

--
Glynn Clements <glynn@gclements.plus.com>

In CVS, it is now:

   if (grays[row][col] < 0 || grays[row][col] > PGM_MAXMAXVAL)
      G_fatal_error ("Too many categories (found: %i, max: %i). Try to
      rescale or reclassify the map", grays[row][col], PGM_MAXMAXVAL);

The check is good, but the -2147483648 problem is still that the module
doesn't understand NULLs? This stops the Segfault but the module still
doesn't work right. Or does the texture algorithm forbid NULLs?

r.univar reports that the output of r.rescale as having a finite number
of NULLs, so r.texture has the problem?

?
Hamish

Hamish wrote:

> if (grays[row][col] < 0 || grays[row][col] > PGM_MAXMAXVAL)
> G_fatal_error ("Too many categories (found: %i, max: %i). Try to
> rescale or reclassify the map", grays[row][col], PGM_MAXMAXVAL);

The check is good, but the -2147483648 problem is still that the module
doesn't understand NULLs?

Correct.

This stops the Segfault but the module still
doesn't work right. Or does the texture algorithm forbid NULLs?

Correct.

If your data has nulls, you have to get rid of them, e.g. using
r.fillnulls. Ditto for out-of-range values (<0 or >255).

--
Glynn Clements <glynn@gclements.plus.com>

On Thu, Jul 07, 2005 at 04:39:38PM +0100, Glynn Clements wrote:

Hamish wrote:

> > if (grays[row][col] < 0 || grays[row][col] > PGM_MAXMAXVAL)
> > G_fatal_error ("Too many categories (found: %i, max: %i). Try to
> > rescale or reclassify the map", grays[row][col], PGM_MAXMAXVAL);
>
>
> The check is good, but the -2147483648 problem is still that the module
> doesn't understand NULLs?

Correct.

> This stops the Segfault but the module still
> doesn't work right. Or does the texture algorithm forbid NULLs?

Correct.

If your data has nulls, you have to get rid of them, e.g. using
r.fillnulls. Ditto for out-of-range values (<0 or >255).

Excuse me, I don't understand your comment. Most of the raster
modules do handle NULL data cells. Can't we propagate also
in r.texture a NULL hole to the output map? It's clear that we
cannot calculate texture on NULL data, but we could write out
NULL at that position and move on for the rest of the map.

?

BTW, why is -2147483648 < 0 not true in the if condition?

Markus

Markus Neteler wrote:

> > > if (grays[row][col] < 0 || grays[row][col] > PGM_MAXMAXVAL)
> > > G_fatal_error ("Too many categories (found: %i, max: %i). Try to
> > > rescale or reclassify the map", grays[row][col], PGM_MAXMAXVAL);
> >
> >
> > The check is good, but the -2147483648 problem is still that the module
> > doesn't understand NULLs?
>
> Correct.
>
> > This stops the Segfault but the module still
> > doesn't work right. Or does the texture algorithm forbid NULLs?
>
> Correct.
>
> If your data has nulls, you have to get rid of them, e.g. using
> r.fillnulls. Ditto for out-of-range values (<0 or >255).

Excuse me, I don't understand your comment. Most of the raster
modules do handle NULL data cells. Can't we propagate also
in r.texture a NULL hole to the output map? It's clear that we
cannot calculate texture on NULL data, but we could write out
NULL at that position and move on for the rest of the map.

Yes, however:

1. r.texture doesn't do this at present; someone needs to add code to
do it.

2. r.texture uses a neighbourhood window, so each null cell would
result in a patch of nulls the same size as the window.

?

BTW, why is -2147483648 < 0 not true in the if condition?

From your previous message:

ERROR: Too many categories (found: -2147483648, max: 255). Try to rescale

it appears that it is true (the error is printed if the condition is
true).

--
Glynn Clements <glynn@gclements.plus.com>

On Sat, Jul 09, 2005 at 10:00:51PM +0100, Glynn Clements wrote:

Markus Neteler wrote:

> > > > if (grays[row][col] < 0 || grays[row][col] > PGM_MAXMAXVAL)
> > > > G_fatal_error ("Too many categories (found: %i, max: %i). Try to
> > > > rescale or reclassify the map", grays[row][col], PGM_MAXMAXVAL);
> > >
> > >
> > > The check is good, but the -2147483648 problem is still that the module
> > > doesn't understand NULLs?
> >
> > Correct.
> >
> > > This stops the Segfault but the module still
> > > doesn't work right. Or does the texture algorithm forbid NULLs?
> >
> > Correct.
> >
> > If your data has nulls, you have to get rid of them, e.g. using
> > r.fillnulls. Ditto for out-of-range values (<0 or >255).
>
> Excuse me, I don't understand your comment. Most of the raster
> modules do handle NULL data cells. Can't we propagate also
> in r.texture a NULL hole to the output map? It's clear that we
> cannot calculate texture on NULL data, but we could write out
> NULL at that position and move on for the rest of the map.

Yes, however:

1. r.texture doesn't do this at present; someone needs to add code to
do it.

2. r.texture uses a neighbourhood window, so each null cell would
result in a patch of nulls the same size as the window.

> ?
>
> BTW, why is -2147483648 < 0 not true in the if condition?

From your previous message:

> ERROR: Too many categories (found: -2147483648, max: 255). Try to rescale

it appears that it is true (the error is printed if the condition is
true).

Ok. For now I have split the error message for the > 255 and the < 0 cases.

Markus