Re: Better shared data structure management and resizable shared data structures

Hi Ashutosh,
Attached is a test-portability fix on top of v20260724-0006 to unblock
CFbot [1], plus the small diagnostic patch I used to root-cause it, in
case it's useful later.

v20260810-0001 = your v20260724-0006, unchanged, renumbered so
CFbot picks up the set as one series
v20260810-0002 = the proposed fix (test module only; no changes
outside src/test/modules/test_shmem/)
v20260810-0003 = diagnostic elog around the existing smaps walker
(please don't commit this one; it's here so the
same instrumentation can be reused later)

Also attached:
smaps-trace-64bit.txt full 64-bit trace, 12 pids
smaps-trace-32bit.txt per-pid header + first 8 include lines +
elided marker + stop/result; the full trace
is available by rerunning 0003

Method
------

To root-cause this rather than guess from the CFbot failure messages,
I forked off the CF tip (50911a16240 with v20260724-0006 applied),
added logging only to test_shmem_usage() (0003), pushed to trigger a
GitHub Actions run [2], and read the archived server logs. The green
run of 0001+0002 is at [3] (9 jobs pass: Linux 32, Linux 64, macOS,
Windows MinGW, both Windows VS slices, plus SanityCheck,
CompilerWarnings, Linux Autoconf).

Diff summary of 0002 vs your patch:

src/test/modules/test_shmem/t/002_resizable_shmem_struct.pl | 13 +++++-
src/test/modules/test_shmem/test_shmem.c | 50
++++++++--------
2 files changed, 35 insertions(+), 28 deletions(-)

Findings
--------

1. test_shmem_usage() smaps walker crosses the mapping boundary
---------------------------------------------------------------

The walker only checks address contiguity (start == prev_end) between
adjacent VMAs, so the first VMA of any kind that happens to abut the
target /dev/zero (deleted) segment is silently pulled into the RSS
sum. The comment on the loop mentions mprotect() splitting the
mapping, but the check itself is unaware of the split's identity.

On Linux 64-bit CI, for the "shrink to minimum, session 1" scenario
(one of the smallest overages, +139264 bytes over sum(allocated_size)):

include target VMA: 7f1b84b0c000-7f1b84de6000 rw-s ... 10315
/dev/zero (deleted) Rss=1116 kB, total=1116 kB
include contiguous VMA: 7f1b84de6000-7f1b9dde5000 ---s ... 10315
/dev/zero (deleted) Rss=0 kB, total=1116 kB
include contiguous VMA: 7f1b9dde5000-7f1b9de00000 rw-s ... 10315
/dev/zero (deleted) Rss=0 kB, total=1116 kB
include contiguous VMA: 7f1b9de00000-7f1b9e600000 rw-p 00000000 00:00 0
Rss=2048 kB, total=3164 kB <-- adjacent private anonymous VMA
stop before VMA: 7f1b9e6f7000-7f1b9f200000 rw-p 00000000 00:00 0
result: rss=3164 kB swap=0 kB shared_hugetlb=0 kB bytes=3239936

Reported shmem_usage is 3164 kB; real shared-segment RSS is 1116 kB.
The 2048 kB delta is one adjacent private anonymous VMA that happens
to sit right after the shared segment. Across the 12 failing
accounting scenarios on 64-bit, the same 2048 kB VMA at
7f1b9de00000-7f1b9e600000 is wrongly included every time, and the
walker's final "bytes=" matches the TAP-reported shmem_usage exactly
(full log in smaps-trace-64bit.txt).

On 32-bit the same bug is much worse. The walker crosses out of the
target /dev/zero segment, through one anonymous VMA, and then straight
into the writable segments of loaded libraries: libcap-ng, libresolv,
libc, libssl, libcrypto, libm, libicu*, libkrb5*, libldap, liburing,
libxml2, libzstd, libstdc++, libsystemd, libubsan, and several more,
before it finally stops at another SysV shared segment. Per-pid,
roughly 170 adjacent VMAs are wrongly included; overage is about 2 MB
per scenario. Header excerpt for one pid (target=0xd35d9700), full
per-pid summary in smaps-trace-32bit.txt:

include target VMA: d3325000-d99da000 rw-s 00000000 00:01 8414
/dev/zero (deleted)
include contiguous VMA: d99da000-ec5d9000 ---s 066b5000 00:01 8414
/dev/zero (deleted)
include contiguous VMA: ec5d9000-ec5f3000 rw-s 192b4000 00:01 8414
/dev/zero (deleted)
include contiguous VMA: ec5f3000-ec5f9000 rw-p 00000000 00:00 0
include contiguous VMA: ec5f9000-ec5fa000 r--p ...
/usr/lib/i386-linux-gnu/libcap-ng.so.0.0.0
...
... (168 more contiguous VMAs elided) ...
stop before VMA: f02dc000-f02dd000 rw-s ... /SYSV0088a83c (deleted)
result: rss=<garbage, see Finding #2> bytes=109752320

What 0002 proposes (test_shmem.c): also check permissions[3] (the 's'
vs 'p' character) when continuing across a split. This still handles
the case the loop comment cares about, one MAP_SHARED region split by
mprotect() into a chain of 'rw-s' / '---s' / 'rw-s' adjacent VMAs
backed by the same inode, while excluding adjacent private mappings.

One thing I'd like your opinion on: the 's' check by itself is not
airtight. If a *different* MAP_SHARED mapping (different inode /
pathname) happened to sit immediately contiguous to our target
segment, the walker would still include it. I haven't hit this on the
CI runners we tested, but on more crowded address spaces (a backend
that has attached to additional SysV segments, for example) it seems
plausible. One option would be to also match on the inode+pathname of
the target VMA (parse them from the same sscanf) and stop as soon as
they change. That's a bit more parsing, not much. Happy to add it to
0002 or leave it as a follow-up, whichever you'd prefer.

2. sscanf("%ld", int64 *) is a width mismatch on MSVC and 32-bit Linux
----------------------------------------------------------------------

The CF-base test_shmem_usage() has:

int64 val;
...
if (sscanf(line, "Rss: %ld kB", &val) == 1)
total_rss_kb += val;

sscanf("%ld", ...) writes a sizeof(long) value, but val is int64. On
platforms where long is 32 bits (MSVC on any target, 32-bit Linux),
sscanf writes only the low 32 bits and leaves the upper 32 bits of
val uninitialized. This is what makes the totals in the 32-bit trace
look like garbage: 0003 uses INT64_FORMAT and prints val correctly,
so what you see is the CF-base sscanf's half-populated int64.

The returned bytes= is still correct because total_rss_kb passes
through mul_size()/add_size() (unsigned Size, which is 32 bits on
32-bit Linux) and the upper garbage gets truncated. So the accounting
failure isn't hidden -- the delta that TAP compares is right, only
the diagnostic printout looks wrong.

0002 switches the four accounting variables (total_rss_kb,
total_swap_kb, total_shared_hugetlb_kb, val) from int64 to plain
'long', which matches %ld on every supported platform and is more
than wide enough for kB-per-VMA counts.

3. read-past-current-size test: compiler elides the read loop
-------------------------------------------------------------

The read side of resizable_shmem_access_beyond_size() had:

for (int i = resizable_shmem->num_entries; i < test_max_entries; i++)
sink = resizable_shmem->data[i];

At -O2 the compiler can legally elide most of that loop: 'sink' is a
plain static int32, resizable_shmem->data is a non-volatile pointer,
and only the last store to sink is externally observable. The CF-base
comment ("Return the last read value so that compiler doesn't optimize
away the assignment to sink") predicts the risk but doesn't prevent
it -- returning sink from the SQL function only pins the last
iteration.

When the read is elided, the backend never faults, so tests 45-47
fail like this (base + 0003 diagnostic run on Linux 64-bit):

not ok 45 - read past current size crashes the backend
not ok 46 - read crash reports lost connection
got: '' (empty stderr -- backend didn't crash)
not ok 47 - read succeeds after crash recovery
got: 'f' expected: 't' (no crash, so no recovery)

0002 fixes this by casting through a volatile pointer:

volatile int32 *volatile_data = resizable_shmem->data;
for (int i = resizable_shmem->num_entries; i < test_max_entries; i++)
sink = volatile_data[i];

Each volatile_data[i] load is now a required side-effect and can't be
elided, so the read actually faults.

4. Windows: skip 002_resizable_shmem_struct.pl
----------------------------------------------

Resizable shared memory isn't supported on Windows yet, and the test
relies on /proc/self/smaps, which is Linux-only. 0002 adds an early
`plan skip_all => 'resizable shared memory tests are not supported on
Windows'` so the two Windows CFbot jobs stop failing. If a future
revision brings Windows support, we can drop the skip.

[1] https://commitfest.postgresql.org/patch/6652/
[2] Diagnostic run (CF base + 0003 elog only, expected failure):
https://github.com/palak-chaturvedi/postgres/actions/runs/31178059423
[3] All-green run of 0001+0002 across 9 jobs:
https://github.com/palak-chaturvedi/postgres/actions/runs/31190837956

Thanks,
Palak