Closed Bug 1811057 Opened 1 year ago Closed 10 months ago

Ship Change Array by Copy Feature

Categories

(Core :: JavaScript Engine, task, P3)

task

Tracking

()

RESOLVED FIXED
115 Branch
Tracking Status
relnote-firefox --- 115+
firefox115 --- fixed

People

(Reporter: mgaudet, Assigned: tjc)

References

(Blocks 1 open bug)

Details

(Keywords: dev-doc-complete)

Attachments

(1 file)

After we've fuzzed it, we should ship the change array by copy proposal on nightly.

We'll need to send an Intent to Ship before that.

Blocks: 1811059
Severity: -- → S3
Priority: -- → P3
Keywords: dev-doc-needed

Set the --enable-change-array-by-copy shell flag and the
javascript.options.experimental.enable_change_array_by_copy pref to true
by default in nightly builds.

Assignee: nobody → tjc
Status: NEW → ASSIGNED
Summary: Ship Change Array by Copy Feature to Nightly → Ship Change Array by Copy Feature
Duplicate of this bug: 1811059
Attachment #9327009 - Attachment description: Bug 1811057 - Ship Change Array by Copy Feature to Nightly r?mgaudet → Bug 1811057 - Ship Change Array by Copy Feature r?mgaudet
Blocks: 1826643
Attachment #9327009 - Attachment description: Bug 1811057 - Ship Change Array by Copy Feature r?mgaudet → Bug 1811057 - Ship Change Array by Copy Feature r?mgaudet,nchevobbe,peterv
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/cb6db65172c6
Ship Change Array by Copy Feature r=mgaudet,devtools-reviewers,peterv

It looks like this is an intermittent failure unrelated to this change?

https://bugzilla.mozilla.org/show_bug.cgi?id=1716416

Flags: needinfo?(tjc)
Flags: needinfo?(tjc)

I can reproduce this locally; it's crashing here:

(rr) bt
#0  mozilla::Maybe<js::frontend::ScriptIndexRange>::operator->() (this=0x7fff6007cd88) at /home/matthew/unified/obj-opt-browser-as-release-x86_64-pc-linux-gnu/dist/include/mozilla/Maybe.h:783
#1  js::GlobalObject::getSelfHostedFunction(JSContext*, JS::Handle<js::GlobalObject*>, JS::Handle<js::PropertyName*>, JS::Handle<JSAtom*>, unsigned int, JS::MutableHandle<JS::Value>)
    (cx=0x7f783c124e00, global=(js::GlobalObject * const) 0x32eed4e3d030 [object BackstagePass], selfHostedName="ArrayToString", name="toString", nargs=<optimized out>, funVal=$JS::UndefinedValue())
    at /home/matthew/unified/js/src/vm/GlobalObject.cpp:868
#2  0x00007f78422882db in JS::NewFunctionFromSpec(JSContext*, JSFunctionSpec const*, JS::Handle<JS::PropertyKey>)
    (cx=cx@entry=0x7f783c124e00, fs=fs@entry=0x7f7846678fc8 <array_methods+40>, id=id@entry=$jsid("toString")) at /home/matthew/unified/js/src/jsapi.cpp:2225
#3  0x00007f78420fad4b in DefineFunctionFromSpec(JSContext*, JS::Handle<JSObject*>, JSFunctionSpec const*)
    (cx=0x7f783c124e00, obj=(JSObject * const) 0x32eed4e4e030 [object Array], fs=0x7f7846678fc8 <array_methods+40>) at /home/matthew/unified/js/src/vm/JSObject.cpp:2287
#4  js::DefineFunctions(JSContext*, JS::Handle<JSObject*>, JSFunctionSpec const*) (cx=0x7f783c124e00, cx@entry=0x7fff6007cf50, obj=(JSObject * const) 0x32eed4e4e030 [object Array], 
    obj@entry=(JSObject * const) 0x7f784512c407 Cannot access memory at address 0x6843007961727241, fs=0x7f7846678fc8 <array_methods+40>, fs@entry=0x3) at /home/matthew/unified/js/src/vm/JSObject.cpp:2299
#5  0x00007f7842139076 in JS_DefineFunctions(JSContext*, JS::Handle<JSObject*>, JSFunctionSpec const*) (cx=0x1e8, cx@entry=0x7f783c124e00, obj=<error reading variable: Cannot access memory at address 0x400>, 
    obj@entry=(JSObject * const) 0x32eed4e4e030 [object Array], fs=0x32eed4e3adc0) at /home/matthew/unified/js/src/vm/PropertyAndElement.cpp:949
#6  0x00007f78420b6381 in js::GlobalObject::resolveConstructor(JSContext*, JS::Handle<js::GlobalObject*>, JSProtoKey, js::GlobalObject::IfClassIsDisabled)
    (cx=0x7f783c124e00, global=(js::GlobalObject * const) 0x32eed4e3d030 [object BackstagePass], key=JSProto_Array, mode=<optimized out>) at /home/matthew/unified/js/src/vm/GlobalObject.cpp:347
#7  0x00007f784200c6c4 in js::GlobalObject::ensureConstructor(JSContext*, JS::Handle<js::GlobalObject*>, JSProtoKey)
    (cx=0x7f783c124e00, global=(js::GlobalObject * const) 0x32eed4e3d030 [object BackstagePass], key=JSProto_Array) at /home/matthew/unified/js/src/vm/GlobalObject.h:337
#8  js::GlobalObject::getOrCreateArrayPrototype(JSContext*, JS::Handle<js::GlobalObject*>) (cx=0x7f783c124e00, global=(js::GlobalObject * const) 0x32eed4e3d030 [object BackstagePass])
    at /home/matthew/unified/js/src/vm/GlobalObject.h:513
#9  js::GlobalObject::createArrayShapeWithDefaultProto(JSContext*) (cx=0x7f783c124e00) at /home/matthew/unified/js/src/builtin/Array.cpp:5081
#10 js::GlobalObject::getArrayShapeWithDefaultProto(JSContext*) (cx=0x7f783c124e00) at /home/matthew/unified/js/src/vm/GlobalObject.h:1039
#11 NewArray<4294967295u>(JSContext*, unsigned int, js::NewObjectKind, js::gc::AllocSite*) (cx=0x7f783c124e00, site=0x0, length=<optimized out>, newKind=<optimized out>)
    at /home/matthew/unified/js/src/builtin/Array.cpp:5100
#12 js::NewDenseCopiedArray(JSContext*, unsigned int, JS::Value const*, js::NewObjectKind) (cx=cx@entry=0x7f783c124e00, length=2, values=0x7f7839b13650, newKind=newKind@entry=js::TenuredObject)
    at /home/matthew/unified/js/src/builtin/Array.cpp:5250
#13 0x00007f78423d5516 in js::InterpretObjLiteralArray(JSContext*, js::frontend::CompilationAtomCache const&, mozilla::Span<unsigned char const, 18446744073709551615ul>, unsigned int)
    (cx=0x7f783c124e00, atomCache=..., literalInsns=..., propertyCount=<optimized out>) at /home/matthew/unified/js/src/frontend/ObjLiteral.cpp:211
#14 js::ObjLiteralStencil::create(JSContext*, js::frontend::CompilationAtomCache const&) const (this=<optimized out>, cx=0x7f783c124e00, atomCache=...) at /home/matthew/unified/js/src/frontend/ObjLiteral.cpp:361
#15 0x00007f78423bf067 in js::frontend::EmitScriptThingsVector(JSContext*, js::frontend::CompilationAtomCache const&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, mozilla::Span<js::frontend::TaggedScriptThingIndex const, 18446744073709551615ul>, mozilla::Span<JS::GCCellPtr, 18446744073709551615ul>) (cx=0x400, 
    cx@entry=0x7f783c124e00, atomCache=..., stencil=..., gcOutput=..., things=..., output=...) at /home/matthew/unified/js/src/frontend/BytecodeSection.cpp:90
#16 0x00007f7842106dc6 in js::PrivateScriptData::InitFromStencil(JSContext*, JS::Handle<JSScript*>, js::frontend::CompilationAtomCache const&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, js::frontend::TypedIndex<js::frontend::ScriptStencil>) (cx=cx@entry=0x7f783c124e00, script=script@entry=0x32eed4e4d100, atomCache=..., stencil=..., gcOutput=..., scriptIndex=scriptIndex@entry=...)
    at /home/matthew/unified/js/src/vm/JSScript.cpp:2293
#17 0x00007f78421071cc in JSScript::fullyInitFromStencil(JSContext*, js::frontend::CompilationAtomCache const&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, JS::Handle<JSScript*>, js::frontend::TypedIndex<js::frontend::ScriptStencil>) (cx=cx@entry=0x7f783c124e00, atomCache=<optimized out>, stencil=..., gcOutput=..., script=script@entry=0x32eed4e4d100, scriptIndex=scriptIndex@entry=...)
    at /home/matthew/unified/js/src/vm/JSScript.cpp:2417
#18 0x00007f78421077b1 in JSScript::fromStencil(JSContext*, js::frontend::CompilationAtomCache&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&, js::frontend::TypedIndex<js::frontend::ScriptStencil>) (cx=0x7f783c124e00, atomCache=..., stencil=..., gcOutput=..., scriptIndex=...) at /home/matthew/unified/js/src/vm/JSScript.cpp:2505
#19 0x00007f78423f4ad1 in InstantiateTopLevel(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&)
    (cx=0x7f783c124e00, input=..., stencil=..., gcOutput=...) at /home/matthew/unified/js/src/frontend/Stencil.cpp:2145
#20 0x00007f78423f3cd6 in js::frontend::CompilationStencil::instantiateStencilAfterPreparation(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&) (cx=0x1e8, cx@entry=0x7f783c124e00, input=..., stencil=..., gcOutput=...) at /home/matthew/unified/js/src/frontend/Stencil.cpp:2487
#21 0x00007f78423f3150 in js::frontend::CompilationStencil::instantiateStencils(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&)
    (cx=0x7f783c124e00, input=..., stencil=..., gcOutput=...) at /home/matthew/unified/js/src/frontend/Stencil.cpp:2414
#22 0x00007f78423a6beb in js::frontend::InstantiateStencils(JSContext*, js::frontend::CompilationInput&, js::frontend::CompilationStencil const&, js::frontend::CompilationGCOutput&)
    (cx=cx@entry=0x7f783c124e00, input=..., stencil=..., gcOutput=...) at /home/matthew/unified/js/src/frontend/BytecodeCompiler.cpp:448
#23 0x00007f78423fceda in JS::InstantiateModuleStencil(JSContext*, JS::InstantiateOptions const&, js::frontend::CompilationStencil*, JS::InstantiationStorage*)
    (cx=cx@entry=0x7f783c124e00, options=..., stencil=0x7f7839c6f710, storage=storage@entry=0x0) at /home/matthew/unified/js/src/frontend/Stencil.cpp:5307
#24 0x00007f783e6a7ad5 in mozJSModuleLoader::InstantiateStencil(JSContext*, js::frontend::CompilationStencil*, bool) (aCx=0x7f783c124e00, aStencil=0x32eed4e3adc0, aIsModule=<optimized out>)
    at /home/matthew/unified/js/xpconnect/loader/mozJSModuleLoader.cpp:1085
#25 mozJSModuleLoader::GetScriptForLocation(JSContext*, ModuleLoaderInfo&, nsIFile*, bool, JS::MutableHandle<JSScript*>, char**)
     (aCx=aCx@entry=0x7f783c124e00, aInfo=..., aModuleFile=0x7f783b119d40, aUseMemMap=false, aScriptOut=aScriptOut@entry=0x0, aLocationOut=aLocationOut@entry=0x0) at /home/matthew/unified/js/xpconnect/loader/mozJSModuleLoader.cpp:1007
#26 0x00007f783e6a7080 in mozJSModuleLoader::LoadSingleModuleScript(mozilla::loader::ComponentModuleLoader*, JSContext*, JS::loader::ModuleLoadRequest*, JS::MutableHandle<JSScript*>)
    (aModuleLoader=<optimized out>, aCx=0x7f783c124e00, aRequest=<optimized out>, aScriptOut=0x0) at /home/matthew/unified/js/xpconnect/loader/mozJSModuleLoader.cpp:689
#27 0x00007f783e6ae39a in mozilla::loader::ComponentModuleLoader::StartFetch(JS::loader::ModuleLoadRequest*) (this=0x7f783b112fc0, aRequest=0x7f78491e4270)
    at /home/matthew/unified/js/xpconnect/loader/ComponentModuleLoader.cpp:104
#28 0x00007f783e6917aa in JS::loader::ModuleLoaderBase::StartOrRestartModuleLoad(JS::loader::ModuleLoadRequest*, JS::loader::ModuleLoaderBase::RestartRequest)
    (this=0x7f783b112fc0, aRequest=0x7f78491e4270, aRestart=JS::loader::ModuleLoaderBase::RestartRequest::No) at /home/matthew/unified/js/loader/ModuleLoaderBase.cpp:418
#29 0x00007f783e6ab30f in JS::loader::ModuleLoadRequest::StartModuleLoad() (this=<optimized out>)
    at /home/matthew/unified/obj-opt-browser-as-release-x86_64-pc-linux-gnu/dist/include/js/loader/ModuleLoadRequest.h:105
#30 mozJSModuleLoader::ImportESModule(JSContext*, nsTSubstring<char> const&, JS::MutableHandle<JSObject*>, mozilla::loader::SkipCheckForBrokenURLOrZeroSized)
    (this=0x7f783b110740, aCx=0x7f783c124e00, aLocation=<optimized out>, aModuleNamespace=0x0, aSkipCheck=<optimized out>) at /home/matthew/unified/js/xpconnect/loader/mozJSModuleLoader.cpp:1822
#31 0x00007f783df660e5 in mozilla::xpcom::ConstructJSMOrESMComponent<(mozilla::xpcom::ComponentType)1>(nsTSubstring<char> const&, char const*, nsISupports**)
    (aURI=<optimized out>, aConstructor=<optimized out>, aResult=<optimized out>) at StaticComponents.cpp:2052
#32 mozilla::xpcom::ConstructESModuleComponent(nsTSubstring<char> const&, char const*, nsISupports**)
    (aURI="resource://gre/modules/MainProcessSingleton.sys.mjs", aConstructor=0x7f7843b2a93d "MainProcessSingleton", aResult=aResult@entry=0x7fff6007df98) at StaticComponents.cpp:2080
#33 0x00007f783df65e5b in mozilla::xpcom::CreateInstanceImpl(mozilla::xpcom::ModuleID, nsID const&, void**) (aID=<optimized out>, aIID=..., aResult=0x7fff6007dff0) at StaticComponents.cpp:9777
#34 0x00007f783df6d9f6 in (anonymous namespace)::EntryWrapper::CreateInstance(nsID const&, void**) (this=0x7fff6007e0e8, aIID=..., aResult=0x7fff6007dff0)
    at /home/matthew/unified/xpcom/components/nsComponentManager.cpp:184
#35 nsComponentManagerImpl::GetServiceLocked(mozilla::Maybe<mozilla::detail::BaseMonitorAutoLock<mozilla::Monitor> >&, (anonymous namespace)::EntryWrapper&, nsID const&, void**)
     (this=this@entry=0x7f78491a4c00, aLock=..., aEntry=..., aIID=..., aResult=aResult@entry=0x7fff6007e158) at /home/matthew/unified/xpcom/components/nsComponentManager.cpp:971
#36 0x00007f783df6e1a1 in nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**)
    (this=0x7f78491a4c00, aContractID=0x7fff6007e304 "@mozilla.org/main-process-singleton;1", aIID=..., aResult=aResult@entry=0x7fff6007e158) at /home/matthew/unified/xpcom/components/nsComponentManager.cpp:1160
#37 0x00007f783df6fc01 in CallGetService(char const*, nsID const&, void**) (aContractID=0x400 <error: Cannot access memory at address 0x400>, aIID=..., aResult=0x7fff6007e158)
    at /home/matthew/unified/xpcom/components/nsComponentManagerUtils.cpp:61
#38 nsGetServiceByContractID::operator()(nsID const&, void**) const (this=0x7f7843b0006d, aIID=..., aInstancePtr=0x7fff6007e158) at /home/matthew/unified/xpcom/components/nsComponentManagerUtils.cpp:240
#39 0x00007f783defd220 in nsCOMPtr_base::assign_from_gs_contractid(nsGetServiceByContractID, nsID const&) (this=0x7fff6007e1c8, aGS=..., aIID=...) at /home/matthew/unified/xpcom/base/nsCOMPtr.cpp:82
#40 0x00007f783df6b41f in nsCOMPtr<nsISupports>::nsCOMPtr(nsGetServiceByContractID) (this=0x7fff6007e1c8, aGS=..., aGS@entry=...) at /home/matthew/unified/xpcom/base/nsCOMPtr.h:1009
#41 NS_CreateServicesFromCategory(char const*, nsISupports*, char const*, char16_t const*)
    (aCategory=0x7f7844d18bbe "app-startup", aOrigin=aOrigin@entry=0x7f783b1bb670, aObserverTopic=0x7f7844d18bbe "app-startup", aObserverData=aObserverData@entry=0x0)
    at /home/matthew/unified/xpcom/components/nsCategoryManager.cpp:662
#42 0x00007f7841f3d5a1 in XREMain::XRE_mainRun() (this=this@entry=0x7fff6007e5c0) at /home/matthew/unified/toolkit/xre/nsAppRunner.cpp:5398
#43 0x00007f7841f3e3b2 in XREMain::XRE_main(int, char**, mozilla::BootstrapConfig const&) (this=this@entry=0x7fff6007e5c0, argc=argc@entry=6, argv=argv@entry=0x7fff6007f8b8, aConfig=...)
    at /home/matthew/unified/toolkit/xre/nsAppRunner.cpp:5859
#44 0x00007f7841f3e75b in XRE_main(int, char**, mozilla::BootstrapConfig const&) (argc=6, argv=0x7fff6007f8b8, aConfig=...) at /home/matthew/unified/toolkit/xre/nsAppRunner.cpp:5915
#45 0x000055cf00af431c in do_main(int, char**, char**) (argc=6, argv=0x7fff6007f8b8, envp=0x7fff6007f8f0) at /home/matthew/unified/browser/app/nsBrowserApp.cpp:227
#46 main(int, char**, char**) (argc=<optimized out>, argv=<optimized out>, envp=0x7fff6007f8f0) at /home/matthew/unified/browser/app/nsBrowserApp.cpp:445

So for some reason getSelfHostedScriptIndexRange is failing to find "ArrayToString" as a self hosted function, which is so unexpected we crash. The fact that this is a BackstagePass global feels relevant here, but I'm not quite sure yet what the fix is.

I can verify that the realm has the changeArrayByCopy creation option set...

Oh, and I can reproduce by running a regular speedometer run:

./mach raptor -t speedometer-desktop --browsertime-arg headless=true --page-cycles 1 --setenv MOZ_DISABLE_CONTENT_SANDBOX=1 

using the following mozconfig:

# Apparently required by jstestbrowser
ac_add_options --enable-js-shell

ac_add_options --enable-optimize
ac_add_options --disable-debug
ac_add_options --enable-warnings-as-errors
ac_add_options --enable-jitspew
ac_add_options --as-milestone=release
ac_add_options --without-wasm-sandboxed-libraries

# Dump opt builds into another dir.
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-opt-browser-as-release-@CONFIG_GUESS@

# For browser builds, because I do them infrequently, enable autoclobber
mk_add_options AUTOCLOBBER=1

With a debug build, I can see we fail an assertion:

0x00007f239d99768e in JSRuntime::getSelfHostedScriptIndexRange (this=0x7f238dd23000, name="ArrayToString") at /home/matthew/unified/js/src/vm/SelfHosting.cpp:2673
2673      MOZ_ASSERT(name->isPermanentAndMayBeShared());
(rr) print name
$1 = "ArrayToString"
(rr) print name->isPermanentAndMayBeShared()
$2 = false
(rr) 

This is definitely surprising...

So this diff fixes it:

diff --git a/js/src/builtin/Array.js b/js/src/builtin/Array.js
--- a/js/src/builtin/Array.js
+++ b/js/src/builtin/Array.js
@@ -407,6 +407,8 @@ function ArrayGroupToMap(callbackfn /*, 
   return map;
 }
 
+#endif
+
 /* ES5 15.4.4.21. */
 function ArrayReduce(callbackfn /*, initialValue*/) {
   /* Step 1. */
@@ -1489,8 +1491,6 @@ function ArrayToSorted(comparefn) {
   return sorted;
 }
 
-#endif
-
 // https://github.com/tc39/proposal-array-find-from-last
 // Array.prototype.findLast ( predicate, thisArg )
 function ArrayFindLast(predicate /*, thisArg*/) {

It's like a merge resolution happened wrong somewhere, but wasn't detectable from tests because it only occurred on release. Thank goodness for these -as-release benchmark runs.

I was able to reproduce this locally as well, without raptor; just building with the mozconfig in comment 9 and running:

MOZ_CRASHREPORTER_NO_REPORT="1" obj-opt-browser-as-release-x86_64-pc-linux-gnu/dist/bin/firefox  "-headless" 

Applying the diff fixed it. I'm surprised that mach try auto doesn't run any release builds.

Anyway, I've updated https://phabricator.services.mozilla.com/D174699 with that patch. Thanks for finding it!

Flags: needinfo?(tjc)
Pushed by mgaudet@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f49b182be8f3
Ship Change Array by Copy Feature r=mgaudet,devtools-reviewers,peterv
Pushed by ctuns@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/d4fbaaf73793
Backed out changeset f49b182be8f3 for causing conflicts with merge back, will reland.
https://hg.mozilla.org/integration/autoland/rev/4893955e0bdf
Ship Change Array by Copy Feature r=mgaudet,devtools-reviewers,peterv CLOSED TREE
Status: ASSIGNED → RESOLVED
Closed: 10 months ago
Resolution: --- → FIXED
Target Milestone: --- → 115 Branch

Release Note Request (optional, but appreciated)
[Why is this notable]: New Javascript Array features
[Affects Firefox for Android]: Yes
[Suggested wording]: Provides additional methods on Array.prototype and TypedArray.prototype to enable changes on the array by returning a new copy of it with the change.
[Links (documentation, blog post, etc)]: https://github.com/tc39/proposal-change-array-by-copy

relnote-firefox: --- → ?

Thanks, added to the Nightly release notes. Keeping the relnote? flag open to keep it on the radar for inclusion in our final release notes.

Regressions: 1835639
Duplicate of this bug: 1836292

MDN FF115 docs work for this can be tracked in https://github.com/mdn/content/issues/27175.
The new methods were already documented, so mostly this is an update to compatibility data and an MDN Firefox release note.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: