1266649 - Assertion failure: !lookup(l).found(), at dist/include/js/HashTable.h:1733

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

The following testcase crashes on mozilla-central revision 0891f0fa044c (build with --enable-debug --enable-more-deterministic, run with --fuzzing-safe --no-threads --no-ion):

// Adapted from randomly chosen test: js/src/jit-test/tests/tracelogger/bug1174542.js
new Debugger().setupTraceLogger({
    Scripts: true
})
// Adapted from randomly chosen test: js/src/jit-test/tests/gc/bug-1253124.js
oomTest(() => function(){});

Backtrace:

0   js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001000d8e6d void js::detail::HashTable<js::HashMapEntry<unsigned int, js::TraceLoggerEventPayload*>, js::HashMap<unsigned int, js::TraceLoggerEventPayload*, js::DefaultHasher<unsigned int>, js::SystemAllocPolicy>::MapHashPolicy, js::SystemAllocPolicy>::putNewInfallible<unsigned int&, js::TraceLoggerEventPayload*&>(unsigned int const&, unsigned int&&&, js::TraceLoggerEventPayload*&&&) + 157 (HashTable.h:1733)
1   js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001000d43b3 js::TraceLoggerThread::getOrCreateEventPayload(TraceLoggerTextId, char const*, unsigned long, unsigned long, void const*) + 899 (HashTable.h:1749)
2   js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001000d38fc js::TraceLoggerEvent::TraceLoggerEvent(js::TraceLoggerThread*, TraceLoggerTextId, JSScript*) + 76 (TraceLogging.cpp:970)
3   js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001007ba875 Interpret(JSContext*, js::RunState&) + 853 (TraceLogging.h:413)
4   js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001007ba457 js::RunScript(JSContext*, js::RunState&) + 519 (Interpreter.cpp:426)
5   js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001007d123d js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) + 605 (Interpreter.cpp:498)
6   js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001007aafce js::Call(JSContext*, JS::Handle<JS::Value>, JS::Handle<JS::Value>, js::AnyInvokeArgs const&, JS::MutableHandle<JS::Value>) + 46 (Interpreter.cpp:544)
7   js-dbg-64-dm-clang-darwin-0891f0fa044c	0x0000000100593af1 JS_CallFunction(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSFunction*>, JS::HandleValueArray const&, JS::MutableHandle<JS::Value>) + 737 (jsapi.cpp:2876)
8   js-dbg-64-dm-clang-darwin-0891f0fa044c	0x000000010099722a OOMTest(JSContext*, unsigned int, JS::Value*) + 1162 (TestingFunctions.cpp:1309)
9   js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001007e0d0e js::CallJSNative(JSContext*, bool (*)(JSContext*, unsigned int, JS::Value*), JS::CallArgs const&) + 222 (jscntxtinlines.h:236)
10  js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001007d129e js::InternalCallOrConstruct(JSContext*, JS::CallArgs const&, js::MaybeConstruct) + 702 (Interpreter.cpp:468)
11  js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001007c61e2 Interpret(JSContext*, js::RunState&) + 48322 (Interpreter.cpp:2831)
12  js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001007ba457 js::RunScript(JSContext*, js::RunState&) + 519 (Interpreter.cpp:426)
13  js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001007d2a54 js::ExecuteKernel(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value const&, js::AbstractFramePtr, JS::Value*) + 1124 (Interpreter.cpp:704)
14  js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001007d2dd5 js::Execute(JSContext*, JS::Handle<JSScript*>, JSObject&, JS::Value*) + 469 (RootingAPI.h:667)
15  js-dbg-64-dm-clang-darwin-0891f0fa044c	0x0000000100599141 ExecuteScript(JSContext*, JS::Handle<JSObject*>, JS::Handle<JSScript*>, JS::Value*) + 417 (jsapi.cpp:4392)
16  js-dbg-64-dm-clang-darwin-0891f0fa044c	0x00000001005993b2 JS_ExecuteScript(JSContext*, JS::Handle<JSScript*>) + 82 (RootingAPI.h:667)
17  js-dbg-64-dm-clang-darwin-0891f0fa044c	0x0000000100020799 Process(JSContext*, char const*, bool, FileKind) + 3609 (js.cpp:530)
18  js-dbg-64-dm-clang-darwin-0891f0fa044c	0x0000000100005c64 main + 11748 (js.cpp:6752)
19  js-dbg-64-dm-clang-darwin-0891f0fa044c	0x0000000100000ec4 start + 52

Comment 1

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

autoBisect shows this is probably related to the following changeset:

The first bad revision is:
changeset:   https://hg.mozilla.org/mozilla-central/rev/d9ebe0f67883
user:        Shu-yu Guo
date:        Wed Apr 20 14:52:12 2016 -0700
summary:     Bug 1265956 - Assert that no entry is found in HashTable::putNew. (r=terrence)

Shu-yu, is bug 1265956 a likely regressor?

Comment 2

[Shu-yu Guo [:shu]]

Bug 1265956 most likely uncovered a real bug. Hannes, mind taking a look?

Comment 3

[Hannes Verschore [:h4writer]]

Attachment: bug1266649

Upon failure to add something in the pointerMap, we had already added something to textIdPayloads. Since we didn't increase the "nextId" we were reusing the same id for another payload. Ergo this assert.

Fixed by upon succeeding to add something to textIdPayloads, to do all necessary things. That way failure to add in pointerMap isn't an issue.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Comment on attachment 8744900 [details] [diff] [review]
bug1266649

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

Thank you for the patch.

::: js/src/jit-test/tests/tracelogger/bug1266649.js
@@ +3,5 @@
> +if (typeof du.setupTraceLogger === "function") {
> +    du.setupTraceLogger({
> +        Scripts: true
> +    })
> +    oomTest(() => function(){});

guard for oomTest too!

::: js/src/vm/TraceLogging.cpp
@@ +383,5 @@
>  
>      nextTextId++;
>  
> +    if (!pointerMap.add(p, text, payload))
> +        return nullptr;

Out of curiosity, why can't we just do, much above: textId = nextTextId++; ?
I think there was a reason for that, but it looks like it would solve this issue more cleanly and make the code easier to follow.

Comment 6

[Hannes Verschore [:h4writer]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d2995bc1288fa84a7db26eb7afcd84c5198befc1
Bug 1266649: TraceLogger - Handle failing to add to pointermap gracefully, r=bbouvier

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/d2995bc1288f