Chromium Code Reviews
|
|
Chromium Code Reviews|
Created:
4 years, 10 months ago by mmoroz Modified:
4 years, 10 months ago Reviewers:
tkent, Oliver Chang, Martin Barbella, inferno CC:
chromium-reviews, blink-reviews, blink-reviews-wtf_chromium.org, Mikhail Base URL:
https://chromium.googlesource.com/chromium/src.git@master Target Ref:
refs/pending/heads/master Project:
chromium Visibility:
Public. |
DescriptionChange assert to release assert for WTF::double_conversion::Vector to prevent OOB memory access.
R=inferno@chromium.org, mbarbella@chromium.org, ochang@chromium.org, tkent@chromium.org
BUG=574802
Committed: https://crrev.com/ad618c3357e189fb685ea88fe4b8b25ac34f2c75
Cr-Commit-Position: refs/heads/master@{#374424}
Patch Set 1 #
Total comments: 2
Patch Set 2 : fix comment #
Created: 4 years, 10 months ago
Messages
Total messages: 15 (3 generated)
lgtm
Can we remove this Vector and use WTF::Vector?
On 2016/02/09 01:31:13, tkent wrote: > Can we remove this Vector and use WTF::Vector? As I see, this light-weight Vector implementation is used only inside double_conversion namespace. May be it is the reason why it was originally implemented - to do not use much complex object with unnecessary features.
anyway, this is trivial. lgtm
lgtm https://codereview.chromium.org/1677363002/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/wtf/dtoa/utils.h (right): https://codereview.chromium.org/1677363002/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/wtf/dtoa/utils.h:166: // Access individual vector elements - checks bounds in debug mode. Please fix the comment.
Thanks! https://codereview.chromium.org/1677363002/diff/1/third_party/WebKit/Source/w... File third_party/WebKit/Source/wtf/dtoa/utils.h (right): https://codereview.chromium.org/1677363002/diff/1/third_party/WebKit/Source/w... third_party/WebKit/Source/wtf/dtoa/utils.h:166: // Access individual vector elements - checks bounds in debug mode. On 2016/02/09 04:30:12, inferno wrote: > Please fix the comment. Done.
The CQ bit was checked by mmoroz@chromium.org
The patchset sent to the CQ was uploaded after l-g-t-m from tkent@chromium.org, ochang@chromium.org, inferno@chromium.org Link to the patchset: https://codereview.chromium.org/1677363002/#ps20001 (title: "fix comment")
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1677363002/20001 View timeline at https://chromium-cq-status.appspot.com/patch-timeline/1677363002/20001
Message was sent while issue was closed.
Committed patchset #2 (id:20001)
Message was sent while issue was closed.
Description was changed from ========== Change assert to release assert for WTF::double_conversion::Vector to prevent OOB memory access. R=inferno@chromium.org, mbarbella@chromium.org, ochang@chromium.org, tkent@chromium.org BUG=574802 ========== to ========== Change assert to release assert for WTF::double_conversion::Vector to prevent OOB memory access. R=inferno@chromium.org, mbarbella@chromium.org, ochang@chromium.org, tkent@chromium.org BUG=574802 Committed: https://crrev.com/ad618c3357e189fb685ea88fe4b8b25ac34f2c75 Cr-Commit-Position: refs/heads/master@{#374424} ==========
Message was sent while issue was closed.
Patchset 2 (id:??) landed as https://crrev.com/ad618c3357e189fb685ea88fe4b8b25ac34f2c75 Cr-Commit-Position: refs/heads/master@{#374424}
Message was sent while issue was closed.
On 2016/02/09 18:51:42, commit-bot: I haz the power wrote: > Patchset 2 (id:??) landed as > https://crrev.com/ad618c3357e189fb685ea88fe4b8b25ac34f2c75 > Cr-Commit-Position: refs/heads/master@{#374424} Link to performance dashboard (is empty right now): https://chromeperf.appspot.com/group_report?rev=374424
Message was sent while issue was closed.
A revert of this CL (patchset #2 id:20001) has been created in https://codereview.chromium.org/1694093002/ by inferno@chromium.org. The reason for reverting is: Perf failures. Speculative revert to see if it fixes. BUG=586581,574802. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
