From 2de91d914cc5ed4ed7ea8e70d5e06c46e991f39f Mon Sep 17 00:00:00 2001 From: Paul Floyd Date: Fri, 23 Dec 2022 16:49:20 +0100 Subject: [PATCH] Bug 444488 - Use glibc.pthread.stack_cache_size tunable Try to use GLIBC_TUNABLES to disable the pthread stack cache. --- NEWS | 1 + coregrind/m_clientstate.c | 2 + coregrind/m_initimg/initimg-linux.c | 61 +++++++++++++++++++++++------ coregrind/m_redir.c | 19 ++++++++- coregrind/m_scheduler/scheduler.c | 32 ++++++++++++++- coregrind/pub_core_clientstate.h | 2 + 6 files changed, 102 insertions(+), 15 deletions(-) diff --git a/NEWS b/NEWS index c9376dcca9..4dd33a364e 100644 --- a/NEWS +++ b/NEWS @@ -25,6 +25,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 170510 Don't warn about ioctl of size 0 without direction hint 351857 confusing error message about valid command line option 444110 priv/guest_ppc_toIR.c:36198:31: warning: duplicated 'if' condition. +444488 Use glibc.pthread.stack_cache_size tunable 459476 vgdb: allow address reuse to avoid "address already in use" errorsuse" errors 462830 WARNING: unhandled amd64-freebsd syscall: 474 463027 broken check for MPX instruction support in assembler diff --git a/coregrind/m_clientstate.c b/coregrind/m_clientstate.c index 23c846d6b8..93662dcb3d 100644 --- a/coregrind/m_clientstate.c +++ b/coregrind/m_clientstate.c @@ -121,6 +121,8 @@ Addr VG_(client__dl_sysinfo_int80) = 0; in nptl/allocatestack.c */ SizeT* VG_(client__stack_cache_actsize__addr) = 0; +client__gnu_get_libc_version_type VG_(client__gnu_get_libc_version_addr) = 0; + #if defined(VGO_solaris) /* Address of variable vg_vfork_fildes in vgpreload_core.so.0 (vg_preloaded.c). */ diff --git a/coregrind/m_initimg/initimg-linux.c b/coregrind/m_initimg/initimg-linux.c index 48df8c1225..4da9a8b976 100644 --- a/coregrind/m_initimg/initimg-linux.c +++ b/coregrind/m_initimg/initimg-linux.c @@ -120,19 +120,19 @@ static void load_client ( /*MOD*/ExeInfo* info, If this needs to handle any more variables it should be hacked into something table driven. The copy is VG_(malloc)'d space. */ -static HChar** setup_client_env ( HChar** origenv, const HChar* toolname) +static HChar** setup_client_env ( HChar** origenv, const HChar* toolname, Bool use_stack_cache_tunable) { vg_assert(origenv); vg_assert(toolname); - const HChar* preload_core = "vgpreload_core"; - const HChar* ld_preload = "LD_PRELOAD="; - const HChar* v_launcher = VALGRIND_LAUNCHER "="; - Int ld_preload_len = VG_(strlen)( ld_preload ); - Int v_launcher_len = VG_(strlen)( v_launcher ); - Bool ld_preload_done = False; - Int vglib_len = VG_(strlen)(VG_(libdir)); - Bool debug = False; + const HChar* preload_core = "vgpreload_core"; + const HChar* ld_preload = "LD_PRELOAD="; + const HChar* v_launcher = VALGRIND_LAUNCHER "="; + Int ld_preload_len = VG_(strlen)( ld_preload ); + Int v_launcher_len = VG_(strlen)( v_launcher ); + Bool ld_preload_done = False; + Int vglib_len = VG_(strlen)(VG_(libdir)); + Bool debug = False; HChar** cpp; HChar** ret; @@ -175,9 +175,10 @@ static HChar** setup_client_env ( HChar** origenv, const HChar* toolname) if (debug) VG_(printf)("XXXXXXXXX: BEFORE %s\n", *cpp); } - /* Allocate a new space */ + /* Allocate a new space + * Size is envc + 1 new entry + maybe one for GLIBC_TUNABLES + NULL */ ret = VG_(malloc) ("initimg-linux.sce.3", - sizeof(HChar *) * (envc+1+1)); /* 1 new entry + NULL */ + sizeof(HChar *) * (envc+1+1+(use_stack_cache_tunable ? 1 : 0))); /* copy it over */ for (cpp = ret; *origenv; ) { @@ -201,6 +202,18 @@ static HChar** setup_client_env ( HChar** origenv, const HChar* toolname) ld_preload_done = True; } + if (use_stack_cache_tunable) { + /* overwrite value found with zeroes */ + const HChar* search_string = "glibc.pthread.stack_cache_size="; + HChar* val; + if ((val = VG_(strstr)(*cpp, search_string))) { + val += VG_(strlen)(search_string); + while (*val != '\0' && *val != ':') { + *val++ = '0'; + } + use_stack_cache_tunable = False; + } + } if (debug) VG_(printf)("XXXXXXXXX: MASH %s\n", *cpp); } @@ -215,6 +228,10 @@ static HChar** setup_client_env ( HChar** origenv, const HChar* toolname) if (debug) VG_(printf)("XXXXXXXXX: ADD %s\n", cp); } + if (use_stack_cache_tunable) { + ret[envc++] = VG_(strdup)("initimg-linux.sce.6", "GLIBC_TUNABLES=glibc.pthread.stack_cache_size=0"); + } + /* ret[0 .. envc-1] is live now. */ /* Find and remove a binding for VALGRIND_LAUNCHER. */ for (i = 0; i < envc; i++) @@ -1004,6 +1021,26 @@ static void setup_client_dataseg ( SizeT max_size ) vg_assert(sr_Res(sres) == anon_start); } +/* + * In glibc 2.34 we need to use the TUNABLE mechanism to + * disable stack cache when --sim-hints=no-nptl-pthread-stackcache + * is specified. This needs to be done in the same manner + * as LD_PRELOAD. + * + * See https://bugs.kde.org/show_bug.cgi?id=444488 + */ +static Bool need_stack_cache_tunable(HChar** argv) +{ + while (argv && *argv) { + if (VG_(strncmp)(*argv, "--sim-hints=", VG_(strlen)("--sim-hints=")) == 0) { + if (VG_(strstr)(*argv, "no-nptl-pthread-stackcache")) { + return True; + } + } + ++argv; + } + return False; +} /*====================================================================*/ /*=== TOP-LEVEL: VG_(setup_client_initial_image) ===*/ @@ -1046,7 +1083,7 @@ IIFinaliseImageInfo VG_(ii_create_image)( IICreateImageInfo iicii, // p: get_helprequest_and_toolname [for toolname] //-------------------------------------------------------------- VG_(debugLog)(1, "initimg", "Setup client env\n"); - env = setup_client_env(iicii.envp, iicii.toolname); + env = setup_client_env(iicii.envp, iicii.toolname, need_stack_cache_tunable(iicii.argv)); //-------------------------------------------------------------- // Setup client stack, eip, and VG_(client_arg[cv]) diff --git a/coregrind/m_redir.c b/coregrind/m_redir.c index 66a3c0c4f9..37c67f4c13 100644 --- a/coregrind/m_redir.c +++ b/coregrind/m_redir.c @@ -405,6 +405,8 @@ void VG_(redir_notify_new_DebugInfo)( const DebugInfo* newdi ) const HChar* const pthread_soname = "libpthread.so.0"; const HChar* const pthread_stack_cache_actsize_varname = "stack_cache_actsize"; + const HChar* const libc_soname = "libc.so.6"; + const HChar* const libc_gnu_get_libc_version_funcname = "gnu_get_libc_version"; #if defined(VGO_solaris) Bool vg_vfork_fildes_var_search = False; const HChar* const vg_preload_core_soname = "vgpreload_core.so.0"; @@ -506,7 +508,8 @@ void VG_(redir_notify_new_DebugInfo)( const DebugInfo* newdi ) dehacktivate_pthread_stack_cache_var_search = SimHintiS(SimHint_no_nptl_pthread_stackcache, VG_(clo_sim_hints)) - && 0 == VG_(strcmp)(newdi_soname, pthread_soname); + && (0 == VG_(strcmp)(newdi_soname, pthread_soname) || + 0 == VG_(strcmp)(newdi_soname, libc_soname)); #if defined(VGO_solaris) vg_vfork_fildes_var_search = @@ -529,6 +532,20 @@ void VG_(redir_notify_new_DebugInfo)( const DebugInfo* newdi ) &demangled_sopatt, &demangled_fnpatt, &isWrap, &becTag, &becPrio ); + + if (isText && dehacktivate_pthread_stack_cache_var_search) { + if (0 == VG_(strcmp)(*names, libc_gnu_get_libc_version_funcname)) { + if ( VG_(clo_verbosity) > 1 ) { + VG_(message)( Vg_DebugMsg, + "deactivate nptl pthread stackcache via tunable:" + " found symbol %s at addr %p\n", + *names, (void*) sym_avmas.main); + } + VG_(client__gnu_get_libc_version_addr) = (client__gnu_get_libc_version_type) sym_avmas.main; + dehacktivate_pthread_stack_cache_var_search = False; + } + } + /* ignore data symbols */ if (!isText) { /* But search for dehacktivate stack cache var if needed. */ diff --git a/coregrind/m_scheduler/scheduler.c b/coregrind/m_scheduler/scheduler.c index 00cc0c6889..027560c2ad 100644 --- a/coregrind/m_scheduler/scheduler.c +++ b/coregrind/m_scheduler/scheduler.c @@ -1348,8 +1348,36 @@ VgSchedReturnCode VG_(scheduler) ( ThreadId tid ) to be added without risk of overflow. */ } } else { - VG_(debugLog)(0,"sched", - "WARNING: pthread stack cache cannot be disabled!\n"); + /* + * glibc 2.34 no longer has stack_cache_actsize as a visible variable + * so we switch to using the GLIBC_TUNABLES env var. Processing for that + * is done in initimg-linux.c / setup_client_env for all glibc + * + * If we don't detect stack_cache_actsize we want to be able to tell + * whether it is an unexpected error or if it is no longer there. + * In the latter case we don't print a warning. + */ + Bool print_warning = True; + if (VG_(client__gnu_get_libc_version_addr) != NULL) { + const HChar* gnu_libc_version = VG_(client__gnu_get_libc_version_addr)(); + if (gnu_libc_version != NULL) { + HChar* glibc_version_tok = VG_(strdup)("scheduler.1", gnu_libc_version); + const HChar* str_major = VG_(strtok)(glibc_version_tok, "."); + Long major = VG_(strtoll10)(str_major, NULL); + const HChar* str_minor = VG_(strtok)(NULL, "."); + Long minor = VG_(strtoll10)(str_minor, NULL); + if (major >= 2 && minor >= 34) { + print_warning = False; + } + VG_(free)(glibc_version_tok); + } + } else { + + } + if (print_warning) { + VG_(debugLog)(0,"sched", + "WARNING: pthread stack cache cannot be disabled!\n"); + } VG_(clo_sim_hints) &= ~SimHint2S(SimHint_no_nptl_pthread_stackcache); /* Remove SimHint_no_nptl_pthread_stackcache from VG_(clo_sim_hints) to avoid having a msg for all following threads. */ diff --git a/coregrind/pub_core_clientstate.h b/coregrind/pub_core_clientstate.h index fb83a9ea23..824ce1e05f 100644 --- a/coregrind/pub_core_clientstate.h +++ b/coregrind/pub_core_clientstate.h @@ -118,6 +118,8 @@ extern Addr VG_(get_initial_client_SP)(void); It would be much cleaner to have a documented and supported way to disable the pthread stack cache. */ extern SizeT* VG_(client__stack_cache_actsize__addr); +typedef const HChar* (*client__gnu_get_libc_version_type)(void); +extern client__gnu_get_libc_version_type VG_(client__gnu_get_libc_version_addr); #if defined(VGO_solaris) /* Address of variable vg_vfork_fildes in vgpreload_core.so.0 -- 2.47.2