Skip to content

Commit

Permalink
Fix potentially clobbered variables in PG_TRY/PG_CATCH blocks
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
KnightMurloc committed Nov 7, 2024
1 parent 92ca25b commit a8f3c90
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 21 deletions.
2 changes: 1 addition & 1 deletion CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 7 additions & 7 deletions src/diskquota.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();

Expand Down
6 changes: 3 additions & 3 deletions src/diskquota_utility.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();
{
Expand Down
2 changes: 1 addition & 1 deletion src/enforcement.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
18 changes: 9 additions & 9 deletions src/quotamodel.c
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();
{
Expand All @@ -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();
Expand Down Expand Up @@ -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();

Expand Down

0 comments on commit a8f3c90

Please sign in to comment.