1. On an iOS device, load https://www.seas.upenn.edu/~cis194/fall16/hw/01-intro.html. 2. Zoom in & out and scroll up & down randomly. Should crash fairly easily. The issue is that we are not invalidating the dataMemoryTemp and cachedMemoryTemp registers when we transition masm.m_allowScratchRegister. These transitions happen in the classes AllowMacroScratchRegisterUsage, AllowMacroScratchRegisterUsageIf and DisallowMacroScratchRegisterUsage.
<rdar://problem/39066024>
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>