Summary: | WebContent crash loading page on seas.upenn.edu @ JavaScriptCore: vmEntryToJavaScript | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Michael Saboff <msaboff> | ||||||
Component: | JavaScriptCore | Assignee: | Michael Saboff <msaboff> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | ews-watchlist, keith_miller, mark.lam, rniwa, saam, webkit-bug-importer | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=185554 | ||||||||
Attachments: |
|
Description
Michael Saboff
2018-05-02 18:10:41 PDT
Created attachment 339423 [details]
Patch
Attachment 339423 [details] did not pass style-queue:
ERROR: Source/JavaScriptCore/b3/air/testair.cpp:1920: Consider using CHECK_EQ instead of CHECK(a == b) [readability/check] [2]
Total errors found: 1 in 5 files
If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 339423 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=339423&action=review r=me > Source/JavaScriptCore/ChangeLog:10 > + We weren't clearing the scratch register cache when switching back and forth between > + allowing scratch register usage. Now we clear when we transition from disallow to > + allow. It's worth explaining why this is a problem. > Source/JavaScriptCore/assembler/AllowMacroScratchRegisterUsageIf.h:45 > +#if CPU(ARM64) > + if (!m_oldValueOfAllowScratchRegister) > + m_masm.invalidateAllTempRegisters(); > +#endif Shouldn't this invalidation happen on any transition? (In reply to Saam Barati from comment #4) > Comment on attachment 339423 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=339423&action=review > > r=me > > > Source/JavaScriptCore/ChangeLog:10 > > + We weren't clearing the scratch register cache when switching back and forth between > > + allowing scratch register usage. Now we clear when we transition from disallow to > > + allow. > > It's worth explaining why this is a problem. > > > Source/JavaScriptCore/assembler/AllowMacroScratchRegisterUsageIf.h:45 > > +#if CPU(ARM64) > > + if (!m_oldValueOfAllowScratchRegister) > > + m_masm.invalidateAllTempRegisters(); > > +#endif > > Shouldn't this invalidation happen on any transition? Updated the ChangeLog to We weren't clearing the scratch register cache when switching back and forth between allowing scratch register usage. We disallow scratch register usage when we are in code that will freely allocate and use any register. Such usage can change the contents of scratch registers. For ARM64, where we cache the contents of scratch registers to reuse some or all of the contained values, we need to invalidate these caches. We do this when re-enabling scratch register usage, that is when we transition from disallow to allow scratch register usage. Comment on attachment 339423 [details] Patch Attachment 339423 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/7552870 New failing tests: fast/mediastream/delayed-permission-allowed.html Created attachment 339434 [details]
Archive of layout-test-results from ews104 for mac-sierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Committed r231317: <https://trac.webkit.org/changeset/231317> |