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

Issue 975443003: Catch corrupted pointers earlier. (Closed)

Created:
5 years, 9 months ago by koda
Modified:
5 years, 9 months ago
Reviewers:
Ivan Posva
CC:
reviews_dartlang.org, vm-dev_dartlang.org
Visibility:
Public.

Description

Catch corrupted pointers earlier. Lack of pointer validation made issues 22549 and 22621 more difficult to track down. Before this change, ScavengePointer would ignore any sufficiently large garbage values, even if they were obviously not well-formed pointers. Remove obsolete unit test with "foreign" pointers; the zero-size scavenger case is already thoroughly exercised by the VM isolate. BUG=22621 R=iposva@google.com Committed: https://code.google.com/p/dart/source/detail?r=44147

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Total comments: 2

Patch Set 4 : #

Patch Set 5 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -12 lines) Patch
M runtime/vm/disassembler_ia32.cc View 1 2 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/disassembler_x64.cc View 1 2 3 4 1 chunk +1 line, -0 lines 0 comments Download
M runtime/vm/raw_object.h View 2 chunks +7 lines, -0 lines 0 comments Download
M runtime/vm/scavenger.cc View 1 2 3 2 chunks +1 line, -4 lines 0 comments Download
M runtime/vm/scavenger_test.cc View 1 2 1 chunk +0 lines, -8 lines 0 comments Download

Messages

Total messages: 6 (0 generated)
koda
5 years, 9 months ago (2015-03-02 16:28:41 UTC) #1
koda
This is actually triggering some unit test failures; looking into whether they are actual bugs...
5 years, 9 months ago (2015-03-02 17:04:13 UTC) #2
koda
On 2015/03/02 17:04:13, koda wrote: > This is actually triggering some unit test failures; looking ...
5 years, 9 months ago (2015-03-02 18:31:13 UTC) #3
Ivan Posva
LGTM -ip https://codereview.chromium.org/975443003/diff/40001/runtime/vm/scavenger.cc File runtime/vm/scavenger.cc (right): https://codereview.chromium.org/975443003/diff/40001/runtime/vm/scavenger.cc#newcode159 runtime/vm/scavenger.cc:159: // TODO(iposva): Add an appropriate assert here ...
5 years, 9 months ago (2015-03-02 18:38:48 UTC) #4
koda
https://codereview.chromium.org/975443003/diff/40001/runtime/vm/scavenger.cc File runtime/vm/scavenger.cc (right): https://codereview.chromium.org/975443003/diff/40001/runtime/vm/scavenger.cc#newcode159 runtime/vm/scavenger.cc:159: // TODO(iposva): Add an appropriate assert here or in ...
5 years, 9 months ago (2015-03-02 19:06:26 UTC) #5
koda
5 years, 9 months ago (2015-03-02 20:40:39 UTC) #6
Message was sent while issue was closed.
Committed patchset #5 (id:80001) manually as r44147 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698