Skip to content

Commit 304b96b

Browse files
ViralBShahclaude
andcommitted
ci: add -Werror strict lane and auto-discovered regression harness
Two pieces of CI hardening (#315): * WERROR opt-in (Make.inc) and a dedicated `strict-werror` CI job that builds with -Werror under both gcc and clang on a pinned ubuntu-24.04. It is off by default and pinned so a future compiler upgrade surfaces as one isolated failure rather than breaking the whole matrix. Fixes the warnings this surfaces in openlibm's own code: - irint(): mark the header-defined helper __unused (it is only used by e_rem_pio2*.c, so it is unused in every other TU). - lgamma_r/lgammal_r: zero-init nadj, which is written and read under the same hx<0 guard (-Wmaybe-uninitialized false positive). Clang-only -Wimplicit-int-float-conversion (intentional lround boundary trick) and -Wignored-attributes (pre-existing extern-inline fenv redeclarations) are documented and suppressed for the lane. * Regression harness: test/regression/*.c are self-contained programs (exit 0=pass, 77=skip, else=fail) auto-discovered and run by `make test` from the repo root, so later fixes can land a regression test by dropping in one file with no Makefile changes. The previously orphaned test-211.c (never built or run) is moved in as the first one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 3329022 commit 304b96b

11 files changed

Lines changed: 131 additions & 15 deletions

File tree

.github/workflows/ci.yml

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,25 @@ jobs:
5252
install: base-devel mingw-w64-${{matrix.env}}-toolchain
5353
- run: make -j$(nproc)
5454
- run: make test
55+
strict-warnings:
56+
# Dedicated -Werror lane. Pinned to a fixed image (not ubuntu-latest) so a
57+
# compiler upgrade surfaces here as an explicit, isolated failure instead of
58+
# breaking the whole matrix. See WERROR in Make.inc.
59+
name: strict-werror (${{ matrix.cc }})
60+
runs-on: ubuntu-24.04
61+
timeout-minutes: 10
62+
strategy:
63+
fail-fast: false
64+
matrix:
65+
include:
66+
- { cc: gcc, flags: "" }
67+
- { cc: clang, flags: "USECLANG=1 USEGCC=0" }
68+
steps:
69+
- uses: actions/checkout@v7
70+
- name: Build with -Werror
71+
run: make -j$(nproc) WERROR=1 ${{ matrix.flags }}
72+
- name: Test
73+
run: make test ${{ matrix.flags }}
5574
code-coverage:
5675
runs-on: ubuntu-latest
5776
timeout-minutes: 10

Make.inc

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,21 @@ SFLAGS_add += $(SFLAGS_arch)
159159
LDFLAGS_add += $(LDFLAGS_arch)
160160

161161
CFLAGS_add += -std=c99 -Wall -I$(OPENLIBM_HOME) -I$(OPENLIBM_HOME)/include -I$(OPENLIBM_HOME)/$(ARCH) -I$(OPENLIBM_HOME)/src -DASSEMBLER -D__BSD_VISIBLE -Wno-implicit-function-declaration
162+
163+
# Treat warnings as errors. Off by default so a compiler upgrade can't break a
164+
# normal build; a dedicated CI lane sets WERROR=1 to catch new warnings early.
165+
WERROR ?= 0
166+
ifeq ($(WERROR),1)
167+
CFLAGS_add += -Werror
168+
# Two clang-only diagnostics are silenced for the strict lane (gcc ignores
169+
# unknown -Wno-* options, so this is safe for both compilers):
170+
# * -Wimplicit-int-float-conversion: s_lround/s_llround deliberately round
171+
# DTYPE_MAX through (dtype)x; the boundary arithmetic is correct by design.
172+
# * -Wignored-attributes: the extern-inline fenv re-declarations in
173+
# openlibm_fenv_*.h trip a harmless attribute-placement diagnostic; tracked
174+
# separately rather than reworked under the warnings sweep.
175+
CFLAGS_add += -Wno-implicit-int-float-conversion -Wno-ignored-attributes
176+
endif
162177
ifneq ($(filter $(ARCH),i387 amd64 powerpc),)
163178
CFLAGS_add += -I$(OPENLIBM_HOME)/ld80
164179
else

Makefile

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,9 +49,15 @@ endif
4949
LDFLAGS_add += -Wl,$(SONAME_FLAG),libopenlibm.$(OLM_MAJOR_SHLIB_EXT)
5050
endif
5151

52-
.PHONY: all check test clean distclean \
52+
.PHONY: all check test test-regression clean distclean \
5353
install install-static install-shared install-pkgconfig install-headers
5454

55+
# Per-issue regression programs live in test/regression/*.c. They are run from
56+
# the repo root (like test-double/test-float) so the freshly built shared
57+
# library is found the same way on every platform.
58+
REGRESSION_SRCS := $(sort $(wildcard test/regression/*.c))
59+
REGRESSION_BINS := $(REGRESSION_SRCS:.c=)
60+
5561

5662
OLM_LIBS := libopenlibm.a
5763
ifneq ($(ARCH), wasm32)
@@ -63,6 +69,21 @@ all : $(OLM_LIBS)
6369
check test: test/test-double test/test-float
6470
test/test-double
6571
test/test-float
72+
$(MAKE) test-regression
73+
74+
# Build (in test/) then run each regression program from the repo root.
75+
# Exit status 0 = pass, 77 = skipped (not applicable here), anything else = fail.
76+
test-regression: libopenlibm.$(OLM_MAJOR_MINOR_SHLIB_EXT)
77+
$(MAKE) -C test regression-build
78+
@pass=0; skip=0; fail=0; \
79+
for t in $(REGRESSION_BINS); do \
80+
./$$t; rc=$$?; \
81+
if [ "$$rc" = "0" ]; then echo " PASS $$t"; pass=`expr $$pass + 1`; \
82+
elif [ "$$rc" = "77" ]; then echo " SKIP $$t"; skip=`expr $$skip + 1`; \
83+
else echo " FAIL $$t (exit $$rc)"; fail=`expr $$fail + 1`; fi; \
84+
done; \
85+
echo "regression: $$pass passed, $$skip skipped, $$fail failed"; \
86+
[ "$$fail" = "0" ]
6687

6788
libopenlibm.a: $(OBJS)
6889
$(AR) -rcs libopenlibm.a $(OBJS)

ld80/e_lgammal_r.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -271,6 +271,7 @@ lgammal_r(long double x, int *signgamp)
271271
long double t, y, z, nadj, p, p1, p2, q, r, w;
272272
int i, ix;
273273
u_int32_t se, i0, i1;
274+
nadj = 0; /* only set & used when hx<0; init silences -Wmaybe-uninitialized */
274275

275276
*signgamp = 1;
276277
GET_LDOUBLE_WORDS (se, i0, i1, x);

src/e_lgamma_r.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,7 @@ __ieee754_lgamma_r(double x, int *signgamp)
208208
double t,y,z,nadj,p,p1,p2,p3,q,r,w;
209209
int32_t hx;
210210
int i,lx,ix;
211+
nadj = 0; /* only set & used when hx<0; init silences -Wmaybe-uninitialized */
211212

212213
EXTRACT_WORDS(hx,lx,x);
213214

src/math_private.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -253,7 +253,7 @@ void __scan_nan(u_int32_t *__words, int __num_words, const char *__s);
253253
/* Asm versions of some functions. */
254254

255255
#ifdef __amd64__
256-
static __inline int
256+
static __inline int __attribute__((__unused__))
257257
irint(double x)
258258
{
259259
int n;
@@ -265,7 +265,7 @@ irint(double x)
265265
#endif
266266

267267
#ifdef __i386__
268-
static __inline int
268+
static __inline int __attribute__((__unused__))
269269
irint(double x)
270270
{
271271
int n;

test/.gitignore

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,8 @@
77
/bench-openlibm
88
/bench-syslibm
99
/*.exe
10+
# Built regression programs (keep the .c sources and the shared header).
11+
/regression/test-*
12+
!/regression/test-*.c
13+
/regression/*.exe
14+
/regression/*.dSYM

test/Makefile

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,17 @@ endif
1313

1414
all: test-double test-float # test-double-system test-float-system
1515

16+
# Per-issue regression tests: every test/regression/*.c is a self-contained
17+
# program named after the issue it covers. New files are picked up automatically
18+
# (no Makefile edit needed); the top-level Makefile runs them via `make test`.
19+
REGRESSION_SRCS := $(sort $(wildcard regression/*.c))
20+
REGRESSION_BINS := $(REGRESSION_SRCS:.c=)
21+
22+
regression-build: $(REGRESSION_BINS)
23+
24+
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 $@
26+
1627
bench: bench-syslibm bench-openlibm
1728

1829
test-double: test-double.c libm-test.c libm-test-ulps.h
@@ -35,3 +46,6 @@ bench-syslibm: libm-bench.cpp
3546

3647
clean:
3748
rm -fr test-double test-float test-double-system test-float-system bench-openlibm bench-syslibm *.dSYM
49+
rm -fr $(REGRESSION_BINS) $(addsuffix .exe,$(REGRESSION_BINS)) regression/*.dSYM
50+
51+
.PHONY: all bench regression-build clean

test/regression/regress-util.h

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/*
2+
* Minimal helpers for openlibm standalone regression tests.
3+
*
4+
* Each regression test is a self-contained program named after the issue it
5+
* covers (test/regression/test-<issue>.c). The harness in test/Makefile builds
6+
* and runs every file in this directory and interprets the exit status as:
7+
*
8+
* 0 the test passed
9+
* 77 the test does not apply on this platform and was skipped
10+
* * the test failed
11+
*
12+
* Use REGRESS_SKIP for the skip status and CHECK() for assertions so a failing
13+
* test prints which condition broke and where.
14+
*/
15+
#ifndef OPENLIBM_REGRESS_UTIL_H
16+
#define OPENLIBM_REGRESS_UTIL_H
17+
18+
#include <stdio.h>
19+
20+
#define REGRESS_SKIP 77
21+
22+
#define CHECK(cond) \
23+
do { \
24+
if (!(cond)) { \
25+
fprintf(stderr, " check failed: %s (%s:%d)\n", \
26+
#cond, __FILE__, __LINE__); \
27+
return 1; \
28+
} \
29+
} while (0)
30+
31+
#endif /* OPENLIBM_REGRESS_UTIL_H */

test/regression/test-211.c

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/*
2+
* Regression test for issue #211: loss of precision in powf.
3+
* https://github.com/JuliaMath/openlibm/issues/211
4+
*
5+
* Fixed in 98f8713 by importing the latest msun powf; this pins the exact
6+
* result so the regression cannot silently return.
7+
*/
8+
#include <math.h>
9+
10+
#include "regress-util.h"
11+
12+
int
13+
main(void)
14+
{
15+
float x = 0xd.65874p-4f;
16+
float y = 4.0f;
17+
float z = powf(x, y);
18+
19+
CHECK(z == 0x1.f74424p-2f);
20+
return 0;
21+
}

0 commit comments

Comments
 (0)