Closed
Bug 1266649
Opened 9 years ago
Closed 9 years ago
Assertion failure: !lookup(l).found(), at dist/include/js/HashTable.h:1733
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla49
Tracking | Status | |
---|---|---|
firefox47 | --- | unaffected |
firefox48 | --- | affected |
firefox49 | --- | fixed |
People
(Reporter: gkw, Assigned: h4writer)
References
Details
(Keywords: assertion, testcase, Whiteboard: [jsbugmon:update])
Attachments
(1 file)
(deleted),
patch
|
bbouvier
:
review+
|
Details | Diff | Splinter Review |
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
Reporter | ||
Comment 1•9 years ago
|
||
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?
Blocks: 1265956
Flags: needinfo?(shu)
Reporter | ||
Updated•9 years ago
|
status-firefox47:
--- → unaffected
Comment 2•9 years ago
|
||
Bug 1265956 most likely uncovered a real bug. Hannes, mind taking a look?
Flags: needinfo?(shu) → needinfo?(hv1989)
Reporter | ||
Updated•9 years ago
|
Summary: Assertion failure: !lookup(l).found(), at /Users/skywalker/shell-cache/js-dbg-64-dm-clang-darwin-0891f0fa044c/objdir-js/dist/include/js/HashTable.h:1733 → Assertion failure: !lookup(l).found(), at dist/include/js/HashTable.h:1733
Assignee | ||
Comment 3•9 years ago
|
||
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•9 years ago
|
||
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.
Attachment #8744900 -
Flags: review?(bbouvier) → review+
Assignee | ||
Comment 6•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d2995bc1288fa84a7db26eb7afcd84c5198befc1
Bug 1266649: TraceLogger - Handle failing to add to pointermap gracefully, r=bbouvier
Comment 7•9 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox49:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla49
You need to log in
before you can comment on or make changes to this bug.
Description
•