Skip to content

Commit a38a638

Browse files
committed
Add regression tests
1 parent 09b66ca commit a38a638

3 files changed

Lines changed: 328 additions & 8 deletions

File tree

.github/workflows/Linux-libretro-common-samples.yml

Lines changed: 54 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,8 @@ jobs:
8080
task_queue_title_error_test
8181
tpool_wait_test
8282
retro_atomic_test
83+
retro_atomic_extern_c_linkage_test
84+
retro_atomic_extern_c_linkage_test_cxx
8385
retro_spsc_test
8486
)
8587
@@ -212,6 +214,25 @@ jobs:
212214
# behavioural SPSC stress is already covered by the C test
213215
# binary above on this same host, and the C++11 backend bottoms
214216
# out through the same libstdc++ __atomic_* builtins.
217+
#
218+
# Two fixtures share the g++/clang++ x c++11/14/17 sweep:
219+
#
220+
# cxx_smoke.cpp : header included at C++ linkage,
221+
# the "ordinary" C++ TU shape.
222+
# cxx_smoke_extern_c.cpp : header included from inside an
223+
# extern "C" { ... } wrap, the
224+
# shape ui_qt.cpp uses under
225+
# !CXX_BUILD. This is the
226+
# regression case for the
227+
# RETRO_BEGIN_DECLS_CXX shield in
228+
# retro_atomic.h's C++11 backend.
229+
# The standalone build of the
230+
# sample at samples/atomic/
231+
# retro_atomic_extern_c_linkage/
232+
# covers the same case under the
233+
# default toolchain; this matrix
234+
# extends coverage to clang++ and
235+
# the higher language standards.
215236
set -u
216237
set -o pipefail
217238
@@ -254,16 +275,41 @@ jobs:
254275
}
255276
EOF
256277
278+
# Regression fixture: include retro_atomic.h from inside an
279+
# extern "C" wrap. Faithful mirror of ui_qt.cpp's pattern.
280+
# Pre-fix retro_atomic.h would emit ~80 "template with C
281+
# linkage" errors here; the RETRO_BEGIN_DECLS_CXX shield
282+
# around <atomic> makes this compile cleanly.
283+
cat > "$tmpdir/cxx_smoke_extern_c.cpp" <<'EOF'
284+
#include <cstdio>
285+
#include <cstddef>
286+
287+
extern "C" {
288+
#include <retro_atomic.h>
289+
}
290+
291+
int main(void) {
292+
retro_atomic_int_t ai; retro_atomic_int_init(&ai, 0);
293+
retro_atomic_store_release_int(&ai, 7);
294+
int v = retro_atomic_load_acquire_int(&ai);
295+
std::printf("backend: %s\n", RETRO_ATOMIC_BACKEND_NAME);
296+
std::puts(v == 7 ? "ALL OK" : "FAIL");
297+
return v == 7 ? 0 : 1;
298+
}
299+
EOF
300+
257301
for cxx in g++ clang++; do
258302
for std in c++11 c++14 c++17; do
259-
echo "==> compile-test with $cxx -std=$std"
260-
$cxx -std=$std -Wall -Wextra -pedantic -O2 \
261-
-I include \
262-
"$tmpdir/cxx_smoke.cpp" \
263-
-o "$tmpdir/cxx_smoke" \
264-
|| { echo "::error title=C++ compile failed::$cxx -std=$std"; exit 1; }
265-
"$tmpdir/cxx_smoke" \
266-
|| { echo "::error title=C++ smoke failed::$cxx -std=$std"; exit 1; }
303+
for fixture in cxx_smoke cxx_smoke_extern_c; do
304+
echo "==> compile-test with $cxx -std=$std ($fixture)"
305+
$cxx -std=$std -Wall -Wextra -pedantic -O2 \
306+
-I include \
307+
"$tmpdir/$fixture.cpp" \
308+
-o "$tmpdir/$fixture" \
309+
|| { echo "::error title=C++ compile failed::$cxx -std=$std $fixture"; exit 1; }
310+
"$tmpdir/$fixture" \
311+
|| { echo "::error title=C++ smoke failed::$cxx -std=$std $fixture"; exit 1; }
312+
done
267313
done
268314
done
269315
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
TARGET := retro_atomic_extern_c_linkage_test
2+
TARGET_TEST := retro_atomic_extern_c_linkage_test_cxx
3+
4+
LIBRETRO_COMM_DIR := ../../..
5+
6+
# Two binaries from one source. The source mirrors ui_qt.cpp's pattern
7+
# of wrapping a RetroArch header include in extern "C" { ... }, gated
8+
# on !CXX_BUILD. We build it once each way:
9+
#
10+
# $(TARGET) : non-unity / separate-TU build (extern "C" wrap is
11+
# active). This is the configuration that triggered
12+
# the original "template with C linkage" failure on
13+
# g++ before the retro_common_api.h fix.
14+
#
15+
# $(TARGET_TEST) : unity / griffin build (CXX_BUILD defined, extern
16+
# "C" wrappers compile out). Validates that the
17+
# fix did not regress the unity-build path.
18+
#
19+
# Both must compile and pass. Build failure on $(TARGET) is the
20+
# regression signal for the original bug.
21+
22+
SOURCE := retro_atomic_extern_c_linkage_test.cpp
23+
24+
# C++11 is the minimum that engages retro_atomic.h's CXX11 backend.
25+
# -Wall -Wextra to catch any incidental warning the test's macro
26+
# expansions might produce on a future toolchain.
27+
CXXFLAGS += -Wall -Wextra -pedantic -std=c++11 -g -O0 \
28+
-I$(LIBRETRO_COMM_DIR)/include
29+
30+
ifneq ($(SANITIZER),)
31+
CXXFLAGS := -fsanitize=$(SANITIZER) -fno-omit-frame-pointer $(CXXFLAGS)
32+
LDFLAGS := -fsanitize=$(SANITIZER) $(LDFLAGS)
33+
endif
34+
35+
# The unity-build target gets CXX_BUILD defined, exactly as
36+
# Makefile.griffin does for griffin_cpp.cpp's TUs.
37+
$(TARGET_TEST): CXXFLAGS += -DCXX_BUILD
38+
39+
all: $(TARGET) $(TARGET_TEST)
40+
41+
$(TARGET): $(SOURCE)
42+
$(CXX) -o $@ $< $(CXXFLAGS) $(LDFLAGS)
43+
44+
$(TARGET_TEST): $(SOURCE)
45+
$(CXX) -o $@ $< $(CXXFLAGS) $(LDFLAGS)
46+
47+
clean:
48+
rm -f $(TARGET) $(TARGET_TEST)
49+
50+
.PHONY: all clean
Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
/* Copyright (C) 2010-2026 The RetroArch team
2+
*
3+
* ---------------------------------------------------------------------------------------
4+
* The following license statement only applies to this file (retro_atomic_extern_c_linkage_test.cpp).
5+
* ---------------------------------------------------------------------------------------
6+
*
7+
* Permission is hereby granted, free of charge,
8+
* to any person obtaining a copy of this software and associated documentation files (the "Software"),
9+
* to deal in the Software without restriction, including without limitation the rights to
10+
* use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software,
11+
* and to permit persons to whom the Software is furnished to do so, subject to the following conditions:
12+
*
13+
* The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.
14+
*
15+
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED,
16+
* INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
17+
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.
18+
* IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
19+
* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
20+
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
21+
*/
22+
23+
/* Regression test for the linkage hazard in
24+
* libretro-common/include/retro_atomic.h's C++11 backend.
25+
*
26+
* Background
27+
* ----------
28+
* Several C++ TUs in RetroArch wrap their includes of in-tree C
29+
* headers in extern "C" { ... } when CXX_BUILD is not defined. The
30+
* canonical example is ui/drivers/ui_qt.cpp:
31+
*
32+
* #ifndef CXX_BUILD
33+
* extern "C" {
34+
* #endif
35+
* #include "../../menu/menu_driver.h" // pulls in retro_atomic.h
36+
* ...
37+
* #ifndef CXX_BUILD
38+
* }
39+
* #endif
40+
*
41+
* On a C++11+ build this chain reaches retro_atomic.h's CXX11 backend,
42+
* which #include's <atomic>. libstdc++'s <atomic> declares dozens of
43+
* templates and template specializations; under C linkage every one of
44+
* them is rejected with
45+
*
46+
* error: template with C linkage
47+
*
48+
* yielding ~80 errors on a single TU. See the build log on master at
49+
* the time of the original report; same failure on g++ 9 through 13.
50+
*
51+
* Fix
52+
* ---
53+
* retro_common_api.h gained RETRO_BEGIN_DECLS_CXX / RETRO_END_DECLS_CXX
54+
* macros that expand to extern "C++" { ... } in C++ mode (with
55+
* CXX_BUILD off) and to nothing otherwise. retro_atomic.h's C++11
56+
* backend wraps its <atomic> / <cstddef> include in those macros, so
57+
* the standard headers parse at C++ linkage regardless of any
58+
* extern "C" the caller imposes.
59+
*
60+
* What this test asserts
61+
* ----------------------
62+
* 1. Compile-time: a C++ TU that wraps #include <retro_atomic.h>
63+
* in extern "C" { ... } compiles without error. The include
64+
* pattern below is a faithful mirror of ui_qt.cpp's; if the
65+
* header's linkage shield breaks, this file fails to compile
66+
* and that is the regression signal.
67+
*
68+
* 2. Runtime: the macro expansions inside the extern "C" wrap still
69+
* produce live std::atomic<T> objects whose load_acquire /
70+
* store_release / fetch_add / fetch_sub / inc / dec round-trip
71+
* correctly. This guards against a future "fix" that papered
72+
* over the compile error by, say, falling back to a non-atomic
73+
* plain-int typedef under C linkage. exit(0) on success,
74+
* exit(1) on any property failure.
75+
*
76+
* 3. Both build configurations exercise the same source with no
77+
* code-level difference (see Makefile). The Makefile builds
78+
* two targets:
79+
* retro_atomic_extern_c_linkage_test -- !CXX_BUILD path
80+
* retro_atomic_extern_c_linkage_test_cxx -- CXX_BUILD path
81+
* The second one defines CXX_BUILD before including any
82+
* RetroArch headers, which makes the in-tree extern "C" wrappers
83+
* compile out (because RETRO_BEGIN_DECLS / RETRO_END_DECLS expand
84+
* to nothing). Both must build and pass.
85+
*
86+
* What this test does NOT assert
87+
* ------------------------------
88+
* It does not exercise weakly-ordered SMP correctness or thread
89+
* safety -- those are covered by retro_atomic_test (single-thread
90+
* property + SPSC stress) and retro_spsc_test (lock-free SPSC) in
91+
* neighbouring sample directories, and by the cross-arch qemu lane
92+
* in .github/workflows/Linux-libretro-common-samples.yml. This
93+
* test's job is purely to police the include-from-C++ hazard.
94+
*
95+
* Build standalone
96+
* ----------------
97+
* make clean all
98+
* ./retro_atomic_extern_c_linkage_test
99+
* ./retro_atomic_extern_c_linkage_test_cxx
100+
*
101+
* Both binaries exit 0 on success. A pre-fix retro_atomic.h would
102+
* fail at the build step rather than at runtime.
103+
*/
104+
105+
/* Mirror of ui_qt.cpp's include guard pattern. retro_common_api.h
106+
* (transitively included from retro_atomic.h) keys on this:
107+
*
108+
* #ifdef CXX_BUILD
109+
* -- no extern "C" wrappers anywhere in libretro-common headers --
110+
* #else
111+
* -- normal RETRO_BEGIN_DECLS = extern "C" { wrapping --
112+
* #endif
113+
*
114+
* The Makefile builds this file twice, once with CXX_BUILD undefined
115+
* (the non-unity build path that triggered the original bug) and once
116+
* with CXX_BUILD defined (the griffin/unity-build path). Both must
117+
* compile and pass the runtime checks. */
118+
119+
#include <cstdio>
120+
#include <cstddef>
121+
122+
/* This is the regression's trigger pattern: a C++ TU includes a
123+
* RetroArch header from inside its own extern "C" block. On a stock
124+
* (broken) retro_atomic.h, the transitive #include <atomic> here would
125+
* be parsed at C linkage and the build would die. */
126+
#ifndef CXX_BUILD
127+
extern "C" {
128+
#endif
129+
130+
#include <retro_atomic.h>
131+
132+
#ifndef CXX_BUILD
133+
}
134+
#endif
135+
136+
/* Capability flags must be set after the include succeeds. If a
137+
* future refactor accidentally compiles the header out under
138+
* extern "C", these gates fire at preprocessing time with a clearer
139+
* message than the libstdc++ template avalanche. */
140+
#if !defined(HAVE_RETRO_ATOMIC)
141+
# error "retro_atomic.h: HAVE_RETRO_ATOMIC not set after include"
142+
#endif
143+
#if !defined(RETRO_ATOMIC_BACKEND_NAME)
144+
# error "retro_atomic.h: RETRO_ATOMIC_BACKEND_NAME not set after include"
145+
#endif
146+
147+
/* On a C++11+ host the CXX11 backend is the expected selection. If
148+
* something promotes a different backend in this TU we want to know;
149+
* the whole point of the test is to police the C++11 path. */
150+
#if !defined(RETRO_ATOMIC_BACKEND_CXX11)
151+
# error "retro_atomic_extern_c_linkage_test: expected the C++11 <atomic> backend; if a different backend is intentional, update this gate"
152+
#endif
153+
154+
/* ---- Single-threaded property checks --------------------------------- */
155+
/* Each returns 0 on success, 1 on failure. Identical in shape to
156+
* retro_atomic_test.c so a regression in either test points at the
157+
* same place. */
158+
159+
static int check_init(void)
160+
{
161+
retro_atomic_int_t ai; retro_atomic_int_init (&ai, 7);
162+
retro_atomic_size_t as; retro_atomic_size_init(&as, (std::size_t)9);
163+
164+
if (retro_atomic_load_acquire_int (&ai) != 7) return 1;
165+
if (retro_atomic_load_acquire_size(&as) != (std::size_t)9) return 1;
166+
return 0;
167+
}
168+
169+
static int check_store_release_load_acquire(void)
170+
{
171+
retro_atomic_int_t ai; retro_atomic_int_init (&ai, 0);
172+
retro_atomic_size_t as; retro_atomic_size_init(&as, 0);
173+
174+
retro_atomic_store_release_int (&ai, 42);
175+
retro_atomic_store_release_size(&as, (std::size_t)42);
176+
177+
if (retro_atomic_load_acquire_int (&ai) != 42) return 1;
178+
if (retro_atomic_load_acquire_size(&as) != (std::size_t)42) return 1;
179+
return 0;
180+
}
181+
182+
static int check_fetch_add_sub_returns_previous(void)
183+
{
184+
retro_atomic_int_t ai; retro_atomic_int_init (&ai, 10);
185+
retro_atomic_size_t as; retro_atomic_size_init(&as, (std::size_t)10);
186+
187+
/* fetch_add returns the previous value (POSIX convention). */
188+
if (retro_atomic_fetch_add_int (&ai, 5) != 10) return 1;
189+
if (retro_atomic_fetch_add_size(&as, 5) != (std::size_t)10) return 1;
190+
191+
if (retro_atomic_load_acquire_int (&ai) != 15) return 1;
192+
if (retro_atomic_load_acquire_size(&as) != (std::size_t)15) return 1;
193+
194+
if (retro_atomic_fetch_sub_int (&ai, 5) != 15) return 1;
195+
if (retro_atomic_fetch_sub_size(&as, 5) != (std::size_t)15) return 1;
196+
197+
if (retro_atomic_load_acquire_int (&ai) != 10) return 1;
198+
if (retro_atomic_load_acquire_size(&as) != (std::size_t)10) return 1;
199+
return 0;
200+
}
201+
202+
int main(void)
203+
{
204+
int fails = 0;
205+
206+
std::printf("backend: %s\n", RETRO_ATOMIC_BACKEND_NAME);
207+
#ifdef CXX_BUILD
208+
std::puts("config: CXX_BUILD defined (griffin/unity-build path)");
209+
#else
210+
std::puts("config: CXX_BUILD undefined (separate-TU path, ui_qt.cpp shape)");
211+
#endif
212+
213+
fails += check_init();
214+
fails += check_store_release_load_acquire();
215+
fails += check_fetch_add_sub_returns_previous();
216+
217+
if (fails)
218+
{
219+
std::printf("FAIL: %d check(s) failed\n", fails);
220+
return 1;
221+
}
222+
std::puts("ALL OK");
223+
return 0;
224+
}

0 commit comments

Comments
 (0)