Date: Fri, 7 Mar 2025 10:52:10 -0600
From: Nathan Bossart <nathandbossart@gmail.com>
To: Andres Freund <andres@anarazel.de>
Cc: Tomas Vondra <tomas@vondra.me>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: strange valgrind reports about wrapper_handler on 64-bit arm
Message-ID: <Z8skOiYFFNzbPVED@nathan>
References: <ace01111-f9ac-4f61-b1b1-8e9379415444@vondra.me>
 <i73pjuqcqgv5lsprc76z4bug55okadxpzpuijnup34qmqtwexi@gakdkcyjox5l>
 <Z8sgkxMDlXVENIHb@nathan>
 <77dxan5ga3rjxjbnm2vtljntbcpab3wov2zlcdukw75a2qojmu@3mbkzfpv54sd>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="1IwYB9lJQVGT1KOB"
Content-Disposition: inline
In-Reply-To: <77dxan5ga3rjxjbnm2vtljntbcpab3wov2zlcdukw75a2qojmu@3mbkzfpv54sd>
Archived-At: <https://www.postgresql.org/message-id/Z8skOiYFFNzbPVED%40nathan>
Precedence: bulk


--1IwYB9lJQVGT1KOB
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Mar 07, 2025 at 11:41:38AM -0500, Andres Freund wrote:
> On 2025-03-07 10:36:35 -0600, Nathan Bossart wrote:
>> On Fri, Mar 07, 2025 at 11:32:28AM -0500, Andres Freund wrote:
>> > Is it possible that the signal number we're getting called for is above
>> > PG_NSIG? That'd explain why the source value is something fairly random?
>> > 
>> > ISTM that we should add an Assert() to wrapper_handler() that ensures that the
>> > signal arg is below PG_NSIG.
>> 
>> We have such an assertion in pqsignal() before we install wrapper_handler
>> for anything.  Is there another way it could be getting called with a
>> different signo?
> 
> Who the hell knows :).
> 
> One potential way would be that we got SIGNAL_ARGS wrong for the platform and
> are interpreting some random thing as the signal number.  Or something went
> wrong in the windows signal emulation code.  Or ...
> 
> It seems cheap insurance to add it both places.

Good enough for me.  I'll commit/back-patch to v17 the attached soon.

-- 
nathan

--1IwYB9lJQVGT1KOB
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename="assert.patch"

diff --git a/src/port/pqsignal.c b/src/port/pqsignal.c
index 5dd8b76bae8..79b50486175 100644
--- a/src/port/pqsignal.c
+++ b/src/port/pqsignal.c
@@ -87,6 +87,8 @@ wrapper_handler(SIGNAL_ARGS)
 {
 	int			save_errno = errno;
 
+	Assert(postgres_signal_arg < PG_NSIG);
+
 #ifndef FRONTEND
 
 	/*

--1IwYB9lJQVGT1KOB--