From a8f3c90fbe23809be755492b2839cba28901dc8d Mon Sep 17 00:00:00 2001
From: Viktor Kurilko <v.kurilko@arenadata.io>
Date: Thu, 7 Nov 2024 16:47:55 +0700
Subject: [PATCH] Fix potentially clobbered variables in PG_TRY/PG_CATCH blocks

Some local variables may have an undefined value after longjmp, which can lead
to undefined behavior.

This patch adds volatile to such variables based on a compiler warning.
Fix a potential memory leak related to creating a hashmap inside a try/catch
block.
Add a compiler flag for throwing warnings related to clobbering variables.
---
 CMakeLists.txt          |  2 +-
 src/diskquota.c         | 14 +++++++-------
 src/diskquota_utility.c |  6 +++---
 src/enforcement.c       |  2 +-
 src/quotamodel.c        | 18 +++++++++---------
 5 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 70b825d4..0dc52727 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -35,7 +35,7 @@ if (APPLE)
     set(CMAKE_MODULE_LINKER_FLAGS "${CMAKE_MODULE_LINKER_FLAGS} -bundle_loader ${PG_BIN_DIR}/postgres")
 endif()
 # set c and ld flags for all projects
-set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} ${PG_C_FLAGS}")
+set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -Wclobbered ${PG_C_FLAGS}")
 
 # generate version
 if(NOT DEFINED DISKQUOTA_VERSION)
diff --git a/src/diskquota.c b/src/diskquota.c
index 50eb0260..8bce2743 100644
--- a/src/diskquota.c
+++ b/src/diskquota.c
@@ -958,10 +958,10 @@ disk_quota_launcher_main(Datum main_arg)
 static void
 create_monitor_db_table(void)
 {
-	const char *sql;
-	bool        connected_in_this_function = false;
-	bool        pushed_active_snap         = false;
-	bool        ret                        = true;
+	const char   *sql;
+	volatile bool connected_in_this_function = false;
+	volatile bool pushed_active_snap         = false;
+	volatile bool ret                        = true;
 
 	/*
 	 * Create function diskquota.diskquota_fetch_table_stat in launcher
@@ -1167,9 +1167,9 @@ process_extension_ddl_message()
 static void
 do_process_extension_ddl_message(MessageResult *code, ExtensionDDLMessage local_extension_ddl_message)
 {
-	int  old_num_db         = num_db;
-	bool pushed_active_snap = false;
-	bool ret                = true;
+	int           old_num_db         = num_db;
+	volatile bool pushed_active_snap = false;
+	volatile bool ret                = true;
 
 	StartTransactionCommand();
 
diff --git a/src/diskquota_utility.c b/src/diskquota_utility.c
index 252d32cd..535393bd 100644
--- a/src/diskquota_utility.c
+++ b/src/diskquota_utility.c
@@ -1430,9 +1430,9 @@ relation_size_local(PG_FUNCTION_ARGS)
 Relation
 diskquota_relation_open(Oid relid)
 {
-	Relation rel;
-	bool     success_open               = false;
-	int32    SavedInterruptHoldoffCount = InterruptHoldoffCount;
+	volatile Relation rel;
+	volatile bool     success_open               = false;
+	int32             SavedInterruptHoldoffCount = InterruptHoldoffCount;
 
 	PG_TRY();
 	{
diff --git a/src/enforcement.c b/src/enforcement.c
index 4568db39..cde8246b 100644
--- a/src/enforcement.c
+++ b/src/enforcement.c
@@ -44,7 +44,7 @@ init_disk_quota_enforcement(void)
 static bool
 quota_check_ExecCheckRTPerms(List *rangeTable, bool ereport_on_violation)
 {
-	ListCell *l;
+	ListCell *volatile l;
 
 	foreach (l, rangeTable)
 	{
diff --git a/src/quotamodel.c b/src/quotamodel.c
index 2f45180c..858913c1 100644
--- a/src/quotamodel.c
+++ b/src/quotamodel.c
@@ -673,9 +673,9 @@ vacuum_disk_quota_model(uint32 id)
 bool
 check_diskquota_state_is_ready(void)
 {
-	bool is_ready           = false;
-	bool pushed_active_snap = false;
-	bool ret                = true;
+	volatile bool is_ready           = false;
+	volatile bool pushed_active_snap = false;
+	volatile bool ret                = true;
 
 	StartTransactionCommand();
 
@@ -795,9 +795,9 @@ refresh_disk_quota_model(bool is_init)
 static void
 refresh_disk_quota_usage(bool is_init)
 {
-	bool  pushed_active_snap          = false;
-	bool  ret                         = true;
-	HTAB *local_active_table_stat_map = NULL;
+	volatile bool pushed_active_snap           = false;
+	volatile bool ret                          = true;
+	HTAB *volatile local_active_table_stat_map = NULL;
 
 	StartTransactionCommand();
 
@@ -836,7 +836,6 @@ refresh_disk_quota_usage(bool is_init)
 		 */
 		if (is_init || (diskquota_hardlimit && (reject_map_changed || hasActiveTable)))
 			dispatch_rejectmap(local_active_table_stat_map);
-		hash_destroy(local_active_table_stat_map);
 	}
 	PG_CATCH();
 	{
@@ -849,6 +848,7 @@ refresh_disk_quota_usage(bool is_init)
 		RESUME_INTERRUPTS();
 	}
 	PG_END_TRY();
+	if (local_active_table_stat_map) hash_destroy(local_active_table_stat_map);
 	if (pushed_active_snap) PopActiveSnapshot();
 	if (ret)
 		CommitTransactionCommand();
@@ -1402,8 +1402,8 @@ truncateStringInfo(StringInfo str, int nchars)
 static bool
 load_quotas(void)
 {
-	bool pushed_active_snap = false;
-	bool ret                = true;
+	volatile bool pushed_active_snap = false;
+	volatile bool ret                = true;
 
 	StartTransactionCommand();