[GRASS-dev] Addons: r.sun2 segfault

[Markus Neteler]

On Wed, Oct 22, 2008 at 4:31 PM, Glynn Clements
<glynn at gclements.plus.com> wrote:
>            for (row = m - offset - 1; row >= finalRow; row--) {
>
>                row_rev = m - row - 1;
>                rowrevoffset = row_rev - offset;
>
>                horizonpointer = horizonarray + arrayNumInt * n * rowrevoffset;
>
> The worst-case situation is when row == 0:
>
>        m = cellhd.rows = 19509
>        n = cellhd.cols = 23522
>        arrayNumInt = 360 / horizonStep = 360/15 = 24
>        row_rev = m - row - 1 = 19509 - 0 - 1 = 19508 (for row==0)
>        rowrevoffset = row_rev - offset = 19508 - 0 = 19508
>
>        arrayNumInt * n * rowrevoffset = 24 * 23522 * 19508 = 11,012,812,224
>
> arrayNumInt, n and rowrevoffset are all "int"s, which is presumably
> only 32 bits. After 2GiB, the calculation will wrap and produce large
> negative values.
>
> If this is the problem, the immediate segfault can be fixed by casting
> to a wider type, e.g.:
>
>                horizonpointer = horizonarray + (ssize_t) arrayNumInt * n * rowrevoffset;
>
> But the code could be littered with such issues (calculating offsets
> which won't fit into an "int").

The suggested change seems to help (running on a 16GB RAM box now):

top - 17:21:14 up 2 days,  3:00,  1 user,  load average: 1.00, 0.94, 0.60
Tasks: 166 total,   1 running, 165 sleeping,   0 stopped,   0 zombie
Cpu(s):  2.7%us,  0.0%sy,  0.0%ni, 87.2%id,  9.7%wa,  0.0%hi,  0.4%si,  0.0%st
Mem:  16439276k total, 16161516k used,   277760k free,   131824k buffers
Swap:        0k total,        0k used,        0k free,  2974596k cached

  PID USER      PR  NI  VIRT  RES  SHR S %CPU %MEM    TIME+  COMMAND
16788 neteler   18   0 17.1g  12g 2772 D   22 77.9   2:55.57 r.sun2
...


> Most of GRASS is immune to this issue by virtue of only holding a
> small number of rows in memory at any given time. But any module which
> stores an entire map (or, more generally, stores data proportional to
> rows*cols) is at risk of having similar issues with large maps.

Once finished (still at 0%), I'll report back to decide if we want to submit
this change to SVN.

Thanks so far,
Markus