On Wed, Dec 10, 2025 at 11:02 PM Chao Li <li.evan.chao@gmail.com> wrote:
>
> While reviewing Melanie's patch [1], I found this bug where presult is not initialized. Let me explain the logic.
Thanks for looking closely at the code.
> static int
> lazy_scan_prune(LVRelState *vacrel,
> presult, &prstate); <== immediately pass uninitialized presult to prune_freeze_setup
>
> Then in prune_freeze_setup():
>
> prstate->deadoffsets = (OffsetNumber *) presult->deadoffsets; <== presult->deadoffsets could be a random value
>
> Attached is a simple fix by just initializing presult in the first place with {0}.
I don't think zero-initializing deadoffsets is needed. We don't read
offsets from it in heap_page_prune_and_freeze() -- it's a result
variable. We only set offsets in it (see heap_prune_record_dead()).
And because we track exactly how many are initialized in
prstate->lpdead_items, the caller (lazy_scan_heap() via
lazy_scan_prune()) will only access those dead offsets which have been
initialized. I think this is a pretty common pattern in C. We don't
zero-initialize the other arrays of offsets in the PruneState
(redirected, dead, etc) and, for example, lazy_scan_noprune() doesn't
zero-initialize the deadoffsets array that it fills in.
The reason PruneFreezeResult is passed into prune_freeze_setup() is
that we save a pointer to the deadoffsets array in the PruneState
instead of having a copy of the whole array (to save stack space and
effort copying the array from PruneState into PruneFreezeResult at the
end).
Other than that, we wait to initialize PruneFreezeResult's members
until the end of heap_page_prune_and_freeze() to make it clear that we
are actually setting all the members. If we filled them out throughout
the various functions and helpers, it would be less clear that we have
filled in all the return values.
I could add a comment to prune_freeze_setup() where we save the
deadoffsets pointer that explains why we are doing that instead of
just having a deadoffsets array in the PruneState. Would that help
with the confusion?
- Melanie
