Skip to content

r.clump: Fail with the clump limit instead of overflowing the index - #7853

Merged
echoix merged 2 commits into
OSGeo:mainfrom
Pranav-error:fix-rclump-label-overflow
Sep 23, 2026
Merged

echoix merged 2 commits into
OSGeo:mainfrom
Pranav-error:fix-rclump-label-overflow

Conversation

@Pranav-error

@Pranav-error Pranav-error commented Aug 26, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #6412.

r.clump stopped with

ERROR: G_realloc: unable to allocate 18446744065119617024 bytes of memory at raster/r.clump/clump.c:633

which reads as a memory problem. It is not one, and the reporter went looking for a disk-based mode because of it.

Where the number comes from

label is a CELL and nalloc an int, both 32 bit:

label++;
cur_clump[col] = label;
if (label >= nalloc) {
    nalloc += INCR;                                    /* INCR is 1024 */
    index = (CELL *)G_realloc(index, nalloc * sizeof(CELL));
}

nalloc starts at INCR and only ever grows by INCR, so it is always a multiple of 1024 and lands exactly on 2³¹, which as a signed 32 bit value is INT_MIN. nalloc * sizeof(CELL) then converts INT_MIN * 4 to size_t:

(2**64) - 2147483648 * 4 = 18446744065119617024

That is the number in the report, so this is the path taken rather than a guess. label itself overflows one step earlier, which is undefined behaviour and would give a negative clump ID.

The fix

@metzm said on the issue that the real problem is the clump count, and that the limit "can not be raised" — clump IDs are the raster values, so the ceiling is one CELL. So this reports the limit instead of overflowing past it, at both places that start a new clump.

MAX_LABEL stops INCR short of INT_MAX rather than at it, so nalloc — which moves in INCR steps and must exceed label — cannot overflow either. The cost is 1024 of 2.1 billion possible clumps.

The new message says what the limit is and what to do about it, since the previous one sent the reporter after the wrong problem.

Verification

I cannot add a regression test for this: reaching it needs a raster with more than 2³¹ clumps, so the reporter's 220000 × 240000 region is roughly the smallest case. Rather than assert it untested, the mechanism above is arithmetic that anyone can check against the reported number.

What I did check: builds clean, clang-format reports no changes, and the four existing tests in raster/r.clump/tests/test_clump.py still pass.

r.clump asked G_realloc() for 18446744065119617024 bytes and stopped with
a memory error, which reads as a memory problem when it is not one.

label and nalloc are both 32 bit. nalloc starts at INCR and grows in INCR
steps, so it lands exactly on 2^31 and wraps to INT_MIN, and
nalloc * sizeof(CELL) then converts to (size_t)(INT_MIN * 4), which is
18446744065119617024 - the number in the report.

Clump IDs are raster values of type CELL, so as @metzm said on the issue
the limit of one CELL cannot be raised. Report it as the limit it is
instead, and stop INCR short of INT_MAX so nalloc cannot overflow either.

Fixes OSGeo#6412
@github-actions github-actions Bot added raster Related to raster data processing C Related code is in C module labels Aug 26, 2026
Comment thread raster/r.clump/clump.c Resolved
Comment thread raster/r.clump/clump.c Resolved

@metzm metzm left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A small suggested change to the error message, otherwise this is fine, thanks!

Per review: say "the maximum supported number of clumps". Re-wrapped the
string so the lines stay within 80 columns.
@Pranav-error

Copy link
Copy Markdown
Contributor Author

Thanks — applied your wording in both places. I re-wrapped the string so it stays within 80 columns, so the line breaks fall differently from the suggestion but the text is yours verbatim:

Too many clumps: the maximum supported number of clumps is %d. Reduce the
extent or the resolution of the computational region, or use r.reclass to
renumber the clumps.

This is ready on my side, but I cannot take it out of draft: new contributors here are limited to five open non-draft PRs and I am at that limit, so markPullRequestReadyForReview is refused. It will come out of draft as soon as one of my others is merged or closed — or feel free to undraft it yourself if you would rather not wait.

@metzm

metzm commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

I can not mark this PR as ready for review, sorry! I guess we have to wait for some of your other PRs to be merged or closed.

@metzm
metzm marked this pull request as ready for review September 23, 2026 17:00
@echoix
echoix merged commit 8229439 into OSGeo:main Sep 23, 2026
26 checks passed
@metzm

metzm commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

@echoix backport to 8.5? It's a bugfix.

@echoix

echoix commented Sep 23, 2026

Copy link
Copy Markdown
Member

Yes you can backport it if you wish.

@echoix echoix added the backport to 8.5 PR needs to be backported to release branch 8.5 label Sep 23, 2026
metzm pushed a commit that referenced this pull request Sep 23, 2026
…7853)

* r.clump: Fail with the clump limit instead of overflowing the index

r.clump asked G_realloc() for 18446744065119617024 bytes and stopped with
a memory error, which reads as a memory problem when it is not one.

label and nalloc are both 32 bit. nalloc starts at INCR and grows in INCR
steps, so it lands exactly on 2^31 and wraps to INT_MIN, and
nalloc * sizeof(CELL) then converts to (size_t)(INT_MIN * 4), which is
18446744065119617024 - the number in the report.

Clump IDs are raster values of type CELL, so as @metzm said on the issue
the limit of one CELL cannot be raised. Report it as the limit it is
instead, and stop INCR short of INT_MAX so nalloc cannot overflow either.

Fixes #6412

* r.clump: Reword the clump limit message

Per review: say "the maximum supported number of clumps". Re-wrapped the
string so the lines stay within 80 columns.
@metzm

metzm commented Sep 23, 2026

Copy link
Copy Markdown
Contributor

OK, backported to 8.5.

@metzm metzm removed the backport to 8.5 PR needs to be backported to release branch 8.5 label Sep 23, 2026
@github-actions github-actions Bot added this to the 8.6.0 milestone Sep 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

C Related code is in C module raster Related to raster data processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] r.clump G_realloc error

3 participants