[PATCH v6 1/2] elf: Do not duplicate the GLIBC_TUNABLES string
H.J. Lu
hjl.tools@gmail.com
Wed Dec 20 00:06:22 GMT 2023
On Tue, Dec 19, 2023 at 1:33 PM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Dec 12, 2023 at 11:16 AM Siddhesh Poyarekar
> <siddhesh@sourceware.org> wrote:
> >
> > On 2023-12-06 08:24, Adhemerval Zanella wrote:
> > > The tunable parsing duplicates the tunable environment variable so it
> > > null-terminates each one since it simplifies the later parsing. It has
> > > the drawback of adding another point of failure (__minimal_malloc
> > > failing), and the memory copy requires tuning the compiler to avoid mem
> > > operations calls.
> > >
> > > The parsing now tracks the tunable start and its size. The
> > > dl-tunable-parse.h adds helper functions to help parsing, like a strcmp
> > > that also checks for size and an iterator for suboptions that are
> > > comma-separated (used on hwcap parsing by x86, powerpc, and s390x).
> > >
> > > Since the environment variable is allocated on the stack by the kernel,
> > > it is safe to keep the references to the suboptions for later parsing
> > > of string tunables (as done by set_hwcaps by multiple architectures).
> > >
> > > Checked on x86_64-linux-gnu, powerpc64le-linux-gnu, and
> > > aarch64-linux-gnu.
> > > ---
> >
> > LGTM.
> >
> > Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> >
> >
> > > elf/dl-tunables.c | 80 ++++-----
> > > elf/dl-tunables.h | 6 +-
> > > elf/tst-tunables.c | 66 ++++++-
> > > sysdeps/generic/dl-tunables-parse.h | 134 ++++++++++++++
> > > sysdeps/s390/cpu-features.c | 165 +++++++-----------
> > > .../unix/sysv/linux/aarch64/cpu-features.c | 33 ++--
> > > .../unix/sysv/linux/powerpc/cpu-features.c | 45 ++---
> > > .../sysv/linux/powerpc/tst-hwcap-tunables.c | 6 +-
> > > sysdeps/x86/Makefile | 4 +-
> > > sysdeps/x86/cpu-tunables.c | 118 +++++--------
> > > sysdeps/x86/tst-hwcap-tunables.c | 148 ++++++++++++++++
> > > 11 files changed, 524 insertions(+), 281 deletions(-)
> > > create mode 100644 sysdeps/generic/dl-tunables-parse.h
> > > create mode 100644 sysdeps/x86/tst-hwcap-tunables.c
> > >
> > > diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c
> > > index 644d21d1b0..3d41e8e28e 100644
> > > --- a/elf/dl-tunables.c
> > > +++ b/elf/dl-tunables.c
> > > @@ -36,31 +36,8 @@
> > > #define TUNABLES_INTERNAL 1
> > > #include "dl-tunables.h"
> > >
> > > -#include <not-errno.h>
> > > -
> > > -static char *
> > > -tunables_strdup (const char *in)
> > > -{
> > > - size_t i = 0;
> > > -
> > > - while (in[i++] != '\0');
> > > - char *out = __minimal_malloc (i + 1);
> > > -
> > > - /* For most of the tunables code, we ignore user errors. However,
> > > - this is a system error - and running out of memory at program
> > > - startup should be reported, so we do. */
> > > - if (out == NULL)
> > > - _dl_fatal_printf ("failed to allocate memory to process tunables\n");
> > > -
> > > - while (i-- > 0)
> > > - out[i] = in[i];
> > > -
> > > - return out;
> > > -}
> > > -
> > > static char **
> > > -get_next_env (char **envp, char **name, size_t *namelen, char **val,
> > > - char ***prev_envp)
> > > +get_next_env (char **envp, char **name, char **val, char ***prev_envp)
> > > {
> > > while (envp != NULL && *envp != NULL)
> > > {
> > > @@ -76,7 +53,6 @@ get_next_env (char **envp, char **name, size_t *namelen, char **val,
> > > continue;
> > >
> > > *name = envline;
> > > - *namelen = len;
> > > *val = &envline[len + 1];
> > > *prev_envp = prev;
> > >
> > > @@ -134,14 +110,14 @@ do_tunable_update_val (tunable_t *cur, const tunable_val_t *valp,
> > > /* Validate range of the input value and initialize the tunable CUR if it looks
> > > good. */
> > > static void
> > > -tunable_initialize (tunable_t *cur, const char *strval)
> > > +tunable_initialize (tunable_t *cur, const char *strval, size_t len)
> > > {
> > > - tunable_val_t val;
> > > + tunable_val_t val = { 0 };
> > >
> > > if (cur->type.type_code != TUNABLE_TYPE_STRING)
> > > val.numval = (tunable_num_t) _dl_strtoul (strval, NULL);
> > > else
> > > - val.strval = strval;
> > > + val.strval = (struct tunable_str_t) { strval, len };
> > > do_tunable_update_val (cur, &val, NULL, NULL);
> > > }
> > >
> > > @@ -165,29 +141,29 @@ struct tunable_toset_t
> > > {
> > > tunable_t *t;
> > > const char *value;
> > > + size_t len;
> > > };
> > >
> > > enum { tunables_list_size = array_length (tunable_list) };
> > >
> > > /* Parse the tunable string VALSTRING and set TUNABLES with the found tunables
> > > - and their respective strings. VALSTRING is a duplicated values, where
> > > - delimiters ':' are replaced with '\0', so string tunables are null
> > > - terminated.
> > > + and their respective values. The VALSTRING is parsed in place, with the
> > > + tunable start and size recorded in TUNABLES.
> > > Return the number of tunables found (including 0 if the string is empty)
> > > or -1 if for an ill-formatted definition. */
> > > static int
> > > -parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
> > > +parse_tunables_string (const char *valstring, struct tunable_toset_t *tunables)
> > > {
> > > if (valstring == NULL || *valstring == '\0')
> > > return 0;
> > >
> > > - char *p = valstring;
> > > + const char *p = valstring;
> > > bool done = false;
> > > int ntunables = 0;
> > >
> > > while (!done)
> > > {
> > > - char *name = p;
> > > + const char *name = p;
> > >
> > > /* First, find where the name ends. */
> > > while (*p != '=' && *p != ':' && *p != '\0')
> > > @@ -209,7 +185,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
> > > /* Skip the '='. */
> > > p++;
> > >
> > > - char *value = p;
> > > + const char *value = p;
> > >
> > > while (*p != '=' && *p != ':' && *p != '\0')
> > > p++;
> > > @@ -218,8 +194,6 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
> > > return -1;
> > > else if (*p == '\0')
> > > done = true;
> > > - else
> > > - *p++ = '\0';
> > >
> > > /* Add the tunable if it exists. */
> > > for (size_t i = 0; i < tunables_list_size; i++)
> > > @@ -228,7 +202,8 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
> > >
> > > if (tunable_is_name (cur->name, name))
> > > {
> > > - tunables[ntunables++] = (struct tunable_toset_t) { cur, value };
> > > + tunables[ntunables++] =
> > > + (struct tunable_toset_t) { cur, value, p - value };
> > > break;
> > > }
> > > }
> > > @@ -238,7 +213,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables)
> > > }
> > >
> > > static void
> > > -parse_tunables (char *valstring)
> > > +parse_tunables (const char *valstring)
> > > {
> > > struct tunable_toset_t tunables[tunables_list_size];
> > > int ntunables = parse_tunables_string (valstring, tunables);
> > > @@ -250,7 +225,7 @@ parse_tunables (char *valstring)
> > > }
> > >
> > > for (int i = 0; i < ntunables; i++)
> > > - tunable_initialize (tunables[i].t, tunables[i].value);
> > > + tunable_initialize (tunables[i].t, tunables[i].value, tunables[i].len);
> > > }
> > >
> > > /* Initialize the tunables list from the environment. For now we only use the
> > > @@ -261,19 +236,20 @@ __tunables_init (char **envp)
> > > {
> > > char *envname = NULL;
> > > char *envval = NULL;
> > > - size_t len = 0;
> > > char **prev_envp = envp;
> > >
> > > /* Ignore tunables for AT_SECURE programs. */
> > > if (__libc_enable_secure)
> > > return;
> > >
> > > - while ((envp = get_next_env (envp, &envname, &len, &envval,
> > > - &prev_envp)) != NULL)
> > > + while ((envp = get_next_env (envp, &envname, &envval, &prev_envp)) != NULL)
> > > {
> > > + /* The environment variable is allocated on the stack by the kernel, so
> > > + it is safe to keep the references to the suboptions for later parsing
> > > + of string tunables. */
> > > if (tunable_is_name ("GLIBC_TUNABLES", envname))
> > > {
> > > - parse_tunables (tunables_strdup (envval));
> > > + parse_tunables (envval);
> > > continue;
> > > }
> > >
> > > @@ -291,7 +267,11 @@ __tunables_init (char **envp)
> > > /* We have a match. Initialize and move on to the next line. */
> > > if (tunable_is_name (name, envname))
> > > {
> > > - tunable_initialize (cur, envval);
> > > + size_t envvallen = 0;
> > > + /* The environment variable is always null-terminated. */
> > > + for (const char *p = envval; *p != '\0'; p++, envvallen++);
> > > +
> > > + tunable_initialize (cur, envval, envvallen);
> > > break;
> > > }
> > > }
> > > @@ -305,7 +285,7 @@ __tunables_print (void)
> > > {
> > > const tunable_t *cur = &tunable_list[i];
> > > if (cur->type.type_code == TUNABLE_TYPE_STRING
> > > - && cur->val.strval == NULL)
> > > + && cur->val.strval.str == NULL)
> > > _dl_printf ("%s:\n", cur->name);
> > > else
> > > {
> > > @@ -331,7 +311,9 @@ __tunables_print (void)
> > > (size_t) cur->type.max);
> > > break;
> > > case TUNABLE_TYPE_STRING:
> > > - _dl_printf ("%s\n", cur->val.strval);
> > > + _dl_printf ("%.*s\n",
> > > + (int) cur->val.strval.len,
> > > + cur->val.strval.str);
> > > break;
> > > default:
> > > __builtin_unreachable ();
> > > @@ -364,7 +346,7 @@ __tunable_get_default (tunable_id_t id, void *valp)
> > > }
> > > case TUNABLE_TYPE_STRING:
> > > {
> > > - *((const char **)valp) = cur->def.strval;
> > > + *((const struct tunable_str_t **)valp) = &cur->def.strval;
> > > break;
> > > }
> > > default:
> > > @@ -399,7 +381,7 @@ __tunable_get_val (tunable_id_t id, void *valp, tunable_callback_t callback)
> > > }
> > > case TUNABLE_TYPE_STRING:
> > > {
> > > - *((const char **)valp) = cur->val.strval;
> > > + *((const struct tunable_str_t **) valp) = &cur->val.strval;
> > > break;
> > > }
> > > default:
> > > diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h
> > > index 0df4dde24e..5d5ee2c3aa 100644
> > > --- a/elf/dl-tunables.h
> > > +++ b/elf/dl-tunables.h
> > > @@ -30,7 +30,11 @@ typedef intmax_t tunable_num_t;
> > > typedef union
> > > {
> > > tunable_num_t numval;
> > > - const char *strval;
> > > + struct tunable_str_t
> > > + {
> > > + const char *str;
> > > + size_t len;
> > > + } strval;
> > > } tunable_val_t;
> > >
> > > typedef void (*tunable_callback_t) (tunable_val_t *);
> > > diff --git a/elf/tst-tunables.c b/elf/tst-tunables.c
> > > index e1ad44f27c..188345b070 100644
> > > --- a/elf/tst-tunables.c
> > > +++ b/elf/tst-tunables.c
> > > @@ -31,7 +31,8 @@ static int restart;
> > >
> > > static const struct test_t
> > > {
> > > - const char *env;
> > > + const char *name;
> > > + const char *value;
> > > int32_t expected_malloc_check;
> > > size_t expected_mmap_threshold;
> > > int32_t expected_perturb;
> > > @@ -39,12 +40,14 @@ static const struct test_t
> > > {
> > > /* Expected tunable format. */
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=2",
> > > 2,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
> > > 2,
> > > 4096,
> > > @@ -52,6 +55,7 @@ static const struct test_t
> > > },
> > > /* Empty tunable are ignored. */
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=2::glibc.malloc.mmap_threshold=4096",
> > > 2,
> > > 4096,
> > > @@ -59,6 +63,7 @@ static const struct test_t
> > > },
> > > /* As well empty values. */
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=:glibc.malloc.mmap_threshold=4096",
> > > 0,
> > > 4096,
> > > @@ -66,18 +71,21 @@ static const struct test_t
> > > },
> > > /* Tunable are processed from left to right, so last one is the one set. */
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=1:glibc.malloc.check=2",
> > > 2,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=1:glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
> > > 2,
> > > 4096,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096:glibc.malloc.check=1",
> > > 1,
> > > 4096,
> > > @@ -85,12 +93,14 @@ static const struct test_t
> > > },
> > > /* 0x800 is larger than tunable maxval (0xff), so the tunable is unchanged. */
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.perturb=0x800",
> > > 0,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.perturb=0x55",
> > > 0,
> > > 0,
> > > @@ -98,6 +108,7 @@ static const struct test_t
> > > },
> > > /* Out of range values are just ignored. */
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.perturb=0x800:glibc.malloc.mmap_threshold=4096",
> > > 0,
> > > 4096,
> > > @@ -105,24 +116,28 @@ static const struct test_t
> > > },
> > > /* Invalid keys are ignored. */
> > > {
> > > + "GLIBC_TUNABLES",
> > > ":glibc.malloc.garbage=2:glibc.malloc.check=1",
> > > 1,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.perturb=0x800:not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
> > > 0,
> > > 4096,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.not_valid.check=2:glibc.malloc.mmap_threshold=4096",
> > > 0,
> > > 4096,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "not_valid.malloc.check=2:glibc.malloc.mmap_threshold=4096",
> > > 0,
> > > 4096,
> > > @@ -130,24 +145,28 @@ static const struct test_t
> > > },
> > > /* Invalid subkeys are ignored. */
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096:glibc.malloc.check=2",
> > > 2,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=4:glibc.malloc.garbage=2:glibc.maoc.mmap_threshold=4096",
> > > 0,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "not_valid.malloc.check=2",
> > > 0,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.not_valid.check=2",
> > > 0,
> > > 0,
> > > @@ -156,6 +175,7 @@ static const struct test_t
> > > /* An ill-formatted tunable in the for key=key=value will considere the
> > > value as 'key=value' (which can not be parsed as an integer). */
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096",
> > > 0,
> > > 0,
> > > @@ -163,41 +183,77 @@ static const struct test_t
> > > },
> > > /* Ill-formatted tunables string is not parsed. */
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.mmap_threshold=glibc.malloc.mmap_threshold=4096:glibc.malloc.check=2",
> > > 0,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=2=2",
> > > 0,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=2=2:glibc.malloc.mmap_threshold=4096",
> > > 0,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=2=2:glibc.malloc.check=2",
> > > 0,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
> > > 0,
> > > 0,
> > > 0,
> > > },
> > > {
> > > + "GLIBC_TUNABLES",
> > > "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096=4096",
> > > 0,
> > > 0,
> > > 0,
> > > },
> > > + /* Also check some tunable aliases. */
> > > + {
> > > + "MALLOC_CHECK_",
> > > + "2",
> > > + 2,
> > > + 0,
> > > + 0,
> > > + },
> > > + {
> > > + "MALLOC_MMAP_THRESHOLD_",
> > > + "4096",
> > > + 0,
> > > + 4096,
> > > + 0,
> > > + },
> > > + {
> > > + "MALLOC_PERTURB_",
> > > + "0x55",
> > > + 0,
> > > + 0,
> > > + 0x55,
> > > + },
> > > + /* 0x800 is larger than tunable maxval (0xff), so the tunable is unchanged. */
> > > + {
> > > + "MALLOC_PERTURB_",
> > > + "0x800",
> > > + 0,
> > > + 0,
> > > + 0,
> > > + },
> > > };
> > >
> > > static int
> > > @@ -245,13 +301,17 @@ do_test (int argc, char *argv[])
> > > {
> > > snprintf (nteststr, sizeof nteststr, "%d", i);
> > >
> > > - printf ("[%d] Spawned test for %s\n", i, tests[i].env);
> > > - setenv ("GLIBC_TUNABLES", tests[i].env, 1);
> > > + printf ("[%d] Spawned test for %s=%s\n",
> > > + i,
> > > + tests[i].name,
> > > + tests[i].value);
> > > + setenv (tests[i].name, tests[i].value, 1);
> > > struct support_capture_subprocess result
> > > = support_capture_subprogram (spargv[0], spargv);
> > > support_capture_subprocess_check (&result, "tst-tunables", 0,
> > > sc_allow_stderr);
> > > support_capture_subprocess_free (&result);
> > > + unsetenv (tests[i].name);
> > > }
> > >
> > > return 0;
> > > diff --git a/sysdeps/generic/dl-tunables-parse.h b/sysdeps/generic/dl-tunables-parse.h
> > > new file mode 100644
> > > index 0000000000..b37be0443b
> > > --- /dev/null
> > > +++ b/sysdeps/generic/dl-tunables-parse.h
> > > @@ -0,0 +1,134 @@
> > > +/* Helper functions to handle tunable strings.
> > > + Copyright (C) 2023 Free Software Foundation, Inc.
> > > + This file is part of the GNU C Library.
> > > +
> > > + The GNU C Library is free software; you can redistribute it and/or
> > > + modify it under the terms of the GNU Lesser General Public
> > > + License as published by the Free Software Foundation; either
> > > + version 2.1 of the License, or (at your option) any later version.
> > > +
> > > + The GNU C Library is distributed in the hope that it will be useful,
> > > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > + Lesser General Public License for more details.
> > > +
> > > + You should have received a copy of the GNU Lesser General Public
> > > + License along with the GNU C Library; if not, see
> > > + <https://www.gnu.org/licenses/>. */
> > > +
> > > +#ifndef _DL_TUNABLES_PARSE_H
> > > +#define _DL_TUNABLES_PARSE_H 1
> > > +
> > > +#include <assert.h>
> > > +#include <string.h>
> > > +
> > > +/* Compare the contents of STRVAL with STR of size LEN. The STR might not
> > > + be null-terminated. */
> > > +static __always_inline bool
> > > +tunable_strcmp (const struct tunable_str_t *strval, const char *str,
> > > + size_t len)
> > > +{
> > > + return strval->len == len && memcmp (strval->str, str, len) == 0;
> > > +}
> > > +#define tunable_strcmp_cte(__tunable, __str) \
> > > + tunable_strcmp (&__tunable->strval, __str, sizeof (__str) - 1)
> > > +
> > > +/*
> > > + Helper functions to iterate over a tunable string composed by multiple
> > > + suboptions separated by commaxi; this is a common pattern for CPU. Each
> > > + suboptions is return in the form of { address, size } (no null terminated).
> > > + For instance:
> > > +
> > > + struct tunable_str_comma_t ts;
> > > + tunable_str_comma_init (&ts, valp);
> > > +
> > > + struct tunable_str_t t;
> > > + while (tunable_str_comma_next (&ts, &t))
> > > + {
> > > + _dl_printf ("[%s] %.*s (%d)\n",
> > > + __func__,
> > > + (int) tstr.len,
> > > + tstr.str,
> > > + (int) tstr.len);
> > > +
> > > + if (tunable_str_comma_strcmp (&t, opt, opt1_len))
> > > + {
> > > + [...]
> > > + }
> > > + else if (tunable_str_comma_strcmp_cte (&t, "opt2"))
> > > + {
> > > + [...]
> > > + }
> > > + }
> > > +
> > > + NB: These function are expected to be called from tunable callback
> > > + functions along with tunable_val_t with string types.
> > > +*/
> > > +
> > > +struct tunable_str_comma_state_t
> > > +{
> > > + const char *p;
> > > + size_t plen;
> > > + size_t maxplen;
> > > +};
> > > +
> > > +struct tunable_str_comma_t
> > > +{
> > > + const char *str;
> > > + size_t len;
> > > + bool disable;
> > > +};
> > > +
> > > +static inline void
> > > +tunable_str_comma_init (struct tunable_str_comma_state_t *state,
> > > + tunable_val_t *valp)
> > > +{
> > > + assert (valp->strval.str != NULL);
> > > + state->p = valp->strval.str;
> > > + state->plen = 0;
> > > + state->maxplen = valp->strval.len;
> > > +}
> > > +
> > > +static inline bool
> > > +tunable_str_comma_next (struct tunable_str_comma_state_t *state,
> > > + struct tunable_str_comma_t *str)
> > > +{
> > > + if (*state->p == '\0' || state->plen >= state->maxplen)
> > > + return false;
> > > +
> > > + const char *c;
> > > + for (c = state->p; *c != ','; c++, state->plen++)
> > > + if (*c == '\0' || state->plen == state->maxplen)
> > > + break;
> > > +
> > > + str->str = state->p;
> > > + str->len = c - state->p;
> > > +
> > > + if (str->len > 0)
> > > + {
> > > + str->disable = *str->str == '-';
> > > + if (str->disable)
> > > + {
> > > + str->str = str->str + 1;
> > > + str->len = str->len - 1;
> > > + }
> > > + }
> > > +
> > > + state->p = c + 1;
> > > + state->plen++;
> > > +
> > > + return true;
> > > +}
> > > +
> > > +/* Compare the contents of T with STR of size LEN. The STR might not be
> > > + null-terminated. */
> > > +static __always_inline bool
> > > +tunable_str_comma_strcmp (const struct tunable_str_comma_t *t, const char *str,
> > > + size_t len)
> > > +{
> > > + return t->len == len && memcmp (t->str, str, len) == 0;
> > > +}
> > > +#define tunable_str_comma_strcmp_cte(__t, __str) \
> > > + tunable_str_comma_strcmp (__t, __str, sizeof (__str) - 1)
> > > +
> > > +#endif
> > > diff --git a/sysdeps/s390/cpu-features.c b/sysdeps/s390/cpu-features.c
> > > index 55449ba07f..06c1cab0fd 100644
> > > --- a/sysdeps/s390/cpu-features.c
> > > +++ b/sysdeps/s390/cpu-features.c
> > > @@ -22,6 +22,7 @@
> > > #include <ifunc-memcmp.h>
> > > #include <string.h>
> > > #include <dl-symbol-redir-ifunc.h>
> > > +#include <dl-tunables-parse.h>
> > >
> > > #define S390_COPY_CPU_FEATURES(SRC_PTR, DEST_PTR) \
> > > (DEST_PTR)->hwcap = (SRC_PTR)->hwcap; \
> > > @@ -51,33 +52,14 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > struct cpu_features cpu_features_curr;
> > > S390_COPY_CPU_FEATURES (cpu_features, &cpu_features_curr);
> > >
> > > - const char *token = valp->strval;
> > > - do
> > > + struct tunable_str_comma_state_t ts;
> > > + tunable_str_comma_init (&ts, valp);
> > > +
> > > + struct tunable_str_comma_t t;
> > > + while (tunable_str_comma_next (&ts, &t))
> > > {
> > > - const char *token_end, *feature;
> > > - bool disable;
> > > - size_t token_len;
> > > - size_t feature_len;
> > > -
> > > - /* Find token separator or end of string. */
> > > - for (token_end = token; *token_end != ','; token_end++)
> > > - if (*token_end == '\0')
> > > - break;
> > > -
> > > - /* Determine feature. */
> > > - token_len = token_end - token;
> > > - if (*token == '-')
> > > - {
> > > - disable = true;
> > > - feature = token + 1;
> > > - feature_len = token_len - 1;
> > > - }
> > > - else
> > > - {
> > > - disable = false;
> > > - feature = token;
> > > - feature_len = token_len;
> > > - }
> > > + if (t.len == 0)
> > > + continue;
> > >
> > > /* Handle only the features here which are really used in the
> > > IFUNC-resolvers. All others are ignored as the values are only used
> > > @@ -85,86 +67,64 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > bool reset_features = false;
> > > unsigned long int hwcap_mask = 0UL;
> > > unsigned long long stfle_bits0_mask = 0ULL;
> > > + bool disable = t.disable;
> > >
> > > - if ((*feature == 'z' || *feature == 'a'))
> > > + if (tunable_str_comma_strcmp_cte (&t, "zEC12")
> > > + || tunable_str_comma_strcmp_cte (&t, "arch10"))
> > > + {
> > > + reset_features = true;
> > > + disable = true;
> > > + hwcap_mask = HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT
> > > + | HWCAP_S390_VXRS_EXT2;
> > > + stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> > > + }
> > > + else if (tunable_str_comma_strcmp_cte (&t, "z13")
> > > + || tunable_str_comma_strcmp_cte (&t, "arch11"))
> > > + {
> > > + reset_features = true;
> > > + disable = true;
> > > + hwcap_mask = HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
> > > + stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> > > + }
> > > + else if (tunable_str_comma_strcmp_cte (&t, "z14")
> > > + || tunable_str_comma_strcmp_cte (&t, "arch12"))
> > > + {
> > > + reset_features = true;
> > > + disable = true;
> > > + hwcap_mask = HWCAP_S390_VXRS_EXT2;
> > > + stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> > > + }
> > > + else if (tunable_str_comma_strcmp_cte (&t, "z15")
> > > + || tunable_str_comma_strcmp_cte (&t, "z16")
> > > + || tunable_str_comma_strcmp_cte (&t, "arch13")
> > > + || tunable_str_comma_strcmp_cte (&t, "arch14"))
> > > {
> > > - if ((feature_len == 5 && *feature == 'z'
> > > - && memcmp (feature, "zEC12", 5) == 0)
> > > - || (feature_len == 6 && *feature == 'a'
> > > - && memcmp (feature, "arch10", 6) == 0))
> > > - {
> > > - reset_features = true;
> > > - disable = true;
> > > - hwcap_mask = HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT
> > > - | HWCAP_S390_VXRS_EXT2;
> > > - stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> > > - }
> > > - else if ((feature_len == 3 && *feature == 'z'
> > > - && memcmp (feature, "z13", 3) == 0)
> > > - || (feature_len == 6 && *feature == 'a'
> > > - && memcmp (feature, "arch11", 6) == 0))
> > > - {
> > > - reset_features = true;
> > > - disable = true;
> > > - hwcap_mask = HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
> > > - stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> > > - }
> > > - else if ((feature_len == 3 && *feature == 'z'
> > > - && memcmp (feature, "z14", 3) == 0)
> > > - || (feature_len == 6 && *feature == 'a'
> > > - && memcmp (feature, "arch12", 6) == 0))
> > > - {
> > > - reset_features = true;
> > > - disable = true;
> > > - hwcap_mask = HWCAP_S390_VXRS_EXT2;
> > > - stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> > > - }
> > > - else if ((feature_len == 3 && *feature == 'z'
> > > - && (memcmp (feature, "z15", 3) == 0
> > > - || memcmp (feature, "z16", 3) == 0))
> > > - || (feature_len == 6
> > > - && (memcmp (feature, "arch13", 6) == 0
> > > - || memcmp (feature, "arch14", 6) == 0)))
> > > - {
> > > - /* For z15 or newer we don't have to disable something,
> > > - but we have to reset to the original values. */
> > > - reset_features = true;
> > > - }
> > > + /* For z15 or newer we don't have to disable something, but we have
> > > + to reset to the original values. */
> > > + reset_features = true;
> > > }
> > > - else if (*feature == 'H')
> > > + else if (tunable_str_comma_strcmp_cte (&t, "HWCAP_S390_VXRS"))
> > > {
> > > - if (feature_len == 15
> > > - && memcmp (feature, "HWCAP_S390_VXRS", 15) == 0)
> > > - {
> > > - hwcap_mask = HWCAP_S390_VXRS;
> > > - if (disable)
> > > - hwcap_mask |= HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
> > > - }
> > > - else if (feature_len == 19
> > > - && memcmp (feature, "HWCAP_S390_VXRS_EXT", 19) == 0)
> > > - {
> > > - hwcap_mask = HWCAP_S390_VXRS_EXT;
> > > - if (disable)
> > > - hwcap_mask |= HWCAP_S390_VXRS_EXT2;
> > > - else
> > > - hwcap_mask |= HWCAP_S390_VXRS;
> > > - }
> > > - else if (feature_len == 20
> > > - && memcmp (feature, "HWCAP_S390_VXRS_EXT2", 20) == 0)
> > > - {
> > > - hwcap_mask = HWCAP_S390_VXRS_EXT2;
> > > - if (!disable)
> > > - hwcap_mask |= HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT;
> > > - }
> > > + hwcap_mask = HWCAP_S390_VXRS;
> > > + if (t.disable)
> > > + hwcap_mask |= HWCAP_S390_VXRS_EXT | HWCAP_S390_VXRS_EXT2;
> > > }
> > > - else if (*feature == 'S')
> > > + else if (tunable_str_comma_strcmp_cte (&t, "HWCAP_S390_VXRS_EXT"))
> > > {
> > > - if (feature_len == 10
> > > - && memcmp (feature, "STFLE_MIE3", 10) == 0)
> > > - {
> > > - stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> > > - }
> > > + hwcap_mask = HWCAP_S390_VXRS_EXT;
> > > + if (t.disable)
> > > + hwcap_mask |= HWCAP_S390_VXRS_EXT2;
> > > + else
> > > + hwcap_mask |= HWCAP_S390_VXRS;
> > > + }
> > > + else if (tunable_str_comma_strcmp_cte (&t, "HWCAP_S390_VXRS_EXT2"))
> > > + {
> > > + hwcap_mask = HWCAP_S390_VXRS_EXT2;
> > > + if (!t.disable)
> > > + hwcap_mask |= HWCAP_S390_VXRS | HWCAP_S390_VXRS_EXT;
> > > }
> > > + else if (tunable_str_comma_strcmp_cte (&t, "STFLE_MIE3"))
> > > + stfle_bits0_mask = S390_STFLE_MASK_ARCH13_MIE3;
> > >
> > > /* Perform the actions determined above. */
> > > if (reset_features)
> > > @@ -187,14 +147,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > else
> > > cpu_features_curr.stfle_bits[0] |= stfle_bits0_mask;
> > > }
> > > -
> > > - /* Jump over current token ... */
> > > - token += token_len;
> > > -
> > > - /* ... and skip token separator for next round. */
> > > - if (*token == ',') token++;
> > > }
> > > - while (*token != '\0');
> > >
> > > /* Copy back the features after checking that no unsupported features were
> > > enabled by user. */
> > > diff --git a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> > > index a11a86efab..c57f154b48 100644
> > > --- a/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> > > +++ b/sysdeps/unix/sysv/linux/aarch64/cpu-features.c
> > > @@ -16,10 +16,12 @@
> > > License along with the GNU C Library; if not, see
> > > <https://www.gnu.org/licenses/>. */
> > >
> > > +#include <array_length.h>
> > > #include <cpu-features.h>
> > > #include <sys/auxv.h>
> > > #include <elf/dl-hwcaps.h>
> > > #include <sys/prctl.h>
> > > +#include <dl-tunables-parse.h>
> > >
> > > #define DCZID_DZP_MASK (1 << 4)
> > > #define DCZID_BS_MASK (0xf)
> > > @@ -33,25 +35,28 @@
> > > struct cpu_list
> > > {
> > > const char *name;
> > > + size_t len;
> > > uint64_t midr;
> > > };
> > >
> > > -static struct cpu_list cpu_list[] = {
> > > - {"thunderxt88", 0x430F0A10},
> > > - {"thunderx2t99", 0x431F0AF0},
> > > - {"thunderx2t99p1", 0x420F5160},
> > > - {"ares", 0x411FD0C0},
> > > - {"emag", 0x503F0001},
> > > - {"kunpeng920", 0x481FD010},
> > > - {"a64fx", 0x460F0010},
> > > - {"generic", 0x0}
> > > +static const struct cpu_list cpu_list[] =
> > > +{
> > > +#define CPU_LIST_ENTRY(__str, __num) { __str, sizeof (__str) - 1, __num }
> > > + CPU_LIST_ENTRY ("thunderxt88", 0x430F0A10),
> > > + CPU_LIST_ENTRY ("thunderx2t99", 0x431F0AF0),
> > > + CPU_LIST_ENTRY ("thunderx2t99p1", 0x420F5160),
> > > + CPU_LIST_ENTRY ("ares", 0x411FD0C0),
> > > + CPU_LIST_ENTRY ("emag", 0x503F0001),
> > > + CPU_LIST_ENTRY ("kunpeng920", 0x481FD010),
> > > + CPU_LIST_ENTRY ("a64fx", 0x460F0010),
> > > + CPU_LIST_ENTRY ("generic", 0x0),
> > > };
> > >
> > > static uint64_t
> > > -get_midr_from_mcpu (const char *mcpu)
> > > +get_midr_from_mcpu (const struct tunable_str_t *mcpu)
> > > {
> > > - for (int i = 0; i < sizeof (cpu_list) / sizeof (struct cpu_list); i++)
> > > - if (strcmp (mcpu, cpu_list[i].name) == 0)
> > > + for (int i = 0; i < array_length (cpu_list); i++)
> > > + if (tunable_strcmp (mcpu, cpu_list[i].name, cpu_list[i].len))
> > > return cpu_list[i].midr;
> > >
> > > return UINT64_MAX;
> > > @@ -63,7 +68,9 @@ init_cpu_features (struct cpu_features *cpu_features)
> > > register uint64_t midr = UINT64_MAX;
> > >
> > > /* Get the tunable override. */
> > > - const char *mcpu = TUNABLE_GET (glibc, cpu, name, const char *, NULL);
> > > + const struct tunable_str_t *mcpu = TUNABLE_GET (glibc, cpu, name,
> > > + struct tunable_str_t *,
> > > + NULL);
> > > if (mcpu != NULL)
> > > midr = get_midr_from_mcpu (mcpu);
> > >
> > > diff --git a/sysdeps/unix/sysv/linux/powerpc/cpu-features.c b/sysdeps/unix/sysv/linux/powerpc/cpu-features.c
> > > index 7c6e20e702..390b3fd11a 100644
> > > --- a/sysdeps/unix/sysv/linux/powerpc/cpu-features.c
> > > +++ b/sysdeps/unix/sysv/linux/powerpc/cpu-features.c
> > > @@ -20,6 +20,7 @@
> > > #include <stdint.h>
> > > #include <cpu-features.h>
> > > #include <elf/dl-tunables.h>
> > > +#include <dl-tunables-parse.h>
> > > #include <unistd.h>
> > > #include <string.h>
> > >
> > > @@ -43,41 +44,26 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > struct cpu_features *cpu_features = &GLRO(dl_powerpc_cpu_features);
> > > unsigned long int tcbv_hwcap = cpu_features->hwcap;
> > > unsigned long int tcbv_hwcap2 = cpu_features->hwcap2;
> > > - const char *token = valp->strval;
> > > - do
> > > +
> > > + struct tunable_str_comma_state_t ts;
> > > + tunable_str_comma_init (&ts, valp);
> > > +
> > > + struct tunable_str_comma_t t;
> > > + while (tunable_str_comma_next (&ts, &t))
> > > {
> > > - const char *token_end, *feature;
> > > - bool disable;
> > > - size_t token_len, i, feature_len, offset = 0;
> > > - /* Find token separator or end of string. */
> > > - for (token_end = token; *token_end != ','; token_end++)
> > > - if (*token_end == '\0')
> > > - break;
> > > + if (t.len == 0)
> > > + continue;
> > >
> > > - /* Determine feature. */
> > > - token_len = token_end - token;
> > > - if (*token == '-')
> > > - {
> > > - disable = true;
> > > - feature = token + 1;
> > > - feature_len = token_len - 1;
> > > - }
> > > - else
> > > - {
> > > - disable = false;
> > > - feature = token;
> > > - feature_len = token_len;
> > > - }
> > > - for (i = 0; i < array_length (hwcap_tunables); ++i)
> > > + size_t offset = 0;
> > > + for (int i = 0; i < array_length (hwcap_tunables); ++i)
> > > {
> > > const char *hwcap_name = hwcap_names + offset;
> > > size_t hwcap_name_len = strlen (hwcap_name);
> > > /* Check the tunable name on the supported list. */
> > > - if (hwcap_name_len == feature_len
> > > - && memcmp (feature, hwcap_name, feature_len) == 0)
> > > + if (tunable_str_comma_strcmp (&t, hwcap_name, hwcap_name_len))
> > > {
> > > /* Update the hwcap and hwcap2 bits. */
> > > - if (disable)
> > > + if (t.disable)
> > > {
> > > /* Id is 1 for hwcap2 tunable. */
> > > if (hwcap_tunables[i].id)
> > > @@ -98,12 +84,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > }
> > > offset += hwcap_name_len + 1;
> > > }
> > > - token += token_len;
> > > - /* ... and skip token separator for next round. */
> > > - if (*token == ',')
> > > - token++;
> > > }
> > > - while (*token != '\0');
> > > }
> > >
> > > static inline void
> > > diff --git a/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c b/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
> > > index 2631016a3a..049164f841 100644
> > > --- a/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
> > > +++ b/sysdeps/unix/sysv/linux/powerpc/tst-hwcap-tunables.c
> > > @@ -110,7 +110,11 @@ do_test (int argc, char *argv[])
> > > run_test ("-arch_2_06", "__memcpy_power7");
> > > if (hwcap & PPC_FEATURE_ARCH_2_05)
> > > run_test ("-arch_2_06,-arch_2_05","__memcpy_power6");
> > > - run_test ("-arch_2_06,-arch_2_05,-power5+,-power5,-power4", "__memcpy_power4");
> > > + run_test ("-arch_2_06,-arch_2_05,-power5+,-power5,-power4",
> > > + "__memcpy_power4");
> > > + /* Also run with valid, but empty settings. */
> > > + run_test (",-,-arch_2_06,-arch_2_05,-power5+,-power5,,-power4,-",
> > > + "__memcpy_power4");
> > > }
> > > else
> > > {
> > > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> > > index 917c26f116..a64e5f002a 100644
> > > --- a/sysdeps/x86/Makefile
> > > +++ b/sysdeps/x86/Makefile
> > > @@ -12,7 +12,8 @@ CFLAGS-get-cpuid-feature-leaf.o += $(no-stack-protector)
> > >
> > > tests += tst-get-cpu-features tst-get-cpu-features-static \
> > > tst-cpu-features-cpuinfo tst-cpu-features-cpuinfo-static \
> > > - tst-cpu-features-supports tst-cpu-features-supports-static
> > > + tst-cpu-features-supports tst-cpu-features-supports-static \
> > > + tst-hwcap-tunables
> > > tests-static += tst-get-cpu-features-static \
> > > tst-cpu-features-cpuinfo-static \
> > > tst-cpu-features-supports-static
> > > @@ -65,6 +66,7 @@ $(objpfx)tst-isa-level-1.out: $(objpfx)tst-isa-level-mod-1-baseline.so \
> > > endif
> > > tst-ifunc-isa-2-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-SSE4_2,-AVX,-AVX2,-AVX512F
> > > tst-ifunc-isa-2-static-ENV = $(tst-ifunc-isa-2-ENV)
> > > +tst-hwcap-tunables-ARGS = -- $(host-test-program-cmd)
> > > endif
> > >
> > > ifeq ($(subdir),math)
> > > diff --git a/sysdeps/x86/cpu-tunables.c b/sysdeps/x86/cpu-tunables.c
> > > index 5697885226..ef96148d30 100644
> > > --- a/sysdeps/x86/cpu-tunables.c
> > > +++ b/sysdeps/x86/cpu-tunables.c
> > > @@ -24,11 +24,12 @@
> > > #include <string.h>
> > > #include <cpu-features.h>
> > > #include <ldsodefs.h>
> > > +#include <dl-tunables-parse.h>
> > > #include <dl-symbol-redir-ifunc.h>
> > >
> > > #define CHECK_GLIBC_IFUNC_CPU_OFF(f, cpu_features, name, len) \
> > > _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> > > - if (memcmp (f, #name, len) == 0) \
> > > + if (tunable_str_comma_strcmp_cte (&f, #name)) \
> > > { \
> > > CPU_FEATURE_UNSET (cpu_features, name) \
> > > break; \
> > > @@ -38,7 +39,7 @@
> > > which isn't available. */
> > > #define CHECK_GLIBC_IFUNC_PREFERRED_OFF(f, cpu_features, name, len) \
> > > _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> > > - if (memcmp (f, #name, len) == 0) \
> > > + if (tunable_str_comma_strcmp_cte (&f, #name) == 0) \
^^^ Typo.
A patch is at
https://patchwork.sourceware.org/project/glibc/patch/20231220000448.1420614-1-hjl.tools@gmail.com/
> > > { \
> > > cpu_features->preferred[index_arch_##name] \
> > > &= ~bit_arch_##name; \
> > > @@ -46,12 +47,11 @@
> > > }
> > >
> > > /* Enable/disable a preferred feature NAME. */
> > > -#define CHECK_GLIBC_IFUNC_PREFERRED_BOTH(f, cpu_features, name, \
> > > - disable, len) \
> > > +#define CHECK_GLIBC_IFUNC_PREFERRED_BOTH(f, cpu_features, name, len) \
> > > _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> > > - if (memcmp (f, #name, len) == 0) \
> > > + if (tunable_str_comma_strcmp_cte (&f, #name)) \
> > > { \
> > > - if (disable) \
> > > + if (f.disable) \
> > > cpu_features->preferred[index_arch_##name] &= ~bit_arch_##name; \
> > > else \
> > > cpu_features->preferred[index_arch_##name] |= bit_arch_##name; \
> > > @@ -61,11 +61,11 @@
> > > /* Enable/disable a preferred feature NAME. Enable a preferred feature
> > > only if the feature NEED is usable. */
> > > #define CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH(f, cpu_features, name, \
> > > - need, disable, len) \
> > > + need, len) \
> > > _Static_assert (sizeof (#name) - 1 == len, #name " != " #len); \
> > > - if (memcmp (f, #name, len) == 0) \
> > > + if (tunable_str_comma_strcmp_cte (&f, #name)) \
> > > { \
> > > - if (disable) \
> > > + if (f.disable) \
> > > cpu_features->preferred[index_arch_##name] &= ~bit_arch_##name; \
> > > else if (CPU_FEATURE_USABLE_P (cpu_features, need)) \
> > > cpu_features->preferred[index_arch_##name] |= bit_arch_##name; \
> > > @@ -93,38 +93,20 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > NOTE: the IFUNC selection may change over time. Please check all
> > > multiarch implementations when experimenting. */
> > >
> > > - const char *p = valp->strval, *c;
> > > struct cpu_features *cpu_features = &GLRO(dl_x86_cpu_features);
> > > - size_t len;
> > >
> > > - do
> > > - {
> > > - const char *n;
> > > - bool disable;
> > > - size_t nl;
> > > -
> > > - for (c = p; *c != ','; c++)
> > > - if (*c == '\0')
> > > - break;
> > > + struct tunable_str_comma_state_t ts;
> > > + tunable_str_comma_init (&ts, valp);
> > >
> > > - len = c - p;
> > > - disable = *p == '-';
> > > - if (disable)
> > > - {
> > > - n = p + 1;
> > > - nl = len - 1;
> > > - }
> > > - else
> > > - {
> > > - n = p;
> > > - nl = len;
> > > - }
> > > - switch (nl)
> > > + struct tunable_str_comma_t n;
> > > + while (tunable_str_comma_next (&ts, &n))
> > > + {
> > > + switch (n.len)
> > > {
> > > default:
> > > break;
> > > case 3:
> > > - if (disable)
> > > + if (n.disable)
> > > {
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX, 3);
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, CX8, 3);
> > > @@ -135,7 +117,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > }
> > > break;
> > > case 4:
> > > - if (disable)
> > > + if (n.disable)
> > > {
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX2, 4);
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, BMI1, 4);
> > > @@ -149,7 +131,7 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > }
> > > break;
> > > case 5:
> > > - if (disable)
> > > + if (n.disable)
> > > {
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, LZCNT, 5);
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, MOVBE, 5);
> > > @@ -159,12 +141,12 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > }
> > > break;
> > > case 6:
> > > - if (disable)
> > > + if (n.disable)
> > > {
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, POPCNT, 6);
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SSE4_1, 6);
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, SSE4_2, 6);
> > > - if (memcmp (n, "XSAVEC", 6) == 0)
> > > + if (memcmp (n.str, "XSAVEC", 6) == 0)
> > > {
> > > /* Update xsave_state_size to XSAVE state size. */
> > > cpu_features->xsave_state_size
> > > @@ -174,14 +156,14 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > }
> > > break;
> > > case 7:
> > > - if (disable)
> > > + if (n.disable)
> > > {
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512F, 7);
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, OSXSAVE, 7);
> > > }
> > > break;
> > > case 8:
> > > - if (disable)
> > > + if (n.disable)
> > > {
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512CD, 8);
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512BW, 8);
> > > @@ -190,86 +172,72 @@ TUNABLE_CALLBACK (set_hwcaps) (tunable_val_t *valp)
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512PF, 8);
> > > CHECK_GLIBC_IFUNC_CPU_OFF (n, cpu_features, AVX512VL, 8);
> > > }
> > > - CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Slow_BSF,
> > > - disable, 8);
> > > + CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Slow_BSF, 8);
> > > break;
> > > case 11:
> > > {
> > > - CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> > > - Prefer_ERMS,
> > > - disable, 11);
> > > - CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> > > - Prefer_FSRM,
> > > - disable, 11);
> > > + CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Prefer_ERMS,
> > > + 11);
> > > + CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features, Prefer_FSRM,
> > > + 11);
> > > CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH (n, cpu_features,
> > > Slow_SSE4_2,
> > > SSE4_2,
> > > - disable, 11);
> > > + 11);
> > > }
> > > break;
> > > case 15:
> > > {
> > > CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> > > - Fast_Rep_String,
> > > - disable, 15);
> > > + Fast_Rep_String, 15);
> > > }
> > > break;
> > > case 16:
> > > {
> > > CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
> > > - (n, cpu_features, Prefer_No_AVX512, AVX512F,
> > > - disable, 16);
> > > + (n, cpu_features, Prefer_No_AVX512, AVX512F, 16);
> > > }
> > > break;
> > > case 18:
> > > {
> > > CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> > > - Fast_Copy_Backward,
> > > - disable, 18);
> > > + Fast_Copy_Backward, 18);
> > > }
> > > break;
> > > case 19:
> > > {
> > > CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> > > - Fast_Unaligned_Load,
> > > - disable, 19);
> > > + Fast_Unaligned_Load, 19);
> > > CHECK_GLIBC_IFUNC_PREFERRED_BOTH (n, cpu_features,
> > > - Fast_Unaligned_Copy,
> > > - disable, 19);
> > > + Fast_Unaligned_Copy, 19);
> > > }
> > > break;
> > > case 20:
> > > {
> > > CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
> > > - (n, cpu_features, Prefer_No_VZEROUPPER, AVX, disable,
> > > - 20);
> > > + (n, cpu_features, Prefer_No_VZEROUPPER, AVX, 20);
> > > }
> > > break;
> > > case 23:
> > > {
> > > CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
> > > - (n, cpu_features, AVX_Fast_Unaligned_Load, AVX,
> > > - disable, 23);
> > > + (n, cpu_features, AVX_Fast_Unaligned_Load, AVX, 23);
> > > }
> > > break;
> > > case 24:
> > > {
> > > CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
> > > - (n, cpu_features, MathVec_Prefer_No_AVX512, AVX512F,
> > > - disable, 24);
> > > + (n, cpu_features, MathVec_Prefer_No_AVX512, AVX512F, 24);
> > > }
> > > break;
> > > case 26:
> > > {
> > > CHECK_GLIBC_IFUNC_PREFERRED_NEED_BOTH
> > > - (n, cpu_features, Prefer_PMINUB_for_stringop, SSE2,
> > > - disable, 26);
> > > + (n, cpu_features, Prefer_PMINUB_for_stringop, SSE2, 26);
> > > }
> > > break;
> > > }
> > > - p += len + 1;
> > > }
> > > - while (*c != '\0');
> > > }
> > >
> > > #if CET_ENABLED
> > > @@ -277,11 +245,11 @@ attribute_hidden
> > > void
> > > TUNABLE_CALLBACK (set_x86_ibt) (tunable_val_t *valp)
> > > {
> > > - if (memcmp (valp->strval, "on", sizeof ("on")) == 0)
> > > + if (tunable_strcmp_cte (valp, "on"))
> > > GL(dl_x86_feature_control).ibt = cet_always_on;
> > > - else if (memcmp (valp->strval, "off", sizeof ("off")) == 0)
> > > + else if (tunable_strcmp_cte (valp, "off"))
> > > GL(dl_x86_feature_control).ibt = cet_always_off;
> > > - else if (memcmp (valp->strval, "permissive", sizeof ("permissive")) == 0)
> > > + else if (tunable_strcmp_cte (valp, "permissive"))
> > > GL(dl_x86_feature_control).ibt = cet_permissive;
> > > }
> > >
> > > @@ -289,11 +257,11 @@ attribute_hidden
> > > void
> > > TUNABLE_CALLBACK (set_x86_shstk) (tunable_val_t *valp)
> > > {
> > > - if (memcmp (valp->strval, "on", sizeof ("on")) == 0)
> > > + if (tunable_strcmp_cte (valp, "on"))
> > > GL(dl_x86_feature_control).shstk = cet_always_on;
> > > - else if (memcmp (valp->strval, "off", sizeof ("off")) == 0)
> > > + else if (tunable_strcmp_cte (valp, "off"))
> > > GL(dl_x86_feature_control).shstk = cet_always_off;
> > > - else if (memcmp (valp->strval, "permissive", sizeof ("permissive")) == 0)
> > > + else if (tunable_strcmp_cte (valp, "permissive"))
> > > GL(dl_x86_feature_control).shstk = cet_permissive;
> > > }
> > > #endif
> > > diff --git a/sysdeps/x86/tst-hwcap-tunables.c b/sysdeps/x86/tst-hwcap-tunables.c
> > > new file mode 100644
> > > index 0000000000..01a9377f7e
> > > --- /dev/null
> > > +++ b/sysdeps/x86/tst-hwcap-tunables.c
> > > @@ -0,0 +1,148 @@
> > > +/* Tests for x86 GLIBC_TUNABLES=glibc.cpu.hwcaps filter.
> > > + Copyright (C) 2023 Free Software Foundation, Inc.
> > > + This file is part of the GNU C Library.
> > > +
> > > + The GNU C Library is free software; you can redistribute it and/or
> > > + modify it under the terms of the GNU Lesser General Public
> > > + License as published by the Free Software Foundation; either
> > > + version 2.1 of the License, or (at your option) any later version.
> > > +
> > > + The GNU C Library is distributed in the hope that it will be useful,
> > > + but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > > + Lesser General Public License for more details.
> > > +
> > > + You should have received a copy of the GNU Lesser General Public
> > > + License along with the GNU C Library; if not, see
> > > + <http://www.gnu.org/licenses/>. */
> > > +
> > > +#include <array_length.h>
> > > +#include <getopt.h>
> > > +#include <ifunc-impl-list.h>
> > > +#include <spawn.h>
> > > +#include <stdio.h>
> > > +#include <stdlib.h>
> > > +#include <string.h>
> > > +#include <intprops.h>
> > > +#include <support/check.h>
> > > +#include <support/support.h>
> > > +#include <support/xunistd.h>
> > > +#include <support/capture_subprocess.h>
> > > +
> > > +/* Nonzero if the program gets called via `exec'. */
> > > +#define CMDLINE_OPTIONS \
> > > + { "restart", no_argument, &restart, 1 },
> > > +static int restart;
> > > +
> > > +/* Disable everything. */
> > > +static const char *test_1[] =
> > > +{
> > > + "__memcpy_avx512_no_vzeroupper",
> > > + "__memcpy_avx512_unaligned",
> > > + "__memcpy_avx512_unaligned_erms",
> > > + "__memcpy_evex_unaligned",
> > > + "__memcpy_evex_unaligned_erms",
> > > + "__memcpy_avx_unaligned",
> > > + "__memcpy_avx_unaligned_erms",
> > > + "__memcpy_avx_unaligned_rtm",
> > > + "__memcpy_avx_unaligned_erms_rtm",
> > > + "__memcpy_ssse3",
> > > +};
> > > +
> > > +static const struct test_t
> > > +{
> > > + const char *env;
> > > + const char *const *funcs;
> > > + size_t nfuncs;
> > > +} tests[] =
> > > +{
> > > + {
> > > + /* Disable everything. */
> > > + "-Prefer_ERMS,-Prefer_FSRM,-AVX,-AVX2,-AVX_Usable,-AVX2_Usable,"
> > > + "-AVX512F_Usable,-SSE4_1,-SSE4_2,-SSSE3,-Fast_Unaligned_Load,-ERMS,"
> > > + "-AVX_Fast_Unaligned_Load",
> > > + test_1,
> > > + array_length (test_1)
> > > + },
> > > + {
> > > + /* Same as before, but with some empty suboptions. */
> > > + ",-,-Prefer_ERMS,-Prefer_FSRM,-AVX,-AVX2,-AVX_Usable,-AVX2_Usable,"
> > > + "-AVX512F_Usable,-SSE4_1,-SSE4_2,,-SSSE3,-Fast_Unaligned_Load,,-,-ERMS,"
> > > + "-AVX_Fast_Unaligned_Load,-,",
> > > + test_1,
> > > + array_length (test_1)
> > > + }
> > > +};
> > > +
> > > +/* Called on process re-execution. */
> > > +_Noreturn static void
> > > +handle_restart (int ntest)
> > > +{
> > > + struct libc_ifunc_impl impls[32];
> > > + int cnt = __libc_ifunc_impl_list ("memcpy", impls, array_length (impls));
> > > + if (cnt == 0)
> > > + _exit (EXIT_SUCCESS);
> > > + TEST_VERIFY_EXIT (cnt >= 1);
> > > + for (int i = 0; i < cnt; i++)
> > > + {
> > > + for (int f = 0; f < tests[ntest].nfuncs; f++)
> > > + {
> > > + if (strcmp (impls[i].name, tests[ntest].funcs[f]) == 0)
> > > + TEST_COMPARE (impls[i].usable, false);
> > > + }
> > > + }
> > > +
> > > + _exit (EXIT_SUCCESS);
> > > +}
> > > +
> > > +static int
> > > +do_test (int argc, char *argv[])
> > > +{
> > > + /* We must have either:
> > > + - One our fource parameters left if called initially:
> > > + + path to ld.so optional
> > > + + "--library-path" optional
> > > + + the library path optional
> > > + + the application name
> > > + + the test to check */
> > > +
> > > + TEST_VERIFY_EXIT (argc == 2 || argc == 5);
> > > +
> > > + if (restart)
> > > + handle_restart (atoi (argv[1]));
> > > +
> > > + char nteststr[INT_BUFSIZE_BOUND (int)];
> > > +
> > > + char *spargv[10];
> > > + {
> > > + int i = 0;
> > > + for (; i < argc - 1; i++)
> > > + spargv[i] = argv[i + 1];
> > > + spargv[i++] = (char *) "--direct";
> > > + spargv[i++] = (char *) "--restart";
> > > + spargv[i++] = nteststr;
> > > + spargv[i] = NULL;
> > > + }
> > > +
> > > + for (int i = 0; i < array_length (tests); i++)
> > > + {
> > > + snprintf (nteststr, sizeof nteststr, "%d", i);
> > > +
> > > + printf ("[%d] Spawned test for %s\n", i, tests[i].env);
> > > + char *tunable = xasprintf ("glibc.cpu.hwcaps=%s", tests[i].env);
> > > + setenv ("GLIBC_TUNABLES", tunable, 1);
> > > +
> > > + struct support_capture_subprocess result
> > > + = support_capture_subprogram (spargv[0], spargv);
> > > + support_capture_subprocess_check (&result, "tst-tunables", 0,
> > > + sc_allow_stderr);
> > > + support_capture_subprocess_free (&result);
> > > +
> > > + free (tunable);
> > > + }
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +#define TEST_FUNCTION_ARGV do_test
> > > +#include <support/test-driver.c>
>
> I am checking in this:
>
> https://patchwork.sourceware.org/project/glibc/patch/ZYIKpyKCN7YhXDXI@gmail.com/
>
> to fix sysdeps/x86/tst-hwcap-tunables.c.
>
> --
> H.J.
--
H.J.
More information about the Libc-alpha
mailing list