Closed
Bug 1461027
Opened 7 years ago
Closed 7 years ago
Assertion failure: nslots == numFixedSlots() + (hasPrivate() ? 1 : 0), at js/src/vm/NativeObject-inl.h:36
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla62
People
(Reporter: gkw, Assigned: jonco)
References
Details
(4 keywords, Whiteboard: [jsbugmon:][adv-main62+])
Attachments
(2 files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
sfink
:
review+
|
Details | Diff | Splinter Review |
The following testcase crashes on mozilla-central revision 4303d49c5393 (build with --32 --enable-debug, run with --fuzzing-safe --no-threads --no-baseline --no-ion):
// Adapted from randomly chosen test: js/src/jit-test/tests/ion/inlining/TypedObject-TypeDescrIsSimpleType.js
for (var i = 0; i < 99; i++) {
w = new TypedObject.ArrayType(TypedObject.int32, 100).build(function() {});
}
// jsfunfuzz-generated
relazifyFunctions();
Backtrace:
#0 0x56d48f16 in js::NativeObject::fixedData (this=0xf6a77070, nslots=4) at js/src/vm/NativeObject-inl.h:36
#1 0x56d2f2f0 in js::ArrayBufferObject::inlineDataPointer (this=0xf6a77070) at js/src/vm/ArrayBufferObject.cpp:952
#2 0x56b55359 in js::ArrayBufferObject::hasInlineData (this=0xf6a77070) at js/src/vm/ArrayBufferObject.h:372
#3 js::OutlineTypedObject::obj_trace (trc=0xffee093c, object=0xf6a90020) at js/src/builtin/TypedObject.cpp:1628
#4 0x56e2190b in js::Class::doTrace (this=<optimized out>, obj=0xf6a90020, trc=0xffee093c) at /home/ubuntu/shell-cache/js-dbg-32-linux-4303d49c5393/objdir-js/dist/include/js/Class.h:869
/snip
For detailed crash information, see attachment.
ArrayBuffers are on the stack, setting s-s as a start.
Reporter | ||
Comment 1•7 years ago
|
||
Reporter | ||
Comment 2•7 years ago
|
||
This was compiled with:
'sh' '/home/ubuntu/trees/mozilla-central/js/src/configure' '--target=i686-pc-linux' '--enable-debug' '--with-ccache' '--enable-gczeal' '--enable-debug-symbols' '--disable-tests'
python -u -m funfuzz.js.compile_shell -b "--32 --enable-debug" -r 4303d49c5393
Updated•7 years ago
|
Whiteboard: [jsbugmon:update] → [jsbugmon:]
Comment 3•7 years ago
|
||
JSBugMon: Cannot process bug: Unable to automatically reproduce, please track manually.
Reporter | ||
Comment 4•7 years ago
|
||
autobisectjs shows this is probably related to the following changeset:
The first bad revision is:
changeset: https://hg.mozilla.org/mozilla-central/rev/9b7cc103ce95
user: Jon Coppeard
date: Thu May 10 10:09:27 2018 +0100
summary: Bug 1457703 - Don't fixup an associated object's shape when updating moved pointers in another object r=sfink a=abillings
Jon, is bug 1457703 a likely regressor?
Assignee | ||
Comment 6•7 years ago
|
||
Fix another place we can call numFixedSlots() on an associated object while traching during a moving GC.
Attachment #8975440 -
Flags: review?(sphink)
Comment 7•7 years ago
|
||
Comment on attachment 8975440 [details] [diff] [review]
bug1461027-fixed-data
Review of attachment 8975440 [details] [diff] [review]:
-----------------------------------------------------------------
I used my callgraph traversal to look for more instances. Here's one:
#178229 = void js::ArrayBufferViewObject::trace(JSTracer*, JSObject*)
#15471 = void* js::NativeObject::getPrivate() const
#15390 = uint32 js::NativeObject::numFixedSlots() const
similarly
#178229 = void js::ArrayBufferViewObject::trace(JSTracer*, JSObject*)
#15475 = void js::NativeObject::setPrivateUnbarriered(void*)
#15390 = uint32 js::NativeObject::numFixedSlots() const
and while we're there,
#178229 = void js::ArrayBufferViewObject::trace(JSTracer*, JSObject*)
#15476 = void js::NativeObject::initPrivate(void*)
#15390 = uint32 js::NativeObject::numFixedSlots() const
Then there's
#165462 = void js::ScriptSourceObject::trace(JSTracer*, JSObject*)
#15434 = JS::Value* js::NativeObject::getReservedSlot(uint32) const
#15418 = JS::Value* js::NativeObject::getSlot(uint32) const
#15390 = uint32 js::NativeObject::numFixedSlots() const
paired with
#178194 = void js::ArrayBufferObject::trace(JSTracer*, JSObject*)
#15427 = void js::NativeObject::setSlot(uint32, JS::Value*)
#15393 = uint8 js::NativeObject::slotInRange(uint32, uint32) const
#15390 = uint32 js::NativeObject::numFixedSlots() const
Hopping over to RegExps:
#244301 = void js::RegExpObject::trace(JSTracer*)
#18595 = js::ReadBarriered<js::RegExpShared*>& js::RegExpObject::sharedRef()
#15468 = void** js::NativeObject::privateRef(uint32) const
#15390 = uint32 js::NativeObject::numFixedSlots() const
I hope the flow makes this one safe, but I didn't check:
#307657 = void js::gc::StoreBuffer::SlotsEdge::trace(js::TenuringTracer*) const
#307659 = void js::TenuringTracer::traceObjectSlots(js::NativeObject*, uint32, uint32)
#15392 = void js::NativeObject::getSlotRange(uint32, uint32, js::HeapSlot**, js::HeapSlot**, js::HeapSlot**, js::HeapSlot**)
#15393 = uint8 js::NativeObject::slotInRange(uint32, uint32) const
#15390 = uint32 js::NativeObject::numFixedSlots() const
I'll skip things similar to that. I'm not sure what to make of this:
#33538 = void NoteWeakMapsTracer::trace(JSObject*, JS::GCCellPtr, JS::GCCellPtr)
#307694 = void JS::TraceChildren(JSTracer*, JS::GCCellPtr)
#105943 = void js::TraceChildren(JSTracer*, void*, int32)
#313776 = void (TraceChildrenFunctor, int32, JSTracer**, void**)<JSObject>((Forward<Args>)(JS::DispatchTraceKindTyped::args)...)) JS::DispatchTraceKindTyped(F, JS::TraceKind, Args&& ...) [with F = TraceChildrenFunctor; Args = {JSTracer*&, void*&}; decltype (f.operator()<JSObject>((Forward<Args>)(JS::DispatchTraceKindTyped::args)...)) = void]
#314678 = void TraceChildrenFunctor::operator(JSTracer*, void*)(JSTracer*, void*) [with T = JSObject]
#162570 = void JSObject::traceChildren(JSTracer*)
#15423 = js::HeapSlot* js::NativeObject::getSlotRef(uint32)
#15393 = uint8 js::NativeObject::slotInRange(uint32, uint32) const
#15390 = uint32 js::NativeObject::numFixedSlots() const
Probably nothing or impossible:
#169217 = void JS::GCHashSet<T, HashPolicy, AllocPolicy>::trace(JSTracer*) [with T = jsid; HashPolicy = js::DefaultHasher<jsid>; AllocPolicy = js::TempAllocPolicy]
#169608 = js::detail::HashTable<T, HashPolicy, AllocPolicy>::Enum::~Enum() [with T = const jsid; HashPolicy = js::HashSet<jsid, js::DefaultHasher<jsid>, js::TempAllocPolicy>::SetOps; AllocPolicy = js::TempAllocPolicy]
#169801 = js::detail::HashTable<T, HashPolicy, AllocPolicy>::Enum::~Enum() [with T = const jsid; HashPolicy = js::HashSet<jsid, js::DefaultHasher<jsid>, js::TempAllocPolicy>::SetOps; AllocPolicy = js::TempAllocPolicy]
#169803 = void js::detail::HashTable<T, HashPolicy, AllocPolicy>::compactIfUnderloaded() [with T = const jsid; HashPolicy = js::HashSet<jsid, js::DefaultHasher<jsid>, js::TempAllocPolicy>::SetOps; AllocPolicy = js::TempAllocPolicy]
#168049 = js::detail::HashTable<T, HashPolicy, AllocPolicy>::RebuildStatus js::detail::HashTable<T, HashPolicy, AllocPolicy>::changeTableSize(int, js::detail::HashTable<T, HashPolicy, AllocPolicy>::FailureBehavior) [with T = const jsid; HashPolicy = js::HashSet<jsid, js::DefaultHasher<jsid>, js::TempAllocPolicy>::SetOps; AllocPolicy = js::TempAllocPolicy]
#13224 = void js::TempAllocPolicy::reportAllocOverflow() const
#17966 = void js::ReportAllocationOverflow(JSContext*)
(IN LIMITED 1) #29390 = void JS_ReportErrorNumberASCII(JSContext*, (JSErrorFormatString*)(void*,uint32)*, void*, uint32)
#153327 = void JS_ReportErrorNumberASCIIVA(JSContext*, (JSErrorFormatString*)(void*,uint32)*, void*, uint32, __va_list_tag*)
#153328 = uint8 js::ReportErrorNumberVA(JSContext*, uint32, (JSErrorFormatString*)(void*,uint32)*, void*, uint32, uint32, __va_list_tag*)
#154214 = jscntxt.cpp:void ReportError(JSContext*, JSErrorReport*, (JSErrorFormatString*)(void*,uint32)*, void*)
#154216 = void js::ErrorToException(JSContext*, JSErrorReport*, (JSErrorFormatString*)(void*,uint32)*, void*)
#155890 = js::ErrorObject* js::ErrorObject::create(JSContext*, uint32, class JS::Handle<JSObject*>, class JS::Handle<JSString*>, uint32, uint32, struct js::ScopedJSFreePtr<JSErrorReport>*, class JS::Handle<JSString*>, class JS::Handle<JSObject*>)
#239297 = uint8 js::ErrorObject::init(JSContext*, class JS::Handle<js::ErrorObject*>, uint32, struct js::ScopedJSFreePtr<JSErrorReport>*, class JS::Handle<JSString*>, class JS::Handle<JSObject*>, uint32, uint32, class JS::Handle<JSString*>)
#15437 = void js::NativeObject::initReservedSlot(uint32, JS::Value*)
#15428 = void js::NativeObject::initSlot(uint32, JS::Value*)
#15393 = uint8 js::NativeObject::slotInRange(uint32, uint32) const
#15390 = uint32 js::NativeObject::numFixedSlots() const
It doesn't report anything more if I suppress things resembling any of the above.
Attachment #8975440 -
Flags: review?(sphink)
Assignee | ||
Comment 8•7 years ago
|
||
(In reply to Steve Fink [:sfink] [:s:] (PTO June 31) from comment #7)
> I used my callgraph traversal to look for more instances. Here's one:
This looks really useful, is this available publicly somehow?
> #178229 = void js::ArrayBufferViewObject::trace(JSTracer*, JSObject*)
> #15471 = void* js::NativeObject::getPrivate() const
> #15390 = uint32 js::NativeObject::numFixedSlots() const
It's OK to call numFixedSlots() on the object that's being traced, because it's already had its shape updated. The problem comes when doing this for some other object (e.g. a view of an array buffer being traced). So I think all of these are safe.
> #165462 = void js::ScriptSourceObject::trace(JSTracer*, JSObject*)
This doesn't seem to exist any more.
> #169217 = void JS::GCHashSet<T, HashPolicy, AllocPolicy>::trace(JSTracer*) [with T = jsid; HashPolicy = js::DefaultHasher<jsid>; AllocPolicy = js::TempAllocPolicy]
Hmm, hopefully that's impossible. Oh, we should be using Range rather than Enum, which doesn't have this possibility.
Updated•7 years ago
|
Has Regression Range: yes → no
Updated•7 years ago
|
Has Regression Range: no → yes
Comment 9•7 years ago
|
||
Comment on attachment 8975440 [details] [diff] [review]
bug1461027-fixed-data
Review of attachment 8975440 [details] [diff] [review]:
-----------------------------------------------------------------
Oh right, that makes sense. I wonder if we could stick the object being traced into the JSTracer if DEBUG, so we could assert in numFixedSlots().
re: the callgraph tool, I thought I had sent you instructions for using it at some point in the past when I was going on PTO? I can look for it.
But quick instructions:
1. grab the script at https://bitbucket.org/sfink/sfink-tools/raw/default/bin/traverse.py?at=default&fileviewer=file-view-default
2. find a recent hazard build. It'll have a build artifact named callgraph.txt. (The one I used here was pretty old, as you noticed from the missing ScriptSourceObject::trace). You'll need to uncompress it.
3. python traverse.py callgraph.txt
4. run 'help'. Many of the commands are kind of cryptic. Some of them don't work since I keep changing stuff and I don't have tests. It'll take a while to load in the full callgraph (you can do a -p linux64-shell-haz push to get a much smaller shell-only one, but it's not *that* bad.)
I have a Rust version that loads the callgraph in < 1sec, but I haven't implemented any of the traversals for it yet.
As you point out, though, it's not really the right tool for this particular problem since it can't distinguish which object you're looking at. Though come to think of it, I wonder how huge it would get if I recorded every field access in every method, so you could ask questions like "are all accesses to SomeClass.somefield done with this RAII lock held?" or simply "where are all the places we directly access this struct's field?" Hmm...
Attachment #8975440 -
Flags: review+
Assignee | ||
Comment 10•7 years ago
|
||
Comment 11•7 years ago
|
||
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla62
Updated•7 years ago
|
Group: javascript-core-security → core-security-release
status-firefox60:
--- → wontfix
status-firefox61:
--- → wontfix
status-firefox-esr52:
--- → wontfix
status-firefox-esr60:
--- → wontfix
Flags: in-testsuite+
Updated•6 years ago
|
Keywords: regression
Updated•6 years ago
|
Whiteboard: [jsbugmon:] → [jsbugmon:][adv-main62+]
Updated•5 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•