| Message ID | 1494514306-4167-6-git-send-email-siddhesh@sourceware.org |
|---|---|
| State | New |
| Headers | show |
On 11/05/2017 11:51, Siddhesh Poyarekar wrote: > Drop _dl_hwcap_mask when building with tunables. This completes the > transition of hwcap_mask reading from _dl_hwcap_mask to tunables. > > * elf/dl-cache.c: Include dl-tunables.h. > (_dl_load_cache_lookup)[HAVE_TUNABLES]: Read > glibc.tune.hwcap_mask. > * elf/dl-hwcaps.c: Include dl-tunables.h. > (_dl_important_hwcaps)[HAVE_TUNABLES]: Read and update > glibc.tune.hwcap_mask. > * sysdeps/sparc/sparc32/dl-machine.h: Likewise. > * elf/dl-support.c (_dl_hwcap2)[HAVE_TUNABLES]: Drop > _dl_hwcap_mask. > * elf/dl-tunables.c (__tunable_set_val): Make a hidden alias. > * elf/dl-tunables.h (__tunable_set_val): Likewise. > * elf/rtld.c (rtld_global_ro)[HAVE_TUNABLES]: Drop > _dl_hwcap_mask. > (process_envvars)[HAVE_TUNABLES]: Likewise. > * sysdeps/generic/ldsodefs.h (rtld_global_ro)[HAVE_TUNABLES]: > Likewise. > * sysdeps/x86/cpu-features.c (init_cpu_features): Don't > initialize dl_hwcap_mask when tunables are enabled. I would recommend to incorporate the fix for LD_HWCAP_MASK on tst-end-setuid you sent before [1] to avoid this fail on some configuration (mine for instance). Also I think we should open a bugzilla to iron out the elf/dl-hwcaps.c allocation scheme which is triggering this issue. [1] https://sourceware.org/ml/libc-alpha/2017-05/msg00340.html > diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h > index f26a8b2..695ac24 100644 > --- a/sysdeps/generic/ldsodefs.h > +++ b/sysdeps/generic/ldsodefs.h > @@ -515,8 +515,10 @@ struct rtld_global_ro > /* Mask for hardware capabilities that are available. */ > EXTERN uint64_t _dl_hwcap; > > +#if !HAVE_TUNABLES > /* Mask for important hardware capabilities we honour. */ > EXTERN uint64_t _dl_hwcap_mask; > +#endif > > #ifdef HAVE_AUX_VECTOR > /* Pointer to the auxv list supplied to the program at startup. */ > diff --git a/sysdeps/sparc/sparc32/dl-machine.h b/sysdeps/sparc/sparc32/dl-machine.h > index cf7272f..6923e47 100644 > --- a/sysdeps/sparc/sparc32/dl-machine.h > +++ b/sysdeps/sparc/sparc32/dl-machine.h > @@ -39,7 +39,12 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr) > /* XXX The following is wrong! Dave Miller rejected to implement it > correctly. If this causes problems shoot *him*! */ > #ifdef SHARED > - return GLRO(dl_hwcap) & GLRO(dl_hwcap_mask) & HWCAP_SPARC_V9; > +# if HAVE_TUNABLES > + uint64_t hwcap_mask = TUNABLE_GET (glibc, tune, hwcap_mask); You are missing '#include <elf/dl-tunables.h>' and the final type argument for TUNABLE_GET (it fails to build for sparcv9 with tunable enabled). > +# else > + uint64_t hwcap_mask = GLRO(dl_hwcap_mask); > +# endif > + return GLRO(dl_hwcap) & hwcap_mask & HWCAP_SPARC_V9; > #else > return GLRO(dl_hwcap) & HWCAP_SPARC_V9; > #endif > diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c > index b481f50..4fe58bf 100644 > --- a/sysdeps/x86/cpu-features.c > +++ b/sysdeps/x86/cpu-features.c > @@ -316,7 +316,11 @@ no_cpuid: > /* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */ > GLRO(dl_platform) = NULL; > GLRO(dl_hwcap) = 0; > +#if !HAVE_TUNABLES > + /* The glibc.tune.hwcap_mask tunable is initialized already, so no need to do > + this. */ > GLRO(dl_hwcap_mask) = HWCAP_IMPORTANT; > +#endif > > # ifdef __x86_64__ > if (cpu_features->kind == arch_kind_intel) >
On 11/05/2017 11:51, Siddhesh Poyarekar wrote: > Drop _dl_hwcap_mask when building with tunables. This completes the > transition of hwcap_mask reading from _dl_hwcap_mask to tunables. > > * elf/dl-cache.c: Include dl-tunables.h. > (_dl_load_cache_lookup)[HAVE_TUNABLES]: Read > glibc.tune.hwcap_mask. > * elf/dl-hwcaps.c: Include dl-tunables.h. > (_dl_important_hwcaps)[HAVE_TUNABLES]: Read and update > glibc.tune.hwcap_mask. > * sysdeps/sparc/sparc32/dl-machine.h: Likewise. > * elf/dl-support.c (_dl_hwcap2)[HAVE_TUNABLES]: Drop > _dl_hwcap_mask. > * elf/dl-tunables.c (__tunable_set_val): Make a hidden alias. > * elf/dl-tunables.h (__tunable_set_val): Likewise. > * elf/rtld.c (rtld_global_ro)[HAVE_TUNABLES]: Drop > _dl_hwcap_mask. > (process_envvars)[HAVE_TUNABLES]: Likewise. > * sysdeps/generic/ldsodefs.h (rtld_global_ro)[HAVE_TUNABLES]: > Likewise. > * sysdeps/x86/cpu-features.c (init_cpu_features): Don't > initialize dl_hwcap_mask when tunables are enabled. I would recommend to incorporate the fix for LD_HWCAP_MASK on tst-end-setuid you sent before [1] to avoid this fail on some configuration (mine for instance). Also I think we should open a bugzilla to iron out the elf/dl-hwcaps.c allocation scheme which is triggering this issue. [1] https://sourceware.org/ml/libc-alpha/2017-05/msg00340.html > diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h > index f26a8b2..695ac24 100644 > --- a/sysdeps/generic/ldsodefs.h > +++ b/sysdeps/generic/ldsodefs.h > @@ -515,8 +515,10 @@ struct rtld_global_ro > /* Mask for hardware capabilities that are available. */ > EXTERN uint64_t _dl_hwcap; > > +#if !HAVE_TUNABLES > /* Mask for important hardware capabilities we honour. */ > EXTERN uint64_t _dl_hwcap_mask; > +#endif > > #ifdef HAVE_AUX_VECTOR > /* Pointer to the auxv list supplied to the program at startup. */ > diff --git a/sysdeps/sparc/sparc32/dl-machine.h b/sysdeps/sparc/sparc32/dl-machine.h > index cf7272f..6923e47 100644 > --- a/sysdeps/sparc/sparc32/dl-machine.h > +++ b/sysdeps/sparc/sparc32/dl-machine.h > @@ -39,7 +39,12 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr) > /* XXX The following is wrong! Dave Miller rejected to implement it > correctly. If this causes problems shoot *him*! */ > #ifdef SHARED > - return GLRO(dl_hwcap) & GLRO(dl_hwcap_mask) & HWCAP_SPARC_V9; > +# if HAVE_TUNABLES > + uint64_t hwcap_mask = TUNABLE_GET (glibc, tune, hwcap_mask); You are missing '#include <elf/dl-tunables.h>' and the final type argument for TUNABLE_GET (it fails to build for sparcv9 with tunable enabled). > +# else > + uint64_t hwcap_mask = GLRO(dl_hwcap_mask); > +# endif > + return GLRO(dl_hwcap) & hwcap_mask & HWCAP_SPARC_V9; > #else > return GLRO(dl_hwcap) & HWCAP_SPARC_V9; > #endif > diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c > index b481f50..4fe58bf 100644 > --- a/sysdeps/x86/cpu-features.c > +++ b/sysdeps/x86/cpu-features.c > @@ -316,7 +316,11 @@ no_cpuid: > /* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */ > GLRO(dl_platform) = NULL; > GLRO(dl_hwcap) = 0; > +#if !HAVE_TUNABLES > + /* The glibc.tune.hwcap_mask tunable is initialized already, so no need to do > + this. */ > GLRO(dl_hwcap_mask) = HWCAP_IMPORTANT; > +#endif > > # ifdef __x86_64__ > if (cpu_features->kind == arch_kind_intel) >
On Wednesday 17 May 2017 08:58 PM, Adhemerval Zanella wrote: > I would recommend to incorporate the fix for LD_HWCAP_MASK on tst-end-setuid > you sent before [1] to avoid this fail on some configuration (mine for > instance). Also I think we should open a bugzilla to iron out the > elf/dl-hwcaps.c allocation scheme which is triggering this issue. > > [1] https://sourceware.org/ml/libc-alpha/2017-05/msg00340.html Pushed fix to the test case: https://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=ce79740bdbccea312df6cfcf70689efb57792fc9 and reported bug: https://sourceware.org/bugzilla/show_bug.cgi?id=21502 Siddhesh > > >> diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h >> index f26a8b2..695ac24 100644 >> --- a/sysdeps/generic/ldsodefs.h >> +++ b/sysdeps/generic/ldsodefs.h >> @@ -515,8 +515,10 @@ struct rtld_global_ro >> /* Mask for hardware capabilities that are available. */ >> EXTERN uint64_t _dl_hwcap; >> >> +#if !HAVE_TUNABLES >> /* Mask for important hardware capabilities we honour. */ >> EXTERN uint64_t _dl_hwcap_mask; >> +#endif >> >> #ifdef HAVE_AUX_VECTOR >> /* Pointer to the auxv list supplied to the program at startup. */ >> diff --git a/sysdeps/sparc/sparc32/dl-machine.h b/sysdeps/sparc/sparc32/dl-machine.h >> index cf7272f..6923e47 100644 >> --- a/sysdeps/sparc/sparc32/dl-machine.h >> +++ b/sysdeps/sparc/sparc32/dl-machine.h >> @@ -39,7 +39,12 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr) >> /* XXX The following is wrong! Dave Miller rejected to implement it >> correctly. If this causes problems shoot *him*! */ >> #ifdef SHARED >> - return GLRO(dl_hwcap) & GLRO(dl_hwcap_mask) & HWCAP_SPARC_V9; >> +# if HAVE_TUNABLES >> + uint64_t hwcap_mask = TUNABLE_GET (glibc, tune, hwcap_mask); > > You are missing '#include <elf/dl-tunables.h>' and the final type argument for > TUNABLE_GET (it fails to build for sparcv9 with tunable enabled). > >> +# else >> + uint64_t hwcap_mask = GLRO(dl_hwcap_mask); >> +# endif >> + return GLRO(dl_hwcap) & hwcap_mask & HWCAP_SPARC_V9; >> #else >> return GLRO(dl_hwcap) & HWCAP_SPARC_V9; >> #endif >> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c >> index b481f50..4fe58bf 100644 >> --- a/sysdeps/x86/cpu-features.c >> +++ b/sysdeps/x86/cpu-features.c >> @@ -316,7 +316,11 @@ no_cpuid: >> /* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */ >> GLRO(dl_platform) = NULL; >> GLRO(dl_hwcap) = 0; >> +#if !HAVE_TUNABLES >> + /* The glibc.tune.hwcap_mask tunable is initialized already, so no need to do >> + this. */ >> GLRO(dl_hwcap_mask) = HWCAP_IMPORTANT; >> +#endif >> >> # ifdef __x86_64__ >> if (cpu_features->kind == arch_kind_intel) >>
diff --git a/elf/dl-cache.c b/elf/dl-cache.c index 017c78a..b7ae05c 100644 --- a/elf/dl-cache.c +++ b/elf/dl-cache.c @@ -24,6 +24,7 @@ #include <dl-procinfo.h> #include <stdint.h> #include <_itoa.h> +#include <dl-tunables.h> #ifndef _DL_PLATFORMS_COUNT # define _DL_PLATFORMS_COUNT 0 @@ -258,8 +259,14 @@ _dl_load_cache_lookup (const char *name) if (platform != (uint64_t) -1) platform = 1ULL << platform; +#if HAVE_TUNABLES + uint64_t hwcap_mask = TUNABLE_GET (glibc, tune, hwcap_mask, uint64_t); +#else + uint64_t hwcap_mask = GLRO(dl_hwcap_mask); +#endif + #define _DL_HWCAP_TLS_MASK (1LL << 63) - uint64_t hwcap_exclude = ~((GLRO(dl_hwcap) & GLRO(dl_hwcap_mask)) + uint64_t hwcap_exclude = ~((GLRO(dl_hwcap) & hwcap_mask) | _DL_HWCAP_PLATFORM | _DL_HWCAP_TLS_MASK); /* Only accept hwcap if it's for the right platform. */ diff --git a/elf/dl-hwcaps.c b/elf/dl-hwcaps.c index c437397..6a46441a 100644 --- a/elf/dl-hwcaps.c +++ b/elf/dl-hwcaps.c @@ -24,6 +24,7 @@ #include <ldsodefs.h> #include <dl-procinfo.h> +#include <dl-tunables.h> #ifdef _DL_FIRST_PLATFORM # define _DL_FIRST_EXTRA (_DL_FIRST_PLATFORM + _DL_PLATFORMS_COUNT) @@ -37,8 +38,13 @@ internal_function _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz, size_t *max_capstrlen) { +#if HAVE_TUNABLES + uint64_t hwcap_mask = TUNABLE_GET (glibc, tune, hwcap_mask, uint64_t); +#else + uint64_t hwcap_mask = GLRO(dl_hwcap_mask); +#endif /* Determine how many important bits are set. */ - uint64_t masked = GLRO(dl_hwcap) & GLRO(dl_hwcap_mask); + uint64_t masked = GLRO(dl_hwcap) & hwcap_mask; size_t cnt = platform != NULL; size_t n, m; size_t total; @@ -125,7 +131,12 @@ _dl_important_hwcaps (const char *platform, size_t platform_len, size_t *sz, LD_HWCAP_MASK environment variable (or default HWCAP_IMPORTANT). So there is no way to request ignoring an OS-supplied dsocap string and bit like you can ignore an OS-supplied HWCAP bit. */ - GLRO(dl_hwcap_mask) |= (uint64_t) mask << _DL_FIRST_EXTRA; + hwcap_mask |= (uint64_t) mask << _DL_FIRST_EXTRA; +#if HAVE_TUNABLES + TUNABLE_UPDATE_VAL (glibc, tune, hwcap_mask, &hwcap_mask); +#else + GLRO(dl_hwcap_mask) = hwcap_mask; +#endif size_t len; for (const char *p = dsocaps; p < dsocaps + dsocapslen; p += len + 1) { diff --git a/elf/dl-support.c b/elf/dl-support.c index 3c46a7a..c22be85 100644 --- a/elf/dl-support.c +++ b/elf/dl-support.c @@ -164,6 +164,7 @@ uint64_t _dl_hwcap2 __attribute__ ((nocommon)); /* The value of the FPU control word the kernel will preset in hardware. */ fpu_control_t _dl_fpu_control = _FPU_DEFAULT; +#if !HAVE_TUNABLES /* This is not initialized to HWCAP_IMPORTANT, matching the definition of _dl_important_hwcaps, below, where no hwcap strings are ever used. This mask is still used to mediate the lookups in the cache @@ -171,6 +172,7 @@ fpu_control_t _dl_fpu_control = _FPU_DEFAULT; LD_HWCAP_MASK environment variable here), there is no real point in setting _dl_hwcap nonzero below, but we do anyway. */ uint64_t _dl_hwcap_mask __attribute__ ((nocommon)); +#endif /* Prevailing state of the stack. Generally this includes PF_X, indicating it's * executable but this isn't true for all platforms. */ diff --git a/elf/dl-tunables.c b/elf/dl-tunables.c index abf10e5..be7733f 100644 --- a/elf/dl-tunables.c +++ b/elf/dl-tunables.c @@ -497,3 +497,4 @@ cb: if (callback) callback (&cur->val); } +rtld_hidden_def (__tunable_set_val) diff --git a/elf/dl-tunables.h b/elf/dl-tunables.h index 7a1124a..79a29a2 100644 --- a/elf/dl-tunables.h +++ b/elf/dl-tunables.h @@ -70,6 +70,8 @@ extern void __tunables_init (char **); extern void __tunable_set_val (tunable_id_t, void *, tunable_callback_t); extern void __tunable_update_val (tunable_id_t, void *); +rtld_hidden_proto (__tunable_set_val) + /* Get and return a tunable value. */ # define TUNABLE_GET(__top, __ns, __id, __type) \ ({ \ diff --git a/elf/rtld.c b/elf/rtld.c index 319ef06..3746653 100644 --- a/elf/rtld.c +++ b/elf/rtld.c @@ -159,7 +159,9 @@ struct rtld_global_ro _rtld_global_ro attribute_relro = ._dl_debug_fd = STDERR_FILENO, ._dl_use_load_bias = -2, ._dl_correct_cache_id = _DL_CACHE_DEFAULT_ID, +#if !HAVE_TUNABLES ._dl_hwcap_mask = HWCAP_IMPORTANT, +#endif ._dl_lazy = 1, ._dl_fpu_control = _FPU_DEFAULT, ._dl_pagesize = EXEC_PAGESIZE, @@ -2402,6 +2404,7 @@ process_envvars (enum mode *modep) _dl_show_auxv (); break; +#if !HAVE_TUNABLES case 10: /* Mask for the important hardware capabilities. */ if (!__libc_enable_secure @@ -2409,6 +2412,7 @@ process_envvars (enum mode *modep) GLRO(dl_hwcap_mask) = __strtoul_internal (&envline[11], NULL, 0, 0); break; +#endif case 11: /* Path where the binary is found. */ diff --git a/sysdeps/generic/ldsodefs.h b/sysdeps/generic/ldsodefs.h index f26a8b2..695ac24 100644 --- a/sysdeps/generic/ldsodefs.h +++ b/sysdeps/generic/ldsodefs.h @@ -515,8 +515,10 @@ struct rtld_global_ro /* Mask for hardware capabilities that are available. */ EXTERN uint64_t _dl_hwcap; +#if !HAVE_TUNABLES /* Mask for important hardware capabilities we honour. */ EXTERN uint64_t _dl_hwcap_mask; +#endif #ifdef HAVE_AUX_VECTOR /* Pointer to the auxv list supplied to the program at startup. */ diff --git a/sysdeps/sparc/sparc32/dl-machine.h b/sysdeps/sparc/sparc32/dl-machine.h index cf7272f..6923e47 100644 --- a/sysdeps/sparc/sparc32/dl-machine.h +++ b/sysdeps/sparc/sparc32/dl-machine.h @@ -39,7 +39,12 @@ elf_machine_matches_host (const Elf32_Ehdr *ehdr) /* XXX The following is wrong! Dave Miller rejected to implement it correctly. If this causes problems shoot *him*! */ #ifdef SHARED - return GLRO(dl_hwcap) & GLRO(dl_hwcap_mask) & HWCAP_SPARC_V9; +# if HAVE_TUNABLES + uint64_t hwcap_mask = TUNABLE_GET (glibc, tune, hwcap_mask); +# else + uint64_t hwcap_mask = GLRO(dl_hwcap_mask); +# endif + return GLRO(dl_hwcap) & hwcap_mask & HWCAP_SPARC_V9; #else return GLRO(dl_hwcap) & HWCAP_SPARC_V9; #endif diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c index b481f50..4fe58bf 100644 --- a/sysdeps/x86/cpu-features.c +++ b/sysdeps/x86/cpu-features.c @@ -316,7 +316,11 @@ no_cpuid: /* Reuse dl_platform, dl_hwcap and dl_hwcap_mask for x86. */ GLRO(dl_platform) = NULL; GLRO(dl_hwcap) = 0; +#if !HAVE_TUNABLES + /* The glibc.tune.hwcap_mask tunable is initialized already, so no need to do + this. */ GLRO(dl_hwcap_mask) = HWCAP_IMPORTANT; +#endif # ifdef __x86_64__ if (cpu_features->kind == arch_kind_intel)