0003-Imporve-pg_re_throw-check-if-sigjmp_buf-is-valid-and.patch

From e217c57624fd9bf952efff1476fec32604ef56d4 Mon Sep 17 00:00:00 2001
From: wangxiaoran <fanfuxiaoran@gmail.com>
Date: Mon, 19 Aug 2024 11:44:00 +0800
Subject: [PATCH] Imporve pg_re_throw: check if sigjmp_buf is valid and report
 error

If the code in PG_TRY contains any non local control flow other than
ereport(ERROR) like goto, break etc., the PG_CATCH or PG_END_TRY cannot
be called, then the PG_exception_stack will point to the memory whose
stack frame has been released. So after that, when the pg_re_throw
called, __longjmp() will crash and report Segmentation fault error.

In that case, to help developers to figure out the root cause easily, it is
better to report that 'the sigjmp_buf is invalid' rather than letting
the __longjmp report any error.

Addition to sigjmp_buf, add another field 'int magic' which is next to
the sigjum_buf in the local stack frame memory. The magic's value is always
'PG_exception_magic 0x12345678'. And in 'pg_re_throw' routine, check if
the magic's value is still '0x12345678', if not, that means the memory
where the 'PG_exception_stack' points to has been released, and the 'sigbuf'
must be invalid.
---
 src/backend/postmaster/autovacuum.c        |  8 ++++----
 src/backend/postmaster/bgworker.c          |  4 ++--
 src/backend/postmaster/bgwriter.c          |  4 ++--
 src/backend/postmaster/checkpointer.c      |  5 ++---
 src/backend/postmaster/pgarch.c            |  4 ++--
 src/backend/postmaster/walsummarizer.c     |  4 ++--
 src/backend/postmaster/walwriter.c         |  4 ++--
 src/backend/replication/logical/slotsync.c |  4 ++--
 src/backend/tcop/postgres.c                |  5 ++---
 src/backend/utils/error/elog.c             | 12 ++++++++++--
 src/include/utils/elog.h                   | 14 ++++++++++----
 11 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 7d0877c95e..4076d99938 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -361,7 +361,7 @@ static void avl_sigusr2_handler(SIGNAL_ARGS);
 void
 AutoVacLauncherMain(char *startup_data, size_t startup_data_len)
 {
-	sigjmp_buf	local_sigjmp_buf;
+	PG_exception	local_sigjmp_buf = {PG_exception_magic};
 
 	Assert(startup_data_len == 0);
 
@@ -436,7 +436,7 @@ AutoVacLauncherMain(char *startup_data, size_t startup_data_len)
 	 * call redundant, but it is not since InterruptPending might be set
 	 * already.
 	 */
-	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
+	if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
 	{
 		/* since not using PG_TRY, must reset error stack by hand */
 		error_context_stack = NULL;
@@ -1359,7 +1359,7 @@ avl_sigusr2_handler(SIGNAL_ARGS)
 void
 AutoVacWorkerMain(char *startup_data, size_t startup_data_len)
 {
-	sigjmp_buf	local_sigjmp_buf;
+	PG_exception    local_sigjmp_buf = {PG_exception_magic};
 	Oid			dbid;
 
 	Assert(startup_data_len == 0);
@@ -1421,7 +1421,7 @@ AutoVacWorkerMain(char *startup_data, size_t startup_data_len)
 	 * seem that this policy makes the HOLD_INTERRUPTS() call redundant, but
 	 * it is not since InterruptPending might be set already.
 	 */
-	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
+	if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
 	{
 		/* since not using PG_TRY, must reset error stack by hand */
 		error_context_stack = NULL;
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index b83967cda3..d9cb01cd3a 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -716,7 +716,7 @@ bgworker_die(SIGNAL_ARGS)
 void
 BackgroundWorkerMain(char *startup_data, size_t startup_data_len)
 {
-	sigjmp_buf	local_sigjmp_buf;
+	PG_exception	local_sigjmp_buf = {PG_exception_magic};
 	BackgroundWorker *worker;
 	bgworker_main_type entrypt;
 
@@ -781,7 +781,7 @@ BackgroundWorkerMain(char *startup_data, size_t startup_data_len)
 	 *
 	 * We just need to clean up, report the error, and go away.
 	 */
-	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
+	if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
 	{
 		/* Since not using PG_TRY, must reset error stack by hand */
 		error_context_stack = NULL;
diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index 0f75548759..445b56ac76 100644
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -86,7 +86,7 @@ static XLogRecPtr last_snapshot_lsn = InvalidXLogRecPtr;
 void
 BackgroundWriterMain(char *startup_data, size_t startup_data_len)
 {
-	sigjmp_buf	local_sigjmp_buf;
+	PG_exception    local_sigjmp_buf = {PG_exception_magic};
 	MemoryContext bgwriter_context;
 	bool		prev_hibernate;
 	WritebackContext wb_context;
@@ -150,7 +150,7 @@ BackgroundWriterMain(char *startup_data, size_t startup_data_len)
 	 * call redundant, but it is not since InterruptPending might be set
 	 * already.
 	 */
-	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
+	if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
 	{
 		/* Since not using PG_TRY, must reset error stack by hand */
 		error_context_stack = NULL;
diff --git a/src/backend/postmaster/checkpointer.c b/src/backend/postmaster/checkpointer.c
index 199f008bcd..e2a2c7336e 100644
--- a/src/backend/postmaster/checkpointer.c
+++ b/src/backend/postmaster/checkpointer.c
@@ -172,7 +172,7 @@ static void ReqCheckpointHandler(SIGNAL_ARGS);
 void
 CheckpointerMain(char *startup_data, size_t startup_data_len)
 {
-	sigjmp_buf	local_sigjmp_buf;
+	PG_exception    local_sigjmp_buf = {PG_exception_magic};
 	MemoryContext checkpointer_context;
 
 	Assert(startup_data_len == 0);
@@ -248,7 +248,7 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
 	 * call redundant, but it is not since InterruptPending might be set
 	 * already.
 	 */
-	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
+	if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
 	{
 		/* Since not using PG_TRY, must reset error stack by hand */
 		error_context_stack = NULL;
@@ -311,7 +311,6 @@ CheckpointerMain(char *startup_data, size_t startup_data_len)
 
 	/* We can now handle ereport(ERROR) */
 	PG_exception_stack = &local_sigjmp_buf;
-
 	/*
 	 * Unblock signals (they were blocked when the postmaster forked us)
 	 */
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 02f91431f5..3a40334bc4 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -515,7 +515,7 @@ pgarch_ArchiverCopyLoop(void)
 static bool
 pgarch_archiveXlog(char *xlog)
 {
-	sigjmp_buf	local_sigjmp_buf;
+	PG_exception    local_sigjmp_buf = {PG_exception_magic};
 	MemoryContext oldcontext;
 	char		pathname[MAXPGPATH];
 	char		activitymsg[MAXFNAMELEN + 16];
@@ -544,7 +544,7 @@ pgarch_archiveXlog(char *xlog)
 	 * PgArchiverMain() and use PG_TRY/PG_CATCH here, but the extra code to
 	 * avoid the odd archiver restart doesn't seem worth it.
 	 */
-	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
+	if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
 	{
 		/* Since not using PG_TRY, must reset error stack by hand */
 		error_context_stack = NULL;
diff --git a/src/backend/postmaster/walsummarizer.c b/src/backend/postmaster/walsummarizer.c
index daa7909382..da58b26aa8 100644
--- a/src/backend/postmaster/walsummarizer.c
+++ b/src/backend/postmaster/walsummarizer.c
@@ -210,7 +210,7 @@ WalSummarizerShmemInit(void)
 void
 WalSummarizerMain(char *startup_data, size_t startup_data_len)
 {
-	sigjmp_buf	local_sigjmp_buf;
+	PG_exception    local_sigjmp_buf = {PG_exception_magic};
 	MemoryContext context;
 
 	/*
@@ -273,7 +273,7 @@ WalSummarizerMain(char *startup_data, size_t startup_data_len)
 	/*
 	 * If an exception is encountered, processing resumes here.
 	 */
-	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
+	if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
 	{
 		/* Since not using PG_TRY, must reset error stack by hand */
 		error_context_stack = NULL;
diff --git a/src/backend/postmaster/walwriter.c b/src/backend/postmaster/walwriter.c
index 6e7918a78d..c18e7eb024 100644
--- a/src/backend/postmaster/walwriter.c
+++ b/src/backend/postmaster/walwriter.c
@@ -88,7 +88,7 @@ int			WalWriterFlushAfter = DEFAULT_WAL_WRITER_FLUSH_AFTER;
 void
 WalWriterMain(char *startup_data, size_t startup_data_len)
 {
-	sigjmp_buf	local_sigjmp_buf;
+	PG_exception    local_sigjmp_buf = {PG_exception_magic};
 	MemoryContext walwriter_context;
 	int			left_till_hibernate;
 	bool		hibernating;
@@ -147,7 +147,7 @@ WalWriterMain(char *startup_data, size_t startup_data_len)
 	 * call redundant, but it is not since InterruptPending might be set
 	 * already.
 	 */
-	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
+	if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
 	{
 		/* Since not using PG_TRY, must reset error stack by hand */
 		error_context_stack = NULL;
diff --git a/src/backend/replication/logical/slotsync.c b/src/backend/replication/logical/slotsync.c
index 2d1914ce08..c35c54edab 100644
--- a/src/backend/replication/logical/slotsync.c
+++ b/src/backend/replication/logical/slotsync.c
@@ -1333,7 +1333,7 @@ ReplSlotSyncWorkerMain(char *startup_data, size_t startup_data_len)
 	WalReceiverConn *wrconn = NULL;
 	char	   *dbname;
 	char	   *err;
-	sigjmp_buf	local_sigjmp_buf;
+	PG_exception    local_sigjmp_buf = {PG_exception_magic};
 	StringInfoData app_name;
 
 	Assert(startup_data_len == 0);
@@ -1366,7 +1366,7 @@ ReplSlotSyncWorkerMain(char *startup_data, size_t startup_data_len)
 	 * operates at the bottom of the exception stack, ERRORs turn into FATALs.
 	 * Therefore, we create our own exception handler to catch ERRORs.
 	 */
-	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
+	if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
 	{
 		/* since not using PG_TRY, must reset error stack by hand */
 		error_context_stack = NULL;
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 8bc6bea113..c2108ef72e 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -4220,7 +4220,7 @@ PostgresSingleUserMain(int argc, char *argv[],
 void
 PostgresMain(const char *dbname, const char *username)
 {
-	sigjmp_buf	local_sigjmp_buf;
+	PG_exception    local_sigjmp_buf = {PG_exception_magic};
 
 	/* these must be volatile to ensure state is preserved across longjmp: */
 	volatile bool send_ready_for_query = true;
@@ -4422,7 +4422,7 @@ PostgresMain(const char *dbname, const char *username)
 	 * were inside a transaction.
 	 */
 
-	if (sigsetjmp(local_sigjmp_buf, 1) != 0)
+	if (sigsetjmp(local_sigjmp_buf.buf, 1) != 0)
 	{
 		/*
 		 * NOTE: if you are tempted to add more code in this if-block,
@@ -4537,7 +4537,6 @@ PostgresMain(const char *dbname, const char *username)
 
 	/* We can now handle ereport(ERROR) */
 	PG_exception_stack = &local_sigjmp_buf;
-
 	if (!ignore_till_sync)
 		send_ready_for_query = true;	/* initially, or after error */
 
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 943d8588f3..09fd4a829d 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -93,7 +93,7 @@
 /* Global variables */
 ErrorContextCallback *error_context_stack = NULL;
 
-sigjmp_buf *PG_exception_stack = NULL;
+PG_exception *PG_exception_stack = NULL;
 
 /*
  * Hook for intercepting messages before they are sent to the server log.
@@ -1985,7 +1985,15 @@ pg_re_throw(void)
 {
 	/* If possible, throw the error to the next outer setjmp handler */
 	if (PG_exception_stack != NULL)
-		siglongjmp(*PG_exception_stack, 1);
+	{
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wunused"
+		void *stackTop = NULL;
+#pragma clang diagnostic pop
+		Assert((void *)PG_exception_stack > &stackTop ||
+				PG_exception_stack->magic == PG_exception_magic);
+		siglongjmp(PG_exception_stack->buf, 1);
+	}
 	else
 	{
 		/*
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 054dd2bf62..d8dc45c1ff 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -383,11 +383,11 @@ extern PGDLLIMPORT ErrorContextCallback *error_context_stack;
  */
 #define PG_TRY(...)  \
 	do { \
-		sigjmp_buf *_save_exception_stack##__VA_ARGS__ = PG_exception_stack; \
+		PG_exception *_save_exception_stack##__VA_ARGS__ = PG_exception_stack; \
 		ErrorContextCallback *_save_context_stack##__VA_ARGS__ = error_context_stack; \
-		sigjmp_buf _local_sigjmp_buf##__VA_ARGS__; \
+		PG_exception _local_sigjmp_buf##__VA_ARGS__ = {PG_exception_magic}; \
 		bool _do_rethrow##__VA_ARGS__ = false; \
-		if (sigsetjmp(_local_sigjmp_buf##__VA_ARGS__, 0) == 0) \
+		if (sigsetjmp(_local_sigjmp_buf##__VA_ARGS__.buf, 0) == 0) \
 		{ \
 			PG_exception_stack = &_local_sigjmp_buf##__VA_ARGS__
 
@@ -426,7 +426,13 @@ extern PGDLLIMPORT ErrorContextCallback *error_context_stack;
 	(pg_re_throw(), pg_unreachable())
 #endif
 
-extern PGDLLIMPORT sigjmp_buf *PG_exception_stack;
+#define PG_exception_magic 0x12345678
+typedef struct PG_exception
+{
+	int magic;
+	sigjmp_buf buf;
+} PG_exception;
+extern PGDLLIMPORT PG_exception *PG_exception_stack;
 
 
 /* Stuff that error handlers might want to use */
-- 
2.39.3 (Apple Git-146)