Bug 161115 - js/regress/put-by-id-transition-with-indexing-header.html and svg/carto.net/window.svg fail in debug after r204854
Summary: js/regress/put-by-id-transition-with-indexing-header.html and svg/carto.net/w...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
: 161114 (view as bug list)
Depends on:
Blocks:
 
Reported: 2016-08-23 16:04 PDT by Filip Pizlo
Modified: 2016-08-24 09:57 PDT (History)
11 users (show)

See Also:


Attachments
skipping patch (1.27 KB, patch)
2016-08-23 16:16 PDT, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (3.76 KB, patch)
2016-08-23 23:21 PDT, Filip Pizlo
keith_miller: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2016-08-23 16:04:27 PDT
Patch forthcoming.
Comment 1 Filip Pizlo 2016-08-23 16:16:44 PDT
Created attachment 286802 [details]
skipping patch
Comment 2 Filip Pizlo 2016-08-23 16:24:19 PDT
Skipped in http://trac.webkit.org/changeset/204867
Comment 3 Filip Pizlo 2016-08-23 19:27:32 PDT
I'm pretty sure I know what is going on: if we put a new butterfly with more out-of-line capacity (or more pre-capacity) into an object with an old structure (or with m_indexBias reflecting the old pre-capacity) then we won't quite know how to find the base, since that calculation currently relies on the structure and m_indexBias.

This is the code that causes this:

char* JIT_OPERATION operationReallocateButterflyToGrowPropertyStorage(ExecState* exec, JSObject* object, size_t newSize)
{
    VM& vm = exec->vm();
    NativeCallFrameTracer tracer(&vm, exec);

    DeferGC deferGC(vm.heap);
    Butterfly* result = object->growOutOfLineStorage(vm, object->structure()->outOfLineCapacity(), newSize);
    object->setButterflyWithoutChangingStructure(vm, result);
    return reinterpret_cast<char*>(result);
}

Intriguingly, the use of DeferGC is one of the causes. It causes GC to run after we have already set the butterfly, rather than in a state where the object still points to the "right" butterfly for its structure.

I'm tempted to say that the solution is to simply remove the DeferGC!  If we do that then the GC will happen exactly where we want it to: inside growOutOfLineStorage().  That's a fine place to GC, since the object will still be in a sane state in that method.
Comment 4 Filip Pizlo 2016-08-23 21:32:04 PDT
*** Bug 161114 has been marked as a duplicate of this bug. ***
Comment 5 Filip Pizlo 2016-08-23 21:32:41 PDT
I have fixes for these crashes, I'm testing them now.
Comment 6 Filip Pizlo 2016-08-23 23:21:39 PDT
Created attachment 286835 [details]
the patch
Comment 7 Keith Miller 2016-08-24 09:55:43 PDT
Comment on attachment 286835 [details]
the patch

r=me.
Comment 8 Filip Pizlo 2016-08-24 09:57:50 PDT
Landed in https://trac.webkit.org/changeset/204901