Summary: | Structure::create should call didBecomePrototype() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robin Morisset <rmorisset> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Component: | JavaScriptCore | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Severity: | Normal | CC: | cdumez, commit-queue, cz18811105578, darin, darkfloyd, ews-watchlist, fpizlo, keith_miller, kenwong20052006, mark.lam, msaboff, ryanhaddad, saam, sroberts, tsavell, tzagallo, webkit-bot-watchers-bugzilla, ysuzuki | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||||||||||||||||||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=199139 https://bugs.webkit.org/show_bug.cgi?id=199312 |
||||||||||||||||||||||||||||||||||||||||||||||||||||||
Bug Depends on: | 197334, 199179 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Bug Blocks: | |||||||||||||||||||||||||||||||||||||||||||||||||||||||
Attachments: |
|
Description
Robin Morisset
2019-03-27 13:48:58 PDT
Created attachment 366101 [details]
Patch
Comment on attachment 366101 [details]
Patch
Nice, r=me
Comment on attachment 366101 [details] Patch Attachment 366101 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11687701 Number of test failures exceeded the failure limit. Created attachment 366108 [details]
Archive of layout-test-results from ews116 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 366136 [details]
Patch
Added a missing exception check that was causing failures.
Also added fixed the wasm prototypes (not sure why they were not found by the prototypeVerifier, maybe they are not always allocated when there is no wasm code?)
Comment on attachment 366136 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366136&action=review > Source/JavaScriptCore/runtime/JSGlobalObject.cpp:2104 > + JSValue prototype = object->get(m_exec, m_vm.propertyNames->prototype); > + if (UNLIKELY(scope.exception())) > + scope.clearException(); This isn't what you want. You want JSObject::getPrototypeDirect. This is like asking for o.__proto__. Doing what you're doing is o.prototype Comment on attachment 366136 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366136&action=review > Source/JavaScriptCore/runtime/ObjectPrototype.cpp:72 > + didBecomePrototype(); this is called above. Comment on attachment 366136 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366136&action=review > Source/JavaScriptCore/runtime/JSGlobalObject.cpp:2135 > +#ifndef NDEBUG I prefer: #if !ASSERT_DISABLED Comment on attachment 366136 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366136&action=review > Source/JavaScriptCore/runtime/JSGlobalObject.cpp:2099 > + if (!isJSCellKind(kind) || !static_cast<JSCell*>(cell)->isObject()) > + return IterationStatus::Continue; I think we should also have a check here for Structure*. Something like: if (Structure* structure = jsDynamicCast<Structure*>(cell)) { if (structure->isMonoProto()) { if (JSObject* proto = structure->storedPrototypeObject()) RELEASE_ASSERT(proto->mayBePrototype()); } } Comment on attachment 366136 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366136&action=review >> Source/JavaScriptCore/runtime/JSGlobalObject.cpp:2135 >> +#ifndef NDEBUG > > I prefer: > #if !ASSERT_DISABLED I'd also just move this to init(). I guess there is also a world where we could call this before we GC. That way, we ensure we're always in a consistent state to some degree as we execute programs. Perhaps that's a bit overkill though. Created attachment 366853 [details]
Patch
Fixed the assert.
It found several more issues, including FunctionConstructor and several classes related to TypedArrays, that I also fixed.
Thanks to Saam for his help with this.
Comment on attachment 366853 [details] Patch Attachment 366853 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11786046 Number of test failures exceeded the failure limit. Created attachment 366861 [details]
Archive of layout-test-results from ews114 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 366853 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=366853&action=review r=me > Source/JavaScriptCore/runtime/InternalFunction.h:56 > + { revert > Source/JavaScriptCore/runtime/JSGlobalObject.h:1037 > + revert > Source/JavaScriptCore/runtime/JSObject.cpp:3912 > + revert > Source/JavaScriptCore/runtime/Structure.cpp:214 > + if (prototype.isCell()) > + RELEASE_ASSERT(prototype.getObject()->mayBePrototype()); There is more than one kind of structure constructor. Perhaps do it somewhere where all are covered? Probably in finishCreation. Probably also worth doing in Structure::changePrototypeTransition Thanks for the review. > somewhere where all are covered? Probably in finishCreation. The other two constructors respectively use a null prototype and copy the prototype of another struct. So I don't think covering them would bring anything. > Probably also worth doing in Structure::changePrototypeTransition Will do. (In reply to Robin Morisset from comment #16) > Thanks for the review. > > > somewhere where all are covered? Probably in finishCreation. > > The other two constructors respectively use a null prototype and copy the > prototype of another struct. So I don't think covering them would bring > anything. I agree that this is good enough for the state of the world today, but it's better to put these kinds of assertions in a place where we won't miss it when adding new constructors to Structure in the future. > > > Probably also worth doing in Structure::changePrototypeTransition > > Will do. Created attachment 367056 [details]
Patch
Applied Saam's suggestions, and fixed one more missing didBecomePrototype, found by EWS.
Waiting for EWS to confirm this was the last one before landing.
Created attachment 367057 [details]
Patch
Missing file and forgotten OOPS in changelog.
Attachment 367057 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/ChangeLog:7: Line contains tab character. [whitespace/tab] [5]
Total errors found: 1 in 36 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 367057 [details] Patch Attachment 367057 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11820335 Number of test failures exceeded the failure limit. Created attachment 367061 [details]
Archive of layout-test-results from ews116 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews116 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 367081 [details]
Patch
Yet another missing didBecomePrototype, this one in webcore bindings generated by a perl script.
Sending it for another round of EWS.
Comment on attachment 367081 [details] Patch Attachment 367081 [details] did not pass bindings-ews (mac): Output: https://webkit-queues.webkit.org/results/11823194 New failing tests: (JS) JSTestCallTracer.cpp (JS) JSTestCEReactions.cpp (JS) JSTestCEReactionsStringifier.cpp (JS) JSTestClassWithJSBuiltinConstructor.cpp (JS) JSTestActiveDOMObject.cpp (JS) JSTestDOMJIT.cpp (JS) JSTestEnabledBySetting.cpp (JS) JSTestEventConstructor.cpp (JS) JSTestEventTarget.cpp (JS) JSTestException.cpp (JS) JSTestGenerateIsReachable.cpp (JS) JSTestGlobalObject.h (JS) JSTestIndexedSetterNoIdentifier.cpp (JS) JSTestIndexedSetterThrowingException.cpp (JS) JSTestIndexedSetterWithIdentifier.cpp (JS) JSTestInterface.cpp (JS) JSTestInterfaceLeadingUnderscore.cpp (JS) JSTestIterable.cpp (JS) JSTestJSBuiltinConstructor.cpp (JS) JSMapLike.cpp (JS) JSTestMediaQueryListListener.cpp (JS) JSTestNamedAndIndexedSetterNoIdentifier.cpp (JS) JSTestNamedAndIndexedSetterThrowingException.cpp (JS) JSTestNamedAndIndexedSetterWithIdentifier.cpp (JS) JSTestNamedConstructor.cpp (JS) JSTestNamedDeleterNoIdentifier.cpp (JS) JSTestNamedDeleterThrowingException.cpp (JS) JSTestNamedDeleterWithIdentifier.cpp (JS) JSTestNamedDeleterWithIndexedGetter.cpp (JS) JSTestNamedGetterCallWith.cpp (JS) JSTestNamedGetterNoIdentifier.cpp (JS) JSTestNamedGetterWithIdentifier.cpp (JS) JSTestNamedSetterNoIdentifier.cpp (JS) JSTestNamedSetterThrowingException.cpp (JS) JSTestNamedSetterWithIdentifier.cpp (JS) JSTestNamedSetterWithIndexedGetter.cpp (JS) JSTestNamedSetterWithIndexedGetterAndSetter.cpp (JS) JSTestNamedSetterWithOverrideBuiltins.cpp (JS) JSTestNamedSetterWithUnforgableProperties.cpp (JS) JSTestNamedSetterWithUnforgablePropertiesAndOverrideBuiltins.cpp (JS) JSTestNode.cpp (JS) JSTestObj.cpp (JS) JSTestOverloadedConstructors.cpp (JS) JSTestOverloadedConstructorsWithSequence.cpp (JS) JSTestOverrideBuiltins.cpp (JS) JSTestPluginInterface.cpp (JS) JSTestPromiseRejectionEvent.cpp (JS) JSReadOnlyMapLike.cpp (JS) JSInterfaceName.cpp (JS) JSTestSerialization.cpp (JS) JSTestSerializationIndirectInheritance.cpp (JS) JSTestSerializationInherit.cpp (JS) JSTestSerializationInheritFinal.cpp (JS) JSTestSerializedScriptValueInterface.cpp (JS) JSTestStringifier.cpp (JS) JSTestStringifierAnonymousOperation.cpp (JS) JSTestStringifierNamedOperation.cpp (JS) JSTestStringifierOperationImplementedAs.cpp (JS) JSTestStringifierOperationNamedToString.cpp (JS) JSTestStringifierReadOnlyAttribute.cpp (JS) JSTestStringifierReadWriteAttribute.cpp (JS) JSTestTypedefs.cpp Created attachment 367089 [details]
Patch
Updating the binding tests that otherwise failed.
Comment on attachment 367089 [details] Patch Attachment 367089 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11824624 Number of test failures exceeded the failure limit. Created attachment 367097 [details]
Archive of layout-test-results from ews114 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews114 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 367140 [details]
Patch
Trying to fix yet another missing didBecomePrototype in code generated by CodeGenerator.pm, sending (again) to EWS for verification.
Created attachment 367363 [details]
Patch
Comment on attachment 367363 [details] Patch Attachment 367363 [details] did not pass bindings-ews (mac): Output: https://webkit-queues.webkit.org/results/11859081 New failing tests: (JS) JSTestDOMJIT.cpp (JS) JSTestEventConstructor.cpp (JS) JSTestEventTarget.cpp (JS) JSTestNode.cpp (JS) JSTestPromiseRejectionEvent.cpp (JS) JSTestSerializationIndirectInheritance.cpp (JS) JSTestSerializationInherit.cpp (JS) JSTestSerializationInheritFinal.cpp Comment on attachment 367363 [details] Patch Attachment 367363 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11859270 Number of test failures exceeded the failure limit. Created attachment 367369 [details]
Archive of layout-test-results from ews115 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-highsierra Platform: Mac OS X 10.13.6
Created attachment 367423 [details]
Patch
Fix another missing didBecomePrototype (this one in JSGlobalObject.cpp::createConsoleProperty), and update the bindings tests.
Sending for yet another round of EWS testing, maybe this time it will pass?
(In reply to Robin Morisset from comment #33) > Created attachment 367423 [details] > Patch > > Fix another missing didBecomePrototype (this one in > JSGlobalObject.cpp::createConsoleProperty), and update the bindings tests. > Sending for yet another round of EWS testing, maybe this time it will pass? Of note: every macOS Debug WK1 EWS bot that tried to process this patch got stuck and failed to continue testing. It may be worth trying to build this locally and run debug WK1 layout tests to see if it is reproducible. https://webkit-queues.webkit.org/patch/367423/mac-debug-ews (In reply to Ryan Haddad from comment #34) > (In reply to Robin Morisset from comment #33) > > Created attachment 367423 [details] > > Patch > > > > Fix another missing didBecomePrototype (this one in > > JSGlobalObject.cpp::createConsoleProperty), and update the bindings tests. > > Sending for yet another round of EWS testing, maybe this time it will pass? > Of note: every macOS Debug WK1 EWS bot that tried to process this patch got > stuck and failed to continue testing. It may be worth trying to build this > locally and run debug WK1 layout tests to see if it is reproducible. > > https://webkit-queues.webkit.org/patch/367423/mac-debug-ews Hi Robin, this is still happening. https://webkit-queues.webkit.org/queue-status/mac-debug-ews/bots/ews117 (ews112-117) (ews116 is offline, not related) The bots will continually stall after processing this patch. I am going to obsolete your patch so they can move forward. (In reply to Shawn Roberts from comment #35) > (In reply to Ryan Haddad from comment #34) > > (In reply to Robin Morisset from comment #33) > > > Created attachment 367423 [details] > > > Patch > > > > > > Fix another missing didBecomePrototype (this one in > > > JSGlobalObject.cpp::createConsoleProperty), and update the bindings tests. > > > Sending for yet another round of EWS testing, maybe this time it will pass? > > Of note: every macOS Debug WK1 EWS bot that tried to process this patch got > > stuck and failed to continue testing. It may be worth trying to build this > > locally and run debug WK1 layout tests to see if it is reproducible. > > > > https://webkit-queues.webkit.org/patch/367423/mac-debug-ews > > Hi Robin, this is still happening. > > https://webkit-queues.webkit.org/queue-status/mac-debug-ews/bots/ews117 > (ews112-117) (ews116 is offline, not related) > > The bots will continually stall after processing this patch. I am going to > obsolete your patch so they can move forward. Oh sorry, I thought the bots had automatically given up after failing to process this patch. I agree that it should be obsoleted until I find the time to test it locally and understand what is wrong with it. Created attachment 368352 [details]
Patch
I was able to build this patch without issues on my MacBook Pro, so I have no idea why the bots choked on it.
So I rebased it on ToT, and decided to try it again. I will watch the bots more carefully this time, and cancel/obsolete it if they break again.
Hopefully it was just a transient failure, or maybe the patch file itself got slightly corrupted.
(In reply to Robin Morisset from comment #37) > Created attachment 368352 [details] > Patch > > I was able to build this patch without issues on my MacBook Pro, so I have > no idea why the bots choked on it. > So I rebased it on ToT, and decided to try it again. I will watch the bots > more carefully this time, and cancel/obsolete it if they break again. > Hopefully it was just a transient failure, or maybe the patch file itself > got slightly corrupted. The issues we were having back then were actually unrelated to your patch. The team identified an issue that was causing EWS to hang after processing patches with failures. that issue is resolved now. Any errors you get will be actual, and we should not need to obsolete patches any longer. (In reply to Shawn Roberts from comment #38) > (In reply to Robin Morisset from comment #37) > > Created attachment 368352 [details] > > Patch > > > > I was able to build this patch without issues on my MacBook Pro, so I have > > no idea why the bots choked on it. > > So I rebased it on ToT, and decided to try it again. I will watch the bots > > more carefully this time, and cancel/obsolete it if they break again. > > Hopefully it was just a transient failure, or maybe the patch file itself > > got slightly corrupted. > > The issues we were having back then were actually unrelated to your patch. > The team identified an issue that was causing EWS to hang after processing > patches with failures. that issue is resolved now. Any errors you get will > be actual, and we should not need to obsolete patches any longer. Oh ok! I had been trying to debug the patch locally and could not (obviously, with hindsight) find anything wrong with it. Comment on attachment 368352 [details] Patch Clearing flags on attachment: 368352 Committed r244708: <https://trac.webkit.org/changeset/244708> All reviewed patches have been landed. Closing bug. Oops, the commit queue did not wait for the debug bot, and this patch seems to have broken the build in debug mode. Trying to fix it real fast, will ask for a roll-out if I can't fix it by the end of this afternoon. The specific error message, which happens for basically every test in LayoutTests is: dyld: Symbol not found: __ZN3JSC10DisallowGC19s_scopeReentryCountE Referenced from: /Users/rmorisset/Webkit/OpenSource/WebKitBuild/Debug/jsc Expected in: /System/Library/Frameworks/JavaScriptCore.framework/Versions/A/JavaScriptCore in /Users/rmorisset/Webkit/OpenSource/WebKitBuild/Debug/jsc This is quite surprising for two reasons: I have not touched that code at all, and JSC::DisallowGC::s_scopeReentryCount is already marked as JS_EXPORT_PRIVATE. Re-opened since this is blocked by bug 197334 *** Bug 196896 has been marked as a duplicate of this bug. *** (In reply to Robin Morisset from comment #43) > The specific error message, which happens for basically every test in > LayoutTests is: > dyld: Symbol not found: __ZN3JSC10DisallowGC19s_scopeReentryCountE > Referenced from: /Users/rmorisset/Webkit/OpenSource/WebKitBuild/Debug/jsc > Expected in: > /System/Library/Frameworks/JavaScriptCore.framework/Versions/A/JavaScriptCore > in /Users/rmorisset/Webkit/OpenSource/WebKitBuild/Debug/jsc > > This is quite surprising for two reasons: I have not touched that code at > all, and JSC::DisallowGC::s_scopeReentryCount is already marked as > JS_EXPORT_PRIVATE. That’s a symbol missing in the *system* copy of JavaScriptCore, not the just-built version. The issue here is that the jsc tool is loading the system copy of the JavaScriptCore framework, not the one that was built alongside it. It’s not surprising that it needs to run the correct version since it uses internals and inline functions from the framework, and doesn’t use public API or SPI. And also, for testing, clearly we want to run the framework we are trying to test, not a different one. The question is how this is supposed to work. What is supposed to make jsc load the correct version of JavaScriptCore? Perhaps DYLD_FRAMEOWRK_PATH environment variable is supposed to be set? This is what needs to be investigated. Created attachment 368983 [details]
Patch
I was able to fix my local problem, it was just a missing DYLD_FRAMEWORK (run-jsc automatically does it, but visibly not run-layout-jsc).
However, I still cannot reproduce the EWS failures, so sending it again for a full round of EWS, maybe this time I will get some useful error messages.
Comment on attachment 368983 [details] Patch Attachment 368983 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12093558 New failing tests: legacy-animation-engine/fast/layers/no-clipping-overflow-hidden-hardware-acceleration.html http/tests/css/filters-on-iframes.html Created attachment 369025 [details]
Archive of layout-test-results from ews212 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews212 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
Comment on attachment 368983 [details]
Patch
The test failure on windows also happens without my patch, so let's try landing it.
Comment on attachment 368983 [details] Patch Clearing flags on attachment: 368983 Committed r245031: <https://trac.webkit.org/changeset/245031> All reviewed patches have been landed. Closing bug. I don’t understand. The patch that landed is just change logs, no code change? Comment on attachment 368983 [details]
Patch
This patch is only change logs, no code change.
(In reply to Darin Adler from comment #53) > I don’t understand. The patch that landed is just change logs, no code > change? Oops, thank you very much for catching this. I obviously made some mistake when sending this patch, and forgot to check it before landing it. Created attachment 369414 [details]
Patch for landing
This time with the actual contents
The commit-queue encountered the following flaky tests while processing attachment 369414 [details]: fetch/fetch-worker-crash.html bug 187257 (author: youennf@gmail.com) The commit-queue is continuing to process your patch. Comment on attachment 369414 [details] Patch for landing Clearing flags on attachment: 369414 Committed r245068: <https://trac.webkit.org/changeset/245068> All reviewed patches have been landed. Closing bug. Layout tests are exiting early on macOS and iOS debug bots due to the following assertion failure: ASSERTION FAILED: m_prototype.get().isEmpty() || isValidPrototype(m_prototype.get()) /Volumes/Data/slave/mojave-debug/build/WebKitBuild/Debug/JavaScriptCore.framework/PrivateHeaders/Structure.h(145) : void JSC::Structure::finishCreation(JSC::VM &) 1 0x211182679 WTFCrash 2 0x1f800728b WTFCrashWithInfo(int, char const*, char const*, int) 3 0x1f81fe6d9 JSC::Structure::finishCreation(JSC::VM&) 4 0x1f81fe4c9 JSC::Structure::create(JSC::VM&, JSC::JSGlobalObject*, JSC::JSValue, JSC::TypeInfo const&, JSC::ClassInfo const*, unsigned char, unsigned int) 5 0x1f94e831a WebCore::JSDOMIterator<WebCore::JSURLSearchParams, WebCore::URLSearchParamsIteratorTraits>::createStructure(JSC::VM&, JSC::JSGlobalObject*, JSC::JSValue) 6 0x1f94e7d5a JSC::Structure* WebCore::getDOMStructure<WebCore::JSDOMIterator<WebCore::JSURLSearchParams, WebCore::URLSearchParamsIteratorTraits> >(JSC::VM&, WebCore::JSDOMGlobalObject&) 7 0x1f94e7c25 JSC::JSValue WebCore::iteratorCreate<WebCore::JSDOMIterator<WebCore::JSURLSearchParams, WebCore::URLSearchParamsIteratorTraits> >(WebCore::JSDOMIterator<WebCore::JSURLSearchParams, WebCore::URLSearchParamsIteratorTraits>::Wrapper&, WebCore::IterationKind) 8 0x1f94e7b62 WebCore::jsURLSearchParamsPrototypeFunctionEntriesCaller(JSC::ExecState*, WebCore::JSURLSearchParams*, JSC::ThrowScope&) 9 0x1f94d4b50 long long WebCore::IDLOperation<WebCore::JSURLSearchParams>::call<&(WebCore::jsURLSearchParamsPrototypeFunctionEntriesCaller(JSC::ExecState*, WebCore::JSURLSearchParams*, JSC::ThrowScope&)), (WebCore::CastedThisErrorBehavior)0>(JSC::ExecState&, char const*) 10 0x1f94d483c WebCore::jsURLSearchParamsPrototypeFunctionEntries(JSC::ExecState*) 11 0x2f729f60116b 12 0x2116a03e4 llint_entry 13 0x21168d263 vmEntryToJavaScript 14 0x2123190b7 JSC::JITCode::execute(JSC::VM*, JSC::ProtoCallFrame*) 15 0x212318690 JSC::Interpreter::executeProgram(JSC::SourceCode const&, JSC::ExecState*, JSC::JSObject*) 16 0x212645ee5 JSC::evaluate(JSC::ExecState*, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) 17 0x2126460a1 JSC::profiledEvaluate(JSC::ExecState*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) 18 0x1fa22efe8 WebCore::JSExecState::profiledEvaluate(JSC::ExecState*, JSC::ProfilingReason, JSC::SourceCode const&, JSC::JSValue, WTF::NakedPtr<JSC::Exception>&) 19 0x1fa22ed06 WebCore::ScriptController::evaluateInWorld(WebCore::ScriptSourceCode const&, WebCore::DOMWrapperWorld&, WebCore::ExceptionDetails*) 20 0x1fa22f0bd WebCore::ScriptController::evaluate(WebCore::ScriptSourceCode const&, WebCore::ExceptionDetails*) 21 0x1fa8dab21 WebCore::ScriptElement::executeClassicScript(WebCore::ScriptSourceCode const&) 22 0x1fa8d8f88 WebCore::ScriptElement::prepareScript(WTF::TextPosition const&, WebCore::ScriptElement::LegacyTypeSupport) 23 0x1fad7ef67 WebCore::HTMLScriptRunner::runScript(WebCore::ScriptElement&, WTF::TextPosition const&) 24 0x1fad7ed8f WebCore::HTMLScriptRunner::execute(WTF::Ref<WebCore::ScriptElement, WTF::DumbPtrTraits<WebCore::ScriptElement> >&&, WTF::TextPosition const&) 25 0x1fad5bbdb WebCore::HTMLDocumentParser::runScriptsForPausedTreeBuilder() 26 0x1fad5c03d WebCore::HTMLDocumentParser::pumpTokenizerLoop(WebCore::HTMLDocumentParser::SynchronousMode, bool, WebCore::PumpSession&) 27 0x1fad5b3f4 WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) 28 0x1fad5ad7d WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode) 29 0x1fad5d452 WebCore::HTMLDocumentParser::resumeParsingAfterScriptExecution() 30 0x1fad5d83d WebCore::HTMLDocumentParser::notifyFinished(WebCore::PendingScript&) 31 0x1fa8b7427 WebCore::PendingScript::notifyClientFinished() LEAK: 2 WebPageProxy https://build.webkit.org/results/Apple%20Mojave%20Debug%20WK2%20(Tests)/r245068%20(2551)/results.html (In reply to Ryan Haddad from comment #60) > Layout tests are exiting early on macOS and iOS debug bots due to the > following assertion failure: > > ASSERTION FAILED: m_prototype.get().isEmpty() || > isValidPrototype(m_prototype.get()) > /Volumes/Data/slave/mojave-debug/build/WebKitBuild/Debug/JavaScriptCore. > framework/PrivateHeaders/Structure.h(145) : void > JSC::Structure::finishCreation(JSC::VM &) It looks like Mac-debug EWS was seeing crashes before this patch was landed. I will go ahead and roll it out to unblock EWS and trunk debug bots. Reverted r245068 for reason: Caused debug layout tests to exit early due to an assertion failure. Committed r245082: <https://trac.webkit.org/changeset/245082> Created attachment 369598 [details]
Patch
Trying again to get EWS to test it, this time not forgetting JSDOMIteratorPrototype.
Comment on attachment 369598 [details]
Patch
I visibly got the Changelog wrong, so even if EWS is happy with this patch I will need to fix that before landing it.
Comment on attachment 369598 [details] Patch Attachment 369598 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/12157027 New failing tests: fast/dom/Window/window-function-name-getter-precedence.html http/tests/security/cross-frame-access-frames.html fast/dom/Window/es52-globals.html http/tests/security/isolatedWorld/all-window-prototypes.html http/tests/security/cross-frame-access-get-override.html http/tests/security/cross-frame-access-getOwnPropertyDescriptor.html http/tests/security/cross-frame-access-name-getter.html http/tests/security/cross-frame-access-custom.html js/dom/activation-proto.html http/tests/security/cross-frame-access-first-time.html fast/loader/window-clearing.html js/dom/Object-defineProperty.html fast/dom/Window/window-function-frame-getter-precedence.html http/tests/security/cross-frame-access-call.html http/tests/security/cross-frame-access-get.html Created attachment 369622 [details]
Archive of layout-test-results from ews115 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 369598 [details] Patch Attachment 369598 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/12160045 New failing tests: svg/dom/viewspec-parser-1.html svg/dom/SVGViewSpec-defaults.html svg/dom/viewspec-parser-5.html svg/dom/viewspec-parser-7.html svg/dom/viewspec-parser-2.html svg/dom/viewspec-parser-4.html svg/dom/viewspec-parser-3.html svg/dom/viewspec-parser-6.html svg/custom/js-svg-constructors.svg svg/dom/SVGViewSpec.html Created attachment 369640 [details]
Archive of layout-test-results from ews214 for win-future
The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews214 Port: win-future Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit
*** Bug 198259 has been marked as a duplicate of this bug. *** Start rebaselining. I think JSGlobalObject's reset prototype things are missing (maybe). If reopened, may I ask whether the previous patch is not completely resolving the bug? JSDOMWindowProperties should be a prototype. Add ASSERT to Structure::setPrototypeWithoutTransition. Created attachment 372674 [details]
Patch
For EWS
Created attachment 372675 [details]
Patch
For EWS
Committed r246714: <https://trac.webkit.org/changeset/246714> It looks like the changes in https://trac.webkit.org/changeset/246714/webkit Broke 7 API tests on Mac debug. Build: https://build.webkit.org/builders/Apple%20Mojave%20Debug%20WK2%20%28Tests%29/builds/3224 I was able to reproduce this locally, the 7 API tests fail on 246714 but not 246713 (In reply to Truitt Savell from comment #78) > It looks like the changes in https://trac.webkit.org/changeset/246714/webkit > Broke 7 API tests on Mac debug. > > Build: > https://build.webkit.org/builders/Apple%20Mojave%20Debug%20WK2%20%28Tests%29/ > builds/3224 > > I was able to reproduce this locally, the 7 API tests fail on 246714 but not > 246713 Thanks! I'll land a follow-up patch. Yusuke, Saam and I talked about this offline and we came to the conclusion we should set the didBecomePrototype bit in Structure::create instead. I'll revert this change for now. Re-opened since this is blocked by bug 199179 *** Bug 199139 has been marked as a duplicate of this bug. *** Created attachment 372851 [details]
Patch
Comment on attachment 372851 [details] Patch Clearing flags on attachment: 372851 Committed r246801: <https://trac.webkit.org/changeset/246801> All reviewed patches have been landed. Closing bug. Debug bug fixes in: https://bugs.webkit.org/show_bug.cgi?id=199202. *** Bug 197557 has been marked as a duplicate of this bug. *** *** Bug 145763 has been marked as a duplicate of this bug. *** *** Bug 196677 has been marked as a duplicate of this bug. *** |