[compiler-rt] r179012 - [msan] Intercept glob().

[Chandler Carruth]

On Mon, Apr 8, 2013 at 2:03 AM, Evgeniy Stepanov
<eugeni.stepanov at gmail.com>wrote:

> Author: eugenis
> Date: Mon Apr  8 04:03:00 2013
> New Revision: 179012
>
> URL: http://llvm.org/viewvc/llvm-project?rev=179012&view=rev
> Log:
> [msan] Intercept glob().
>
> Modified:
>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
>
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_platform_interceptors.h
>     compiler-rt/trunk/lib/tsan/rtl/tsan_stat.cc
>     compiler-rt/trunk/lib/tsan/rtl/tsan_stat.h
>

Err, no tests at all? This isn't acceptable. Please commit code like this
with proper testing. There is at least one obvious bug in the code, and I
don't know how many others or how to write unittests for it as you've not
laid out the groundwork in this commit...

I've reverted this commit in r179054, unbreaking roughly all users of ASan.

See the comments below for the bug we spotted, I've no idea if there are
more lurking here:


> Modified:
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
> URL:
> http://llvm.org/viewvc/llvm-project/compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc?rev=179012&r1=179011&r2=179012&view=diff
>
> ==============================================================================
> ---
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
> (original)
> +++
> compiler-rt/trunk/lib/sanitizer_common/sanitizer_common_interceptors.inc
> Mon Apr  8 04:03:00 2013
> @@ -549,6 +549,53 @@ INTERCEPTOR(int, setitimer, int which, c
>  #endif
>
>
> +#if SANITIZER_INTERCEPT_GLOB
> +struct sanitizer_glob_t {
> +  SIZE_T gl_pathc;
> +  char **gl_pathv;
> +};
> +
> +static void unpoison_glob_t(void *ctx, sanitizer_glob_t *pglob) {
> +  COMMON_INTERCEPTOR_WRITE_RANGE(ctx, pglob, sizeof(*pglob));
> +  // +1 for NULL pointer at the end.
> +  COMMON_INTERCEPTOR_WRITE_RANGE(
> +      ctx, pglob->gl_pathv,
> +      (sizeof(pglob->gl_pathc) + 1) * sizeof(*pglob->gl_pathv));
>

I don't think you want 'sizeof(pglob->gl_pathc) + 1', I think you just want
'pglob->gl_pathc + 1' as that is the count of things, and we're about to
scale it by the size of the things...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130408/0804097b/attachment.html>