[PATCH] test: Fix and stabilize tst-wcsmbs-clone-overflow test
Carlos O'Donell
carlos@redhat.com
Tue May 26 12:13:44 GMT 2026
On 5/26/26 7:45 AM, Frédéric Bérat wrote:
> The test tst-wcsmbs-clone-overflow was initially added to tests-static.
> However, this causes the test to be unstable because gconv modules
> dynamically load libc.so. Any discrepancy between the statically linked
> version and the dynamically loaded one can lead to a crash.
LGTM.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
Correct, I don't think we noticed that this will be the case for all
stdio tests that do conversions because they must dlopen the converter
modules, an this loads the system glibc but with the env-var specified
converters.
> By removing the test from tests-static, it relies on dynamic linking,
> safely bypassing the dlopen crash. Since the test is now dynamically
> linked, it cannot use the internal thread-local symbol
OK.
> _NL_CURRENT_DATA(LC_CTYPE) because _nl_current_LC_CTYPE is hidden in
> libc.so, leading to undefined references. Thus, the test now uses
> newlocale and uselocale, safely extracting the locale data from the
> returned locale_t object.
OK.
> Furthermore, using newlocale requires the gconv-modules configuration to
> be built and available so that the ISO8859-1.so module can be
> dynamically loaded. Otherwise, glibc falls back to the built-in C locale
> conversions, leaving __shlib_handle as NULL and silently bypassing the
> reference counter increment.
OK.
> A new Makefile fragment, gen-gconv-modules.mk, is introduced to ensure
> the gconv-modules are built before the test runs, and an explicit check
> for __shlib_handle != NULL is added to the test.
OK.
> ---
> gen-gconv-modules.mk | 6 ++++++
> localedata/Makefile | 6 ++----
> wcsmbs/Makefile | 7 ++-----
> wcsmbs/tst-wcsmbs-clone-overflow.c | 12 +++++++++---
> 4 files changed, 19 insertions(+), 12 deletions(-)
> create mode 100644 gen-gconv-modules.mk
>
> diff --git a/gen-gconv-modules.mk b/gen-gconv-modules.mk
> new file mode 100644
> index 0000000000..046721a7a7
> --- /dev/null
> +++ b/gen-gconv-modules.mk
> @@ -0,0 +1,6 @@
> +# defines target $(gen-gconv-modules) that ensures gconv-modules are available
> +
> +gen-gconv-modules := $(common-objpfx)iconvdata/gconv-modules
> +
> +$(gen-gconv-modules):
> + $(MAKE) -C ../iconvdata subdir=iconvdata $@
OK. Nice, this is like generating locales, but generates the module file.
> diff --git a/localedata/Makefile b/localedata/Makefile
> index 6c9fe7c5c4..0fb1d78c29 100644
> --- a/localedata/Makefile
> +++ b/localedata/Makefile
> @@ -287,7 +287,7 @@ install-others := $(addprefix $(inst_i18ndir)/, \
> $(locales))
> endif
>
> -tests: $(objdir)/iconvdata/gconv-modules
> +tests: $(gen-gconv-modules)
OK.
>
> tests-static += \
> tst-langinfo-newlocale-static \
> @@ -412,6 +412,7 @@ LOCALES := \
> # LOCALES
>
> include ../gen-locales.mk
> +include ../gen-gconv-modules.mk
OK. Clearly shows we need gen-locales and gen-gconv-modules.
>
> $(objpfx)tst-iconv-emojis-trans.out: $(gen-locales)
>
> @@ -587,6 +588,3 @@ $(objpfx)mtrace-tst-leaks.out: $(objpfx)tst-leaks.out
>
> bug-setlocale1-ENV-only = LOCPATH=$(objpfx) LC_CTYPE=de_DE.UTF-8
> bug-setlocale1-static-ENV-only = $(bug-setlocale1-ENV-only)
> -
> -$(objdir)/iconvdata/gconv-modules:
> - $(MAKE) -C ../iconvdata subdir=iconvdata $@
OK.
> diff --git a/wcsmbs/Makefile b/wcsmbs/Makefile
> index ee3638c0e5..f4f89c030f 100644
> --- a/wcsmbs/Makefile
> +++ b/wcsmbs/Makefile
> @@ -213,10 +213,6 @@ xtests += test-wcsncmp-nonarray
> tests-internal += \
> tst-wcsmbs-clone-overflow
>
> -tests-static += \
> - tst-wcsmbs-clone-overflow
> -
> -
OK. Remove from static tests.
> include ../Rules
>
> ifeq ($(run-built-tests),yes)
> @@ -233,6 +229,7 @@ LOCALES := \
> zh_TW.EUC-TW \
> # LOCALES
> include ../gen-locales.mk
> +include ../gen-gconv-modules.mk
OK. Included to indicate we need this too.
>
> $(objpfx)tst-btowc.out: $(gen-locales)
> $(objpfx)tst-c16c32-1.out: $(gen-locales)
> @@ -247,7 +244,7 @@ $(objpfx)tst-c32-state.out: $(gen-locales)
> $(objpfx)test-c8rtomb.out: $(gen-locales)
> $(objpfx)test-mbrtoc8.out: $(gen-locales)
> $(objpfx)tst-wscanf-to_inpunct.out: $(gen-locales)
> -$(objpfx)tst-wcsmbs-clone-overflow.out: $(gen-locales)
> +$(objpfx)tst-wcsmbs-clone-overflow.out: $(gen-locales) $(gen-gconv-modules)
OK. Nice, clearly specified test dependency.
> endif
>
> $(objpfx)tst-wcstod-round: $(libm)
> diff --git a/wcsmbs/tst-wcsmbs-clone-overflow.c b/wcsmbs/tst-wcsmbs-clone-overflow.c
> index adfd4fa61d..99461ea087 100644
> --- a/wcsmbs/tst-wcsmbs-clone-overflow.c
> +++ b/wcsmbs/tst-wcsmbs-clone-overflow.c
> @@ -30,8 +30,11 @@
> static int
> do_test (void)
> {
> - if (setlocale (LC_ALL, "de_DE.ISO-8859-1") == NULL)
> - FAIL_EXIT1 ("setlocale failed, check if de_DE.ISO-8859-1 is generated");
> + locale_t loc_obj = newlocale (LC_ALL_MASK, "de_DE.ISO-8859-1", NULL);
OK. New locale object inheriting everything (LC_ALL_MASK).
> + if (loc_obj == NULL)
> + FAIL_EXIT1 ("newlocale failed, check if de_DE.ISO-8859-1 is generated");
OK.
> +
> + uselocale (loc_obj);
OK.
>
> wchar_t buf[32] = L"123";
> int j;
> @@ -41,7 +44,7 @@ do_test (void)
> FAIL_EXIT1 ("swscanf failed");
>
> /* Retrieve the current gconv_fcts from the LC_CTYPE locale data. */
> - struct __locale_data *loc = _NL_CURRENT_DATA (LC_CTYPE);
> + struct __locale_data *loc = loc_obj->__locales[LC_CTYPE];
OK. Looking at internal details, but the test is "open box" style.
> struct lc_ctype_data *ctype = loc->private;
> const struct gconv_fcts *fcts = ctype->fcts;
>
> @@ -51,6 +54,9 @@ do_test (void)
> /* Capture the reference counter. */
> int initial_counter = fcts->towc->__counter;
>
> + if (fcts->towc->__shlib_handle == NULL)
> + FAIL_EXIT1 ("__shlib_handle is NULL!");
OK. Yes, verify that we actually loaded something, this was the problem
previously with `make test t=` runs which didn't have the gconv modules loaded.
> +
> /* Perform a second iteration of swscanf. If the stack-allocated FILE
> leaks the gconv reference, the counter will increment. */
> if (swscanf (buf, L"%d", &j) < 1)
--
Cheers,
Carlos.
More information about the Libc-alpha
mailing list