1058500 - Support MSan (MemorySanitizer) in the JS shell

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Christian Holler (:decoder)]

The JS shell is a low hanging fruit for getting MSan (MemorySanitizer) to work, because it has only one external library (zlib) that we need to take care of. I managed to get that working even without recompiling zlib by marking the memory we pass to zlib as initialized (MSan won't see the initialization in zlib happening).

I'll attach a patch here soon (it will also need build peer review since we need to add generic MSan support to the build system).

Together with bug 1056233, this will allow for a JS shell build (with --disable-ion) that also passes at least the jit-tests, I haven't tried more.

Once we have our ARM64 simulator working, we can even test parts of the JIT then.

Comment 1

[Christian Holler (:decoder)]

Attachment: jsshell-msan-build.patch

Build system support for MSan.

This adds --enable-memory-sanitizer similar to --enable-address-sanitizer.

Furthermore, it adds the basic poison/unpoison functions to mfbt/MemoryChecking.h. I tried making the three MOZ_MAKE_MEM_* macros useful as well under MSan. Since MSan and ASan can never be used together, it does not make sense to provide a combined interface. With this interface, working with NOACCESS or UNDEFINED memory will cause MSan to report an error.

Ted, can you review this in one, or does the mfbt part need additional review?

Comment 2

[Christian Holler (:decoder)]

Attachment: jsshell-msan-zlib.patch

This patch marks memory that we pass to zlib as initialized. With this patch, we don't need to build zlib with MSan, initial tests show that this works fine :)

The downside is of course that we won't see if zlib really initializes all our memory, but that's a fair trade-off for making the build/use easier I guess.

Comment 3

[Sergey Matveev]

(In reply to Christian Holler (:decoder) from comment #2)
> Created attachment 8479953 [details] [diff] [review]
> jsshell-msan-zlib.patch
> 
> This patch marks memory that we pass to zlib as initialized. With this
> patch, we don't need to build zlib with MSan, initial tests show that this
> works fine :)

Note that this approach is not guaranteed to prevent spurious reports from a non-instrumented DSO. E.g. some code in zlib may allocate memory, write to it and then pass it to a libc call. MSan will not register the write (because zlib is not instrumented), but will intercept the libc call and bark at the "uninitialized" argument.

Comment 4

[Christian Holler (:decoder)]

Thanks for pointing that out. I didn't think about that. I think that for the particular functions we use in zlib, this won't happen though (I guess). We just compress/decompress our source code, that's all.

Comment 5

[(not currently active) Ted Mielczarek]

Comment on attachment 8479951 [details] [diff] [review]
jsshell-msan-build.patch

Review of attachment 8479951 [details] [diff] [review]:
-----------------------------------------------------------------

You need someone else to review the MFBT changes. The build changes are fine.

Comment 6

[Christian Holler (:decoder)]

Comment on attachment 8479951 [details] [diff] [review]
jsshell-msan-build.patch

Waldo, can you look at these easy MFBT additions for MSan? Thanks!

Comment 7

[Nicholas Nethercote [inactive]]

Comment on attachment 8479953 [details] [diff] [review]
jsshell-msan-zlib.patch

Review of attachment 8479953 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/vm/Compression.cpp
@@ +106,5 @@
>  js::DecompressString(const unsigned char *inp, size_t inplen, unsigned char *out, size_t outlen)
>  {
>      JS_ASSERT(inplen <= UINT32_MAX);
> +
> +    // Mark the memory we pass to zlib as initialized for MSan

'.' at end of sentence, please :)

Comment 8

[Christian Holler (:decoder)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/12cc55488b9c
https://hg.mozilla.org/integration/mozilla-inbound/rev/b4e8253d215d

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/12cc55488b9c
https://hg.mozilla.org/mozilla-central/rev/b4e8253d215d