WebKit Bugzilla
New
Browse
Search+
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
136141
r171362
accidentally increased the size of InlineCallFrame
https://bugs.webkit.org/show_bug.cgi?id=136141
Summary
r171362 accidentally increased the size of InlineCallFrame
Mark Lam
Reported
2014-08-21 17:50:02 PDT
r171362
increased the size of InlineCallFrame::kind to 2 bits. This increased the size of InlineCallFrame from 72 to 80 though not intentionally. The fix is to reduce the size of InlineCallFrame::stackOffset to 29 bits.
Attachments
The patch
(3.39 KB, patch)
2014-08-21 17:54 PDT
,
Mark Lam
mark.lam
: review-
Details
Formatted Diff
Diff
patch 2: with correct limits
(3.41 KB, patch)
2014-08-21 17:58 PDT
,
Mark Lam
no flags
Details
Formatted Diff
Diff
patch 3: with better assertion.
(3.28 KB, patch)
2014-08-21 21:44 PDT
,
Mark Lam
no flags
Details
Formatted Diff
Diff
Show Obsolete
(2)
View All
Add attachment
proposed patch, testcase, etc.
Mark Lam
Comment 1
2014-08-21 17:54:34 PDT
Created
attachment 236948
[details]
The patch
Mark Lam
Comment 2
2014-08-21 17:57:10 PDT
Comment on
attachment 236948
[details]
The patch View in context:
https://bugs.webkit.org/attachment.cgi?id=236948&action=review
> Source/JavaScriptCore/bytecode/CodeOrigin.h:164 > + static const int maxStackOffset = (1 << 29) - 1; > + static const int minStackOffset = (~0 << 29);
Wrong limits.
Mark Lam
Comment 3
2014-08-21 17:58:21 PDT
Created
attachment 236949
[details]
patch 2: with correct limits
Filip Pizlo
Comment 4
2014-08-21 18:17:46 PDT
Comment on
attachment 236949
[details]
patch 2: with correct limits View in context:
https://bugs.webkit.org/attachment.cgi?id=236949&action=review
> Source/JavaScriptCore/bytecode/CodeOrigin.h:207 > + void setStackOffset(signed offset) > + { > + RELEASE_ASSERT(minStackOffset <= offset && offset <= maxStackOffset); > + stackOffset = offset; > + }
Why can't this just be: void setStackOffset(signed offset) { stackOffset = offset; RELEASE_ASSERT(static_cast<signed>(stackOffset) == offset); } Then you can get rid of the minStackOffset/maxStackOffset constants.
Mark Lam
Comment 5
2014-08-21 21:11:18 PDT
(In reply to
comment #4
)
> Why can't this just be: > > void setStackOffset(signed offset) > { > stackOffset = offset; > RELEASE_ASSERT(static_cast<signed>(stackOffset) == offset); > } > > Then you can get rid of the minStackOffset/maxStackOffset constants.
That is an excellent and superior solution. Will fix.
Mark Lam
Comment 6
2014-08-21 21:44:29 PDT
Created
attachment 236964
[details]
patch 3: with better assertion.
WebKit Commit Bot
Comment 7
2014-08-21 22:30:14 PDT
Comment on
attachment 236964
[details]
patch 3: with better assertion. Clearing flags on attachment: 236964 Committed
r172853
: <
http://trac.webkit.org/changeset/172853
>
WebKit Commit Bot
Comment 8
2014-08-21 22:30:17 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug