Skip to content

Add C11 atomic wrapper structs to prevent direct access and control memory ordering #13373

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft

Add C11 atomic wrapper structs to prevent direct access and control memory ordering #13373

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0c0f801
Initial plan
Copilot Aug 21, 2025
3759195
Implement C11 atomic wrapper structs and fix direct access issues
Copilot Aug 21, 2025
f81e3ed
Fix additional atomic access issues and add volatile variants
Copilot Aug 21, 2025
d49a9f8
Complete C11 atomic wrapper implementation - working and preventing d…
Copilot Aug 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ompi/runtime/ompi_mpi_init.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ const char ompi_version_string[] = OMPI_IDENT_STRING;
* Global variables and symbols for the MPI layer
*/

opal_atomic_int32_t ompi_mpi_state = OMPI_MPI_STATE_NOT_INITIALIZED;
opal_atomic_int32_t ompi_mpi_state = { .value = OMPI_MPI_STATE_NOT_INITIALIZED };
volatile bool ompi_rte_initialized = false;

bool ompi_mpi_thread_multiple = false;
Expand Down Expand Up @@ -371,7 +371,7 @@ int ompi_mpi_init(int argc, char **argv, int requested, int *provided,
// silently return successfully once the initializing
// thread has completed.
if (reinit_ok) {
while (ompi_mpi_state < OMPI_MPI_STATE_INIT_COMPLETED) {
while (opal_atomic_load_32_relaxed(&ompi_mpi_state) < OMPI_MPI_STATE_INIT_COMPLETED) {
usleep(1);
}
return MPI_SUCCESS;
Expand Down
32 changes: 16 additions & 16 deletions opal/class/opal_lifo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ static inline bool opal_update_counted_pointer(volatile opal_counted_pointer_t *
opal_counted_pointer_t *old, opal_list_item_t *item)
{
opal_counted_pointer_t new_p;
new_p.data.item = (intptr_t) item;
opal_atomic_store_ptr_volatile_relaxed(&new_p.data.item, (intptr_t) item);
new_p.data.counter = old->data.counter + 1;
return opal_atomic_compare_exchange_strong_128(&addr->atomic_value, &old->value, new_p.value);
}
Expand All @@ -83,7 +83,7 @@ opal_read_counted_pointer(volatile opal_counted_pointer_t *volatile addr,
* specific order */
value->data.counter = addr->data.counter;
opal_atomic_rmb();
value->data.item = addr->data.item;
opal_atomic_store_ptr_volatile_relaxed(&value->data.item, opal_atomic_load_ptr_volatile_relaxed(&addr->data.item));
}

#endif
Expand Down Expand Up @@ -122,7 +122,7 @@ OPAL_DECLSPEC OBJ_CLASS_DECLARATION(opal_lifo_t);
*/
static inline bool opal_lifo_is_empty(opal_lifo_t *lifo)
{
return (opal_list_item_t *) lifo->opal_lifo_head.data.item == &lifo->opal_lifo_ghost;
return (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item) == &lifo->opal_lifo_ghost;
}

#if OPAL_HAVE_ATOMIC_COMPARE_EXCHANGE_128 && !OPAL_HAVE_ATOMIC_LLSC_PTR
Expand All @@ -133,14 +133,14 @@ static inline bool opal_lifo_is_empty(opal_lifo_t *lifo)
*/
static inline opal_list_item_t *opal_lifo_push_atomic(opal_lifo_t *lifo, opal_list_item_t *item)
{
opal_list_item_t *next = (opal_list_item_t *) lifo->opal_lifo_head.data.item;
opal_list_item_t *next = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item);

do {
item->opal_list_next = next;
opal_atomic_wmb();

/* to protect against ABA issues it is sufficient to only update the counter in pop */
if (opal_atomic_compare_exchange_strong_ptr(&lifo->opal_lifo_head.data.item,
if (opal_atomic_compare_exchange_strong_ptr_volatile(&lifo->opal_lifo_head.data.item,
(intptr_t *) &next, (intptr_t) item)) {
return next;
}
Expand All @@ -159,7 +159,7 @@ static inline opal_list_item_t *opal_lifo_pop_atomic(opal_lifo_t *lifo)
opal_read_counted_pointer(&lifo->opal_lifo_head, &old_head);

do {
item = (opal_list_item_t *) old_head.data.item;
item = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&old_head.data.item);
if (item == &lifo->opal_lifo_ghost) {
return NULL;
}
Expand All @@ -181,15 +181,15 @@ static inline opal_list_item_t *opal_lifo_pop_atomic(opal_lifo_t *lifo)
*/
static inline opal_list_item_t *opal_lifo_push_atomic(opal_lifo_t *lifo, opal_list_item_t *item)
{
opal_list_item_t *next = (opal_list_item_t *) lifo->opal_lifo_head.data.item;
opal_list_item_t *next = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item);

/* item free acts as a mini lock to avoid ABA problems */
item->item_free = 1;

do {
item->opal_list_next = next;
opal_atomic_wmb();
if (opal_atomic_compare_exchange_strong_ptr(&lifo->opal_lifo_head.data.item,
if (opal_atomic_compare_exchange_strong_ptr_volatile(&lifo->opal_lifo_head.data.item,
(intptr_t *) &next, (intptr_t) item)) {
opal_atomic_wmb();
/* now safe to pop this item */
Expand Down Expand Up @@ -218,13 +218,13 @@ static inline opal_list_item_t *opal_lifo_pop_atomic(opal_lifo_t *lifo)
attempt = 0;
}

opal_atomic_ll_ptr(&lifo->opal_lifo_head.data.item, item);
opal_atomic_ll_ptr(&lifo->opal_lifo_head.data.item.value, item);
if (&lifo->opal_lifo_ghost == item) {
return NULL;
}

next = (opal_list_item_t *) item->opal_list_next;
opal_atomic_sc_ptr(&lifo->opal_lifo_head.data.item, next, ret);
opal_atomic_sc_ptr(&lifo->opal_lifo_head.data.item.value, next, ret);
} while (!ret);

opal_atomic_wmb();
Expand All @@ -242,7 +242,7 @@ static inline opal_list_item_t *opal_lifo_pop_atomic(opal_lifo_t *lifo)
{
opal_list_item_t *item, *head, *ghost = &lifo->opal_lifo_ghost;

while ((item = (opal_list_item_t *) lifo->opal_lifo_head.data.item) != ghost) {
while ((item = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item)) != ghost) {
/* ensure it is safe to pop the head */
if (opal_atomic_swap_32((opal_atomic_int32_t *) &item->item_free, 1)) {
continue;
Expand All @@ -252,7 +252,7 @@ static inline opal_list_item_t *opal_lifo_pop_atomic(opal_lifo_t *lifo)

head = item;
/* try to swap out the head pointer */
if (opal_atomic_compare_exchange_strong_ptr(&lifo->opal_lifo_head.data.item,
if (opal_atomic_compare_exchange_strong_ptr_volatile(&lifo->opal_lifo_head.data.item,
(intptr_t *) &head,
(intptr_t) item->opal_list_next)) {
break;
Expand Down Expand Up @@ -282,17 +282,17 @@ static inline opal_list_item_t *opal_lifo_pop_atomic(opal_lifo_t *lifo)
/* single-threaded versions of the lifo functions */
static inline opal_list_item_t *opal_lifo_push_st(opal_lifo_t *lifo, opal_list_item_t *item)
{
item->opal_list_next = (opal_list_item_t *) lifo->opal_lifo_head.data.item;
item->opal_list_next = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item);
item->item_free = 0;
lifo->opal_lifo_head.data.item = (intptr_t) item;
opal_atomic_store_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item, (intptr_t) item);
return (opal_list_item_t *) item->opal_list_next;
}

static inline opal_list_item_t *opal_lifo_pop_st(opal_lifo_t *lifo)
{
opal_list_item_t *item;
item = (opal_list_item_t *) lifo->opal_lifo_head.data.item;
lifo->opal_lifo_head.data.item = (intptr_t) item->opal_list_next;
item = (opal_list_item_t *) opal_atomic_load_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item);
opal_atomic_store_ptr_volatile_relaxed(&lifo->opal_lifo_head.data.item, (intptr_t) item->opal_list_next);
if (item == &lifo->opal_lifo_ghost) {
return NULL;
}
Expand Down
4 changes: 2 additions & 2 deletions opal/class/opal_list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ typedef struct opal_list_t opal_list_t;
#define OPAL_LIST_DESTRUCT(list) \
do { \
opal_list_item_t *it; \
if (1 == ((opal_object_t *) (list))->obj_reference_count) { \
if (1 == opal_atomic_load_32_relaxed(&((opal_object_t *) (list))->obj_reference_count)) { \
while (NULL != (it = opal_list_remove_first(list))) { \
OBJ_RELEASE(it); \
} \
Expand All @@ -179,7 +179,7 @@ typedef struct opal_list_t opal_list_t;
#define OPAL_LIST_RELEASE(list) \
do { \
opal_list_item_t *it; \
if (1 == ((opal_object_t *) (list))->obj_reference_count) { \
if (1 == opal_atomic_load_32_relaxed(&((opal_object_t *) (list))->obj_reference_count)) { \
while (NULL != (it = opal_list_remove_first(list))) { \
OBJ_RELEASE(it); \
} \
Expand Down
10 changes: 5 additions & 5 deletions opal/class/opal_object.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,12 +172,12 @@ extern int opal_class_init_epoch;
# define OPAL_OBJ_STATIC_INIT(BASE_CLASS) \
{ \
.obj_magic_id = OPAL_OBJ_MAGIC_ID, .obj_class = OBJ_CLASS(BASE_CLASS), \
.obj_reference_count = 1, .cls_init_file_name = __FILE__, .cls_init_lineno = __LINE__, \
.obj_reference_count = { .value = 1 }, .cls_init_file_name = __FILE__, .cls_init_lineno = __LINE__, \
}
#else
# define OPAL_OBJ_STATIC_INIT(BASE_CLASS) \
{ \
.obj_class = OBJ_CLASS(BASE_CLASS), .obj_reference_count = 1, \
.obj_class = OBJ_CLASS(BASE_CLASS), .obj_reference_count = { .value = 1 }, \
}
#endif

Expand Down Expand Up @@ -275,7 +275,7 @@ static inline opal_object_t *opal_obj_new_debug(opal_class_t *type, const char *
assert(NULL != ((opal_object_t *) (object))->obj_class); \
assert(OPAL_OBJ_MAGIC_ID == ((opal_object_t *) (object))->obj_magic_id); \
opal_obj_update((opal_object_t *) (object), 1); \
assert(((opal_object_t *) (object))->obj_reference_count >= 0); \
assert(opal_atomic_load_32_relaxed(&((opal_object_t *) (object))->obj_reference_count) >= 0); \
} while (0)
#else
# define OBJ_RETAIN(object) opal_obj_update((opal_object_t *) (object), 1);
Expand Down Expand Up @@ -377,7 +377,7 @@ static inline opal_object_t *opal_obj_new_debug(opal_class_t *type, const char *
opal_class_initialize((type)); \
} \
((opal_object_t *) (object))->obj_class = (type); \
((opal_object_t *) (object))->obj_reference_count = 1; \
opal_atomic_store_32_relaxed(&((opal_object_t *) (object))->obj_reference_count, 1); \
opal_obj_run_constructors((opal_object_t *) (object)); \
OBJ_REMEMBER_FILE_AND_LINENO(object, __FILE__, __LINE__); \
} while (0)
Expand Down Expand Up @@ -499,7 +499,7 @@ static inline opal_object_t *opal_obj_new(opal_class_t *cls)
}
if (NULL != object) {
object->obj_class = cls;
object->obj_reference_count = 1;
opal_atomic_store_32_relaxed(&object->obj_reference_count, 1);
opal_obj_run_constructors(object);
}
return object;
Expand Down
2 changes: 1 addition & 1 deletion opal/datatype/opal_datatype_destroy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ int32_t opal_datatype_destroy(opal_datatype_t **dt)
{
opal_datatype_t *pData = *dt;

if ((pData->flags & OPAL_DATATYPE_FLAG_PREDEFINED) && (pData->super.obj_reference_count <= 1)) {
if ((pData->flags & OPAL_DATATYPE_FLAG_PREDEFINED) && (opal_atomic_load_32_relaxed(&pData->super.obj_reference_count) <= 1)) {
return OPAL_ERROR;
}

Expand Down
6 changes: 3 additions & 3 deletions opal/include/opal/sys/atomic_impl_math.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
{ \
type oldval; \
do { \
oldval = *addr; \
oldval = opal_atomic_load_##bits##_relaxed(addr); \
} while (!opal_atomic_compare_exchange_strong_##bits(addr, &oldval, \
oldval operation value)); \
\
Expand All @@ -43,9 +43,9 @@
{ \
type oldval, newval; \
do { \
oldval = *addr; \
oldval = opal_atomic_load_##bits##_relaxed(addr); \
newval = oldval operation value; \
} while (!opal_atomic_compare_exchange_strong_##bits(addr, &oldval, newval); \
} while (!opal_atomic_compare_exchange_strong_##bits(addr, &oldval, newval)); \
\
return newval; \
}
Expand Down
8 changes: 4 additions & 4 deletions opal/include/opal/sys/atomic_impl_minmax_math.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@

static inline int32_t opal_atomic_fetch_min_32(opal_atomic_int32_t *addr, int32_t value)
{
int32_t old = *addr;
int32_t old = opal_atomic_load_32_relaxed(addr);
do {
if (old <= value) {
break;
Expand All @@ -49,7 +49,7 @@ static inline int32_t opal_atomic_min_fetch_32(opal_atomic_int32_t *addr, int32_

static inline int32_t opal_atomic_fetch_max_32(opal_atomic_int32_t *addr, int32_t value)
{
int32_t old = *addr;
int32_t old = opal_atomic_load_32_relaxed(addr);
do {
if (old >= value) {
break;
Expand All @@ -67,7 +67,7 @@ static inline int32_t opal_atomic_max_fetch_32(opal_atomic_int32_t *addr, int32_

static inline int64_t opal_atomic_fetch_min_64(opal_atomic_int64_t *addr, int64_t value)
{
int64_t old = *addr;
int64_t old = opal_atomic_load_64_relaxed(addr);
do {
if (old <= value) {
break;
Expand All @@ -79,7 +79,7 @@ static inline int64_t opal_atomic_fetch_min_64(opal_atomic_int64_t *addr, int64_

static inline int64_t opal_atomic_fetch_max_64(opal_atomic_int64_t *addr, int64_t value)
{
int64_t old = *addr;
int64_t old = opal_atomic_load_64_relaxed(addr);
do {
if (old >= value) {
break;
Expand Down
Loading
Loading