Skip to content

Fix OOB read in samplePerlinBeta17Terrain and add Beta 1.7 regression…#164

Open
lzray-universe wants to merge 1 commit into
Cubitect:masterfrom
lzray-universe:master
Open

Fix OOB read in samplePerlinBeta17Terrain and add Beta 1.7 regression…#164
lzray-universe wants to merge 1 commit into
Cubitect:masterfrom
lzray-universe:master

Conversation

@lzray-universe
Copy link
Copy Markdown

Summary

This PR fixes an out-of-bounds read in samplePerlinBeta17Terrain() and adds some regression tests for Beta 1.7 biome determinism.

The bug was caused by intermediate Perlin permutation indices being kept as int, but not wrapped back to 8 bits. So values like idx[i1] + i2 could go past the valid range of the 257-entry permutation table d[256 + 1].

This was a pretty annoying bug because it could look like a random determinism issue, especially with different stack layouts / build modes.

Root cause

PerlinNoise::d only has valid indices [0, 256], where d[256] mirrors d[0].

But in samplePerlinBeta17Terrain(), the intermediate indices were computed like this:

int a1 = idx[i1]   + i2;
int b1 = idx[i1+1] + i2;

int a2 = idx[a1]   + i3;
int a3 = idx[a1+1] + i3;
int b2 = idx[b1]   + i3;
int b3 = idx[b1+1] + i3;

Since idx[...], i1, i2, and i3 can all be in [0, 255], expressions such as idx[i1] + i2 may reach 510. Then later reads like idx[a1], idx[a1+1], idx[b1], and idx[b1+1] can go beyond the end of d.

Other Perlin sampling paths already use uint8_t intermediates, or otherwise get the same 8-bit wraparound behavior. So this issue seems specific to samplePerlinBeta17Terrain().

Fix

Mask the intermediate permutation indices with & 0xff, matching the intended 8-bit wrapping:

int a1 = (idx[i1]   + i2) & 0xff;
int b1 = (idx[i1+1] + i2) & 0xff;

int a2 = (idx[a1]   + i3) & 0xff;
int a3 = (idx[a1+1] + i3) & 0xff;
int b2 = (idx[b1]   + i3) & 0xff;
int b3 = (idx[b1+1] + i3) & 0xff;

So all accesses now stay inside [0, 256].

About the seed=262 instability

The previously observed Beta 1.7 instability around seed=262 was not really a seperate uninitialized-variable bug. It was a symptom of this out-of-bounds read.
I also checked the suspected locals:

  • climate[2] is filled by sampleBiomeNoiseBeta() before use.
  • cols[2] is filled by processColumnNoise() before use.
  • SeaLevelColumnNoiseBeta colNoise is initialized by genColumnNoise() before use.
  • SurfaceNoiseBeta snb is initialized by initSurfaceNoiseBeta() before use.

Value-initializing these locals can make the result look fixed, but that only changes the nearby stack memory which the invalid read was accidentally looking at. So it hides the real problem...

Some evidences

I used an instrumented version of samplePerlinBeta17Terrain() to check every idx[...] access.

Before the fix, the test triggered a lot of out-of-bounds reads. For example, with seed=0, 1076 out of 1200 sampled calls accessed indices outside the valid range, and the observed indices reached at least 408.

After the fix, the same instrumentation reports:

Calls: 1200
OOB events: 0

Tests

This also adds Beta 1.7 regression coverage:

  • testBeta17BiomeDeterminism()
    • Repeats the same seed=262 biome query multiple times at several scales.
    • Confirms the result is deterministic.
  • testBeta17BiomeRegression()
    • Computes a regression hash over multiple seeds and coordinates.
  • testBeta17BiomeValidity()
    • Checks 500 seeds over a coordinate grid.
    • Confirms all biome IDs stay in the valid 0..255 range.

Copilot AI review requested due to automatic review settings May 19, 2026 16:32
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an out-of-bounds permutation-table read in Beta 1.7 terrain Perlin sampling and adds Beta 1.7 biome checks intended to guard determinism and validity.

Changes:

  • Wraps intermediate Perlin permutation indices to 8 bits in samplePerlinBeta17Terrain().
  • Adds Beta 1.7 determinism, regression-hash, and biome-ID validity checks to tests.c.
  • Wires the new checks into the test executable’s main().

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
noise.c Masks intermediate Beta 1.7 Perlin indices to keep table accesses in bounds.
tests.c Adds Beta 1.7 biome determinism/regression/validity checks and invokes them from main().

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests.c
Comment on lines +557 to +561
printf(" B1.7 regression hash: %08x\n", hash);

static volatile uint32_t vhash;
vhash = hash;
return 0;
Comment thread tests.c
Comment on lines +641 to 648
printf("Testing Beta 1.7 biome determinism:\n");
testBeta17BiomeDeterminism();
printf("Testing Beta 1.7 biome regression:\n");
testBeta17BiomeRegression();
printf("Testing Beta 1.7 biome validity:\n");
testBeta17BiomeValidity();

return 0;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants