Re: GetBufferDescriptor() being called for local buffers from MarkBufferDirtyHint()

Hi,

On Mon, 29 Jun 2026 at 13:29, Ayush Tiwari <ayushtiwari(dot)slg01(at)gmail(dot)com>
wrote:

>
> Given the discussion here, I tried making the buffer descriptor accessors
> take
> a signed index and adding range assertions.
>
> Most callers already pass signed buffer indexes derived from Buffer values
> (e.g. buffer - 1, or -buffer - 1 for local buffers). The only caller I
> found
> that passes an unsigned value to GetBufferDescriptor() is
> ClockSweepTick(), and
> that normalizes the value before returning it, so it should still be less
> than
> NBuffers.
>
> The change looked fairly straightforward to me, unless I'm missing
> something.
> I also made the same change for GetLocalBufferDescriptor(), since it has
> the
> same uint32 signature and takes local buffer indexes.
>

cfbot flagged v1 in an assertion-enabled (cassert) build: anything that
uses a
temporary table hits the new assertion in GetLocalBufferDescriptor():

TRAP: failed Assert("id >= 0 && id < NLocBuffer"), buf_internals.h
GetLocalBufferDescriptor
InitLocalBuffers
ExtendBufferedRelLocal -> ... -> heap_insert

It looks like InitLocalBuffers() initializes the descriptors in a loop
before
it sets NLocBuffer, so during that loop the assert sees NLocBuffer == 0.
The
access itself seems fine, since the array is already allocated with
num_temp_buffers entries; it's just that this is the one place that runs
before the bound the assert checks against is set. (The shared path doesn't
run into this, since NBuffers is already set when BufferManagerShmemInit()
does the equivalent loop.)

I'm not sure which way would be preferable here if are adding assert for
LocalBufferDescriptor too:

1. set NLocBuffer = nbufs before the init loop, so the loop can keep using
GetLocalBufferDescriptor(); or
2. leave NLocBuffer where it is and index LocalBufferDescriptors[]
directly
in that loop.

Regards,
Ayush