Skip to content

Commit b71e18c

Browse files
committed
Fix some race conditions in xca6ll/hot/amcssth.
* Use memory-model-aware builtins in GCC and Clang when a memory location may be written by one thread and read by another, avoiding race conditions due to out-of-order updates on ARM. * Call dylan_make_wrappers while the test is still single-threaded, preventing multiple threads from racing to call it. * Prevent dylan_init from creating a padding object, as we must not have an exact root pointing at a padding object.
1 parent 136b2b6 commit b71e18c

File tree

3 files changed

+46
-20
lines changed

3 files changed

+46
-20
lines changed

code/amcssth.c

Lines changed: 24 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -108,19 +108,22 @@ static mps_res_t area_scan(mps_ss_t ss, void *base, void *limit, void *closure)
108108

109109
static void churn(mps_ap_t ap, size_t roots_count)
110110
{
111-
size_t i;
112-
size_t r;
111+
size_t i, j, r;
113112

114113
++objs;
115114
r = (size_t)rnd();
116115
if (r & 1) {
116+
mps_addr_t root;
117117
i = (r >> 1) % exactRootsCOUNT;
118-
if (exactRoots[i] != objNULL)
119-
cdie(dylan_check(exactRoots[i]), "dying root check");
120-
exactRoots[i] = make(ap, roots_count);
121-
if (exactRoots[(exactRootsCOUNT-1) - i] != objNULL)
122-
dylan_write(exactRoots[(exactRootsCOUNT-1) - i],
123-
exactRoots, exactRootsCOUNT);
118+
atomic_load(&exactRoots[i], &root);
119+
if (root != objNULL)
120+
cdie(dylan_check(root), "dying root check");
121+
root = make(ap, roots_count);
122+
atomic_store(&exactRoots[i], &root);
123+
j = exactRootsCOUNT - i - 1;
124+
atomic_load(&exactRoots[j], &root);
125+
if (root != objNULL)
126+
dylan_write(root, exactRoots, exactRootsCOUNT);
124127
} else {
125128
i = (r >> 1) % ambigRootsCOUNT;
126129
ambigRoots[(ambigRootsCOUNT-1) - i] = make(ap, roots_count);
@@ -221,9 +224,11 @@ static void test_pool(const char *name, mps_pool_t pool, size_t roots_count)
221224
(unsigned long)collections, objs,
222225
(unsigned long)mps_arena_committed(arena));
223226

224-
for (i = 0; i < exactRootsCOUNT; ++i)
225-
cdie(exactRoots[i] == objNULL || dylan_check(exactRoots[i]),
226-
"all roots check");
227+
for (i = 0; i < exactRootsCOUNT; ++i) {
228+
mps_addr_t root;
229+
atomic_load(&exactRoots[i], &root);
230+
cdie(root == objNULL || dylan_check(root), "all roots check");
231+
}
227232

228233
if (collections >= collectionsCOUNT / 2 && !walked)
229234
{
@@ -248,9 +253,12 @@ static void test_pool(const char *name, mps_pool_t pool, size_t roots_count)
248253
ramping = 0;
249254
/* kill half of the roots */
250255
for(i = 0; i < exactRootsCOUNT; i += 2) {
251-
if (exactRoots[i] != objNULL) {
252-
cdie(dylan_check(exactRoots[i]), "ramp kill check");
253-
exactRoots[i] = objNULL;
256+
mps_addr_t root;
257+
atomic_load(&exactRoots[i], &root);
258+
if (root != objNULL) {
259+
cdie(dylan_check(root), "ramp kill check");
260+
root = objNULL;
261+
atomic_store(&exactRoots[i], &root);
254262
}
255263
}
256264
}
@@ -298,6 +306,8 @@ static void test_arena(void)
298306
mps_pool_t amc_pool, amcz_pool;
299307
void *marker = &marker;
300308

309+
die(dylan_make_wrappers(), "make wrappers");
310+
301311
MPS_ARGS_BEGIN(args) {
302312
MPS_ARGS_ADD(args, MPS_KEY_ARENA_SIZE, testArenaSIZE);
303313
MPS_ARGS_ADD(args, MPS_KEY_ARENA_GRAIN_SIZE, rnd_grain(testArenaSIZE));

code/fmtdytst.c

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ mps_res_t dylan_make_wrappers(void)
6868
return MPS_RES_OK;
6969
}
7070

71-
/* dylan_init -- turn raw memory into initialised dylan-vector (or pad)
71+
/* dylan_init -- turn raw memory into initialised dylan-vector
7272
*
7373
* If the raw memory is large enough, initialises it to a dylan-vector,
7474
* whose slots are initialised to either dylan-ints, or valid refs, at
@@ -78,8 +78,6 @@ mps_res_t dylan_make_wrappers(void)
7878
* and "nr_refs" arguments. If "nr_refs" is 0, all slots are
7979
* initialized to dylan-ints: this may be useful for making leaf
8080
* objects.
81-
*
82-
* (Makes a pad if the raw memory is too small to hold a dylan-vector)
8381
*/
8482

8583
mps_res_t dylan_init(mps_addr_t addr, size_t size,
@@ -93,8 +91,7 @@ mps_res_t dylan_init(mps_addr_t addr, size_t size,
9391
if (res != MPS_RES_OK)
9492
return res;
9593

96-
/* If there is enough room, make a vector, otherwise just */
97-
/* make a padding object. */
94+
/* If there is enough room, make a vector. */
9895
if(size >= sizeof(mps_word_t) * 2) {
9996
mps_word_t *p = (mps_word_t *)addr;
10097
mps_word_t i, t = (size / sizeof(mps_word_t)) - 2;
@@ -110,7 +107,7 @@ mps_res_t dylan_init(mps_addr_t addr, size_t size,
110107
p[2+i] = (mps_word_t)refs[(r >> 1) % nr_refs]; /* random ptr */
111108
}
112109
} else {
113-
dylan_pad(addr, size);
110+
return MPS_RES_UNIMPL;
114111
}
115112

116113
return MPS_RES_OK;

code/testlib.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,25 @@ extern void randomize(int argc, char *argv[]);
292292
extern void testlib_init(int argc, char *argv[]);
293293

294294

295+
/* Memory-model-aware operations */
296+
297+
#if defined(MPS_BUILD_GC) || defined(MPS_BUILD_LL)
298+
299+
/* See <https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html>
300+
* and <https://clang.llvm.org/docs/LanguageExtensions.html> */
301+
#define atomic_load(SRC, DEST) __atomic_load(SRC, DEST, __ATOMIC_ACQUIRE)
302+
#define atomic_store(DEST, SRC) __atomic_store(DEST, SRC, __ATOMIC_RELEASE)
303+
304+
#elif defined(MPS_BUILD_MV)
305+
306+
/* Microsoft Visual C/C++ does not need memory-model-aware load and store as
307+
* loads and stores of register-sized values are atomic on Intel. */
308+
#define atomic_load(SRC, DEST) (*(DEST) = *(SRC))
309+
#define atomic_store(DEST, SRC) (*(DEST) = *(SRC))
310+
311+
#endif
312+
313+
295314
#endif /* testlib_h */
296315

297316

0 commit comments

Comments
 (0)