Skip to content

Commit 43f291e

Browse files
ViralBShahclaude
andauthored
ld80: make powl() thread-safe (#222) (#355)
powl() in ld80/e_powl.c kept its working scratch values (z, w, W, Wa, Wb, ya, yb, u) in file-scope `static` variables. Two threads calling powl() concurrently clobbered each other's scratch, so a call could return arbitrary garbage -- e.g. the reproducer from the issue, powl(0x8.779021e7c2f81b2p+14982L, -0x8.0021b03e1f821c9p-15097L), which is exactly 1, would instead come back as huge values or inf. Move that scratch to local automatic variables inside powl(). The `volatile` on z is preserved (as a local volatile) because it is load-bearing for the rounding/underflow behaviour, exactly like the twom10000 constant. The math is unchanged. reducl() and powil() already used local automatics and were unaffected. Add test/regression/test-222.c: it spawns 8 threads, each repeatedly computing a different power with a known-exact result, and fails if any call ever deviates. Before the fix this fails essentially every run; after it, it is solid. It skips (77) when long double is not the 80-bit type or when threads cannot be started. The regression Makefile rule gains -pthread (harmless for the single-threaded tests). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 2afd671 commit 43f291e

3 files changed

Lines changed: 136 additions & 3 deletions

File tree

ld80/e_powl.c

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,6 @@ static long double R[] = {
189189
static const long double MAXLOGL = 1.1356523406294143949492E4L;
190190
static const long double MINLOGL = -1.13994985314888605586758E4L;
191191
static const long double LOGE2L = 6.9314718055994530941723E-1L;
192-
static volatile long double z;
193-
static long double w, W, Wa, Wb, ya, yb, u;
194192
static const long double huge = 0x1p10000L;
195193
#if 0 /* XXX Prevent gcc from erroneously constant folding this. */
196194
static const long double twom10000 = 0x1p-10000L;
@@ -208,6 +206,13 @@ powl(long double x, long double y)
208206
int i, nflg, iyflg, yoddint;
209207
long e;
210208

209+
/* Local scratch (formerly file-scope statics, which made powl()
210+
* non-reentrant and unsafe to call from multiple threads). The
211+
* volatile on z is load-bearing: it forces the rounding/underflow
212+
* behaviour of the final products, just as twom10000 does below. */
213+
volatile long double z;
214+
long double w, W, Wa, Wb, ya, yb, u;
215+
211216
if( y == 0.0L )
212217
return( 1.0L );
213218

test/Makefile

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,10 @@ REGRESSION_BINS := $(REGRESSION_SRCS:.c=)
2121

2222
regression-build: $(REGRESSION_BINS)
2323

24+
# -pthread lets thread-safety regressions (e.g. test-222) spawn threads. It is
25+
# harmless for the single-threaded tests and is a no-op stub on glibc >= 2.34.
2426
regression/%: regression/%.c regression/regress-util.h
25-
$(CC) $(CPPFLAGS) $(CFLAGS) $(CFLAGS_add) $(LDFLAGS) $(LDFLAGS_arch) $< -D__BSD_VISIBLE -I ../include -I../src $(OPENLIBM_LIB) -o $@
27+
$(CC) $(CPPFLAGS) $(CFLAGS) $(CFLAGS_add) $(LDFLAGS) $(LDFLAGS_arch) $< -D__BSD_VISIBLE -I ../include -I../src $(OPENLIBM_LIB) -pthread -o $@
2628

2729
bench: bench-syslibm bench-openlibm
2830

test/regression/test-222.c

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
/*
2+
* Regression test for issue #222: powl() is not thread-safe.
3+
* https://github.com/JuliaMath/openlibm/issues/222
4+
*
5+
* The ld80 implementation (ld80/e_powl.c) kept its working scratch values
6+
* (z, w, W, Wa, Wb, ya, yb, u) in file-scope `static` variables. When two
7+
* threads ran powl() at the same time they clobbered each other's scratch,
8+
* so a call could return arbitrary garbage -- e.g. the reproducer below,
9+
* powl(0x8.779021e7c2f81b2p+14982L, -0x8.0021b03e1f821c9p-15097L), which is
10+
* exactly 1, would instead come back as huge values or inf.
11+
*
12+
* The fix moves that scratch to local automatic variables. This test drives
13+
* several threads, each repeatedly computing a *different* power whose result
14+
* is known exactly, and fails if any call ever deviates. With the bug the
15+
* cross-thread contention is detected essentially every run; after the fix it
16+
* is rock solid.
17+
*
18+
* Skips (exit 77) where it cannot apply: when long double is not the 80-bit
19+
* extended type this file targets, or when threads cannot be started.
20+
*/
21+
#include <float.h>
22+
#include <math.h>
23+
24+
#include "regress-util.h"
25+
26+
/*
27+
* ld80-specific: the race was in ld80/e_powl.c, which openlibm builds only on
28+
* x86. Restrict to the 80-bit format so every other arch compiles to a bare
29+
* skip with no reference to powl (it may be absent, breaking the link) and the
30+
* pthread code is only built where the test actually runs (natively, never
31+
* under cross-qemu).
32+
*/
33+
#if LDBL_MANT_DIG != 64
34+
35+
int
36+
main(void)
37+
{
38+
return REGRESS_SKIP; /* not the 80-bit long double powl */
39+
}
40+
41+
#else
42+
43+
#include <pthread.h>
44+
45+
#define NTHREADS 8
46+
#define NITERS 20000
47+
48+
/*
49+
* Each job is a (base, exponent, expected) triple chosen so the result is
50+
* representable exactly in 80-bit long double and goes through the full
51+
* mantissa/log path of powl() (rather than an early special-case return),
52+
* which is where the shared scratch lived. Mixing distinct values across
53+
* threads is what makes the old race observable.
54+
*/
55+
struct job {
56+
long double x;
57+
long double y;
58+
long double expect;
59+
};
60+
61+
static const struct job jobs[NTHREADS] = {
62+
/* the exact reproducer from the issue: result is 1 */
63+
{ 0x8.779021e7c2f81b2p+14982L, -0x8.0021b03e1f821c9p-15097L, 1.0L },
64+
{ 2.0L, 10.0L, 1024.0L },
65+
{ 3.0L, 7.0L, 2187.0L },
66+
{ 1.5L, 4.0L, 5.0625L },
67+
{ 10.0L, 3.0L, 1000.0L },
68+
{ 7.0L, 2.0L, 49.0L },
69+
{ 5.0L, 3.0L, 125.0L },
70+
{ 2.0L, -3.0L, 0.125L },
71+
};
72+
73+
static volatile int failed;
74+
75+
static void *
76+
worker(void *arg)
77+
{
78+
const struct job *j = (const struct job *)arg;
79+
long i;
80+
81+
for (i = 0; i < NITERS; i++) {
82+
long double r = powl(j->x, j->y);
83+
if (r != j->expect) {
84+
fprintf(stderr,
85+
" powl(%.20Lg, %.20Lg) = %.20Lg, expected %.20Lg\n",
86+
j->x, j->y, r, j->expect);
87+
failed = 1;
88+
return NULL;
89+
}
90+
}
91+
return NULL;
92+
}
93+
94+
int
95+
main(void)
96+
{
97+
pthread_t t[NTHREADS];
98+
int i, started;
99+
100+
/* Sanity: every job must be correct single-threaded first. */
101+
for (i = 0; i < NTHREADS; i++)
102+
CHECK(powl(jobs[i].x, jobs[i].y) == jobs[i].expect);
103+
104+
started = 0;
105+
for (i = 0; i < NTHREADS; i++) {
106+
if (pthread_create(&t[i], NULL, worker,
107+
(void *)&jobs[i]) != 0)
108+
break;
109+
started++;
110+
}
111+
112+
if (started < 2) {
113+
/* Could not get real concurrency; nothing meaningful to test. */
114+
for (i = 0; i < started; i++)
115+
pthread_join(t[i], NULL);
116+
return REGRESS_SKIP;
117+
}
118+
119+
for (i = 0; i < started; i++)
120+
pthread_join(t[i], NULL);
121+
122+
CHECK(failed == 0);
123+
return 0;
124+
}
125+
126+
#endif /* LDBL_MANT_DIG */

0 commit comments

Comments
 (0)