Fixed zapping of contents

Fixed zapping of contents

BUG=

Committed: https://crrev.com/ae18e6cd8ef6be9e765aa3d8bc97d1b875983703
Cr-Commit-Position: refs/heads/master@{#39946}

[heimbuef, 2016-09-28 07:55:10]

Patchset #1 (id:1) has been deleted

[heimbuef, 2016-09-28 07:56:27]

heimbuef@google.com changed reviewers:
+ michael l@chromium.org, verwaest@chromium.org

[heimbuef, 2016-09-28 07:56:28]

PTAL

[heimbuef, 2016-09-28 08:01:50]

On 2016/09/28 at 07:56:28, heimbuef wrote:
> PTAL

Missing file, can't update the cl right now ... error in depot tools

[heimbuef, 2016-09-28 08:19:44]

heimbuef@google.com changed reviewers:
+ lippautz@chromium.org
- michael l@chromium.org

[heimbuef, 2016-09-28 08:19:59]

heimbuef@google.com changed reviewers:
+ mlippautz@chromium.org
- lippautz@chromium.org

[heimbuef, 2016-09-28 08:21:06]

On 2016/09/28 at 08:01:50, heimbuef wrote:
> On 2016/09/28 at 07:56:28, heimbuef wrote:
> > PTAL
> 
> Missing file, can't update the cl right now ... error in depot tools

Added.

[heimbuef, 2016-09-28 08:21:18]

Patchset #1 (id:20001) has been deleted

[Toon Verwaest, 2016-09-28 08:35:01]

https://codereview.chromium.org/2377943003/diff/40001/src/zone/accounting-all...
File src/zone/accounting-allocator.cc (right):

https://codereview.chromium.org/2377943003/diff/40001/src/zone/accounting-all...
src/zone/accounting-allocator.cc:32: memory->ZapHeader();
If you anyway need to zap something here, why not just zap everything here?

[heimbuef, 2016-09-28 08:47:13]

On 2016/09/28 at 08:35:01, verwaest wrote:
>
https://codereview.chromium.org/2377943003/diff/40001/src/zone/accounting-all...
> File src/zone/accounting-allocator.cc (right):
> 
>
https://codereview.chromium.org/2377943003/diff/40001/src/zone/accounting-all...
> src/zone/accounting-allocator.cc:32: memory->ZapHeader();
> If you anyway need to zap something here, why not just zap everything here?

Are you worried about debug performance?

[Michael Lippautz, 2016-09-28 09:02:28]

fyi: just patched this into yesterdays chromium and I still get the spurious
values. checking right now why this is the case.

[Michael Lippautz, 2016-09-28 09:22:39]

https://codereview.chromium.org/2377943003/diff/40001/src/zone/zone.cc
File src/zone/zone.cc (right):

https://codereview.chromium.org/2377943003/diff/40001/src/zone/zone.cc#newcod...
src/zone/zone.cc:156: memset(segment_head_, kZapDeadByte, size);
I guess this should also be the ZapContents?

[heimbuef, 2016-09-28 09:38:28]

On 2016/09/28 at 09:22:39, mlippautz wrote:
> https://codereview.chromium.org/2377943003/diff/40001/src/zone/zone.cc
> File src/zone/zone.cc (right):
> 
>
https://codereview.chromium.org/2377943003/diff/40001/src/zone/zone.cc#newcod...
> src/zone/zone.cc:156: memset(segment_head_, kZapDeadByte, size);
> I guess this should also be the ZapContents?

Fixed it.

[Michael Lippautz, 2016-09-29 09:44:48]

Deferring the actual review to Toon, since I haven't been much here lately.

https://codereview.chromium.org/2377943003/diff/60001/src/zone/zone.cc
File src/zone/zone.cc (right):

https://codereview.chromium.org/2377943003/diff/60001/src/zone/zone.cc#newcod...
src/zone/zone.cc:127: ASAN_UNPOISON_MEMORY_REGION(start, keep->capacity());
All other unpoison operations are within #ifdef DEBUG.

[Toon Verwaest, 2016-09-29 12:55:15]

I still don't understand why you don't just zap the entire thing when freeing
the segment; and why you do part before you call it, and part when you are in
it. What's the advantage of the added complexity?

[Toon Verwaest, 2016-09-30 10:44:43]

lgtm

[heimbuef, 2016-10-04 08:08:33]

The CQ bit was checked by heimbuef@google.com

[commit-bot: I haz the power, 2016-10-04 08:08:45]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-10-04 08:47:24]

Committed patchset #2 (id:60001)

[commit-bot: I haz the power, 2016-10-04 08:47:43]

Description was changed from

==========
Fixed zapping of contents

BUG=
==========

to

==========
Fixed zapping of contents

BUG=

Committed: https://crrev.com/ae18e6cd8ef6be9e765aa3d8bc97d1b875983703
Cr-Commit-Position: refs/heads/master@{#39946}
==========

[commit-bot: I haz the power, 2016-10-04 08:47:44]

Patchset 2 (id:??) landed as
https://crrev.com/ae18e6cd8ef6be9e765aa3d8bc97d1b875983703
Cr-Commit-Position: refs/heads/master@{#39946}