That's interesting. I remember rumours that there was a bug
in the env var handling once before, but I could not find it.
I wonder if it got filed as a bug. Dan, one of yours maybe?
J
On Monday, June 14, 2010, Thomas Rast wrote:
> 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;
------------------------------------------------------------------------------
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
