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.
Thanks for a great tool!
- Thomas
Index: coregrind/m_libcproc.c
===================================================================
--- coregrind/m_libcproc.c (revision 11175)
+++ coregrind/m_libcproc.c (working copy)
@@ -182,9 +182,14 @@
entry_start = output+1; /* entry starts after ':' */
}
- *output++ = *varp++;
+ if (*varp)
+ *output++ = *varp++;
}
+ /* Again match against the copied entry; this time we do not have
+ to save the old value of *output as we will zero it anyway */
+ *output = '\0';
+
/* match against the last entry */
if (VG_(string_match)(remove_pattern, entry_start)) {
output = entry_start;
--
Thomas Rast
tr...@{inf,student}.ethz.ch
------------------------------------------------------------------------------
ThinkGeek and WIRED's GeekDad team up for the Ultimate
GeekDad Father's Day Giveaway. ONE MASSIVE PRIZE to the
lucky parental unit. See the prize list and enter to win:
http://p.sf.net/sfu/thinkgeek-promo
_______________________________________________
Valgrind-users mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/valgrind-users
