From 67ee498d3826ea208bfa41e761011e572a619bcf Mon Sep 17 00:00:00 2001 From: Xing Guo Date: Tue, 16 Apr 2024 21:29:14 +0800 Subject: [PATCH] Add missing volatile qualifier. (#7570) Variables being modified in the PG_TRY block and read in the PG_CATCH block should be qualified with volatile. The variable waitEventSet is modified in the PG_TRY block (line 1085) and read in the PG_CATCH block (line 1095). The variable relation is modified in the PG_TRY block (line 500) and read in the PG_CATCH block (line 515). Besides, the variable objectAddress doesn't need the volatile qualifier. Ref: C99 7.13.2.1[^1], > All accessible objects have values, and all other components of the abstract machine have state, as of the time the longjmp function was called, except that the values of objects of automatic storage duration that are local to the function containing the invocation of the corresponding setjmp macro that do not have volatile-qualified type and have been changed between the setjmp invocation and longjmp call are indeterminate. [^1]: https://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf DESCRIPTION: Correctly mark some variables as volatile --------- Co-authored-by: Hong Yi (cherry picked from commit ada3ba25072cc5be055b3bbdedfa2fe936443b0d) --- .../distributed/connection/remote_commands.c | 2 +- .../distributed/utils/citus_depended_object.c | 15 ++++++++++++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/backend/distributed/connection/remote_commands.c b/src/backend/distributed/connection/remote_commands.c index 4b46e96d2..cbd74ff51 100644 --- a/src/backend/distributed/connection/remote_commands.c +++ b/src/backend/distributed/connection/remote_commands.c @@ -883,7 +883,7 @@ WaitForAllConnections(List *connectionList, bool raiseInterrupts) palloc(totalConnectionCount * sizeof(MultiConnection *)); WaitEvent *events = palloc(totalConnectionCount * sizeof(WaitEvent)); bool *connectionReady = palloc(totalConnectionCount * sizeof(bool)); - WaitEventSet *waitEventSet = NULL; + WaitEventSet *volatile waitEventSet = NULL; /* convert connection list to an array such that we can move items around */ MultiConnection *connectionItem = NULL; diff --git a/src/backend/distributed/utils/citus_depended_object.c b/src/backend/distributed/utils/citus_depended_object.c index a160fcd56..7588f8594 100644 --- a/src/backend/distributed/utils/citus_depended_object.c +++ b/src/backend/distributed/utils/citus_depended_object.c @@ -465,8 +465,8 @@ static bool AnyObjectViolatesOwnership(DropStmt *dropStmt) { bool hasOwnershipViolation = false; - volatile ObjectAddress objectAddress = { 0 }; - Relation relation = NULL; + ObjectAddress objectAddress = { 0 }; + volatile Relation relation = NULL; ObjectType objectType = dropStmt->removeType; bool missingOk = dropStmt->missing_ok; @@ -480,8 +480,17 @@ AnyObjectViolatesOwnership(DropStmt *dropStmt) Node *object = NULL; foreach_ptr(object, dropStmt->objects) { + Relation rel = NULL; objectAddress = get_object_address(objectType, object, - &relation, AccessShareLock, missingOk); + &rel, AccessShareLock, missingOk); + + /* + * The object relation is qualified with volatile and its value is obtained from + * get_object_address(). Unless we can qualify the corresponding parameter of + * get_object_address() with volatile (this is a function defined in PostgreSQL), + * we cannot get rid of this assignment. + */ + relation = rel; if (OidIsValid(objectAddress.objectId)) {