|
From: <sv...@va...> - 2016-10-01 11:54:46
|
Author: mjw
Date: Sat Oct 1 12:54:38 2016
New Revision: 15989
Log:
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
"<NO CWD>". 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).
Added:
trunk/none/tests/nocwd.c
trunk/none/tests/nocwd.stderr.exp
trunk/none/tests/nocwd.stdout.exp
trunk/none/tests/nocwd.vgtest
Modified:
trunk/NEWS
trunk/coregrind/m_commandline.c
trunk/coregrind/m_libcfile.c
trunk/coregrind/m_main.c
trunk/coregrind/m_options.c
trunk/coregrind/pub_core_libcfile.h
trunk/drd/drd_error.c
trunk/include/pub_tool_libcfile.h
trunk/none/tests/Makefile.am
Modified: trunk/NEWS
==============================================================================
--- trunk/NEWS (original)
+++ trunk/NEWS Sat Oct 1 12:54:38 2016
@@ -179,6 +179,7 @@
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
Modified: trunk/coregrind/m_commandline.c
==============================================================================
--- trunk/coregrind/m_commandline.c (original)
+++ trunk/coregrind/m_commandline.c Sat Oct 1 12:54:38 2016
@@ -220,9 +220,10 @@
// 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(".") );
}
Modified: trunk/coregrind/m_libcfile.c
==============================================================================
--- trunk/coregrind/m_libcfile.c (original)
+++ trunk/coregrind/m_libcfile.c Sat Oct 1 12:54:38 2016
@@ -548,16 +548,12 @@
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 @@
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 @@
(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;
}
Modified: trunk/coregrind/m_main.c
==============================================================================
--- trunk/coregrind/m_main.c (original)
+++ trunk/coregrind/m_main.c Sat Oct 1 12:54:38 2016
@@ -1853,12 +1853,9 @@
// 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 : "<NO CWD>" );
//============================================================
// Command line argument handling order:
Modified: trunk/coregrind/m_options.c
==============================================================================
--- trunk/coregrind/m_options.c (original)
+++ trunk/coregrind/m_options.c Sat Oct 1 12:54:38 2016
@@ -273,6 +273,10 @@
// 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);
Modified: trunk/coregrind/pub_core_libcfile.h
==============================================================================
--- trunk/coregrind/pub_core_libcfile.h (original)
+++ trunk/coregrind/pub_core_libcfile.h Sat Oct 1 12:54:38 2016
@@ -102,11 +102,10 @@
/* 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
Modified: trunk/drd/drd_error.c
==============================================================================
--- trunk/drd/drd_error.c (original)
+++ trunk/drd/drd_error.c Sat Oct 1 12:54:38 2016
@@ -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) */
Modified: trunk/include/pub_tool_libcfile.h
==============================================================================
--- trunk/include/pub_tool_libcfile.h (original)
+++ trunk/include/pub_tool_libcfile.h Sat Oct 1 12:54:38 2016
@@ -104,7 +104,8 @@
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
Modified: trunk/none/tests/Makefile.am
==============================================================================
--- trunk/none/tests/Makefile.am (original)
+++ trunk/none/tests/Makefile.am Sat Oct 1 12:54:38 2016
@@ -140,6 +140,7 @@
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 @@
manythreads \
mmap_fcntl_bug \
munmap_exe map_unaligned map_unmap mq \
+ nocwd \
pending \
procfs-cmdline-exe \
pselect_alarm \
Added: trunk/none/tests/nocwd.c
==============================================================================
--- trunk/none/tests/nocwd.c (added)
+++ trunk/none/tests/nocwd.c Sat Oct 1 12:54:38 2016
@@ -0,0 +1,45 @@
+#include <limits.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+
+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;
+}
Added: trunk/none/tests/nocwd.stderr.exp
==============================================================================
(empty)
Added: trunk/none/tests/nocwd.stdout.exp
==============================================================================
--- trunk/none/tests/nocwd.stdout.exp (added)
+++ trunk/none/tests/nocwd.stdout.exp Sat Oct 1 12:54:38 2016
@@ -0,0 +1 @@
+Hello World
Added: trunk/none/tests/nocwd.vgtest
==============================================================================
--- trunk/none/tests/nocwd.vgtest (added)
+++ trunk/none/tests/nocwd.vgtest Sat Oct 1 12:54:38 2016
@@ -0,0 +1,2 @@
+prog: nocwd
+vgopts: -q --trace-children=yes
|