From: Mark Wielaard Date: Sat, 1 Oct 2016 11:54:38 +0000 (+0000) Subject: Don't require the current working directory to exist. Bug #369209. X-Git-Tag: svn/VALGRIND_3_13_0~377 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=be052139d67afde163ac38b70001fea94ad29061;p=thirdparty%2Fvalgrind.git Don't require the current working directory to exist. Bug #369209. At startup valgrind fetches the current working directory and stashes it away to be used later (in debug messages, read config files or create log files). But if the current working directory didn't exist (or there was some other error getting its path) then valgrind would go in an endless loop. This was caused by assuming that any error meant a larger buffer needed to be created to store the cwd path (ERANGE). However there could be other reasons calling getcwd failed. Fix this by only looping and resizing the buffer when the error is ERANGE. Any other error just means we cannot fetch and store the current working directory. Fix all callers to check get_startup_wd() returns NULL. Only abort startup if a relative path needs to be used for user supplied relative log files. Debug messages will just show "". And skip reading any config files from the startup_wd if it doesn't exist. Also add a new testcase that tests executing valgrind in a deep, inaccessible and/or non-existing directory (none/tests/nocwd.vgtest). git-svn-id: svn://svn.valgrind.org/valgrind/trunk@15989 --- diff --git a/NEWS b/NEWS index 6956876a76..67a0ecbb94 100644 --- a/NEWS +++ b/NEWS @@ -179,6 +179,7 @@ where XXXXXX is the bug number as listed below. 369000 AMD64 fma4 instructions unsupported. 361253 [s390x] ex_clone.c:42: undefined reference to `pthread_create' 369169 ppc64 fails jm_int_isa_2_07 test +369209 valgrind loops and eats up all memory if cwd doesn't exist. n-i-bz Fix incorrect (or infinite loop) unwind on RHEL7 x86 and amd64 n-i-bz massif --pages-as-heap=yes does not report peak caused by mmap+munmap diff --git a/coregrind/m_commandline.c b/coregrind/m_commandline.c index 67b9277a62..e6209a5be6 100644 --- a/coregrind/m_commandline.c +++ b/coregrind/m_commandline.c @@ -220,9 +220,10 @@ void VG_(split_up_argv)( Int argc, HChar** argv ) // Don't read ./.valgrindrc if "." is the same as "$HOME", else its // contents will be applied twice. (bug #142488) + // Also don't try to read it if there is no cwd. if (home) { const HChar *cwd = VG_(get_startup_wd)(); - f2_clo = ( VG_STREQ(home, cwd) + f2_clo = ( (cwd == NULL || VG_STREQ(home, cwd)) ? NULL : read_dot_valgrindrc(".") ); } diff --git a/coregrind/m_libcfile.c b/coregrind/m_libcfile.c index c413db1282..0d4cef224d 100644 --- a/coregrind/m_libcfile.c +++ b/coregrind/m_libcfile.c @@ -548,16 +548,12 @@ Int VG_(unlink) ( const HChar* file_name ) Hence VG_(record_startup_wd) notes it (in a platform dependent way) and VG_(get_startup_wd) produces the noted value. */ static HChar *startup_wd; -static Bool startup_wd_acquired = False; /* Record the process' working directory at startup. Is intended to be called exactly once, at startup, before the working directory - changes. Return True for success, False for failure, so that the - caller can bomb out suitably without creating module cycles if - there is a problem. */ -Bool VG_(record_startup_wd) ( void ) + changes. */ +void VG_(record_startup_wd) ( void ) { - vg_assert(!startup_wd_acquired); # if defined(VGO_linux) || defined(VGO_solaris) /* Simple: just ask the kernel */ SysRes res; @@ -567,11 +563,15 @@ Bool VG_(record_startup_wd) ( void ) startup_wd = VG_(realloc)("startup_wd", startup_wd, szB); VG_(memset)(startup_wd, 0, szB); res = VG_(do_syscall2)(__NR_getcwd, (UWord)startup_wd, szB-1); - } while (sr_isError(res)); + } while (sr_isError(res) && sr_Err(res) == VKI_ERANGE); + + if (sr_isError(res)) { + VG_(free)(startup_wd); + startup_wd = NULL; + return; + } vg_assert(startup_wd[szB-1] == 0); - startup_wd_acquired = True; - return True; # elif defined(VGO_darwin) /* We can't ask the kernel, so instead rely on launcher-*.c to @@ -585,23 +585,19 @@ Bool VG_(record_startup_wd) ( void ) (Int)VG_(getppid)()); wd = VG_(getenv)( envvar ); if (wd == NULL) - return False; + return; SizeT need = VG_(strlen)(wd) + 1; startup_wd = VG_(malloc)("startup_wd", need); VG_(strcpy)(startup_wd, wd); - startup_wd_acquired = True; - return True; } # else # error Unknown OS # endif } -/* Return the previously acquired startup_wd. */ +/* Return the previously acquired startup_wd or NULL. */ const HChar *VG_(get_startup_wd) ( void ) { - vg_assert(startup_wd_acquired); - return startup_wd; } diff --git a/coregrind/m_main.c b/coregrind/m_main.c index 395bfa29ad..09badd70d3 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -1853,12 +1853,9 @@ Int valgrind_main ( Int argc, HChar **argv, HChar **envp ) // Record the working directory at startup // p: none VG_(debugLog)(1, "main", "Getting the working directory at startup\n"); - { Bool ok = VG_(record_startup_wd)(); - if (!ok) - VG_(err_config_error)( "Can't establish current working " - "directory at startup\n"); - } - VG_(debugLog)(1, "main", "... %s\n", VG_(get_startup_wd)() ); + VG_(record_startup_wd)(); + const HChar *wd = VG_(get_startup_wd)(); + VG_(debugLog)(1, "main", "... %s\n", wd != NULL ? wd : "" ); //============================================================ // Command line argument handling order: diff --git a/coregrind/m_options.c b/coregrind/m_options.c index 83d6018b80..d5e07da850 100644 --- a/coregrind/m_options.c +++ b/coregrind/m_options.c @@ -273,6 +273,10 @@ HChar* VG_(expand_file_name)(const HChar* option_name, const HChar* format) // If 'out' is not an absolute path name, prefix it with the startup dir. if (out[0] != '/') { + if (base_dir == NULL) { + message = "Current working dir doesn't exist, use absolute path\n"; + goto bad; + } len = VG_(strlen)(base_dir) + 1 + VG_(strlen)(out) + 1; HChar *absout = VG_(malloc)("options.efn.4", len); diff --git a/coregrind/pub_core_libcfile.h b/coregrind/pub_core_libcfile.h index caf7ce18a1..9d32b9fb3a 100644 --- a/coregrind/pub_core_libcfile.h +++ b/coregrind/pub_core_libcfile.h @@ -102,11 +102,10 @@ extern Int VG_(mkstemp) ( const HChar* part_of_name, /*OUT*/HChar* fullname ); /* Record the process' working directory at startup. Is intended to be called exactly once, at startup, before the working directory - changes. Return True for success, False for failure, so that the - caller can bomb out suitably without creating module cycles if - there is a problem. The saved value can later be acquired by - calling VG_(get_startup_wd) (in pub_tool_libcfile.h). */ -extern Bool VG_(record_startup_wd) ( void ); + changes. The saved value can later be acquired by calling + VG_(get_startup_wd) (in pub_tool_libcfile.h). Note that might + return if the working directory couldn't be found. */ +extern void VG_(record_startup_wd) ( void ); #endif // __PUB_CORE_LIBCFILE_H diff --git a/drd/drd_error.c b/drd/drd_error.c index fb116f6b25..cec3a2be1c 100644 --- a/drd/drd_error.c +++ b/drd/drd_error.c @@ -32,7 +32,6 @@ #include "pub_tool_basics.h" #include "pub_tool_libcassert.h" /* tl_assert() */ #include "pub_tool_libcbase.h" /* strlen() */ -#include "pub_tool_libcfile.h" /* VG_(get_startup_wd)() */ #include "pub_tool_libcprint.h" /* VG_(printf)() */ #include "pub_tool_machine.h" #include "pub_tool_mallocfree.h" /* VG_(malloc), VG_(free) */ diff --git a/include/pub_tool_libcfile.h b/include/pub_tool_libcfile.h index 4a7346e761..c126e1d9d4 100644 --- a/include/pub_tool_libcfile.h +++ b/include/pub_tool_libcfile.h @@ -104,7 +104,8 @@ extern const HChar* VG_(dirname) ( const HChar* path ); extern const HChar* VG_(tmpdir)(void); /* Return the working directory at startup. The returned string is - persistent. */ + persistent. Might be NULL if the current working directory doesn't + exist. */ extern const HChar *VG_(get_startup_wd) ( void ); #endif // __PUB_TOOL_LIBCFILE_H diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am index 11bc3fa958..ffe55d4e94 100644 --- a/none/tests/Makefile.am +++ b/none/tests/Makefile.am @@ -140,6 +140,7 @@ EXTRA_DIST = \ mq.stderr.exp mq.vgtest \ munmap_exe.stderr.exp munmap_exe.vgtest \ nestedfns.stderr.exp nestedfns.stdout.exp nestedfns.vgtest \ + nocwd.stdout.exp nocwd.stderr.exp nocwd.vgtest \ nodir.stderr.exp nodir.vgtest \ pending.stdout.exp pending.stderr.exp pending.vgtest \ ppoll_alarm.stdout.exp ppoll_alarm.stderr.exp ppoll_alarm.vgtest \ @@ -219,6 +220,7 @@ check_PROGRAMS = \ manythreads \ mmap_fcntl_bug \ munmap_exe map_unaligned map_unmap mq \ + nocwd \ pending \ procfs-cmdline-exe \ pselect_alarm \ diff --git a/none/tests/nocwd.c b/none/tests/nocwd.c new file mode 100644 index 0000000000..d01d623773 --- /dev/null +++ b/none/tests/nocwd.c @@ -0,0 +1,45 @@ +#include +#include +#include +#include +#include +#include +#include + +int +main (int argc, char **argv) +{ + char template[] = "/tmp/wd_test_XXXXXX"; + char *tmpdir = mkdtemp(template); + if (tmpdir == NULL) + { + perror ("Couldn't mkdtemp"); + exit (-1); + } + + if (chdir (tmpdir) != 0) + { + perror ("Couldn't chdir into tmpdir"); + exit (-1); + } + + /* Go deep. */ + int dirslen = PATH_MAX; + while (dirslen > 0) + { + /* We don't do any error checking in case some OS fails. */ + mkdir ("subdir", S_IRWXU); + chdir ("subdir"); + dirslen -= strlen ("subdir"); + } + + /* Make one component inaccessible. */ + chmod(tmpdir, 0); + + /* Remove the current dir (don't check error, might fail). */ + rmdir ("../subdir"); + + execlp ("echo", "echo", "Hello", "World", (char *) NULL); + perror ("Couldn't execlp"); + return -1; +} diff --git a/none/tests/nocwd.stderr.exp b/none/tests/nocwd.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/nocwd.stdout.exp b/none/tests/nocwd.stdout.exp new file mode 100644 index 0000000000..557db03de9 --- /dev/null +++ b/none/tests/nocwd.stdout.exp @@ -0,0 +1 @@ +Hello World diff --git a/none/tests/nocwd.vgtest b/none/tests/nocwd.vgtest new file mode 100644 index 0000000000..74e2b4aba9 --- /dev/null +++ b/none/tests/nocwd.vgtest @@ -0,0 +1,2 @@ +prog: nocwd +vgopts: -q --trace-children=yes