Bug 150532

Summary: r190735 Caused us to maybe trample the base's tag-GPR on 32-bit inline cache when the cache allocates a scratch register and then jumps to the slow path
Product: WebKit Reporter: Saam Barati <saam>
Component: JavaScriptCoreAssignee: Saam Barati <saam>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, dewei_zhu, ggaren
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
patch
ggaren: review+
patch for landing none

Description Saam Barati 2015-10-24 12:21:27 PDT
This register used to show up in the used register set because of
how the DFG kept track of used register. I changed this in my
work on online caching because we don't want to spill these registers
when we have a GetByIdFlush/PutByIdFlush and we use the used register set
as the metric of how to spill. That said, these registers should be locked
and not used as scratch registers by the scratch register allocator. The
reason is that our inline cache may fail and jump to the slow path. The slow
path then uses the base's tag register. If the inline cache used the base's tag
register as a scratch and it fails and jumps to the slow path, we have a problem.
The most obvious solution is to just make StructureStubInfo aware of the base's tag
register so that it can lock it when allocating a scratch.

Note that this doesn't mean that we can trample this register when making a call.
We can totally trample this as long as the inline cache succeeds. The problem is 
only when we trample it and then jump to the slow path
Comment 1 Saam Barati 2015-10-25 15:47:02 PDT
Created attachment 264026 [details]
patch
Comment 2 Geoffrey Garen 2015-10-25 18:06:50 PDT
Comment on attachment 264026 [details]
patch

r=me
Comment 3 Geoffrey Garen 2015-10-25 18:15:10 PDT
...Can has regression test?
Comment 4 Saam Barati 2015-10-25 22:16:12 PDT
(In reply to comment #3)
> ...Can has regression test?
Yeah. 
I was thinking I should create one.
I'll write one tomorrow before landing.
Comment 5 Saam Barati 2015-10-26 12:46:18 PDT
Created attachment 264064 [details]
patch for landing
Comment 6 Saam Barati 2015-10-26 12:49:37 PDT
landed in:
http://trac.webkit.org/changeset/191594