On 7/8/26 02:31, Andreas Karlsson wrote:
> Hi!
>
> This looks useful so I took a stab at continuing this work plus adding
> NOACCESS sentinels to shared memory allocated in the main segment with
> the ShmemAllocator.
>
> For the ShmemAllocator I had originally planned to mark all memory as
> NOACCESS and then have ShmemAllocRaw() mark only the allocated regions
> as accessible but that did not work with legacy allocations (e.g.
> calling ShmemAlloc() directly) and EXEC_BAKCEND as there is no way to
> know where those regions were as they are not in ShmemIndex. I
> personally think the solution I went with instead, of jsut adding 32
> NOACESS bytes at the end of each allocation in shared memory is good
> enough even if it does not cover all padding bytes like my old solution
> used to do.
>
> What do you think? Is the current solution good enough? I preferred my
> original idea of starting out with everything NOACCESS but EXEC_BAKCEND
> made that painful to implement.
>
Hmmm, I agree starting with everything marked as NOACCESS seems like a
cleaner option, but if it doesn't work with EXEC_BACKEND (and if it
can't be made to work) it's irrelevant.
So what happens with EXEC_BACKEND builds? I see the 0002 patch now says:
* When compiled with EXEC_BACKEND we need to recreate all NOACCESS
* regions in each backend, but we can only recreate those with index
* entries and not any of the regions for memory allocated directly with
* ShmemAlloc().
Does it mean EXEC_BACKEND builds won't have any valgrind markings for
the regions allocated by ShmemAlloc()? Or do I misunderstand?
If that's the case, it seems EXEC_BACKEND builds would have limited
checks no matter what. Maybe it'd make sense to use the "clean" approach
for regular builds, and something simpler / limited for EXEC_BACKEND?
It's a bit late over here, so maybe I'm missing something obvious, but
it seems to me marking everything as NOACCESS (the "clean" approach)
would catch even accesses this EXEC_BACKEND-compatible approach would
not catch, right? That does not seem like a great trade off.
> For shm_toc I fixed the issues pointed out be Andres (the padding issue
> and that we can have shm_toc_lookup() set NOACCESS) and made sure we
> clear all NOACCESS sentinels on dsm_deatch(). The clearing of the
> NOACCESS in dsm_detach() felt a bit ugly but I am not sure there is any
> better solution.
>
I don't see Andres mentioning shm_toc_lookup explicitly, but I guess
it's what he meant when he mentioned NOACCESS and shm_toc_attach. Or did
I miss an email in this thread?
The dsm_detach business seems a bit strange to me. Can you explain why
we need to clean the NOACCESS sentinels in dsm_detach (I understand why
dsm_detach seems to be the only place to do that, but why do we need
to)? And is DEFINED the right marking? I'd probably briefly explain that
in the dsm_detach() comment.
I mean, we're about to detach the memory, right? So why should it be
accessible or defined?
But if we need to do this stuff in dsm_detach(), maybe we could do
something in dsm_attach() too?
One comments about shm_toc_lookup - the new block says:
/*
* Since we do not know the size of entries we can only use the
* start of the next entry for setting the no-access sentinel.
*/
Size nextoffset = i == 0 ? toc->toc_total_bytes : toc->toc_entry[i -
1].offset;
That seems hard to read, without the "(i == 0)". More importantly, is it
doing what the comment says? That talks about "next entry", but this
looks at the previous entry, no?
> I have also attached a third patch which contains my own extension which
> I used during development in case anyone would want to use it too. It
> adds three functions which can do out of bounds access to an array of 10
> 32-bit integers. For example the function call below triggers a Valgrind
> error when called with an index > 9.
>
> SELECT valgrind_shmem_shm_toc_fg_get(10);
>
> The extension is just a quick and dirty hack I used to test things out,
> and nothing I think should get committed as we do not have any test
> cases for Valgrind for anything else either.
>
Thanks. Seems useful for manual testing, but I agree it's not something
we'd want to commit.
> On 5/4/26 16:21, Tomas Vondra wrote:
>>> I assume the issue is just that the workers don't have the NOACCESS
>>> markers? I
>>> think you'd need to do them in every process using the shm_toc.
>>> Either by
>>> doing it in shm_toc_attach() or in shm_toc().
>
> I do not think you can do it in shm_toc_attach() because there can be
> new allocations between shm_toc_attach() and shm_toc_lookup() so my
> patch adds the NOACESS marker in shm_toc_lookup() which should be safe.
> So NOACCESS markers are added in both shm_toc_allocate() and
> shm_toc_lookup() in my patch.
>
I'm not following. Why would new allocations (between shm_toc_attach and
shm_toc_lookup) be an issue?
> Sadly it cannot take alignment correctly into account since we do not
> save that in the TOC (see below).
>
>> Right, but it'd also need to know how long the entry is. Which AFAIK it
>> does not. We don't want to redo the calculation in every worker, but we
>> might add a "len" field to the toc entry.
>
> Not without changing the API and unifying shm_toc_allocate() and
> shm_toc_insert() since by the time we create a TOC entry we have thrown
> away the size of the allocation. I would not mind changing this API but
> I think I would need to understand why it is like that before changing
> it. It does not feel good to change an API just for Valgrind.
>
> Any idea why these two functions are split and why they are not just one
> function call?
>
I suppose it's split into two functions because we don't want to add
segments that are not fully initialized. The usual use case looks like
this (this is from brin.c):
sharedquery = (char *) shm_toc_allocate(pcxt->toc, querylen + 1);
memcpy(sharedquery, debug_query_string, querylen + 1);
shm_toc_insert(pcxt->toc, PARALLEL_KEY_QUERY_TEXT, sharedquery);
If we added both attach+insert, we'd be adding empty string and only
then set it. I suppose that'd not be safe in some cases?
I don't think we'd want to merge these two functions into one, but I
suppose we could add the length to shm_toc_insert(). Which I suppose
simply wasn't needed until now.
But I agree it'd not be great to do this just because of valgrind.
There's a fair number of shm_toc_insert calls (~80), but not terrible.
But we'd need to be careful to protect against shm_toc_insert calls with
a different length value.
I'm not sure what to do. I was thinking we might add a shm_toc length
only in valgrind builds. And then we wouldn't need this NOACCESS_BYTES
stuff at all. Bu I didn't realize we'd need to tweak shm_toc_insert.
>>> Which means that you'll often have a up to 32byte pad at the end of the
>>> (ALIGNOF_BUFFER=32) allocation already. I don't care about the waste,
>>> but the
>>> ALIGNOF_BUFFER padding will often prevent detecting smaller out-of-
>>> bounds
>>> accesses.
>>
>> Good point. I forgot about this alignment rounding. I don't care about
>> the waste either (it'd be a bit silly in a valgrind build anyway), but
>> not catching smaller mistakes would not be great.
>
> My version fixes this, but is not able to fix this in shm_toc_lookup()
> as I mentioned above due to the lack of a length field for the
> allocation in the TOC.
>
Yeah.
There was a minor bitrot due to b34fd845e03a, so here's v3.
regards
--
Tomas Vondra
From 2226c23bfe1c7a3553bb34b1ce953130101e332a Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email protected]>
Date: Sat, 4 Jul 2026 13:17:15 +0200
Subject: [PATCH v3 3/3] Toy extension to make it easier to test out of bounds
access
I built this extension just for my personal use while developing so I
can trigger out of bounds array access by accessing an array index
outside of a 10 element array in shared memory.
It can be used to test both the ShmemAllocator and shm_toc, including
access from a background worker
---
contrib/meson.build | 1 +
contrib/valgrind_shmem/meson.build | 18 ++
.../valgrind_shmem/valgrind_shmem--1.0.sql | 16 ++
contrib/valgrind_shmem/valgrind_shmem.c | 227 ++++++++++++++++++
contrib/valgrind_shmem/valgrind_shmem.control | 5 +
5 files changed, 267 insertions(+)
create mode 100644 contrib/valgrind_shmem/meson.build
create mode 100644 contrib/valgrind_shmem/valgrind_shmem--1.0.sql
create mode 100644 contrib/valgrind_shmem/valgrind_shmem.c
create mode 100644 contrib/valgrind_shmem/valgrind_shmem.control
diff --git a/contrib/meson.build b/contrib/meson.build
index ebb7f83d8c5..3159c4084b0 100644
--- a/contrib/meson.build
+++ b/contrib/meson.build
@@ -72,4 +72,5 @@ subdir('tsm_system_time')
subdir('unaccent')
subdir('uuid-ossp')
subdir('vacuumlo')
+subdir('valgrind_shmem')
subdir('xml2')
diff --git a/contrib/valgrind_shmem/meson.build b/contrib/valgrind_shmem/meson.build
new file mode 100644
index 00000000000..5b6206ed0cb
--- /dev/null
+++ b/contrib/valgrind_shmem/meson.build
@@ -0,0 +1,18 @@
+# Copyright (c) 2022-2026, PostgreSQL Global Development Group
+
+valgrind_shmem_sources = files(
+ 'valgrind_shmem.c',
+)
+
+valgrind_shmem = shared_module('valgrind_shmem',
+ valgrind_shmem_sources,
+ c_pch: pch_postgres_h,
+ kwargs: contrib_mod_args,
+)
+contrib_targets += valgrind_shmem
+
+install_data(
+ 'valgrind_shmem--1.0.sql',
+ 'valgrind_shmem.control',
+ kwargs: contrib_data_args,
+)
diff --git a/contrib/valgrind_shmem/valgrind_shmem--1.0.sql b/contrib/valgrind_shmem/valgrind_shmem--1.0.sql
new file mode 100644
index 00000000000..234be9b1b07
--- /dev/null
+++ b/contrib/valgrind_shmem/valgrind_shmem--1.0.sql
@@ -0,0 +1,16 @@
+/* contrib/valgrind_shmem/valgrind_shmem--1.1.sql */
+
+-- complain if script is sourced in psql, rather than via CREATE EXTENSION
+\echo Use "CREATE EXTENSION valgrind_shmem" to load this file. \quit
+
+CREATE FUNCTION valgrind_shmem_main_get(int) RETURNS int
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE;
+
+CREATE FUNCTION valgrind_shmem_shm_toc_fg_get(int) RETURNS int
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE;
+
+CREATE FUNCTION valgrind_shmem_shm_toc_bg_get(int) RETURNS int
+AS 'MODULE_PATHNAME'
+LANGUAGE C STRICT IMMUTABLE PARALLEL SAFE;
diff --git a/contrib/valgrind_shmem/valgrind_shmem.c b/contrib/valgrind_shmem/valgrind_shmem.c
new file mode 100644
index 00000000000..62d783362bc
--- /dev/null
+++ b/contrib/valgrind_shmem/valgrind_shmem.c
@@ -0,0 +1,227 @@
+#include "postgres.h"
+
+#include "fmgr.h"
+#include "miscadmin.h"
+#include "postmaster/bgworker.h"
+#include "storage/dsm.h"
+#include "storage/ipc.h"
+#include "storage/proc.h"
+#include "storage/shm_toc.h"
+#include "storage/shmem.h"
+#include "utils/wait_event.h"
+
+PG_MODULE_MAGIC_EXT(
+ .name = "valgrind_shmem",
+ .version = PG_VERSION
+);
+
+#define PG_TEST_VALGRIND_SHMEM_MAGIC 0x74686f72
+
+PG_FUNCTION_INFO_V1(valgrind_shmem_main_get);
+PG_FUNCTION_INFO_V1(valgrind_shmem_shm_toc_fg_get);
+PG_FUNCTION_INFO_V1(valgrind_shmem_shm_toc_bg_get);
+
+pg_noreturn extern PGDLLEXPORT void valgrind_shmem_main(Datum main_arg);
+
+typedef struct
+{
+ int32 index;
+ bool done;
+ int32 result;
+} WorkerHeader;
+
+typedef struct
+{
+ int32 a[10];
+} ShmemState;
+
+static ShmemState *state;
+static int we_valdgrind_shmem_request = 0;
+
+static void
+shmem_request(void *arg)
+{
+ ShmemRequestStruct(.name = "valgrind_shmem: state",
+ .size = sizeof(ShmemState),
+ .ptr = (void **) &state,
+ );
+}
+
+static void
+shmem_init(void *arg)
+{
+ for (int i = 0; i < 10; i++)
+ state->a[i] = i * 2;
+}
+
+static const ShmemCallbacks shmem_callbacks = {
+ .request_fn = shmem_request,
+ .init_fn = shmem_init,
+};
+
+/*
+ * Gets an array element from a struct in the main shared memory.
+ */
+Datum
+valgrind_shmem_main_get(PG_FUNCTION_ARGS)
+{
+ int32 index = PG_GETARG_INT32(0);
+
+ PG_RETURN_INT32(state->a[index]);
+}
+
+/*
+ * Gets an array element from a struct in a shm_toc entry in DSM.
+ */
+Datum
+valgrind_shmem_shm_toc_fg_get(PG_FUNCTION_ARGS)
+{
+ int32 index = PG_GETARG_INT32(0);
+ shm_toc_estimator e;
+ Size segsize;
+ dsm_segment *seg;
+ shm_toc *toc;
+ ShmemState *state;
+ int32 ret;
+
+ shm_toc_initialize_estimator(&e);
+ shm_toc_estimate_chunk(&e, sizeof(ShmemState));
+ shm_toc_estimate_keys(&e, 1);
+ segsize = shm_toc_estimate(&e);
+
+ seg = dsm_create(shm_toc_estimate(&e), 0);
+ toc = shm_toc_create(PG_TEST_VALGRIND_SHMEM_MAGIC, dsm_segment_address(seg), segsize);
+
+ state = shm_toc_allocate(toc, sizeof(ShmemState));
+ for (int i = 0; i < 10; i++)
+ state->a[i] = i * 3;
+ shm_toc_insert(toc, 1, state);
+
+ ret = state->a[index];
+
+ dsm_detach(seg);
+
+ PG_RETURN_INT32(ret);
+}
+
+/*
+ * Gets an array element from a struct in a shm_toc entry in DSM in a
+ * background worker.
+ */
+Datum
+valgrind_shmem_shm_toc_bg_get(PG_FUNCTION_ARGS)
+{
+ int32 index = PG_GETARG_INT32(0);
+ shm_toc_estimator e;
+ Size segsize;
+ dsm_segment *seg;
+ shm_toc *toc;
+ WorkerHeader *header;
+ ShmemState *state;
+ BackgroundWorker worker = {
+ .bgw_flags = BGWORKER_SHMEM_ACCESS,
+ .bgw_start_time = BgWorkerStart_ConsistentState,
+ .bgw_restart_time = BGW_NEVER_RESTART,
+ .bgw_library_name = "valgrind_shmem",
+ .bgw_function_name = "valgrind_shmem_main",
+ .bgw_type = "valgrind_shmem",
+ .bgw_name = "valgrind_shmem worker",
+ .bgw_notify_pid = MyProcPid
+ };
+ BackgroundWorkerHandle *whandle;
+ int32 ret;
+
+ shm_toc_initialize_estimator(&e);
+ shm_toc_estimate_chunk(&e, sizeof(WorkerHeader));
+ shm_toc_estimate_chunk(&e, sizeof(ShmemState));
+ shm_toc_estimate_keys(&e, 2);
+ segsize = shm_toc_estimate(&e);
+
+ seg = dsm_create(shm_toc_estimate(&e), 0);
+ toc = shm_toc_create(PG_TEST_VALGRIND_SHMEM_MAGIC, dsm_segment_address(seg), segsize);
+
+ header = shm_toc_allocate(toc, sizeof(WorkerHeader));
+ header->index = index;
+ header->done = false;
+ shm_toc_insert(toc, 0, header);
+
+ state = shm_toc_allocate(toc, sizeof(ShmemState));
+ for (int i = 0; i < 10; i++)
+ state->a[i] = i * 3;
+ shm_toc_insert(toc, 1, state);
+
+ worker.bgw_main_arg = UInt32GetDatum(dsm_segment_handle(seg));
+
+ if (!RegisterDynamicBackgroundWorker(&worker, &whandle))
+ ereport(ERROR,
+ errcode(ERRCODE_INSUFFICIENT_RESOURCES),
+ errmsg("could not register background process"),
+ errhint("You may need to increase \"max_worker_processes\"."));
+
+ for (;;)
+ {
+ BgwHandleStatus status;
+ pid_t pid;
+
+ if (header->done)
+ break;
+
+ status = GetBackgroundWorkerPid(whandle, &pid);
+ if (status == BGWH_STOPPED || status == BGWH_POSTMASTER_DIED)
+ ereport(ERROR,
+ errmsg("background worker died unexpectedly"));
+
+ if (we_valdgrind_shmem_request == 0)
+ we_valdgrind_shmem_request = WaitEventExtensionNew("TestValgrindHsmemRequest");
+
+ (void) WaitLatch(MyLatch, WL_LATCH_SET | WL_EXIT_ON_PM_DEATH, 0,
+ we_valdgrind_shmem_request);
+
+ ResetLatch(MyLatch);
+
+ CHECK_FOR_INTERRUPTS();
+ }
+
+ ret = header->result;
+
+ dsm_detach(seg);
+
+ PG_RETURN_INT32(ret);
+}
+
+void
+valgrind_shmem_main(Datum main_arg)
+{
+ dsm_segment *seg;
+ shm_toc *toc;
+ WorkerHeader *header;
+ ShmemState *state;
+
+ seg = dsm_attach(DatumGetUInt32(main_arg));
+ if (seg == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("unable to map dynamic shared memory segment")));
+
+ toc = shm_toc_attach(PG_TEST_VALGRIND_SHMEM_MAGIC, dsm_segment_address(seg));
+ if (toc == NULL)
+ ereport(ERROR,
+ (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+ errmsg("bad magic number in dynamic shared memory segment")));
+
+ header = shm_toc_lookup(toc, 0, false);
+ state = shm_toc_lookup(toc, 1, false);
+
+ pg_usleep(500 * 1000);
+
+ header->result = state->a[header->index];
+ header->done = true;
+
+ proc_exit(0);
+}
+
+void
+_PG_init(void)
+{
+ RegisterShmemCallbacks(&shmem_callbacks);
+}
diff --git a/contrib/valgrind_shmem/valgrind_shmem.control b/contrib/valgrind_shmem/valgrind_shmem.control
new file mode 100644
index 00000000000..b70bd0e2d10
--- /dev/null
+++ b/contrib/valgrind_shmem/valgrind_shmem.control
@@ -0,0 +1,5 @@
+# valigrind_shmem extension
+comment = 'Dummy extension testing valgrind and shared memory'
+default_version = '1.0'
+module_pathname = '$libdir/valgrind_shmem'
+relocatable = true
--
2.54.0
From d8c89655eaeb6a34e11571553b4d112a50f1a298 Mon Sep 17 00:00:00 2001
From: Andreas Karlsson <[email protected]>
Date: Thu, 4 Jun 2026 13:54:22 +0200
Subject: [PATCH v3 2/3] valgrind: Add NOACCESS sentinels to allocations in
main shared memory
When built with USE_VALGRIND we have the ShmemAllocator add a NOACCESS
sentinel after every allocation in shared memory. On EXEC_BACKEND builds
we set up the sentinels when the backend attaches to shared memory, but
then only for things in ShmemIndex and not for allocations done directly
via legacy functions like ShmemAlloc().
Author: Andreas Karlsson <[email protected]>
---
src/backend/storage/ipc/shmem.c | 38 ++++++++++++++++++++++++++++++++-
1 file changed, 37 insertions(+), 1 deletion(-)
diff --git a/src/backend/storage/ipc/shmem.c b/src/backend/storage/ipc/shmem.c
index 1fbba9c3a4c..25a6a7ec899 100644
--- a/src/backend/storage/ipc/shmem.c
+++ b/src/backend/storage/ipc/shmem.c
@@ -142,6 +142,7 @@
#include "storage/shmem_internal.h"
#include "storage/spin.h"
#include "utils/builtins.h"
+#include "utils/memdebug.h"
#include "utils/tuplestore.h"
/*
@@ -279,6 +280,13 @@ static bool AttachShmemIndexEntry(ShmemRequest *request, bool missing_ok);
Datum pg_numa_available(PG_FUNCTION_ARGS);
+/* With valgrind, we want to add a couple NOACCESS bytes */
+#ifdef USE_VALGRIND
+#define NOACCESS_BYTES 32
+#else
+#define NOACCESS_BYTES 0
+#endif
+
/*
* ShmemRequestStruct() --- request a named shared memory area
*
@@ -405,9 +413,14 @@ ShmemGetRequestedSize(void)
/* pad the start address for alignment like ShmemAllocRaw() does */
if (alignment < PG_CACHE_LINE_SIZE)
alignment = PG_CACHE_LINE_SIZE;
+
size = TYPEALIGN(alignment, size);
size = add_size(size, request->options->size);
+
+#if USE_VALGRIND
+ size = add_size(size, NOACCESS_BYTES);
+#endif
}
return size;
@@ -492,6 +505,26 @@ ShmemAttachRequested(void)
callbacks->attach_fn(callbacks->opaque_arg);
}
+#ifdef USE_VALGRIND
+ {
+ HASH_SEQ_STATUS hstat;
+ ShmemIndexEnt *ent;
+
+ hash_seq_init(&hstat, ShmemIndex);
+
+ /*
+ * When compiled with EXEC_BACKEND we need to recreate all NOACCESS
+ * regions in each backend, but we can only recreate those with index
+ * entries and not any of the regions for memory allocated directly with
+ * ShmemAlloc().
+ */
+ while ((ent = (ShmemIndexEnt *) hash_seq_search(&hstat)) != NULL)
+ {
+ VALGRIND_MAKE_MEM_NOACCESS(ent->location + ent->size, NOACCESS_BYTES);
+ }
+ }
+#endif
+
LWLockRelease(ShmemIndexLock);
shmem_request_state = SRS_DONE;
@@ -822,11 +855,14 @@ ShmemAllocRaw(Size size, Size alignment, Size *allocated_size)
rawStart = ShmemAllocator->free_offset;
newStart = TYPEALIGN(alignment, rawStart);
- newFree = newStart + size;
+ newFree = newStart + size + NOACCESS_BYTES;
if (newFree <= ShmemSegHdr->totalsize)
{
newSpace = (char *) ShmemBase + newStart;
ShmemAllocator->free_offset = newFree;
+
+ /* Make the bytes at the end no-access */
+ VALGRIND_MAKE_MEM_NOACCESS(newSpace + size, NOACCESS_BYTES);
}
else
newSpace = NULL;
--
2.54.0
From 71e0624cec05b8e8835b28f3dbd58a8aa2896f41 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Sat, 2 May 2026 23:09:00 +0200
Subject: [PATCH v3 1/3] valgrind: Add NOACCESS sentinels for shm_toc entries
When built with USE_VALGRIND add a couple NOACCESS bytes after each
entry allocated in a shm_toc from the segment. Since markings are not
shared between processes they are either set when a process allocates or
when a process looks up an allocation in the toc.
But as we have not saved the size of an allocation in the toc we need
to, on lookup, base where we set the sentinel on where the start of the
following allocation is so any padding will cause a gap before the
sentinel. This is far from ideal but better than nothing.
We also need to make sure to clear all NOACCESS bytes when detaching
from a DSM segment since a shm_toc does not know when it was destroyed
or deatched.
Author: Tomas Vondra <[email protected]>
Author: Andreas Karlsson <[email protected]>
---
src/backend/storage/ipc/dsm.c | 7 ++++++
src/backend/storage/ipc/shm_toc.c | 39 +++++++++++++++++++++++++++----
2 files changed, 42 insertions(+), 4 deletions(-)
diff --git a/src/backend/storage/ipc/dsm.c b/src/backend/storage/ipc/dsm.c
index 8b69df4ff26..f447d1543e3 100644
--- a/src/backend/storage/ipc/dsm.c
+++ b/src/backend/storage/ipc/dsm.c
@@ -45,6 +45,7 @@
#include "storage/shmem.h"
#include "storage/subsystems.h"
#include "utils/freepage.h"
+#include "utils/memdebug.h"
#include "utils/memutils.h"
#include "utils/resowner.h"
@@ -846,6 +847,12 @@ dsm_detach(dsm_segment *seg)
*/
if (seg->mapped_address != NULL)
{
+ /*
+ * We need to clear up NOACCESS regions set by shm_toc as shm_tocs
+ * have no function for deatching or destroying.
+ */
+ VALGRIND_MAKE_MEM_DEFINED(seg->mapped_address, seg->mapped_size);
+
if (!is_main_region_dsm_handle(seg->handle))
dsm_impl_op(DSM_OP_DETACH, seg->handle, 0, &seg->impl_private,
&seg->mapped_address, &seg->mapped_size, WARNING);
diff --git a/src/backend/storage/ipc/shm_toc.c b/src/backend/storage/ipc/shm_toc.c
index 2217d48c3d9..0157f5c586c 100644
--- a/src/backend/storage/ipc/shm_toc.c
+++ b/src/backend/storage/ipc/shm_toc.c
@@ -16,6 +16,7 @@
#include "port/atomics.h"
#include "storage/shm_toc.h"
#include "storage/spin.h"
+#include "utils/memdebug.h"
typedef struct shm_toc_entry
{
@@ -74,6 +75,13 @@ shm_toc_attach(uint64 magic, void *address)
return toc;
}
+/* With valgrind, we want to add a couple NOACCESS bytes */
+#ifdef USE_VALGRIND
+#define NUM_NOACCESS_BYTES 32
+#else
+#define NUM_NOACCESS_BYTES 0
+#endif
+
/*
* Allocate shared memory from a segment managed by a table of contents.
*
@@ -87,6 +95,7 @@ shm_toc_attach(uint64 magic, void *address)
void *
shm_toc_allocate(shm_toc *toc, Size nbytes)
{
+ Size reqbytes;
Size total_bytes;
Size allocated_bytes;
Size nentry;
@@ -98,7 +107,7 @@ shm_toc_allocate(shm_toc *toc, Size nbytes)
* proper definition for the minimum to make atomic ops safe, but
* BUFFERALIGN ought to be enough.
*/
- nbytes = BUFFERALIGN(nbytes);
+ reqbytes = BUFFERALIGN(nbytes + NUM_NOACCESS_BYTES);
SpinLockAcquire(&toc->toc_mutex);
@@ -109,18 +118,24 @@ shm_toc_allocate(shm_toc *toc, Size nbytes)
+ allocated_bytes;
/* Check for memory exhaustion and overflow. */
- if (toc_bytes + nbytes > total_bytes || toc_bytes + nbytes < toc_bytes)
+ if (toc_bytes + reqbytes > total_bytes || toc_bytes + reqbytes < toc_bytes)
{
SpinLockRelease(&toc->toc_mutex);
ereport(ERROR,
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of shared memory")));
}
- toc->toc_allocated_bytes += nbytes;
+ toc->toc_allocated_bytes += reqbytes;
SpinLockRelease(&toc->toc_mutex);
- return ((char *) toc) + (total_bytes - allocated_bytes - nbytes);
+#ifdef USE_VALGRIND
+ /* Make the bytes at the end no-access */
+ VALGRIND_MAKE_MEM_NOACCESS(((char *) toc) + (total_bytes - allocated_bytes - reqbytes + nbytes),
+ reqbytes - nbytes);
+#endif
+
+ return ((char *) toc) + (total_bytes - allocated_bytes - reqbytes);
}
/*
@@ -249,7 +264,20 @@ shm_toc_lookup(shm_toc *toc, uint64 key, bool noError)
for (i = 0; i < nentry; ++i)
{
if (toc->toc_entry[i].key == key)
+ {
+#ifdef USE_VALGRIND
+ /*
+ * Since we do not know the size of entries we can only use the
+ * start of the next entry for setting the no-access sentinel.
+ */
+ Size nextoffset = i == 0 ? toc->toc_total_bytes : toc->toc_entry[i - 1].offset;
+
+ VALGRIND_MAKE_MEM_NOACCESS(((char *) toc) + nextoffset - NUM_NOACCESS_BYTES,
+ NUM_NOACCESS_BYTES);
+#endif
+
return ((char *) toc) + toc->toc_entry[i].offset;
+ }
}
/* No matching entry was found. */
@@ -272,5 +300,8 @@ shm_toc_estimate(shm_toc_estimator *e)
sz = add_size(sz, mul_size(e->number_of_keys, sizeof(shm_toc_entry)));
sz = add_size(sz, e->space_for_chunks);
+ /* add space for ENOACCESS bytes, NUM_NOACCESS_BYTES per key */
+ sz = add_size(sz, mul_size(e->number_of_keys, NUM_NOACCESS_BYTES));
+
return BUFFERALIGN(sz);
}
--
2.54.0
