Version: (using Devel) OS: Linux Installed from: Compiled sources The command valgrind --trace-children=yes /bin/sh -c "/bin/sh -c env" yields incorrect output on my system (r10990). It seems to be lacking the LANG environment variable, but it has an extra blank line. I first ran into this running wine under valgrind, where the symptom was missing environment variables. I managed to track it down a bit; in coregrind/m_initimg/initimg-linux.c, on entry to setup_client_env(), on about the fourth time through, there was an empty environment variable (no name, no =, no value) inserted after LD_PRELOAD=, i.e. "LD_PRELOAD=" "" "XAUTHORITY=/home/dank/.Xauthority" and wine (instead of printing a blank line as env does) truncated the environment at that point. Could the LD_PRELOAD munging in that file have misbehaved somehow?
Wine users can apply the crude workaround which I posted at http://www.winehq.org/pipermail/wine-patches/2009-November/081801.html but it's not a fix.
I can reproduce this, but it doesn't seem to me to be a bug in Valgrind. Or at least not in the env mangler in m_initimg. In the test case I'm working with, every time the output mangled environment contains a blank line, the input environment contained a blank line too. So the mangler itself isn't creating them. If Wine truncates the environment at a blank line, isn't that a bug in Wine? It really ought to regard the environment as being terminated by a (char*)NULL, not a blank line. I note in passing that I can reproduce this on Ubuntu 9.10 but not on openSUSE 11.0. So perhaps the presence of the blank line is somehow to do with the dash-vs-bash difference? Am leaning towards closing this as invalid.
Created attachment 40622 [details] patch that does debug printing in the env mangler Using this patch, it's possible to see that (at least in Dan's test case) whenever a blank line exists in the final environment ("FINAL: ") then it also exists in the incoming environment ("BEFORE: ").
I'm stuck somewhere with only a Windows box at the moment, but the next step seems to be to try to reproduce without valgrind, and file a bug with the shell?
I still think I should WFM this; nevertheless .. the debug printing got committed as r11041.
This is a bug in mash_colon_env(), as explained in http://sourceforge.net/mailarchive/forum.php?thread_name=201006141813.45168.trast%40student.ethz.ch&forum_name=valgrind-users I'm quoting the resolution part here; the original story of how I hit it is in the thread starter. I'll attach the patch that I'm referring to. Thomas Rast wrote: > 0x7fffc45381fb GIT_DIFF_TOOM=bad-tool > 0x7fffc4538212 LD_PRELOAD= > 0x7fffc453821e > 0x7fffc453821f MORE=-sl > 0x7fffc4538228 GIT_DIFF_TOOK=bad-tool Turns out that this is a bug in mash_colon_env(). Patch at the end. The reasons it triggered in the git test suite (apart from the once-in-a-lifetime bad luck of hitting a crucial env var that makes the testcase fail): * 'git difftool ...' is a perl script, but goes through the git wrapper program and thus involves valgrind. There is no LD_PRELOAD variable at the beginning, but valgrind uses it for its purposes and leaves it set-but-empty at exec() time. * difftool invokes git-diff, which again goes through the wrapper. * valgrind initially sets LD_PRELOAD again with the valgrind libs, but this time it has a trailing : because it started out empty! * mash_colon_env() removes the first components, but in the last iteration here: if (match) { output = entry_start; varp++; /* skip ':' after removed entry */ //[1] } else entry_start = output+1; /* entry starts after ':' */ } *output++ = *varp++; //[2] } varp is first stepped over the trailing : onto the \0 in line [1], then over the \0 in line [2]. Bang, you're dead. This can be fixed by guarding [2] so it only steps (and copies) anything if we're not at the end yet. Then I believe there's another bug; admittedly I haven't looked into how string_match works, but for the last entry, I believe you need to \0-terminate the string starting from entry_start, in the same way that you do for all other matches.
Created attachment 48031 [details] Proposed fix for string overrun.
The patch is clearly correct, so I've committed it as r11178. I can't actually reproduce the bug here however so if somebody else could confirm that it is now fixed that would be great.
Thanks a lot for the quick response times in discussing and committing this! Reproducing the symptom itself is not so hard: Call the following program exec-env-helper.c and compile it. --- 8< --- #include <unistd.h> extern char **environ; int main (int argc, char *argv[]) { char *new_argv[] = { "env", NULL }; return execvp(new_argv[0], new_argv); } --- >8 --- Then run export LD_PRELOAD= # this simulates the effect of one layer of valgrind env > expected valgrind ./exec-env-helper > actual diff -u expected actual The diff will show that a random variable has been deleted. On the other hand, the fixed version merely shuffles things around a bit and updates the bash-magic $_ variable.
Thomas, could you please verify this bug is fixed in the trunk, so that we can then close the report?
Oh, my apologies for not making this explicit in my last comment. I of course ran the testcase I provided there on all versions to verify that the committed fix works. I have just run it again and it also passes in r11217. Thanks!
Thanks. Closing.