1464274 - (msan) use of uninitialized value in HashTableEntry::isFree()

Status: NEW

(Core :: JavaScript Engine, defect, P2)

Description

[Steve Fink [:sfink] [:s:]]

(msan) use of uninitialized value in HashTableEntry::isFree()

https://hg.mozilla.org/integration/mozilla-inbound/file/635e4b97033/js/public/HashTable.h#l866

==10381==WARNING: MemorySanitizer: use-of-uninitialized-value
    #0 0x279369b in isFree /builds/worker/workspace/build/src/obj-spider/dist/include/js/HashTable.h:866:16
    #1 0x279369b in lookup /builds/worker/workspace/build/src/obj-spider/dist/include/js/HashTable.h:1468
    #2 0x279369b in lookupForAdd /builds/worker/workspace/build/src/obj-spider/dist/include/js/HashTable.h:1843
    #3 0x279369b in lookupForAdd /builds/worker/workspace/build/src/obj-spider/dist/include/js/HashTable.h:408
    #4 0x279369b in getOrCreate<(lambda at /builds/worker/workspace/build/src/js/src/vm/SharedImmutableStringsCache.cpp:101:45)> /builds/worker/workspace/build/src/js/src/vm/SharedImmutableStringsCache-inl.h:27
    #5 0x279369b in js::SharedImmutableStringsCache::getOrCreate(mozilla::UniquePtr<char [], JS::FreePolicy>&&, unsigned long) /builds/worker/workspace/build/src/js/src/vm/SharedImmutableStringsCache.cpp:101
    #6 0x251699b in js::SourceCompressionTask::work() /builds/worker/workspace/build/src/js/src/vm/JSScript.cpp:1976:29
    #7 0x237729c in js::HelperThread::handleCompressionWorkload(js::AutoLockHelperThreadState&) /builds/worker/workspace/build/src/js/src/vm/HelperThreads.cpp:2115:15
    #8 0x23723d7 in js::HelperThread::threadLoop() /builds/worker/workspace/build/src/js/src/vm/HelperThreads.cpp:2384:9
    #9 0x23bc7ca in callMain<0> /builds/worker/workspace/build/src/js/src/threading/Thread.h:242:5
    #10 0x23bc7ca in js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start(void*) /builds/worker/workspace/build/src/js/src/threading/Thread.h:235
    #11 0x7ffff7bc7b4f in start_thread /build/eglibc-ZYONVs/eglibc-2.13/nptl/pthread_create.c:304
    #12 0x7ffff646efbc in clone /build/eglibc-ZYONVs/eglibc-2.13/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:112

  Uninitialized value was created by a heap allocation
    #0 0x46241d in __interceptor_malloc /builds/worker/workspace/moz-toolchain/src/llvm/projects/compiler-rt/lib/msan/msan_interceptors.cc:865:3
    #1 0x25162c7 in js_malloc /builds/worker/workspace/build/src/obj-spider/dist/include/js/Utility.h:388:12
    #2 0x25162c7 in js_pod_malloc<char> /builds/worker/workspace/build/src/obj-spider/dist/include/js/Utility.h:578
    #3 0x25162c7 in js::SourceCompressionTask::work() /builds/worker/workspace/build/src/js/src/vm/JSScript.cpp:1921
    #4 0x237729c in js::HelperThread::handleCompressionWorkload(js::AutoLockHelperThreadState&) /builds/worker/workspace/build/src/js/src/vm/HelperThreads.cpp:2115:15
    #5 0x23723d7 in js::HelperThread::threadLoop() /builds/worker/workspace/build/src/js/src/vm/HelperThreads.cpp:2384:9
    #6 0x23bc7ca in callMain<0> /builds/worker/workspace/build/src/js/src/threading/Thread.h:242:5
    #7 0x23bc7ca in js::detail::ThreadTrampoline<void (&)(void*), js::HelperThread*>::Start(void*) /builds/worker/workspace/build/src/js/src/threading/Thread.h:235
    #8 0x7ffff7bc7b4f in start_thread /build/eglibc-ZYONVs/eglibc-2.13/nptl/pthread_create.c:304

Comment 1

[Jeff Walden [:Waldo]]

This UMR was probably a read-of-constant-zero in the days before bug 1462261.  With that lessening of writes, it's now UMR.  This is good!  The zeroing was somehow hiding actual buggy code.

It's a fairly short bit of basically straight-line code to hit this, so a testcase that hits this probably will fire reliably.  It also *might* be easy to debug sans testcase, for the same reason.  Not sure.  I'm looking and trying some of both, unless I get better intel to investigate from.

Comment 2

[Jeff Walden [:Waldo]]

From the looking I did, I'm not certain this isn't something in the compression code that runs between the use-of here and the was-created-by.  Holding off on looking at this til there's an identified testcase at fault -- then I can start trying to figure out why all my attempts to reproduce this failed.  (Even though I was reproducing other of the msan failures reported just around the time this bug was reported.)

Comment 3

[Steve Fink [:sfink] [:s:]]

Compression code? (red flag goes up) One thing that has been a problem in the past is that if you're linking with a zlib that wasn't compiled with asan, then it won't track definedness though it. I would've thought we would be using an in-tree library, but I guess this build is --disable-jemalloc... I don't know what that ends up with.

My attempts at reproducing locally are running into other problems -- tons of reports not reported on try. I fixed one swath of them by setting the definedness bits after calling vsnprintf, which strangely isn't a problem on try despite using the same version of clang. Mysterious.

Comment 4

[Jeff Walden [:Waldo]]

The relevant code, from allocation of memory to its supposed uninitialized use, is:

    // Try to keep the maximum memory usage down by only allocating half the
    // size of the string, first.
    size_t inputBytes = source->length() * sizeof(char16_t);
    size_t firstSize = inputBytes / 2;
    UniqueChars compressed(js_pod_malloc<char>(firstSize)); // MEMORY ALLOCATED HERE
    if (!compressed)
        return;

    const char16_t* chars = source->data.as<ScriptSource::Uncompressed>().string.chars();
    Compressor comp(reinterpret_cast<const unsigned char*>(chars), inputBytes);
    if (!comp.init())
        return;

    comp.setOutput(reinterpret_cast<unsigned char*>(compressed.get()), firstSize);
    bool cont = true;
    bool reallocated = false;
    while (cont) {
        if (shouldCancel())
            return;

        switch (comp.compressMore()) {
          case Compressor::CONTINUE:
            break;
          case Compressor::MOREOUTPUT: {
            if (reallocated) {
                // The compressed string is longer than the original string.
                return;
            }

            // The compressed output is greater than half the size of the
            // original string. Reallocate to the full size.
            if (!reallocUniquePtr(compressed, inputBytes))
                return;

            comp.setOutput(reinterpret_cast<unsigned char*>(compressed.get()), inputBytes);
            reallocated = true;
            break;
          }
          case Compressor::DONE:
            cont = false;
            break;
          case Compressor::OOM:
            return;
        }
    }

    size_t totalBytes = comp.totalBytesNeeded();

    // Shrink the buffer to the size of the compressed data.
    if (!reallocUniquePtr(compressed, totalBytes))
        return;

    comp.finish(compressed.get(), totalBytes);

    if (shouldCancel())
        return;

    auto& strings = runtime_->sharedImmutableStrings();
    resultString_ = strings.getOrCreate(mozilla::Move(compressed), totalBytes);

And the last line is what uses it.  All on one thread, from every appearance.

So, yeah -- this does have the appearance of being a non-asan zlib issue.  Fun.

Comment 5

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last months and this bug has priority 'P2'.
:sdetar, could you have a look please?
For more information, please visit auto_nag documentation.