| Message ID | 20231017130526.2216827-12-adhemerval.zanella@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Improve loader environment variable handling | expand |
On 2023-10-17 09:05, 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. > --- > elf/dl-tunables.c | 66 +++---- > elf/dl-tunables.h | 6 +- > sysdeps/generic/dl-tunables-parse.h | 128 ++++++++++++++ > sysdeps/s390/cpu-features.c | 167 +++++++----------- > .../unix/sysv/linux/aarch64/cpu-features.c | 38 ++-- > .../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 | 150 ++++++++++++++++ > 10 files changed, 455 insertions(+), 273 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 b39c14694c..869846f9da 100644 > --- a/elf/dl-tunables.c > +++ b/elf/dl-tunables.c > @@ -36,28 +36,6 @@ > #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) > @@ -134,14 +112,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); > } > > @@ -158,29 +136,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 respectibles values. VALSTRING is a duplicated values, where > - delimiters ':' are replaced with '\0', so string tunables are null > - terminated. > + and their respectibles values. The VALSTRING is parse in place, with the parsed > + tunable start and size recorded in TUNABLES. > Return the number of tunables found (including 0 if the string is empty) > or -1 if for a 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') > @@ -202,7 +180,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables) > /* Skip the ':' or '='. */ > p++; > > - char *value = p; > + const char *value = p; > > while (*p != '=' && *p != ':' && *p != '\0') > p++; > @@ -211,8 +189,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++) > @@ -221,7 +197,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; > } > } > @@ -231,7 +208,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); > @@ -243,7 +220,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 > @@ -264,9 +241,12 @@ __tunables_init (char **envp) > while ((envp = get_next_env (envp, &envname, &len, &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; > } > > @@ -284,7 +264,7 @@ __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); > + tunable_initialize (cur, envval, 0); Do you want to put an assert in here that the env aliases are only there for numeric tunables? Passing 0 does not seem right for strval tunables. > break; > } > } > @@ -298,7 +278,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 > { > @@ -324,7 +304,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 (); > @@ -359,7 +341,7 @@ __tunable_get_val (tunable_id_t id, void *valp, tunable_callback_t callback) > } > case TUNABLE_TYPE_STRING: > { > - *((const char **)valp) = cur->val.strval; > + *((struct tunable_str_t **) valp) = &cur->val.strval; > break; > } > default: > diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h > index 45c191e021..0e777d7d37 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/sysdeps/generic/dl-tunables-parse.h b/sysdeps/generic/dl-tunables-parse.h > new file mode 100644 > index 0000000000..a2420f9129 > --- /dev/null > +++ b/sysdeps/generic/dl-tunables-parse.h > @@ -0,0 +1,128 @@ > +/* 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 <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) What does cte stand for? > + > +/* > + Helper functions to iterate over a tunable string composed by multiple > + suboptions separated by comma. Each suboptions is return in the form Maybe enhance this comment like: suboptions separated by comma; this is a common pattern for CPU feature tunables. Alternatively, does it make sense to treat this like a list? That would make this abstraction easier to read and reason, e.g.: if (tunable_list_contains (&t, opt, opt1_len)) ... if (tunable_list_contains_cte_any (&t, opt1, opt2, ...)) and so on. You don't actually need the while loop or state maintenance then. In fact, maybe there's a case here to make a LIST tunable type. > + 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")) > + { > + [...] > + } > + } > +*/ > + > +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) > +{ > + 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..8b1466fa7b 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,85 +67,65 @@ 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. */ > @@ -187,14 +149,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 dc09c1c827..3f1a6bcd62 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,28 +35,32 @@ > struct cpu_list > { > const char *name; > + size_t len; > uint64_t midr; > }; > > -static struct cpu_list cpu_list[] = { > - {"falkor", 0x510FC000}, > - {"thunderxt88", 0x430F0A10}, > - {"thunderx2t99", 0x431F0AF0}, > - {"thunderx2t99p1", 0x420F5160}, > - {"phecda", 0x680F0000}, > - {"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 ("falkor", 0x510FC000), > + CPU_LIST_ENTRY ("thunderxt88", 0x430F0A10), > + CPU_LIST_ENTRY ("thunderx2t99", 0x431F0AF0), > + CPU_LIST_ENTRY ("thunderx2t99p1", 0x420F5160), > + CPU_LIST_ENTRY ("phecda", 0x680F0000), > + 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; > } > @@ -65,7 +71,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..0608209768 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) \ > { \ > 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..e9f50ffbe0 > --- /dev/null > +++ b/sysdeps/x86/tst-hwcap-tunables.c > @@ -0,0 +1,150 @@ > +/* Tests for powerpc 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/>. */ > + > +#pragma GCC optimize ("O0") > + > +#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>
On 27/10/23 08:54, Siddhesh Poyarekar wrote: > > > On 2023-10-17 09:05, 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. >> --- >> elf/dl-tunables.c | 66 +++---- >> elf/dl-tunables.h | 6 +- >> sysdeps/generic/dl-tunables-parse.h | 128 ++++++++++++++ >> sysdeps/s390/cpu-features.c | 167 +++++++----------- >> .../unix/sysv/linux/aarch64/cpu-features.c | 38 ++-- >> .../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 | 150 ++++++++++++++++ >> 10 files changed, 455 insertions(+), 273 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 b39c14694c..869846f9da 100644 >> --- a/elf/dl-tunables.c >> +++ b/elf/dl-tunables.c >> @@ -36,28 +36,6 @@ >> #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) >> @@ -134,14 +112,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); >> } >> @@ -158,29 +136,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 respectibles values. VALSTRING is a duplicated values, where >> - delimiters ':' are replaced with '\0', so string tunables are null >> - terminated. >> + and their respectibles values. The VALSTRING is parse in place, with the > > parsed Ack. > >> + tunable start and size recorded in TUNABLES. >> Return the number of tunables found (including 0 if the string is empty) >> or -1 if for a 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') >> @@ -202,7 +180,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables) >> /* Skip the ':' or '='. */ >> p++; >> - char *value = p; >> + const char *value = p; >> while (*p != '=' && *p != ':' && *p != '\0') >> p++; >> @@ -211,8 +189,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++) >> @@ -221,7 +197,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; >> } >> } >> @@ -231,7 +208,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); >> @@ -243,7 +220,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 >> @@ -264,9 +241,12 @@ __tunables_init (char **envp) >> while ((envp = get_next_env (envp, &envname, &len, &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; >> } >> @@ -284,7 +264,7 @@ __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); >> + tunable_initialize (cur, envval, 0); > > Do you want to put an assert in here that the env aliases are only there for numeric tunables? Passing 0 does not seem right for strval tunables. It should be 'tunable_initialize (cur, envval, len)' in fact, since we can keep a pointer to the kernel allocated environment variable to subsequent TUNABLE_CALLBACK queries. I will also add extra aliases checks on tst-tunebles (unfortunately we don't have any tunable aliases to string ones). > >> break; >> } >> } >> @@ -298,7 +278,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 >> { >> @@ -324,7 +304,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 (); >> @@ -359,7 +341,7 @@ __tunable_get_val (tunable_id_t id, void *valp, tunable_callback_t callback) >> } >> case TUNABLE_TYPE_STRING: >> { >> - *((const char **)valp) = cur->val.strval; >> + *((struct tunable_str_t **) valp) = &cur->val.strval; >> break; >> } >> default: >> diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h >> index 45c191e021..0e777d7d37 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/sysdeps/generic/dl-tunables-parse.h b/sysdeps/generic/dl-tunables-parse.h >> new file mode 100644 >> index 0000000000..a2420f9129 >> --- /dev/null >> +++ b/sysdeps/generic/dl-tunables-parse.h >> @@ -0,0 +1,128 @@ >> +/* 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 <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) > > What does cte stand for? 'constant' and it is meant for string literals (where sizeof works correctly). I tried to come up some macros/builtin shenanigans to check if the input is a string literal to either call sizeof of strlen, but in the end I could not find a reliable way and ended up with this. > >> + >> +/* >> + Helper functions to iterate over a tunable string composed by multiple >> + suboptions separated by comma. Each suboptions is return in the form > > Maybe enhance this comment like: > > suboptions separated by comma; this is a common pattern for CPU > feature tunables. Ack. > > Alternatively, does it make sense to treat this like a list? That would make this abstraction easier to read and reason, e.g.: > > if (tunable_list_contains (&t, opt, opt1_len)) > ... > if (tunable_list_contains_cte_any (&t, opt1, opt2, ...)) > > and so on. You don't actually need the while loop or state maintenance then. In fact, maybe there's a case here to make a LIST tunable type. A see two problems with a list: usually each suboption requires an action by the architecture. So if (tunable_list_contains_cte_any (&t, opt1, opt2, ...)) will required also if (tunable_list_contains (&t1, opt1)) // do something And so on. Another issues is each architecture callback will need to check whether each option is set or not; meaning that tunable_list_contains will require to return a list of unbonded returns values (to indicate whether the suboption is enabled or not). So I think an iterator-like seems to be the best option here. > >> + 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")) >> + { >> + [...] >> + } >> + } >> +*/ >> + >> +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) >> +{ >> + 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..8b1466fa7b 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,85 +67,65 @@ 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. */ >> @@ -187,14 +149,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 dc09c1c827..3f1a6bcd62 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,28 +35,32 @@ >> struct cpu_list >> { >> const char *name; >> + size_t len; >> uint64_t midr; >> }; >> -static struct cpu_list cpu_list[] = { >> - {"falkor", 0x510FC000}, >> - {"thunderxt88", 0x430F0A10}, >> - {"thunderx2t99", 0x431F0AF0}, >> - {"thunderx2t99p1", 0x420F5160}, >> - {"phecda", 0x680F0000}, >> - {"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 ("falkor", 0x510FC000), >> + CPU_LIST_ENTRY ("thunderxt88", 0x430F0A10), >> + CPU_LIST_ENTRY ("thunderx2t99", 0x431F0AF0), >> + CPU_LIST_ENTRY ("thunderx2t99p1", 0x420F5160), >> + CPU_LIST_ENTRY ("phecda", 0x680F0000), >> + 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; >> } >> @@ -65,7 +71,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..0608209768 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) \ >> { \ >> 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..e9f50ffbe0 >> --- /dev/null >> +++ b/sysdeps/x86/tst-hwcap-tunables.c >> @@ -0,0 +1,150 @@ >> +/* Tests for powerpc 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/>. */ >> + >> +#pragma GCC optimize ("O0") >> + >> +#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>
diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index b39c14694c..869846f9da 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -36,28 +36,6 @@ #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) @@ -134,14 +112,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); } @@ -158,29 +136,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 respectibles values. VALSTRING is a duplicated values, where - delimiters ':' are replaced with '\0', so string tunables are null - terminated. + and their respectibles values. The VALSTRING is parse 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 a 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') @@ -202,7 +180,7 @@ parse_tunables_string (char *valstring, struct tunable_toset_t *tunables) /* Skip the ':' or '='. */ p++; - char *value = p; + const char *value = p; while (*p != '=' && *p != ':' && *p != '\0') p++; @@ -211,8 +189,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++) @@ -221,7 +197,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; } } @@ -231,7 +208,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); @@ -243,7 +220,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 @@ -264,9 +241,12 @@ __tunables_init (char **envp) while ((envp = get_next_env (envp, &envname, &len, &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; } @@ -284,7 +264,7 @@ __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); + tunable_initialize (cur, envval, 0); break; } } @@ -298,7 +278,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 { @@ -324,7 +304,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 (); @@ -359,7 +341,7 @@ __tunable_get_val (tunable_id_t id, void *valp, tunable_callback_t callback) } case TUNABLE_TYPE_STRING: { - *((const char **)valp) = cur->val.strval; + *((struct tunable_str_t **) valp) = &cur->val.strval; break; } default: diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h index 45c191e021..0e777d7d37 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/sysdeps/generic/dl-tunables-parse.h b/sysdeps/generic/dl-tunables-parse.h new file mode 100644 index 0000000000..a2420f9129 --- /dev/null +++ b/sysdeps/generic/dl-tunables-parse.h @@ -0,0 +1,128 @@ +/* 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 <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 comma. 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")) + { + [...] + } + } +*/ + +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) +{ + 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..8b1466fa7b 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,85 +67,65 @@ 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. */ @@ -187,14 +149,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 dc09c1c827..3f1a6bcd62 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,28 +35,32 @@ struct cpu_list { const char *name; + size_t len; uint64_t midr; }; -static struct cpu_list cpu_list[] = { - {"falkor", 0x510FC000}, - {"thunderxt88", 0x430F0A10}, - {"thunderx2t99", 0x431F0AF0}, - {"thunderx2t99p1", 0x420F5160}, - {"phecda", 0x680F0000}, - {"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 ("falkor", 0x510FC000), + CPU_LIST_ENTRY ("thunderxt88", 0x430F0A10), + CPU_LIST_ENTRY ("thunderx2t99", 0x431F0AF0), + CPU_LIST_ENTRY ("thunderx2t99p1", 0x420F5160), + CPU_LIST_ENTRY ("phecda", 0x680F0000), + 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; } @@ -65,7 +71,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..0608209768 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) \ { \ 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..e9f50ffbe0 --- /dev/null +++ b/sysdeps/x86/tst-hwcap-tunables.c @@ -0,0 +1,150 @@ +/* Tests for powerpc 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/>. */ + +#pragma GCC optimize ("O0") + +#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>