From c1f6e5e64aedc82b415bea515d731061d0d6e76f Mon Sep 17 00:00:00 2001 From: Paul Floyd Date: Sun, 21 Sep 2025 17:08:35 +0200 Subject: [PATCH] Refactor: make try_get_interp extern and multi-plaftorm Previously it was static and defined for Darwin, FreeBSD and Linux. Now it is global VG_(args_the_exename) and has a length check. Also fixed a nasty bug related to VG_(args_the_exename). Initially this is set to point to the name of the client command in Valgrinds own arguments. Later when checking for scripts or binaries VG_(load_script) may get called recursively. If it gets called more than once it sets VG_(args_the_exename) to point to the new name. But that is on the stack. Later, if the stack grows too much the name will get overwritten. I was seeing that with my first versions of this code in the recursive tests in none/tests/scripts. Now I'm allocating VG_(args_the_exename) on the heap. --- coregrind/m_commandline.c | 2 +- coregrind/m_initimg/initimg-darwin.c | 54 +-------------------------- coregrind/m_initimg/initimg-freebsd.c | 48 +----------------------- coregrind/m_initimg/initimg-linux.c | 52 +------------------------- coregrind/m_pathscan.c | 40 ++++++++++++++++++++ coregrind/m_ume/script.c | 10 ++++- coregrind/pub_core_pathscan.h | 4 +- 7 files changed, 56 insertions(+), 154 deletions(-) diff --git a/coregrind/m_commandline.c b/coregrind/m_commandline.c index e9fdb9bb4c..da57d6c8c1 100644 --- a/coregrind/m_commandline.c +++ b/coregrind/m_commandline.c @@ -213,7 +213,7 @@ void VG_(split_up_argv)( Int argc, HChar** argv ) /* Should now be looking at the exe name. */ if (i < argc) { vg_assert(argv[i]); - VG_(args_the_exename) = argv[i]; + VG_(args_the_exename) = VG_(strdup)("commandline.sua.4", argv[i]); i++; } diff --git a/coregrind/m_initimg/initimg-darwin.c b/coregrind/m_initimg/initimg-darwin.c index ca71295f2d..69685ef672 100644 --- a/coregrind/m_initimg/initimg-darwin.c +++ b/coregrind/m_initimg/initimg-darwin.c @@ -259,58 +259,6 @@ static HChar *copy_str(HChar **tab, const HChar *str) return orig; } -/* - * @todo PJF Make this multi-platform - */ -static Bool try_get_interp(const HChar* args_exe, HChar* interp_out) -{ - HChar hdr[4096]; - Int len = sizeof hdr; - SysRes res; - Int fd; - HChar* end; - HChar* cp; - HChar* interp; - - res = VG_(open)(args_exe, VKI_O_RDONLY, 0); - if (sr_isError(res)) { - return False; - } else { - fd = sr_Res(res); - } - - res = VG_(pread)(fd, hdr, len, 0); - - if (sr_isError(res)) { - VG_(close)(fd); - return False; - } else { - len = sr_Res(res); - } - - if (0 != VG_(memcmp)(hdr, "#!", 2)) { - VG_(close)(fd); - return False; - } - - end = hdr + len; - interp = hdr + 2; - while (interp < end && (*interp == ' ' || *interp == '\t')) - interp++; - - for (cp = interp; cp < end && !VG_(isspace)(*cp); cp++) - ; - - *cp = '\0'; - - VG_(sprintf)(interp_out, "%s", interp); - - VG_(close)(fd); - return True; -} - - - /* ---------------------------------------------------------------- This sets up the client's initial stack, containing the args, @@ -503,7 +451,7 @@ Addr setup_client_stack( void* init_sp, if (VG_(resolved_exename) == NULL) { const HChar *exe_name = VG_(find_executable)(VG_(args_the_exename)); HChar interp_name[VKI_PATH_MAX]; - if (try_get_interp(exe_name, interp_name)) { + if (VG_(try_get_interp)(exe_name, interp_name, VKI_PATH_MAX)) { exe_name = interp_name; } HChar resolved_name[VKI_PATH_MAX]; diff --git a/coregrind/m_initimg/initimg-freebsd.c b/coregrind/m_initimg/initimg-freebsd.c index 3e0d07544d..2f8ac82ebb 100644 --- a/coregrind/m_initimg/initimg-freebsd.c +++ b/coregrind/m_initimg/initimg-freebsd.c @@ -337,52 +337,6 @@ static const struct auxv *find_auxv(const UWord* sp) /* * @todo PJF Make this multi-platform */ -static Bool try_get_interp(const HChar* args_exe, HChar* interp_out) -{ - HChar hdr[4096]; - Int len = sizeof hdr; - SysRes res; - Int fd; - HChar* end; - HChar* cp; - HChar* interp; - - res = VG_(open)(args_exe, VKI_O_RDONLY, 0); - if (sr_isError(res)) { - return False; - } else { - fd = sr_Res(res); - } - - res = VG_(pread)(fd, hdr, len, 0); - - if (sr_isError(res)) { - VG_(close)(fd); - return False; - } else { - len = sr_Res(res); - } - - if (0 != VG_(memcmp)(hdr, "#!", 2)) { - VG_(close)(fd); - return False; - } - - end = hdr + len; - interp = hdr + 2; - while (interp < end && (*interp == ' ' || *interp == '\t')) - interp++; - - for (cp = interp; cp < end && !VG_(isspace)(*cp); cp++) - ; - - *cp = '\0'; - - VG_(sprintf)(interp_out, "%s", interp); - - VG_(close)(fd); - return True; -} /* ---------------------------------------------------------------- @@ -468,7 +422,7 @@ static Addr setup_client_stack(const void* init_sp, const HChar *exe_name = VG_(find_executable)(VG_(args_the_exename)); HChar interp_name[VKI_PATH_MAX]; - if (try_get_interp(exe_name, interp_name)) { + if (VG_(try_get_interp)(exe_name, interp_name, VKI_PATH_MAX)) { exe_name = interp_name; } HChar resolved_name[VKI_PATH_MAX]; diff --git a/coregrind/m_initimg/initimg-linux.c b/coregrind/m_initimg/initimg-linux.c index bcca59da74..935f032a40 100644 --- a/coregrind/m_initimg/initimg-linux.c +++ b/coregrind/m_initimg/initimg-linux.c @@ -385,56 +385,6 @@ struct auxv *find_auxv(UWord* sp) return (struct auxv *)sp; } -/* - * @todo PJF Make this multi-platform - */ -static Bool try_get_interp(const HChar* args_exe, HChar* interp_out) -{ - HChar hdr[4096]; - Int len = sizeof hdr; - SysRes res; - Int fd; - HChar* end; - HChar* cp; - HChar* interp; - - res = VG_(open)(args_exe, VKI_O_RDONLY, 0); - if (sr_isError(res)) { - return False; - } else { - fd = sr_Res(res); - } - - res = VG_(pread)(fd, hdr, len, 0); - - if (sr_isError(res)) { - VG_(close)(fd); - return False; - } else { - len = sr_Res(res); - } - - if (0 != VG_(memcmp)(hdr, "#!", 2)) { - VG_(close)(fd); - return False; - } - - end = hdr + len; - interp = hdr + 2; - while (interp < end && (*interp == ' ' || *interp == '\t')) - interp++; - - for (cp = interp; cp < end && !VG_(isspace)(*cp); cp++) - ; - - *cp = '\0'; - - VG_(sprintf)(interp_out, "%s", interp); - - VG_(close)(fd); - return True; -} - static Addr setup_client_stack( void* init_sp, HChar** orig_envp, @@ -1006,7 +956,7 @@ Addr setup_client_stack( void* init_sp, if (VG_(resolved_exename) == NULL) { const HChar *exe_name = VG_(find_executable)(VG_(args_the_exename)); HChar interp_name[VKI_PATH_MAX]; - if (try_get_interp(exe_name, interp_name)) { + if (VG_(try_get_interp)(exe_name, interp_name, VKI_PATH_MAX)) { exe_name = interp_name; } HChar resolved_name[VKI_PATH_MAX]; diff --git a/coregrind/m_pathscan.c b/coregrind/m_pathscan.c index 02b371486e..f667746546 100644 --- a/coregrind/m_pathscan.c +++ b/coregrind/m_pathscan.c @@ -139,6 +139,46 @@ const HChar* VG_(find_executable) ( const HChar* exec ) return executable_name_out; } +Bool VG_(try_get_interp)(const HChar* args_exe, HChar* interp_out, SizeT max_interp_len) +{ + HChar hdr[4096]; + SysRes res; + Int fd; + HChar* end; + HChar* cp; + HChar* interp; + + res = VG_(open)(args_exe, VKI_O_RDONLY, 0); + if (sr_isError(res)) + return False; + fd = sr_Res(res); + + res = VG_(pread)(fd, hdr, sizeof(hdr), 0); + VG_(close)(fd); + if (sr_isError(res)) + return False; + + Int len = sr_Res(res); + if (len < 2 || VG_(memcmp)(hdr, "#!", 2) != 0) + return False; + + end = hdr + len; + interp = hdr + 2; + while (interp < end && (*interp == ' ' || *interp == '\t')) + interp++; + for (cp = interp; cp < end && !VG_(isspace)(*cp); cp++) + ; + + SizeT interp_len = cp - interp; + if (interp_len >= max_interp_len) + return False; + + VG_(memcpy)(interp_out, interp, interp_len); + interp_out[interp_len] = '\0'; + + return True; +} + /*--------------------------------------------------------------------*/ /*--- end ---*/ /*--------------------------------------------------------------------*/ diff --git a/coregrind/m_ume/script.c b/coregrind/m_ume/script.c index 82d2477d3f..aadc2de075 100644 --- a/coregrind/m_ume/script.c +++ b/coregrind/m_ume/script.c @@ -124,7 +124,15 @@ Int VG_(load_script)(Int fd, const HChar* name, ExeInfo* info) if (info->argv && info->argv[0] != NULL) info->argv[0] = name; - VG_(args_the_exename) = name; + vg_assert(VG_(args_the_exename)); + if (name) { + if (name != VG_(args_the_exename)) { + VG_(free)((void*)VG_(args_the_exename)); + VG_(args_the_exename) = VG_(strdup)("ume.ls.3", name); + } + } else { + VG_(args_the_exename) = name; + } if (0) VG_(printf)("#! script: interp_name=\"%s\" interp_args=\"%s\"\n", diff --git a/coregrind/pub_core_pathscan.h b/coregrind/pub_core_pathscan.h index d84953ab21..8c89fe37fc 100644 --- a/coregrind/pub_core_pathscan.h +++ b/coregrind/pub_core_pathscan.h @@ -34,4 +34,6 @@ extern const HChar* VG_(find_executable) ( const HChar* exec ); -#endif // ndef __PUB_CORE_PATHSCAN_H +extern Bool VG_(try_get_interp)(const HChar* args_exe, HChar* interp_out, SizeT max_interp_len); + +#endif // __PUB_CORE_PATHSCAN_H -- 2.47.3