fix 286270 VG_(env_remove_valgrind_env_stuff)

rev 12001 has introduced a regression in VG_(env_remove_valgrind_env_stuff):
to avoid modifying a possibly read-only env string, the string is duplicated,
and the copy is modified. However, mash_env_column modifies the string
"in-place". The modified string was not put back in the env (and could not,
because the src string is only partially copied).

This means that the valgrind preload strings were not cleaned up and
when a 32 bit executable execs a 64 bits (or vice versa: 64 bit execs 32 bits),
LD_PRELOAD contains both the 32 bits and 64 bits versions of Valgrind
vgpreload.... => ld.so then gives an error msg, as it can't preload either
the 32 or the 64 bits version.

The patch fixes this by duplicating the whole env string, and passing
to mash_colon_env a pointer to the correct offset in the whole env string.
The duplicated string is replacing the original entry in envp.

This patch adds two regression tests : none/tests/allexec32 and
none/tests/allexec64. On a bi-arch valgrind, these will be 32bits and 64 bits
executables, exec-ing each other. On a single arch, one will be a symlink
to the other (to avoid different .exp files, and still test exec).

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@12287

diff --git a/coregrind/m_libcproc.c b/coregrind/m_libcproc.c
index e5f9bb85e677615ae6ebdad3fc2d845aa7d7ce86..4462b6a2fc5c3b54538d079a0a38766cfd84ba5e 100644 (file)
--- a/coregrind/m_libcproc.c
+++ b/coregrind/m_libcproc.c
@@ -240,12 +240,18 @@ void VG_(env_remove_valgrind_env_stuff)(Char** envp)
    // - LD_PRELOAD is on Linux, not on Darwin, not sure about AIX
    // - DYLD_INSERT_LIBRARIES and DYLD_SHARED_REGION are Darwin-only
    for (i = 0; envp[i] != NULL; i++) {
-      if (VG_(strncmp)(envp[i], "LD_PRELOAD=", 11) == 0)
-         ld_preload_str = VG_(arena_strdup)(VG_AR_CORE, "libcproc.erves.1", &envp[i][11]);
-      if (VG_(strncmp)(envp[i], "LD_LIBRARY_PATH=", 16) == 0)
-         ld_library_path_str = VG_(arena_strdup)(VG_AR_CORE, "libcproc.erves.2", &envp[i][16]);
-      if (VG_(strncmp)(envp[i], "DYLD_INSERT_LIBRARIES=", 22) == 0)
-         dyld_insert_libraries_str = VG_(arena_strdup)(VG_AR_CORE, "libcproc.erves.3", &envp[i][22]);
+      if (VG_(strncmp)(envp[i], "LD_PRELOAD=", 11) == 0) {
+         envp[i] = VG_(arena_strdup)(VG_AR_CORE, "libcproc.erves.1", envp[i]);
+         ld_preload_str = &envp[i][11];
+      }
+      if (VG_(strncmp)(envp[i], "LD_LIBRARY_PATH=", 16) == 0) {
+         envp[i] = VG_(arena_strdup)(VG_AR_CORE, "libcproc.erves.2", envp[i]);
+         ld_library_path_str = &envp[i][16];
+      }
+      if (VG_(strncmp)(envp[i], "DYLD_INSERT_LIBRARIES=", 22) == 0) {
+         envp[i] = VG_(arena_strdup)(VG_AR_CORE, "libcproc.erves.3", envp[i]);
+         dyld_insert_libraries_str = &envp[i][22];
+      }
    }
 
    buf = VG_(arena_malloc)(VG_AR_CORE, "libcproc.erves.4",

diff --git a/none/tests/Makefile.am b/none/tests/Makefile.am
index a9c498f0e46fca5a92f8b922db72d9a684309a3a..13e8892121a8eac6851670db07e09dc0737c02dc 100644 (file)
--- a/none/tests/Makefile.am
+++ b/none/tests/Makefile.am
@@ -51,6 +51,8 @@ dist_noinst_SCRIPTS = \
 noinst_HEADERS = fdleak.h
 
 EXTRA_DIST = \
+       allexec32.stdout.exp allexec32.stderr.exp allexec32.vgtest\
+       allexec64.stdout.exp allexec64.stderr.exp allexec64.vgtest\
        ansi.stderr.exp ansi.vgtest \
        args.stderr.exp args.stdout.exp args.vgtest \
        async-sigs.stderr.exp async-sigs.vgtest \

diff --git a/none/tests/allexec.c b/none/tests/allexec.c
new file mode 100644 (file)
index 0000000..b7177e8
--- /dev/null
+++ b/none/tests/allexec.c
@@ -0,0 +1,50 @@
+#include <assert.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <unistd.h>
+
+extern char **environ;
+
+#define S(...) (fprintf(stdout, __VA_ARGS__),fflush(stdout))
+#define FORKEXECWAIT(exec_call) do { \
+      int status;\
+      pid_t child = fork(); \
+      if (child == 0) {exec_call; perror ("exec failed");} \
+      else if (child == -1) perror ("cannot fork\n"); \
+      else if (child != wait (&status)) perror ("error waiting child"); \
+      else S("child exited\n"); \
+   } while (0)
+
+void test_allexec (char *exec)
+{
+   FORKEXECWAIT (execlp(exec, exec, NULL));
+   FORKEXECWAIT (execlp(exec, exec, "constant_arg1", "constant_arg2", NULL));
+   FORKEXECWAIT (execve(exec, NULL, environ));
+}
+
+
+/* If a single argument "exec" is given, will execute itself
+   (in bi-arch, a 32 bit and 64 bit variant) via various exec system calls.
+   Note that this test can only be run after the prerequisite have been
+   prepared by allexec_prepare_prereq, which will a.o. make links
+   for the allexec32 and allexec64 executables. On single arch build,
+   these links points to the same executable to ensure this test works
+   everywhere the same.
+   No arguments or more arguments means just print its args. */
+int main(int argc, char **argv, char **envp) 
+{
+   if ( (argc == 2) && (strcmp (argv[1], "exec") == 0)) {
+      S("%s will exec ./allexec32\n", argv[0]);
+      test_allexec ("./allexec32");
+      S("%s will exec ./allexec64\n", argv[0]);
+      test_allexec ("./allexec64");
+   } else {
+      int i;
+      S("program exec-ed:");
+      for (i = 0; i < argc; i++) S(" %s", argv[i]);
+      S("\n");
+   }
+   return 0;
+}

diff --git a/none/tests/allexec32.stderr.exp b/none/tests/allexec32.stderr.exp
new file mode 100644 (file)
index 0000000..5cf5d3d
--- /dev/null
+++ b/none/tests/allexec32.stderr.exp
@@ -0,0 +1,14 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+

diff --git a/none/tests/allexec32.stdout.exp b/none/tests/allexec32.stdout.exp
new file mode 100644 (file)
index 0000000..c6d7337
--- /dev/null
+++ b/none/tests/allexec32.stdout.exp
@@ -0,0 +1,14 @@
+./allexec32 will exec ./allexec32
+program exec-ed: ./allexec32
+child exited
+program exec-ed: ./allexec32 constant_arg1 constant_arg2
+child exited
+program exec-ed: ./allexec32
+child exited
+./allexec32 will exec ./allexec64
+program exec-ed: ./allexec64
+child exited
+program exec-ed: ./allexec64 constant_arg1 constant_arg2
+child exited
+program exec-ed: ./allexec64
+child exited

diff --git a/none/tests/allexec32.vgtest b/none/tests/allexec32.vgtest
new file mode 100644 (file)
index 0000000..7fb1249
--- /dev/null
+++ b/none/tests/allexec32.vgtest
@@ -0,0 +1,4 @@
+prog: allexec32
+args: exec
+vgopts: --trace-children=yes
+prereq: ./allexec_prepare_prereq

diff --git a/none/tests/allexec64.stderr.exp b/none/tests/allexec64.stderr.exp
new file mode 100644 (file)
index 0000000..5cf5d3d
--- /dev/null
+++ b/none/tests/allexec64.stderr.exp
@@ -0,0 +1,14 @@
+
+
+
+
+
+
+
+
+
+
+
+
+
+

diff --git a/none/tests/allexec64.stdout.exp b/none/tests/allexec64.stdout.exp
new file mode 100644 (file)
index 0000000..865cea0
--- /dev/null
+++ b/none/tests/allexec64.stdout.exp
@@ -0,0 +1,14 @@
+./allexec64 will exec ./allexec32
+program exec-ed: ./allexec32
+child exited
+program exec-ed: ./allexec32 constant_arg1 constant_arg2
+child exited
+program exec-ed: ./allexec32
+child exited
+./allexec64 will exec ./allexec64
+program exec-ed: ./allexec64
+child exited
+program exec-ed: ./allexec64 constant_arg1 constant_arg2
+child exited
+program exec-ed: ./allexec64
+child exited

diff --git a/none/tests/allexec64.vgtest b/none/tests/allexec64.vgtest
new file mode 100644 (file)
index 0000000..2cbf113
--- /dev/null
+++ b/none/tests/allexec64.vgtest
@@ -0,0 +1,4 @@
+prog: allexec64
+args: exec
+vgopts: --trace-children=yes
+prereq: ./allexec_prepare_prereq

diff --git a/none/tests/allexec_prepare_prereq b/none/tests/allexec_prepare_prereq
new file mode 100755 (executable)
index 0000000..0cd1738
--- /dev/null
+++ b/none/tests/allexec_prepare_prereq
@@ -0,0 +1,36 @@
+#! /bin/sh
+
+# prepare the hard or soft link allexec32 and allexec64
+# On 'single arch' compiled Valgrind, allexec32 and allexec64 will point
+# to the same executable.
+# On 'bi-arch', they will point respectively to the executable compiled
+# for the revelant arch.
+# This allows to test the various exec system calls the same way.
+
+
+pair()
+{
+  if ../../tests/arch_test $1 || ../../tests/arch_test $2
+  then
+      if ../../tests/arch_test $1
+      then
+         ln -f $1/allexec allexec32
+      else
+         ln -f -s allexec64 allexec32
+      fi
+      if ../../tests/arch_test $2
+      then
+         ln -f $2/allexec allexec64
+      else
+         ln -f -s allexec32 allexec64
+      fi
+  fi
+}
+
+
+pair x86                        amd64
+pair ppc32                      ppc64
+pair s390x_unexisting_in_32bits s390x
+pair arm                        arm_unexisting_in_64bits
+
+exit 0

diff --git a/none/tests/amd64/Makefile.am b/none/tests/amd64/Makefile.am
index 4e06238e2fbb4829b1c4c45e46ed209c5e123d37..535335428ef733141a8daca6ee636ca9207001cd 100644 (file)
--- a/none/tests/amd64/Makefile.am
+++ b/none/tests/amd64/Makefile.am
@@ -71,6 +71,7 @@ EXTRA_DIST = \
        xadd.stderr.exp xadd.stdout.exp xadd.vgtest
 
 check_PROGRAMS = \
+       allexec \
        amd64locked \
        asorep \
        bug127521-64 bug132813-amd64 bug132918 \
@@ -112,6 +113,8 @@ AM_CFLAGS    += @FLAG_M64@
 AM_CXXFLAGS  += @FLAG_M64@
 AM_CCASFLAGS += @FLAG_M64@
 
+allexec_CFLAGS         = $(AM_CFLAGS) @FLAG_W_NO_NONNULL@
+
 # generic C ones
 amd64locked_CFLAGS     = $(AM_CFLAGS) -O
 bug132918_LDADD                = -lm

diff --git a/none/tests/amd64/allexec.c b/none/tests/amd64/allexec.c
new file mode 120000 (symlink)
index 0000000..6d6a9cf
--- /dev/null
+++ b/none/tests/amd64/allexec.c
@@ -0,0 +1 @@
+../allexec.c
\ No newline at end of file

diff --git a/none/tests/arm/Makefile.am b/none/tests/arm/Makefile.am
index ffa52bdd0c0bb2978000de1a2dc954504d04fac5..013215ae80cd741d3361f32e8edaa157687abc44 100644 (file)
--- a/none/tests/arm/Makefile.am
+++ b/none/tests/arm/Makefile.am
@@ -12,6 +12,7 @@ EXTRA_DIST = \
        vfp.stdout.exp vfp.stderr.exp vfp.vgtest
 
 check_PROGRAMS = \
+       allexec \
        neon128 \
        neon64 \
        v6intARM \
@@ -23,6 +24,8 @@ AM_CFLAGS    += @FLAG_M32@
 AM_CXXFLAGS  += @FLAG_M32@
 AM_CCASFLAGS += @FLAG_M32@
 
+allexec_CFLAGS         = $(AM_CFLAGS) @FLAG_W_NO_NONNULL@
+
 # These two are specific to their ARM/Thumb respectively and so we
 # hardwire -marm/-mthumb.  neon64 and neon128 are compilable on both,
 # however, ask for them to be compiled on thumb, as that looks

diff --git a/none/tests/arm/allexec.c b/none/tests/arm/allexec.c
new file mode 120000 (symlink)
index 0000000..6d6a9cf
--- /dev/null
+++ b/none/tests/arm/allexec.c
@@ -0,0 +1 @@
+../allexec.c
\ No newline at end of file

diff --git a/none/tests/ppc32/Makefile.am b/none/tests/ppc32/Makefile.am
index f43e1be90c2cf5d8aedcf94ebc464efe1b18bf83..c04d9eec2fea110901e9450fa62899582ee97ec2 100644 (file)
--- a/none/tests/ppc32/Makefile.am
+++ b/none/tests/ppc32/Makefile.am
@@ -32,6 +32,7 @@ EXTRA_DIST = \
        test_isa_2_06_part3.stderr.exp  test_isa_2_06_part3.stdout.exp  test_isa_2_06_part3.vgtest
 
 check_PROGRAMS = \
+       allexec \
        bug129390-ppc32 \
        bug139050-ppc32 \
        ldstrev lsw jm-insns mftocrf mcrfs round test_fx test_gx \
@@ -44,6 +45,8 @@ AM_CFLAGS    += @FLAG_M32@
 AM_CXXFLAGS  += @FLAG_M32@
 AM_CCASFLAGS += @FLAG_M32@
 
+allexec_CFLAGS         = $(AM_CFLAGS) @FLAG_W_NO_NONNULL@
+
 if HAS_ALTIVEC
 ALTIVEC_FLAG = -DHAS_ALTIVEC
 else

diff --git a/none/tests/ppc32/allexec.c b/none/tests/ppc32/allexec.c
new file mode 120000 (symlink)
index 0000000..6d6a9cf
--- /dev/null
+++ b/none/tests/ppc32/allexec.c
@@ -0,0 +1 @@
+../allexec.c
\ No newline at end of file

diff --git a/none/tests/ppc64/Makefile.am b/none/tests/ppc64/Makefile.am
index 15b4b39bd130c5f0ae80e8bff48aa1b46b801853..8d6e3afb8583c0c69abe98a8eff3591a77599607 100644 (file)
--- a/none/tests/ppc64/Makefile.am
+++ b/none/tests/ppc64/Makefile.am
@@ -20,6 +20,7 @@ EXTRA_DIST = \
        test_isa_2_06_part3.stderr.exp  test_isa_2_06_part3.stdout.exp  test_isa_2_06_part3.vgtest
 
 check_PROGRAMS = \
+       allexec \
        jm-insns lsw round std_reg_imm twi_tdi tw_td power6_bcmp power6_mf_gpr test_isa_2_06_part1 \
        test_isa_2_06_part2 test_isa_2_06_part3
 
@@ -27,6 +28,8 @@ AM_CFLAGS    += @FLAG_M64@
 AM_CXXFLAGS  += @FLAG_M64@
 AM_CCASFLAGS += @FLAG_M64@
 
+allexec_CFLAGS         = $(AM_CFLAGS) @FLAG_W_NO_NONNULL@
+
 if HAS_ALTIVEC
 ALTIVEC_FLAG = -DHAS_ALTIVEC
 else

diff --git a/none/tests/ppc64/allexec.c b/none/tests/ppc64/allexec.c
new file mode 120000 (symlink)
index 0000000..6d6a9cf
--- /dev/null
+++ b/none/tests/ppc64/allexec.c
@@ -0,0 +1 @@
+../allexec.c
\ No newline at end of file

diff --git a/none/tests/s390x/Makefile.am b/none/tests/s390x/Makefile.am
index 95b20048a2acdc2974447b4c4f384e98a3cb74f8..bb0a2811d59e786095f68f821ce17bd2b378cd74 100644 (file)
--- a/none/tests/s390x/Makefile.am
+++ b/none/tests/s390x/Makefile.am
@@ -8,6 +8,7 @@ INSN_TESTS = clc clcle cvb cvd icm lpr tcxb lam_stam xc mvst add sub mul \
              op_exception fgx stck stckf stcke stfle cksm mvcl clcl
 
 check_PROGRAMS = $(INSN_TESTS) \
+                allexec \
                 ex_sig \
                 ex_clone \
                 op00
@@ -26,5 +27,7 @@ AM_CFLAGS    += @FLAG_M64@
 AM_CXXFLAGS  += @FLAG_M64@
 AM_CCASFLAGS += @FLAG_M64@
 
+allexec_CFLAGS         = $(AM_CFLAGS) @FLAG_W_NO_NONNULL@
+
 ex_clone_LDFLAGS = -lpthread
 tcxb_CFLAGS      = $(AM_CFLAGS) -std=gnu99

diff --git a/none/tests/s390x/allexec.c b/none/tests/s390x/allexec.c
new file mode 120000 (symlink)
index 0000000..6d6a9cf
--- /dev/null
+++ b/none/tests/s390x/allexec.c
@@ -0,0 +1 @@
+../allexec.c
\ No newline at end of file

diff --git a/none/tests/x86/Makefile.am b/none/tests/x86/Makefile.am
index 74affee71f2c9b53f7ae5d149ba19e76b8b1875f..99b2fa0e405158a18bb9149cc0d4b775ea8d00a3 100644 (file)
--- a/none/tests/x86/Makefile.am
+++ b/none/tests/x86/Makefile.am
@@ -65,6 +65,7 @@ EXTRA_DIST = \
 
 check_PROGRAMS = \
        aad_aam \
+       allexec \
        badseg \
        bt_everything \
        bt_literal \
@@ -109,6 +110,8 @@ AM_CFLAGS    += @FLAG_M32@ $(FLAG_MMMX) $(FLAG_MSSE)
 AM_CXXFLAGS  += @FLAG_M32@ $(FLAG_MMMX) $(FLAG_MSSE)
 AM_CCASFLAGS += @FLAG_M32@
 
+allexec_CFLAGS         = $(AM_CFLAGS) @FLAG_W_NO_NONNULL@
+
 if VGCONF_OS_IS_DARWIN
 # Some of the tests (bug125959_x86, bug152818_x86, insn_*) need
 # -mdynamic-no-pic.  I tried setting *_CFLAGS separately for all of them,

diff --git a/none/tests/x86/allexec.c b/none/tests/x86/allexec.c
new file mode 120000 (symlink)
index 0000000..6d6a9cf
--- /dev/null
+++ b/none/tests/x86/allexec.c
@@ -0,0 +1 @@
+../allexec.c
\ No newline at end of file