Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(298)

Issue 2132493002: [runtime] Fully remove RUNTIME_ASSERT for good. (Closed)

Created:
4 years, 5 months ago by Michael Starzinger
Modified:
4 years, 5 months ago
Reviewers:
mvstanton, Yang
CC:
v8-reviews_googlegroups.com
Base URL:
https://chromium.googlesource.com/v8/v8.git@local_cleanup-runtime-assert-robust
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

[runtime] Fully remove RUNTIME_ASSERT for good. This fully deprecates all uses of the RUNTIME_ASSERT macro and removes the macro and underlying logging function in question. All uses have been replaces with CHECK macros which crash safely even in production. It makes sure we discover abuse of runtime functions in the wild early and also abort the process safely. Breaking assumptions in any runtime function can no longer accidentally be caught by JavaScript. R=yangguo@chromium.org BUG=v8:5066 Committed: https://crrev.com/04062e92cccb7b9feb3b12a9b44f5c2c4e96bdae Cr-Commit-Position: refs/heads/master@{#37704}

Patch Set 1 #

Patch Set 2 : Fix. #

Patch Set 3 : Rebased. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+32 lines, -121 lines) Patch
M src/base/logging.h View 1 chunk +0 lines, -3 lines 0 comments Download
M src/base/logging.cc View 1 chunk +0 lines, -11 lines 0 comments Download
M src/debug/mirrors.js View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M src/runtime/runtime-strings.cc View 1 chunk +3 lines, -1 line 0 comments Download
M src/runtime/runtime-utils.h View 1 chunk +28 lines, -62 lines 0 comments Download
M test/mjsunit/es6/reflect-define-property.js View 1 1 chunk +0 lines, -22 lines 0 comments Download
M test/mjsunit/object-define-property.js View 1 1 chunk +0 lines, -22 lines 0 comments Download

Messages

Total messages: 13 (7 generated)
Michael Starzinger
4 years, 5 months ago (2016-07-07 11:00:55 UTC) #3
mvstanton
Whoa, fantastic!
4 years, 5 months ago (2016-07-07 11:05:20 UTC) #5
Yang
On 2016/07/07 11:05:20, mvstanton wrote: > Whoa, fantastic! lgtm!
4 years, 5 months ago (2016-07-07 12:49:37 UTC) #6
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2132493002/40001
4 years, 5 months ago (2016-07-13 08:29:31 UTC) #9
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years, 5 months ago (2016-07-13 08:33:28 UTC) #11
commit-bot: I haz the power
4 years, 5 months ago (2016-07-13 08:36:12 UTC) #13
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/04062e92cccb7b9feb3b12a9b44f5c2c4e96bdae
Cr-Commit-Position: refs/heads/master@{#37704}

Powered by Google App Engine
This is Rietveld 408576698