From: Nicholas Nethercote Date: Mon, 9 Aug 2004 12:21:57 +0000 (+0000) Subject: Remove some more global variables from vg_include.h, replacing them with X-Git-Tag: svn/VALGRIND_2_2_0~59 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=82c053aafc96290209e985bfcd9caac059b5d930;p=thirdparty%2Fvalgrind.git Remove some more global variables from vg_include.h, replacing them with (fewer) functions. Also fixed execve() so that it works better with .in_place. Also added a regression test for --trace-children=yes (there were none!) git-svn-id: svn://svn.valgrind.org/valgrind/trunk@2577 --- diff --git a/coregrind/vg_include.h b/coregrind/vg_include.h index 9bcafac14b..183352bd3a 100644 --- a/coregrind/vg_include.h +++ b/coregrind/vg_include.h @@ -1107,7 +1107,6 @@ VALGRIND_INTERNAL_PRINTF_BACKTRACE(char *format, ...) } - /* --------------------------------------------------------------------- Exports of vg_demangle.c ------------------------------------------------------------------ */ @@ -1265,12 +1264,13 @@ extern Addr VG_(valgrind_end); extern vki_rlimit VG_(client_rlimit_data); /* client's original rlimit data */ -/* stage1 executable file descriptor */ -extern Int VG_(vgexecfd); - /* client executable file descriptor */ extern Int VG_(clexecfd); +// Help set up the child used when doing execve() with --trace-children=yes +Char* VG_(build_child_VALGRINDCLO) ( Char* exename ); +Char* VG_(build_child_exename) ( void ); + /* Determine if %esp adjustment must be noted */ extern Bool VG_(need_to_handle_esp_assignment) ( void ); @@ -1279,10 +1279,6 @@ extern Bool VG_(need_to_handle_esp_assignment) ( void ); extern void VG_(unimplemented) ( Char* msg ) __attribute__((__noreturn__)); -/* Valgrind's argc and argv */ -extern Int VG_(vg_argc); -extern Char **VG_(vg_argv); - /* Something of a function looking for a home ... start up debugger. */ extern void VG_(start_debugger) ( Int tid ); diff --git a/coregrind/vg_main.c b/coregrind/vg_main.c index 1c6be6c9e3..465fd9af88 100644 --- a/coregrind/vg_main.c +++ b/coregrind/vg_main.c @@ -117,7 +117,7 @@ vki_rlimit VG_(client_rlimit_data); Bool VG_(have_ssestate); /* stage1 (main) executable */ -Int VG_(vgexecfd) = -1; +static Int vgexecfd = -1; /* client executable */ Int VG_(clexecfd) = -1; @@ -126,8 +126,8 @@ Int VG_(clexecfd) = -1; const Char *VG_(libdir) = VG_LIBDIR; /* our argc/argv */ -Int VG_(vg_argc); -Char **VG_(vg_argv); +static Int vg_argc; +static Char **vg_argv; /* PID of the main thread */ Int VG_(main_pid); @@ -413,7 +413,7 @@ int scan_auxv(void) break; case AT_UME_EXECFD: - VG_(vgexecfd) = auxv->u.a_val; + vgexecfd = auxv->u.a_val; found |= 2; break; } @@ -567,8 +567,8 @@ static char** copy_args( char* s, char** to ) // files. static void augment_command_line(Int* vg_argc_inout, char*** vg_argv_inout) { - int vg_argc = *vg_argc_inout; - char** vg_argv = *vg_argv_inout; + int vg_argc0 = *vg_argc_inout; + char** vg_argv0 = *vg_argv_inout; char* env_clo = getenv(VALGRINDOPTS); char* f1_clo = get_file_clo( getenv("HOME") ); @@ -590,11 +590,11 @@ static void augment_command_line(Int* vg_argc_inout, char*** vg_argv_inout) env_arg_count, f1_arg_count, f2_arg_count); /* +2: +1 for null-termination, +1 for added '--' */ - from = vg_argv; - vg_argv = malloc( (vg_argc + env_arg_count + f1_arg_count + from = vg_argv0; + vg_argv0 = malloc( (vg_argc0 + env_arg_count + f1_arg_count + f2_arg_count + 2) * sizeof(char **)); - vg_assert(vg_argv); - to = vg_argv; + vg_assert(vg_argv0); + to = vg_argv0; /* copy argv[0] */ *to++ = *from++; @@ -620,23 +620,25 @@ static void augment_command_line(Int* vg_argc_inout, char*** vg_argv_inout) /* add -- */ *to++ = "--"; - vg_argc = to - vg_argv; + vg_argc0 = to - vg_argv0; /* copy rest of original command line, then NULL */ while (*from) *to++ = *from++; *to = NULL; } - *vg_argc_inout = vg_argc; - *vg_argv_inout = vg_argv; + *vg_argc_inout = vg_argc0; + *vg_argv_inout = vg_argv0; } +#define VG_CLO_SEP '\01' + static void get_command_line( int argc, char** argv, Int* vg_argc_out, Char*** vg_argv_out, char*** cl_argv_out ) { - int vg_argc; - char** vg_argv; + int vg_argc0; + char** vg_argv0; char** cl_argv; char* env_clo = getenv(VALGRINDCLO); @@ -644,23 +646,26 @@ static void get_command_line( int argc, char** argv, char *cp; char **cpp; - /* OK, we're getting all our arguments from the environment - the - entire command line belongs to the client (including argv[0]) */ - vg_argc = 1; /* argv[0] */ + /* OK, VALGRINDCLO is set, which means we must be a child of another + Valgrind process using --trace-children, so we're getting all our + arguments from VALGRINDCLO, and the entire command line belongs to + the client (including argv[0]) */ + vg_argc0 = 1; /* argv[0] */ for (cp = env_clo; *cp; cp++) - if (*cp == '\01') - vg_argc++; + if (*cp == VG_CLO_SEP) + vg_argc0++; - vg_argv = malloc(sizeof(char **) * (vg_argc + 1)); - vg_assert(vg_argv); + vg_argv0 = malloc(sizeof(char **) * (vg_argc0 + 1)); + vg_assert(vg_argv0); - cpp = vg_argv; + cpp = vg_argv0; *cpp++ = "valgrind"; /* nominal argv[0] */ *cpp++ = env_clo; + // Replace the VG_CLO_SEP args separator with '\0' for (cp = env_clo; *cp; cp++) { - if (*cp == '\01') { + if (*cp == VG_CLO_SEP) { *cp++ = '\0'; /* chop it up in place */ *cpp++ = cp; } @@ -670,31 +675,32 @@ static void get_command_line( int argc, char** argv, } else { /* Count the arguments on the command line. */ - vg_argv = argv; + vg_argv0 = argv; - for (vg_argc = 1; vg_argc < argc; vg_argc++) { - if (argv[vg_argc][0] != '-') /* exe name */ + for (vg_argc0 = 1; vg_argc0 < argc; vg_argc0++) { + if (argv[vg_argc0][0] != '-') /* exe name */ break; - if (VG_STREQ(argv[vg_argc], "--")) { /* dummy arg */ - vg_argc++; + if (VG_STREQ(argv[vg_argc0], "--")) { /* dummy arg */ + vg_argc0++; break; } } - cl_argv = &argv[vg_argc]; + cl_argv = &argv[vg_argc0]; /* Get extra args from VALGRIND_OPTS and .valgrindrc files. - * Note we don't do this if getting args from VALGRINDCLO. */ - augment_command_line(&vg_argc, &vg_argv); + Note we don't do this if getting args from VALGRINDCLO, as + those extra args will already be present in VALGRINDCLO. */ + augment_command_line(&vg_argc0, &vg_argv0); } if (0) { Int i; - for (i = 0; i < vg_argc; i++) - printf("vg_argv[%d]=\"%s\"\n", i, vg_argv[i]); + for (i = 0; i < vg_argc0; i++) + printf("vg_argv0[%d]=\"%s\"\n", i, vg_argv0[i]); } - *vg_argc_out = vg_argc; - *vg_argv_out = (Char**)vg_argv; + *vg_argc_out = vg_argc0; + *vg_argv_out = (Char**)vg_argv0; *cl_argv_out = cl_argv; } @@ -1413,7 +1419,7 @@ static void load_client(char* cl_argv[], const char* exec, Int need_help, /*====================================================================*/ -/*=== Command-line: variables, processing ===*/ +/*=== Command-line: variables, processing, etc ===*/ /*====================================================================*/ /* Define, and set defaults. */ @@ -1569,25 +1575,25 @@ static void pre_process_cmd_line_options UInt i; /* parse the options we have (only the options we care about now) */ - for (i = 1; i < VG_(vg_argc); i++) { + for (i = 1; i < vg_argc; i++) { - if (strcmp(VG_(vg_argv)[i], "--version") == 0) { + if (strcmp(vg_argv[i], "--version") == 0) { printf("valgrind-" VERSION "\n"); exit(0); - } else if (VG_CLO_STREQ(VG_(vg_argv)[i], "--help") || - VG_CLO_STREQ(VG_(vg_argv)[i], "-h")) { + } else if (VG_CLO_STREQ(vg_argv[i], "--help") || + VG_CLO_STREQ(vg_argv[i], "-h")) { *need_help = 1; - } else if (VG_CLO_STREQ(VG_(vg_argv)[i], "--help-debug")) { + } else if (VG_CLO_STREQ(vg_argv[i], "--help-debug")) { *need_help = 2; - } else if (VG_CLO_STREQN(7, VG_(vg_argv)[i], "--tool=") || - VG_CLO_STREQN(7, VG_(vg_argv)[i], "--skin=")) { - *tool = &VG_(vg_argv)[i][7]; + } else if (VG_CLO_STREQN(7, vg_argv[i], "--tool=") || + VG_CLO_STREQN(7, vg_argv[i], "--skin=")) { + *tool = &vg_argv[i][7]; - } else if (VG_CLO_STREQN(7, VG_(vg_argv)[i], "--exec=")) { - *exec = &VG_(vg_argv)[i][7]; + } else if (VG_CLO_STREQN(7, vg_argv[i], "--exec=")) { + *exec = &vg_argv[i][7]; } } @@ -1625,9 +1631,9 @@ static void process_cmd_line_options( UInt* client_auxv, const char* toolname ) } } - for (i = 1; i < VG_(vg_argc); i++) { + for (i = 1; i < vg_argc; i++) { - Char* arg = VG_(vg_argv)[i]; + Char* arg = vg_argv[i]; Char* colon = arg; /* Look for a colon in the switch name */ @@ -1932,8 +1938,8 @@ static void process_cmd_line_options( UInt* client_auxv, const char* toolname ) VG_(message)(Vg_UserMsg, " %s", VG_(client_argv)[i]); VG_(message)(Vg_UserMsg, "Startup, with flags:"); - for (i = 1; i < VG_(vg_argc); i++) { - VG_(message)(Vg_UserMsg, " %s", VG_(vg_argv)[i]); + for (i = 1; i < vg_argc; i++) { + VG_(message)(Vg_UserMsg, " %s", vg_argv[i]); } VG_(message)(Vg_UserMsg, "Contents of /proc/version:"); @@ -1978,6 +1984,75 @@ static void process_cmd_line_options( UInt* client_auxv, const char* toolname ) } } +// Build the string for VALGRINDCLO. +Char* VG_(build_child_VALGRINDCLO)( Char* exename ) +{ + /* If we're tracing the children, then we need to start it + with our starter+arguments, which are copied into VALGRINDCLO, + except the --exec= option is changed if present. + */ + Int i; + Char *exec; + Char *cp; + Char *optvar; + Int optlen, execlen; + + // All these allocated blocks are not free - because we're either + // going to exec, or panic when we fail. + + // Create --exec= option: "--exec=" + exec = VG_(arena_malloc)(VG_AR_CORE, + VG_(strlen)( exename ) + 7/*--exec=*/ + 1/*\0*/); + vg_assert(NULL != exec); + VG_(sprintf)(exec, "--exec=%s", exename); + + // Allocate space for optvar (may overestimate by counting --exec twice, + // no matter) + optlen = 1; + for (i = 0; i < vg_argc; i++) + optlen += VG_(strlen)(vg_argv[i]) + 1; + optlen += VG_(strlen)(exec)+1; + optvar = VG_(arena_malloc)(VG_AR_CORE, optlen); + + // Copy all valgrind args except the old --exec (if present) + // VG_CLO_SEP is the separator. + cp = optvar; + for (i = 1; i < vg_argc; i++) { + Char *arg = vg_argv[i]; + + if (VG_(memcmp)(arg, "--exec=", 7) == 0) { + // don't copy existing --exec= arg + } else if (VG_(strcmp)(arg, "--") == 0) { + // stop at "--" + break; + } else { + // copy non "--exec" arg + Int len = VG_(strlen)(arg); + VG_(memcpy)(cp, arg, len); + cp += len; + *cp++ = VG_CLO_SEP; + } + } + // Add the new --exec= option + execlen = VG_(strlen)(exec); + VG_(memcpy)(cp, exec, execlen); + cp += execlen; + *cp++ = VG_CLO_SEP; + + *cp = '\0'; + + return optvar; +} + +// Build "/proc/self/fd/". +Char* VG_(build_child_exename)( void ) +{ + Char* exename = VG_(arena_malloc)(VG_AR_CORE, 64); + vg_assert(NULL != exename); + VG_(sprintf)(exename, "/proc/self/fd/%d", vgexecfd); + return exename; +} + /*====================================================================*/ /*=== File descriptor setup ===*/ @@ -2007,8 +2082,8 @@ static void setup_file_descriptors(void) /* Update the soft limit. */ VG_(setrlimit)(VKI_RLIMIT_NOFILE, &rl); - if (VG_(vgexecfd) != -1) - VG_(vgexecfd) = VG_(safe_fd)( VG_(vgexecfd) ); + if (vgexecfd != -1) + vgexecfd = VG_(safe_fd)( vgexecfd ); if (VG_(clexecfd) != -1) VG_(clexecfd) = VG_(safe_fd)( VG_(clexecfd) ); } @@ -2725,7 +2800,7 @@ int main(int argc, char **argv) // Pre-process the command line. // p: n/a //-------------------------------------------------------------- - get_command_line(argc, argv, &VG_(vg_argc), &VG_(vg_argv), &cl_argv); + get_command_line(argc, argv, &vg_argc, &vg_argv, &cl_argv); pre_process_cmd_line_options(&need_help, &tool, &exec); //============================================================== @@ -2782,7 +2857,7 @@ int main(int argc, char **argv) if (0) printf("entry=%x client esp=%x vg_argc=%d brkbase=%x\n", - client_eip, esp_at_startup, VG_(vg_argc), VG_(brk_base)); + client_eip, esp_at_startup, vg_argc, VG_(brk_base)); //============================================================== // Finished setting up operating environment. Now initialise diff --git a/coregrind/vg_syscalls.c b/coregrind/vg_syscalls.c index 569ca7661e..f0aa4d4d35 100644 --- a/coregrind/vg_syscalls.c +++ b/coregrind/vg_syscalls.c @@ -1787,66 +1787,14 @@ PRE(execve) } if (VG_(clo_trace_children)) { - /* If we're tracing the children, then we need to start it - with our starter+arguments. - */ - Int i; - Char *exec = (Char *)arg1; - Char **env = (Char **)arg3; - Char *cp; - Char *exename; - Bool sawexec = False; - Char *optvar; - Int optlen; - - optlen = 1; - for(i = 0; i < VG_(vg_argc); i++) - optlen += VG_(strlen)(VG_(vg_argv)[i]) + 1; - - /* All these are leaked - we're either going to exec, or panic - when we fail. */ - exename = VG_(arena_malloc)(VG_AR_CORE, 64); - exec = VG_(arena_malloc)(VG_AR_CORE, VG_(strlen)(exec) + 7 /* --exec= */ + 1 /* \0 */); - - VG_(sprintf)(exec, "--exec=%s", (Char *)arg1); - VG_(sprintf)(exename, "/proc/self/fd/%d", VG_(vgexecfd)); - - optlen += VG_(strlen)(exec)+1; - - optvar = VG_(arena_malloc)(VG_AR_CORE, optlen); - - /* valgrind arguments */ - cp = optvar; - - for(i = 1; i < VG_(vg_argc); i++) { - Char *arg = VG_(vg_argv)[i]; - Int len; - - if (VG_(memcmp)(arg, "--exec=", 7) == 0) { - /* replace existing --exec= arg */ - sawexec = True; - arg = exec; - } else if (VG_(strcmp)(VG_(vg_argv)[i], "--") == 0) - break; - - len = VG_(strlen)(arg); - VG_(memcpy)(cp, arg, len); - cp += len; - *cp++ = '\01'; - } - - if (!sawexec) { - Int execlen = VG_(strlen)(exec); - VG_(memcpy)(cp, exec, execlen); - cp += execlen; - *cp++ = '\01'; - } - *cp = '\0'; + Char* optvar = VG_(build_child_VALGRINDCLO)( (Char*)arg1 ); - VG_(env_setenv)(&env, VALGRINDCLO, optvar); + // Set VALGRINDCLO and VALGRINDLIB in arg3 (the environment) + VG_(env_setenv)( (Char***)&arg3, VALGRINDCLO, optvar); + VG_(env_setenv)( (Char***)&arg3, VALGRINDLIB, VG_(libdir)); - arg1 = (UInt)exename; - arg3 = (UInt)env; + // Create executable name: "/proc/self/fd/", update arg1 + arg1 = (UInt)VG_(build_child_exename)(); } if (0) { @@ -1854,9 +1802,9 @@ PRE(execve) VG_(printf)("exec: %s\n", (Char *)arg1); for(cpp = (Char **)arg2; cpp && *cpp; cpp++) - VG_(printf)("argv: %s\n", *cpp); + VG_(printf)("argv: %s\n", *cpp); for(cpp = (Char **)arg3; cpp && *cpp; cpp++) - VG_(printf)("env: %s\n", *cpp); + VG_(printf)("env: %s\n", *cpp); } /* Set our real sigmask to match the client's sigmask so that the diff --git a/memcheck/tests/.cvsignore b/memcheck/tests/.cvsignore index a32c9c31d4..779e6b7ce7 100644 --- a/memcheck/tests/.cvsignore +++ b/memcheck/tests/.cvsignore @@ -13,6 +13,7 @@ doublefree error_counts errs1 execve +execve2 exitprog filter_leak_check_size filter_stderr diff --git a/memcheck/tests/Makefile.am b/memcheck/tests/Makefile.am index c577e8c1b0..1e37e4a5c5 100644 --- a/memcheck/tests/Makefile.am +++ b/memcheck/tests/Makefile.am @@ -27,6 +27,7 @@ EXTRA_DIST = $(noinst_SCRIPTS) \ errs1.stderr.exp errs1.vgtest \ exitprog.stderr.exp exitprog.vgtest \ execve.stderr.exp execve.vgtest \ + execve2.stderr.exp execve2.vgtest \ fpeflags.stderr.exp fpeflags.vgtest \ fprw.stderr.exp fprw.vgtest \ fwrite.stderr.exp fwrite.stdout.exp fwrite.vgtest \ @@ -77,7 +78,7 @@ EXTRA_DIST = $(noinst_SCRIPTS) \ check_PROGRAMS = \ badaddrvalue badfree badjump badloop badrw brk buflen_check \ clientperm custom_alloc \ - doublefree error_counts errs1 exitprog execve \ + doublefree error_counts errs1 exitprog execve execve2 \ fpeflags fprw fwrite inits inline \ malloc1 malloc2 malloc3 manuel1 manuel2 manuel3 \ memalign_test memcmptest mempool mmaptest nanoleak new_nothrow \ @@ -104,6 +105,7 @@ doublefree_SOURCES = doublefree.c error_counts_SOURCES = error_counts.c errs1_SOURCES = errs1.c execve_SOURCES = execve.c +execve2_SOURCES = execve2.c exitprog_SOURCES = exitprog.c fpeflags_SOURCES = fpeflags.c fprw_SOURCES = fprw.c diff --git a/memcheck/tests/execve2.c b/memcheck/tests/execve2.c new file mode 100644 index 0000000000..c3eae2c2fa --- /dev/null +++ b/memcheck/tests/execve2.c @@ -0,0 +1,9 @@ +#include +#include + +int main(void) +{ + execve(NULL, NULL, NULL); + execve("../../tests/true", NULL, NULL); + assert(0); // shouldn't get here +} diff --git a/memcheck/tests/execve2.stderr.exp b/memcheck/tests/execve2.stderr.exp new file mode 100644 index 0000000000..c2cadf7d93 --- /dev/null +++ b/memcheck/tests/execve2.stderr.exp @@ -0,0 +1,4 @@ +Syscall param execve(filename) contains uninitialised or unaddressable byte(s) + at 0x........: execve (in /...libc...) + by 0x........: main (execve2.c:6) + Address 0x........ is not stack'd, malloc'd or (recently) free'd diff --git a/memcheck/tests/execve2.vgtest b/memcheck/tests/execve2.vgtest new file mode 100644 index 0000000000..9636ad835e --- /dev/null +++ b/memcheck/tests/execve2.vgtest @@ -0,0 +1,2 @@ +prog: execve2 +vgopts: -q --trace-children=yes