Bug 150532 - 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
Summary: r190735 Caused us to maybe trample the base's tag-GPR on 32-bit inline cache ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-10-24 12:21 PDT by Saam Barati
Modified: 2015-10-26 12:49 PDT (History)
3 users (show)

See Also:


Attachments
patch (4.94 KB, patch)
2015-10-25 15:47 PDT, Saam Barati
ggaren: review+
Details | Formatted Diff | Diff
patch for landing (6.43 KB, patch)
2015-10-26 12:46 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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