1659884 - Assertion failure: !lookup(aLookup).found(), at dist/include\mozilla/HashTable.h:2153

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

Attachment: stack

function g(x =
  function() {
    "use asm";
    function f(){}
    return f
  }
) {
  "use asm";
}

Compiled using on Windows / Clang 9 in mozbuild with:

MAKE=mozmake 'LIBCLANG_PATH=<path to>\.mozbuild\clang\bin' sh ./configure --host=x86_64-pc-mingw32 --target=x86_64-pc-mingw32 --enable-debug --enable-gczeal --enable-debug-symbols --disable-tests

Run with:

--fuzzing-safe --no-threads --no-baseline --no-ion

Tested on m-c rev 069bb8bd2356.

Bisecting soon. Does not seem to affect Linux. Not sure if this is bad, but just-in-case the assertion failure message sounds bad.

Comment 1

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/083eaaa198a4
user:        Tooru Fujisawa
date:        Wed Jul 08 19:15:24 2020 +0000
summary:     Bug 1641202 - Part 2: Rewind CompilationInfo.{funcData,functions}. r=tcampbell

:arai, is bug 1641202 a likely regressor?

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Bug 1659884 - Rewind asm.js module. r?tcampbell!

Comment 3

[Tooru Fujisawa [:arai]]

With my analysis, this doesn't cause any trouble except extra memory consumption in non-debug build.

This issue results in adding same entry (JS::WasmModule) into HashTable twice, without checking duplication.
The table will contain same entry in 2 places (assuming they're different entry with hash collision), and the entry added at first will be found for any lookup.
But we add exactly same entry, and the order doesn't matter.

here's details:

the table is CompilationInfo.asmJS.

https://searchfox.org/mozilla-central/rev/23dd7c485a6525bb3a974abeaafaf34bfb43d76b/js/src/frontend/CompilationInfo.h#215

struct MOZ_RAII CompilationInfo : public JS::CustomAutoRooter {
...
  // AsmJS modules generated by parsing.
  HashMap<FunctionIndex, RefPtr<const JS::WasmModule>> asmJS;

CompilationInfo.asmJS is used in the following places:

https://searchfox.org/mozilla-central/rev/23dd7c485a6525bb3a974abeaafaf34bfb43d76b/js/src/frontend/SharedContext.cpp#374

bool FunctionBox::setAsmJSModule(const JS::WasmModule* module) {
...
  return compilationInfo_.asmJS.putNew(index(), module);
}

https://searchfox.org/mozilla-central/rev/23dd7c485a6525bb3a974abeaafaf34bfb43d76b/js/src/frontend/Stencil.cpp#296-297

static JSFunction* CreateFunction(JSContext* cx,
                                  CompilationInfo& compilationInfo,
                                  ScriptStencil& stencil,
                                  FunctionIndex functionIndex) {
...
    RefPtr<const JS::WasmModule> asmJS =
        compilationInfo.asmJS.lookup(functionIndex)->value();

FunctionBox::setAsmJSModule is called only after fully compiling asm.js module, and the resulting module is added to the compilationInfo.asmJS table with the index of the function.

Then, if the enclosing function hits new directive (so, the inner asm.js function is found before the directive), the parser state is rewinded and the enclosing function is parsed again.

The function's index is also rewinded, and the inner asm.js function will get the same function index.
We forgot to remove the asm.js module from compilationInfo.asmJS, and the previous module is kept in the table, when adding newly compiled asm.js module.

Here, functions will get the same index even after rewind, and the compiled asm.js module is also exactly same, we'll add exactly same entry into the table twice.
The entry is retrieved by lookup in CreateFunction, and that will find the first item, while it should find the second item, but given they're same, this doesn't result in another issue.

Comment 4

[Ted Campbell [:tcampbell]]

I agree with Arai's assessment that this is a debug-only issue in practice. The extra WasmModule instances still get cleaned up so this isn't a permanent leak either. Thanks for the report!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/452f89438885
Rewind asm.js module. r=tcampbell

Comment 6

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/452f89438885