New issue
Advanced search Search tips

Issue 896019 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 3
Type: Task

Blocked on:
issue 920440
issue crashpad:262
issue 909720
issue 912500
issue 917804
issue 919207



Sign in to add a comment

Implement GWP-ASan for Windows

Project Member Reported by vtsyrklevich@google.com, Oct 16

Issue description

Tracking ticket for work required to deploy GWP-ASan on Windows.
 
Blockedon: crashpad:262
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 29

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/53fd12919924e80e64b50c984f25eb5de43005da

commit 53fd12919924e80e64b50c984f25eb5de43005da
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Mon Oct 29 20:59:43 2018

GWP-ASan: Add debug allocator

This allocator is adapted from the GuardedPageAllocator in tcmalloc.
The allocator is intended to be a 'debug' allocator to help catch memory
errors like buffer overflows and use-after-frees. It does so by giving
allocations an entire page and left- or right- aligning them between
guard pages (to catch both underflows and overflows). It also sets pages
inaccessible once they have been freed to detect use-after-frees and can
also detect double frees.

Forthcoming changes add an allocator shim to sample allocations to this
allocator and to add additional debugging context when crashes due to
GWP-ASan occur.

GWP-ASan is going to be launched on Windows first; however, if it’s
successful it will be ported to other platforms (and therefore also be
used by different embedders.)

Bug: 896019
Change-Id: I358e1b6f91569d1d448fc7ee16d5df27ed2467aa
Reviewed-on: https://chromium-review.googlesource.com/c/1284699
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Cait Phillips <caitkp@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#603616}
[modify] https://crrev.com/53fd12919924e80e64b50c984f25eb5de43005da/components/BUILD.gn
[add] https://crrev.com/53fd12919924e80e64b50c984f25eb5de43005da/components/gwp_asan/OWNERS
[add] https://crrev.com/53fd12919924e80e64b50c984f25eb5de43005da/components/gwp_asan/README
[add] https://crrev.com/53fd12919924e80e64b50c984f25eb5de43005da/components/gwp_asan/common/BUILD.gn
[add] https://crrev.com/53fd12919924e80e64b50c984f25eb5de43005da/components/gwp_asan/common/guarded_page_allocator.cc
[add] https://crrev.com/53fd12919924e80e64b50c984f25eb5de43005da/components/gwp_asan/common/guarded_page_allocator.h
[add] https://crrev.com/53fd12919924e80e64b50c984f25eb5de43005da/components/gwp_asan/common/guarded_page_allocator_unittest.cc
[add] https://crrev.com/53fd12919924e80e64b50c984f25eb5de43005da/components/gwp_asan/common/guarded_page_allocator_win.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 31

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/1a33ef9ab55ef434f877ed38aa4445073c5921a2

commit 1a33ef9ab55ef434f877ed38aa4445073c5921a2
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Wed Oct 31 18:47:09 2018

GWP-ASan: Change common to be a component

Changing common to a static_library was a mistake--the
GuardedPageAllocator is a singleton and needs to be in a separate
component to ensure that duplicate symbols create multiple singletons in
the process.

Bug: 896019
Change-Id: Iaee22e2e52180b6a8a7e59c106d96b10bb31b922
Reviewed-on: https://chromium-review.googlesource.com/c/1309425
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604351}
[modify] https://crrev.com/1a33ef9ab55ef434f877ed38aa4445073c5921a2/components/gwp_asan/common/BUILD.gn
[add] https://crrev.com/1a33ef9ab55ef434f877ed38aa4445073c5921a2/components/gwp_asan/common/export.h
[modify] https://crrev.com/1a33ef9ab55ef434f877ed38aa4445073c5921a2/components/gwp_asan/common/guarded_page_allocator.h

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 2

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/380a7a6000b83d6841b32b170e977df4bbdb8e6a

commit 380a7a6000b83d6841b32b170e977df4bbdb8e6a
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Nov 02 21:58:59 2018

GWP-ASan: Allow specifying allocation alignment

Allow specifying an allocation alignment to Allocate() that works
similarly to the alignment parameter to aligned_alloc(), e.g. the
alignment parameter must be a power-of-two smaller than size. Unlike
aligned_alloc it doesn't verify that the size is a multiple of alignment
since that's not enforced by some functions like posix_memalign.

Default alignment behavior stays the same if no alignment is specified.

Bug: 896019
Change-Id: I542314aa2405265af03cd6519f5c997ed1e95152
Reviewed-on: https://chromium-review.googlesource.com/c/1310563
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605063}
[modify] https://crrev.com/380a7a6000b83d6841b32b170e977df4bbdb8e6a/components/gwp_asan/common/guarded_page_allocator.cc
[modify] https://crrev.com/380a7a6000b83d6841b32b170e977df4bbdb8e6a/components/gwp_asan/common/guarded_page_allocator.h
[modify] https://crrev.com/380a7a6000b83d6841b32b170e977df4bbdb8e6a/components/gwp_asan/common/guarded_page_allocator_unittest.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 6

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2d07dc4f9a7b07c4872862316b34c0ae9ee26170

commit 2d07dc4f9a7b07c4872862316b34c0ae9ee26170
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Nov 06 00:50:57 2018

GWP-ASan: Change alloc_offset to alloc_ptr

Currently, the SlotMetadata records the allocation offset in the
returned page; however, it's only ever used to calculate the final
allocation address. Instead, just store the allocation address.

Bug: 896019
Change-Id: Id14a15e4a7c9afdeb4a517b85a90ddb9f30f7a9f
Reviewed-on: https://chromium-review.googlesource.com/c/1318404
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#605548}
[modify] https://crrev.com/2d07dc4f9a7b07c4872862316b34c0ae9ee26170/components/gwp_asan/common/guarded_page_allocator.cc
[modify] https://crrev.com/2d07dc4f9a7b07c4872862316b34c0ae9ee26170/components/gwp_asan/common/guarded_page_allocator.h

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c3b89701de8d8e35d6a11c1e8a04f6b45c52097e

commit c3b89701de8d8e35d6a11c1e8a04f6b45c52097e
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Wed Nov 07 23:14:31 2018

GWP-ASan: Refactor allocator singleton.

A subsequent refactor moves the singleton into client/. Remove it from
common, turn :common back to a static_library, and restructure the
constructor back into an Init() method to accomodate the new singleton
design.

Bug: 896019
Change-Id: I7bfe4b996b165ba34831565160c8ef46f3e48498
Reviewed-on: https://chromium-review.googlesource.com/c/1322111
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606194}
[modify] https://crrev.com/c3b89701de8d8e35d6a11c1e8a04f6b45c52097e/components/gwp_asan/common/BUILD.gn
[delete] https://crrev.com/e0c9cfb14856c92047c396368a0b722c640f7d33/components/gwp_asan/common/export.h
[modify] https://crrev.com/c3b89701de8d8e35d6a11c1e8a04f6b45c52097e/components/gwp_asan/common/guarded_page_allocator.cc
[modify] https://crrev.com/c3b89701de8d8e35d6a11c1e8a04f6b45c52097e/components/gwp_asan/common/guarded_page_allocator.h
[modify] https://crrev.com/c3b89701de8d8e35d6a11c1e8a04f6b45c52097e/components/gwp_asan/common/guarded_page_allocator_unittest.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3bd724b508c4f6abb936b710394710ac3194b8ad

commit 3bd724b508c4f6abb936b710394710ac3194b8ad
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Nov 09 18:37:19 2018

GWP-ASan: Create top-level unit test target

TBR=caitkp@chromium.org

Bug: 896019
Change-Id: I59ead55fbc258d981092c7e99d935a2a518a8921
Reviewed-on: https://chromium-review.googlesource.com/c/1324873
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606908}
[modify] https://crrev.com/3bd724b508c4f6abb936b710394710ac3194b8ad/components/BUILD.gn
[add] https://crrev.com/3bd724b508c4f6abb936b710394710ac3194b8ad/components/gwp_asan/BUILD.gn

Project Member

Comment 8 by bugdroid1@chromium.org, Nov 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/308dc79f2663bd785b17afca47462f430dd24965

commit 308dc79f2663bd785b17afca47462f430dd24965
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Nov 09 19:30:10 2018

GWP-ASan: Add allocator shims and client interface

Use Chromium's allocator shim interface to create a shim that samples
allocations to the GuardedPageAllocator. Add a function to enable
GWP-ASan that checks various feature flags to see if GWP-ASan should be
enabled and with what parameters.

In order to optimize the allocation hot-path, sampling is performed
using a thread-local counter to avoid contention. The sampling interval
is sampled from a poisson-distribution with a supplied frequency.

In order to optimize the deallocation hot-path, the GuardedPageAllocator
bounds are stored in global variables and the bounds check referencing
those globals are inlined into the deallocation routine to avoid the
indirection of accessing GlobalPageAllocator::PointerIsMine through a
singleton.

Bug: 896019
Change-Id: Id389d95a23067e2518cd58316db7b12935ec84af
Reviewed-on: https://chromium-review.googlesource.com/c/1306401
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#606928}
[modify] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/BUILD.gn
[add] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/client/BUILD.gn
[add] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/client/export.h
[add] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/client/gwp_asan.cc
[add] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/client/gwp_asan.h
[add] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/client/sampling_allocator_shims.cc
[add] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/client/sampling_allocator_shims.h
[add] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/client/sampling_allocator_shims_unittest.cc
[add] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/client/sampling_allocator_shims_win.h
[modify] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/common/guarded_page_allocator.cc
[modify] https://crrev.com/308dc79f2663bd785b17afca47462f430dd24965/components/gwp_asan/common/guarded_page_allocator.h

Project Member

Comment 9 by bugdroid1@chromium.org, Nov 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/27dc01ba9ec81c18b3e5da23c523c923cbb7af3f

commit 27dc01ba9ec81c18b3e5da23c523c923cbb7af3f
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Thu Nov 15 00:50:44 2018

GWP-ASan: Set crash key with allocator address

The crash handler needs to know where the GuardedPageAllocator in the
crashing process is to determine if the crash is related to a GWP-ASan
allocation. Use a crash key to transmit this information.

Bug: 896019
Change-Id: I93428ec6d46ebefe2939ff74fab9cb9fd35782a2
Reviewed-on: https://chromium-review.googlesource.com/c/1330073
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Albert J. Wong <ajwong@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608200}
[modify] https://crrev.com/27dc01ba9ec81c18b3e5da23c523c923cbb7af3f/components/gwp_asan/client/BUILD.gn
[add] https://crrev.com/27dc01ba9ec81c18b3e5da23c523c923cbb7af3f/components/gwp_asan/client/DEPS
[add] https://crrev.com/27dc01ba9ec81c18b3e5da23c523c923cbb7af3f/components/gwp_asan/client/crash_key.cc
[add] https://crrev.com/27dc01ba9ec81c18b3e5da23c523c923cbb7af3f/components/gwp_asan/client/crash_key.h
[modify] https://crrev.com/27dc01ba9ec81c18b3e5da23c523c923cbb7af3f/components/gwp_asan/client/sampling_allocator_shims.cc
[modify] https://crrev.com/27dc01ba9ec81c18b3e5da23c523c923cbb7af3f/components/gwp_asan/client/sampling_allocator_shims_unittest.cc
[modify] https://crrev.com/27dc01ba9ec81c18b3e5da23c523c923cbb7af3f/components/gwp_asan/common/BUILD.gn
[add] https://crrev.com/27dc01ba9ec81c18b3e5da23c523c923cbb7af3f/components/gwp_asan/common/crash_key_name.h

Project Member

Comment 10 by bugdroid1@chromium.org, Nov 15

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fc0fad5907fdb652a968f15de2d9741f7c56c668

commit fc0fad5907fdb652a968f15de2d9741f7c56c668
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Thu Nov 15 22:28:21 2018

GWP-ASan: Refactor SlotMetadata

Refactor SlotMetadata to break out matching allocation/deallocation info
into a common struct.

Bug: 896019
Change-Id: I6955e43048a30977b502db21f254faf9796d643c
Reviewed-on: https://chromium-review.googlesource.com/c/1338236
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608550}
[modify] https://crrev.com/fc0fad5907fdb652a968f15de2d9741f7c56c668/components/gwp_asan/common/guarded_page_allocator.cc
[modify] https://crrev.com/fc0fad5907fdb652a968f15de2d9741f7c56c668/components/gwp_asan/common/guarded_page_allocator.h

Project Member

Comment 11 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5dd2059a58d1f6f635aa52bddf5f7c7669ac2734

commit 5dd2059a58d1f6f635aa52bddf5f7c7669ac2734
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Nov 16 20:00:14 2018

GWP-ASan: Fix broken unit test

After a refactor I noticed that this unit test was broken.
_aligned_malloc() does not go through the allocator shims on windows,
but because we used EXPECT_TRUE() we didn't catch this failure. Delete
the _aligned_malloc() test and refactor allocationCheck() to return
failure count so that we can track failures across calls and return the
proper exit code.

Bug: 896019
Change-Id: Ibef719eb14980575cc52796167735f5a0ba177fc
Reviewed-on: https://chromium-review.googlesource.com/c/1339319
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608908}
[modify] https://crrev.com/5dd2059a58d1f6f635aa52bddf5f7c7669ac2734/components/gwp_asan/client/sampling_allocator_shims_unittest.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fe2499e4b1a6a5dd926115606688e39ad4a2d735

commit fe2499e4b1a6a5dd926115606688e39ad4a2d735
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Nov 16 21:02:27 2018

GWP-ASan: Refactor SlotMetadata

Get rid of the SlotMetadata constructor/destructor and replace the
destructor with a Destroy() method. Also change alloc_ptr to be a
uintptr_t. This refactor makes SlotMetadata safer to initialize with
out-of-process memory in an upcoming crash handler change, because the
SlotMetadata we read should not be automatically destructed (the members
will be from another memory space). Since alloc_ptr is only used in the
crash handler there's no reason to not make it a uintptr_t to make it
appear less like a pointer in crash handler's memory space.

Bug: 896019
Change-Id: I3f1f22eac6277722de9594a3331b8daef618d29d
Reviewed-on: https://chromium-review.googlesource.com/c/1340868
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608935}
[modify] https://crrev.com/fe2499e4b1a6a5dd926115606688e39ad4a2d735/components/gwp_asan/common/guarded_page_allocator.cc
[modify] https://crrev.com/fe2499e4b1a6a5dd926115606688e39ad4a2d735/components/gwp_asan/common/guarded_page_allocator.h

Project Member

Comment 13 by bugdroid1@chromium.org, Nov 16

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3f565126ff34b7b8d9e3707bed0bd41c53140fee

commit 3f565126ff34b7b8d9e3707bed0bd41c53140fee
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Nov 16 21:03:05 2018

GWP-ASan: Get rid of kFreePagesNumBits

This change is preparing for an upcoming refactor. kFreePagesNumBits
and kGpaMaxPages are equivalent. Get rid of the latter with a
static_assert() that kGpaMaxPages is always equal to the size of the
BitMap.

Bug: 896019
Change-Id: I7e99766d2e6fffa0e3d3eb3a47a2e76038b522ab
Reviewed-on: https://chromium-review.googlesource.com/c/1340526
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#608936}
[modify] https://crrev.com/3f565126ff34b7b8d9e3707bed0bd41c53140fee/components/gwp_asan/common/guarded_page_allocator.cc
[modify] https://crrev.com/3f565126ff34b7b8d9e3707bed0bd41c53140fee/components/gwp_asan/common/guarded_page_allocator.h

Project Member

Comment 14 by bugdroid1@chromium.org, Nov 19

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08f74b8d617131bc8a55009d6f030de4b114ad6d

commit 08f74b8d617131bc8a55009d6f030de4b114ad6d
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Mon Nov 19 17:54:41 2018

GWP-ASan: Refactor SlotMetadata methods

In an upcoming change, I split the GuardedPageAllocator into two
objects. I'm moving these methods into the GuardedPageAllocator because
SlotMetadata lives in the base object but these methods should not.

Bug: 896019
Change-Id: Ia98e966ce6cc8aa843bcaa4ce322c1b355e508a1
Reviewed-on: https://chromium-review.googlesource.com/c/1341062
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#609352}
[modify] https://crrev.com/08f74b8d617131bc8a55009d6f030de4b114ad6d/components/gwp_asan/common/guarded_page_allocator.cc
[modify] https://crrev.com/08f74b8d617131bc8a55009d6f030de4b114ad6d/components/gwp_asan/common/guarded_page_allocator.h

Project Member

Comment 15 by bugdroid1@chromium.org, Nov 26

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ad4c5def83d7da6f7382e6b18824477e7b8ab86

commit 5ad4c5def83d7da6f7382e6b18824477e7b8ab86
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Mon Nov 26 19:00:45 2018

GWP-ASan: Refactor GuardedPageAllocator into two objects

Refactor a trivially copyable (e.g. plain old data) object out of the
current GuardedPageAllocator. This 'base state' object encapsulates
just the information required by the crash handler. Because its
construction/destruction is trivial, a base state object can be
overwritten with out-of-process memory and destructed without concerns
about undefined behavior or destructing complex objects with pointers
from another process.

The GuardedPageAllocator encapsulates all of the remaining data and
allocation logic that does not to be reached by the crash handler. As
such, it's been refactored into client/.

Bug: 896019

Change-Id: I418411b914a61c592f8b585d55058aa5a705acfb
Reviewed-on: https://chromium-review.googlesource.com/c/1339246
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#610902}
[modify] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/BUILD.gn
[modify] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/client/BUILD.gn
[modify] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/client/crash_key.cc
[modify] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/client/crash_key.h
[add] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/client/guarded_page_allocator.cc
[add] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/client/guarded_page_allocator.h
[rename] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/client/guarded_page_allocator_unittest.cc
[rename] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/client/guarded_page_allocator_win.cc
[modify] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/client/gwp_asan.cc
[modify] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/client/sampling_allocator_shims.cc
[modify] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/client/sampling_allocator_shims_unittest.cc
[modify] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/common/BUILD.gn
[add] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/common/allocator_state.cc
[add] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/common/allocator_state.h
[modify] https://crrev.com/5ad4c5def83d7da6f7382e6b18824477e7b8ab86/components/gwp_asan/common/crash_key_name.h
[delete] https://crrev.com/391b36e5fbe266d80338ba6a6fe192087e10ab52/components/gwp_asan/common/guarded_page_allocator.cc
[delete] https://crrev.com/391b36e5fbe266d80338ba6a6fe192087e10ab52/components/gwp_asan/common/guarded_page_allocator.h

Project Member

Comment 16 by bugdroid1@chromium.org, Nov 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/409ceb85a800c61a2188d2bde44dda2e6a483a4f

commit 409ceb85a800c61a2188d2bde44dda2e6a483a4f
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Nov 27 07:03:52 2018

GWP-ASan: Add crash handler hook.

After a GWP-ASan exception occurs, a crash handler hook (e.g. a crashpad
UserStreamDataSource) is responsible for looking at the crash and
1) determining if the exception was related to GWP-ASan, and 2) adding
additional debug information to the minidump if so.

The crash handler hook determines if the exception was related to
GWP-ASan by finding the AllocatorState address (using a crash key),
reading the allocator state and seeing if the exception occurred in the
bounds of the allocator region. If it did, we extract debug information
about that allocation and report it in the minidump.

CQ-DEPEND=CL:1339246

Bug: 896019
Change-Id: I63d12b5137098b20ec946e3bddbdcabaf20e430a
Reviewed-on: https://chromium-review.googlesource.com/c/1330283
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Brian White <bcwhite@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611033}
[modify] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/BUILD.gn
[modify] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/client/guarded_page_allocator_unittest.cc
[modify] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/common/BUILD.gn
[modify] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/common/allocator_state.cc
[modify] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/common/allocator_state.h
[add] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/common/allocator_state_unittest.cc
[add] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/crash_handler/BUILD.gn
[add] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/crash_handler/DEPS
[add] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/crash_handler/crash.proto
[add] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/crash_handler/crash_analyzer.cc
[add] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/crash_handler/crash_analyzer.h
[add] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/crash_handler/crash_analyzer_win.cc
[add] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/crash_handler/crash_handler.cc
[add] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/crash_handler/crash_handler.h
[add] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/components/gwp_asan/crash_handler/crash_handler_unittest.cc
[modify] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/409ceb85a800c61a2188d2bde44dda2e6a483a4f/tools/metrics/histograms/histograms.xml

Project Member

Comment 17 by bugdroid1@chromium.org, Nov 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1f52ab37424d3f01d7cc271ab3d3e1be861949a

commit f1f52ab37424d3f01d7cc271ab3d3e1be861949a
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Nov 27 08:09:58 2018

GWP-ASan: Perform early start-up initialization

GWP-ASan is a debug allocator intended to find memory errors in the
wild. In order to detect as many as bugs as possible, it should start as
early as possible, e.g. soon after it's dependency, FeatureList, has
been initialized. Since content/ doesn't call into ChromeMainDelegate
for child processes after FeatureList has been initialized, create a new
delegate method for post-FeatureList initialization and enable GWP-ASan
at that point.

Bug: 896019
Change-Id: I9c081602cd32a71d5d9eb774bd9818757d9b7e95
Reviewed-on: https://chromium-review.googlesource.com/c/1343347
Reviewed-by: Robert Kaplow <rkaplow@chromium.org>
Reviewed-by: Ken Rockot <rockot@google.com>
Reviewed-by: Avi Drissman <avi@chromium.org>
Reviewed-by: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611041}
[modify] https://crrev.com/f1f52ab37424d3f01d7cc271ab3d3e1be861949a/chrome/BUILD.gn
[modify] https://crrev.com/f1f52ab37424d3f01d7cc271ab3d3e1be861949a/chrome/app/BUILD.gn
[modify] https://crrev.com/f1f52ab37424d3f01d7cc271ab3d3e1be861949a/chrome/app/DEPS
[modify] https://crrev.com/f1f52ab37424d3f01d7cc271ab3d3e1be861949a/chrome/app/chrome_main_delegate.cc
[modify] https://crrev.com/f1f52ab37424d3f01d7cc271ab3d3e1be861949a/chrome/app/chrome_main_delegate.h
[modify] https://crrev.com/f1f52ab37424d3f01d7cc271ab3d3e1be861949a/content/app/content_main_runner_impl.cc
[modify] https://crrev.com/f1f52ab37424d3f01d7cc271ab3d3e1be861949a/content/public/app/content_main_delegate.h

Project Member

Comment 18 by bugdroid1@chromium.org, Nov 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d32ec497929f46da4c89cea930f9d1113bc61b58

commit d32ec497929f46da4c89cea930f9d1113bc61b58
Author: Tim Schumann <tschumann@chromium.org>
Date: Tue Nov 27 12:29:04 2018

Revert "GWP-ASan: Add crash handler hook."

This reverts commit 409ceb85a800c61a2188d2bde44dda2e6a483a4f.

Reason for revert: Introduced flaky tests ( http://crbug.com/908804 )

Original change's description:
> GWP-ASan: Add crash handler hook.
> 
> After a GWP-ASan exception occurs, a crash handler hook (e.g. a crashpad
> UserStreamDataSource) is responsible for looking at the crash and
> 1) determining if the exception was related to GWP-ASan, and 2) adding
> additional debug information to the minidump if so.
> 
> The crash handler hook determines if the exception was related to
> GWP-ASan by finding the AllocatorState address (using a crash key),
> reading the allocator state and seeing if the exception occurred in the
> bounds of the allocator region. If it did, we extract debug information
> about that allocation and report it in the minidump.
> 
> CQ-DEPEND=CL:1339246
> 
> Bug: 896019
> Change-Id: I63d12b5137098b20ec946e3bddbdcabaf20e430a
> Reviewed-on: https://chromium-review.googlesource.com/c/1330283
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Reviewed-by: Brian White <bcwhite@chromium.org>
> Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#611033}

TBR=ajwong@chromium.org,bcwhite@chromium.org,mark@chromium.org,vtsyrklevich@chromium.org,vitalybuka@chromium.org

Change-Id: If79e66f0852b0bb5ff6ea5d97e0197f0c5924ab8
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 896019
Reviewed-on: https://chromium-review.googlesource.com/c/1352173
Reviewed-by: Tim Schumann <tschumann@chromium.org>
Commit-Queue: Tim Schumann <tschumann@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611075}
[modify] https://crrev.com/d32ec497929f46da4c89cea930f9d1113bc61b58/components/gwp_asan/BUILD.gn
[modify] https://crrev.com/d32ec497929f46da4c89cea930f9d1113bc61b58/components/gwp_asan/client/guarded_page_allocator_unittest.cc
[modify] https://crrev.com/d32ec497929f46da4c89cea930f9d1113bc61b58/components/gwp_asan/common/BUILD.gn
[modify] https://crrev.com/d32ec497929f46da4c89cea930f9d1113bc61b58/components/gwp_asan/common/allocator_state.cc
[modify] https://crrev.com/d32ec497929f46da4c89cea930f9d1113bc61b58/components/gwp_asan/common/allocator_state.h
[delete] https://crrev.com/61986d784baacd664a5f0e5013e873278593a523/components/gwp_asan/common/allocator_state_unittest.cc
[delete] https://crrev.com/61986d784baacd664a5f0e5013e873278593a523/components/gwp_asan/crash_handler/BUILD.gn
[delete] https://crrev.com/61986d784baacd664a5f0e5013e873278593a523/components/gwp_asan/crash_handler/DEPS
[delete] https://crrev.com/61986d784baacd664a5f0e5013e873278593a523/components/gwp_asan/crash_handler/crash.proto
[delete] https://crrev.com/61986d784baacd664a5f0e5013e873278593a523/components/gwp_asan/crash_handler/crash_analyzer.cc
[delete] https://crrev.com/61986d784baacd664a5f0e5013e873278593a523/components/gwp_asan/crash_handler/crash_analyzer.h
[delete] https://crrev.com/61986d784baacd664a5f0e5013e873278593a523/components/gwp_asan/crash_handler/crash_analyzer_win.cc
[delete] https://crrev.com/61986d784baacd664a5f0e5013e873278593a523/components/gwp_asan/crash_handler/crash_handler.cc
[delete] https://crrev.com/61986d784baacd664a5f0e5013e873278593a523/components/gwp_asan/crash_handler/crash_handler.h
[delete] https://crrev.com/61986d784baacd664a5f0e5013e873278593a523/components/gwp_asan/crash_handler/crash_handler_unittest.cc
[modify] https://crrev.com/d32ec497929f46da4c89cea930f9d1113bc61b58/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/d32ec497929f46da4c89cea930f9d1113bc61b58/tools/metrics/histograms/histograms.xml

Project Member

Comment 19 by bugdroid1@chromium.org, Nov 27

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9caa221f953cd6e0abf770c452645c8e741137b9

commit 9caa221f953cd6e0abf770c452645c8e741137b9
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Nov 27 21:30:15 2018

Reland "GWP-ASan: Add crash handler hook."

This is a reland of 409ceb85a800c61a2188d2bde44dda2e6a483a4f

Original change's description:
> GWP-ASan: Add crash handler hook.
>
> After a GWP-ASan exception occurs, a crash handler hook (e.g. a crashpad
> UserStreamDataSource) is responsible for looking at the crash and
> 1) determining if the exception was related to GWP-ASan, and 2) adding
> additional debug information to the minidump if so.
>
> The crash handler hook determines if the exception was related to
> GWP-ASan by finding the AllocatorState address (using a crash key),
> reading the allocator state and seeing if the exception occurred in the
> bounds of the allocator region. If it did, we extract debug information
> about that allocation and report it in the minidump.
>
> CQ-DEPEND=CL:1339246
>
> Bug: 896019
> Change-Id: I63d12b5137098b20ec946e3bddbdcabaf20e430a
> Reviewed-on: https://chromium-review.googlesource.com/c/1330283
> Reviewed-by: Mark Mentovai <mark@chromium.org>
> Reviewed-by: Brian White <bcwhite@chromium.org>
> Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#611033}

TBR=bcwhite@chromium.org

Bug: 896019
Change-Id: Ibc35c33ccc816cd2436412e2c6f399908161e7e1
Reviewed-on: https://chromium-review.googlesource.com/c/1351935
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611337}
[modify] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/BUILD.gn
[modify] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/client/guarded_page_allocator_unittest.cc
[modify] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/common/BUILD.gn
[modify] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/common/allocator_state.cc
[modify] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/common/allocator_state.h
[add] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/common/allocator_state_unittest.cc
[add] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/crash_handler/BUILD.gn
[add] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/crash_handler/DEPS
[add] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/crash_handler/crash.proto
[add] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/crash_handler/crash_analyzer.cc
[add] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/crash_handler/crash_analyzer.h
[add] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/crash_handler/crash_analyzer_win.cc
[add] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/crash_handler/crash_handler.cc
[add] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/crash_handler/crash_handler.h
[add] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/components/gwp_asan/crash_handler/crash_handler_unittest.cc
[modify] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/tools/metrics/histograms/enums.xml
[modify] https://crrev.com/9caa221f953cd6e0abf770c452645c8e741137b9/tools/metrics/histograms/histograms.xml

Project Member

Comment 20 by bugdroid1@chromium.org, Nov 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5261640d9852b6ebeab23e77917d72fe6a39e231

commit 5261640d9852b6ebeab23e77917d72fe6a39e231
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Wed Nov 28 00:53:03 2018

GWP-ASan: Wire crash handler into crash component

Add the GWP-ASan crash handler hook to the list of user stream data
sources in the Windows crashpad process. The GWP-ASan crash handler
inspects crashes, and if they are related to a GWP-ASan allocation, adds
additional debugging data to the minidump.

CQ-DEPEND=CL:1330283

Bug: 896019
Change-Id: I0b38c60cd46c029200240560fdb714fbd2087f70
Reviewed-on: https://chromium-review.googlesource.com/c/1338324
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#611472}
[modify] https://crrev.com/5261640d9852b6ebeab23e77917d72fe6a39e231/components/crash/content/app/BUILD.gn
[modify] https://crrev.com/5261640d9852b6ebeab23e77917d72fe6a39e231/components/crash/content/app/DEPS
[modify] https://crrev.com/5261640d9852b6ebeab23e77917d72fe6a39e231/components/crash/content/app/run_as_crashpad_handler_win.cc

Blockedon: 909720
Project Member

Comment 22 by bugdroid1@chromium.org, Nov 30

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9b0c8199762c922f6932a6ce041b865678e8d1ea

commit 9b0c8199762c922f6932a6ce041b865678e8d1ea
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Nov 30 19:23:08 2018

GWP-ASan: Upgrade DCHECK to CHECK

We should never reach this routine with a pointer outside of the
GWP-ASan region.

Bug: 896019
Change-Id: I078443f5634f03d1c7e460db16f61398c6dad999
Reviewed-on: https://chromium-review.googlesource.com/c/1356332
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#612719}
[modify] https://crrev.com/9b0c8199762c922f6932a6ce041b865678e8d1ea/components/gwp_asan/client/guarded_page_allocator.cc

Project Member

Comment 23 by bugdroid1@chromium.org, Dec 4

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08bc052506c74b6f6f3ed4e8ab60fa29d280b9f1

commit 08bc052506c74b6f6f3ed4e8ab60fa29d280b9f1
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Dec 04 06:58:54 2018

GWP-ASan: Add documentation

Add a simple design doc with the current development status.

Bug: 896019
Change-Id: I14f1f4e4a5201c0d5bba2260783c5b1ed0db6e27
Reviewed-on: https://chromium-review.googlesource.com/c/1357568
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#613451}
[rename] https://crrev.com/08bc052506c74b6f6f3ed4e8ab60fa29d280b9f1/components/gwp_asan/README.md
[add] https://crrev.com/08bc052506c74b6f6f3ed4e8ab60fa29d280b9f1/docs/gwp_asan.md

Blockedon: 912500
Project Member

Comment 25 by bugdroid1@chromium.org, Dec 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e

commit 0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Wed Dec 12 23:36:43 2018

allocator: Add Windows _aligned_* shims

On Windows we don’t currently hook the _aligned_* allocation APIs, this
can cause issues because _aligned_realloc can call HeapSize and cause
GWP-ASan crashes similar to  bug 909720 . Unfortunately the
_aligned_realloc API is different enough that it can not be implemented
using the standard POSIX shims, in particular because _aligned_malloc
and _aligned_free don't return valid allocation addresses, they are
offsets into allocations.

I add new Windows platform-specific shims for _aligned_malloc,
_aligned_realloc, and _aligned_free and wire them in for all users of
the allocator shims. I implement these routines on top of the Windows
Heap* API and leave uncommon _aligned_* shims to crash to ensure that
any future uses immediately surface why their use fails.

Bug: 912500, 896019
Change-Id: Ieaa50b816ab277a6ad4b80ee8519027343fa9878
Reviewed-on: https://chromium-review.googlesource.com/c/1367485
Reviewed-by: danakj <danakj@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Alexei Filippov <alph@chromium.org>
Reviewed-by: Erik Chen <erikchen@chromium.org>
Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616106}
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/BUILD.gn
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/allocator_shim.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/allocator_shim.h
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/allocator_shim_default_dispatch_to_glibc.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/allocator_shim_default_dispatch_to_mac_zoned_malloc.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/allocator_shim_default_dispatch_to_tcmalloc.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/allocator_shim_default_dispatch_to_winheap.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/allocator_shim_override_ucrt_symbols_win.h
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/allocator_shim_unittest.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/winheap_stubs_win.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/winheap_stubs_win.h
[add] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/allocator/winheap_stubs_win_unittest.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/debug/thread_heap_usage_tracker.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/base/sampling_heap_profiler/poisson_allocation_sampler.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/components/gwp_asan/client/sampling_allocator_shims.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/components/gwp_asan/client/sampling_allocator_shims_unittest.cc
[modify] https://crrev.com/0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e/components/services/heap_profiling/public/cpp/allocator_shim.cc

Project Member

Comment 26 by bugdroid1@chromium.org, Dec 12

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7486030c6976ca73ad74ca3f4f15a725dba72e4a

commit 7486030c6976ca73ad74ca3f4f15a725dba72e4a
Author: Thomas Anderson <thomasanderson@chromium.org>
Date: Wed Dec 12 23:57:36 2018

Revert "allocator: Add Windows _aligned_* shims"

This reverts commit 0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e.

Reason for revert: Breaks win-rel build: https://ci.chromium.org/p/chromium/builders/luci.chromium.ci/win-rel/7800

Original change's description:
> allocator: Add Windows _aligned_* shims
> 
> On Windows we don’t currently hook the _aligned_* allocation APIs, this
> can cause issues because _aligned_realloc can call HeapSize and cause
> GWP-ASan crashes similar to  bug 909720 . Unfortunately the
> _aligned_realloc API is different enough that it can not be implemented
> using the standard POSIX shims, in particular because _aligned_malloc
> and _aligned_free don't return valid allocation addresses, they are
> offsets into allocations.
> 
> I add new Windows platform-specific shims for _aligned_malloc,
> _aligned_realloc, and _aligned_free and wire them in for all users of
> the allocator shims. I implement these routines on top of the Windows
> Heap* API and leave uncommon _aligned_* shims to crash to ensure that
> any future uses immediately surface why their use fails.
> 
> Bug: 912500, 896019
> Change-Id: Ieaa50b816ab277a6ad4b80ee8519027343fa9878
> Reviewed-on: https://chromium-review.googlesource.com/c/1367485
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
> Reviewed-by: Alexei Filippov <alph@chromium.org>
> Reviewed-by: Erik Chen <erikchen@chromium.org>
> Reviewed-by: Will Harris <wfh@chromium.org>
> Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
> Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#616106}

TBR=danakj@chromium.org,chrisha@chromium.org,primiano@chromium.org,alph@chromium.org,erikchen@chromium.org,wfh@chromium.org,siggi@chromium.org,vtsyrklevich@chromium.org,vitalybuka@chromium.org

Change-Id: Ie9e3246f3a70985cef38263e22f8f86a62002b22
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 912500, 896019
Reviewed-on: https://chromium-review.googlesource.com/c/1374909
Reviewed-by: Thomas Anderson <thomasanderson@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616113}
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/BUILD.gn
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/allocator_shim.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/allocator_shim.h
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/allocator_shim_default_dispatch_to_glibc.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/allocator_shim_default_dispatch_to_mac_zoned_malloc.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/allocator_shim_default_dispatch_to_tcmalloc.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/allocator_shim_default_dispatch_to_winheap.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/allocator_shim_override_ucrt_symbols_win.h
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/allocator_shim_unittest.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/winheap_stubs_win.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/allocator/winheap_stubs_win.h
[delete] https://crrev.com/cd05ee7ce74bcef9438775d777764cd7e3a23b1d/base/allocator/winheap_stubs_win_unittest.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/debug/thread_heap_usage_tracker.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/base/sampling_heap_profiler/poisson_allocation_sampler.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/components/gwp_asan/client/sampling_allocator_shims.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/components/gwp_asan/client/sampling_allocator_shims_unittest.cc
[modify] https://crrev.com/7486030c6976ca73ad74ca3f4f15a725dba72e4a/components/services/heap_profiling/public/cpp/allocator_shim.cc

Project Member

Comment 27 by bugdroid1@chromium.org, Dec 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f

commit 7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Thu Dec 13 02:29:36 2018

Reland "allocator: Add Windows _aligned_* shims"

This is a reland of 0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e with a
missing #if DCHECK_IS_ON() added.

Original change's description:
> allocator: Add Windows _aligned_* shims
>
> On Windows we don’t currently hook the _aligned_* allocation APIs, this
> can cause issues because _aligned_realloc can call HeapSize and cause
> GWP-ASan crashes similar to  bug 909720 . Unfortunately the
> _aligned_realloc API is different enough that it can not be implemented
> using the standard POSIX shims, in particular because _aligned_malloc
> and _aligned_free don't return valid allocation addresses, they are
> offsets into allocations.
>
> I add new Windows platform-specific shims for _aligned_malloc,
> _aligned_realloc, and _aligned_free and wire them in for all users of
> the allocator shims. I implement these routines on top of the Windows
> Heap* API and leave uncommon _aligned_* shims to crash to ensure that
> any future uses immediately surface why their use fails.
>
> Bug: 912500, 896019
> Change-Id: Ieaa50b816ab277a6ad4b80ee8519027343fa9878
> Reviewed-on: https://chromium-review.googlesource.com/c/1367485
> Reviewed-by: danakj <danakj@chromium.org>
> Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
> Reviewed-by: Alexei Filippov <alph@chromium.org>
> Reviewed-by: Erik Chen <erikchen@chromium.org>
> Reviewed-by: Will Harris <wfh@chromium.org>
> Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
> Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#616106}

TBR=danakj@chromium.org,vitalybuka@chromium.org,alph@chromium.org,erikchen@chromium.org,wfh@chromium.org

Bug: 912500, 896019
Change-Id: I05aeb4bc9ef2974f967a81c53cb7bbbcc6e955f3
Reviewed-on: https://chromium-review.googlesource.com/c/1374959
Reviewed-by: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616171}
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/BUILD.gn
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/allocator_shim.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/allocator_shim.h
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/allocator_shim_default_dispatch_to_glibc.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/allocator_shim_default_dispatch_to_mac_zoned_malloc.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/allocator_shim_default_dispatch_to_tcmalloc.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/allocator_shim_default_dispatch_to_winheap.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/allocator_shim_override_ucrt_symbols_win.h
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/allocator_shim_unittest.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/winheap_stubs_win.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/winheap_stubs_win.h
[add] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/allocator/winheap_stubs_win_unittest.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/debug/thread_heap_usage_tracker.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/base/sampling_heap_profiler/poisson_allocation_sampler.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/components/gwp_asan/client/sampling_allocator_shims.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/components/gwp_asan/client/sampling_allocator_shims_unittest.cc
[modify] https://crrev.com/7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f/components/services/heap_profiling/public/cpp/allocator_shim.cc

Project Member

Comment 28 by bugdroid1@chromium.org, Dec 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2729d4f2727ea0bbe4639943fb61be94a5944d84

commit 2729d4f2727ea0bbe4639943fb61be94a5944d84
Author: Elly Fong-Jones <ellyjones@chromium.org>
Date: Thu Dec 13 13:39:14 2018

Revert "Reland "allocator: Add Windows _aligned_* shims""

This reverts commit 7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f.

Reason for revert:
Sheriff suspects this broke win-asan tests, specifically: https://logs.chromium.org/logs/chromium/buildbucket/cr-buildbucket.appspot.com/8927275104247189904/+/steps/media_blink_unittests_on_Windows-10-15063/0/logs/WebMediaPlayerImplTest.LoadAndDestroy/0

e.g.:

    #0 0x7ff7b62b2e1c in __asan_wrap_memmove C:\b\rr\tmph59meq\w\src\third_party\llvm\projects\compiler-rt\lib\sanitizer_common\sanitizer_common_interceptors.inc:775
    #1 0x7ff7bd0736f0 in _aligned_realloc C:/b/swarming/w/ir/cache/builder/src/out/Release_x64\minkernel\crts\ucrt\src\appcrt\heap\align.cpp:622
    #2 0x7ff7b68faf0e in av_realloc_f C:/b/swarming/w/ir/cache/builder/src/out/Release_x64\..\..\third_party\ffmpeg\libavutil\mem.c:158
    #3 0x7ff7b68ee92a in ffio_rewind_with_probe_data C:/b/swarming/w/ir/cache/builder/src/out/Release_x64\..\..\third_party\ffmpeg\libavformat\aviobuf.c:1136
    #4 0x7ff7b7a69966 in av_probe_input_buffer2 C:/b/swarming/w/ir/cache/builder/src/out/Release_x64\..\..\third_party\ffmpeg\libavformat\format.c:304
    #5 0x7ff7b68c8b40 in avformat_open_input C:/b/swarming/w/ir/cache/builder/src/out/Release_x64\..\..\third_party\ffmpeg\libavformat\utils.c:573
    #6 0x7ff7b37437f8 in media::FFmpegGlue::OpenContext C:/b/swarming/w/ir/cache/builder/src/out/Release_x64\..\..\media\filters\ffmpeg_glue.cc:117


Original change's description:
> Reland "allocator: Add Windows _aligned_* shims"
> 
> This is a reland of 0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e with a
> missing #if DCHECK_IS_ON() added.
> 
> Original change's description:
> > allocator: Add Windows _aligned_* shims
> >
> > On Windows we don’t currently hook the _aligned_* allocation APIs, this
> > can cause issues because _aligned_realloc can call HeapSize and cause
> > GWP-ASan crashes similar to  bug 909720 . Unfortunately the
> > _aligned_realloc API is different enough that it can not be implemented
> > using the standard POSIX shims, in particular because _aligned_malloc
> > and _aligned_free don't return valid allocation addresses, they are
> > offsets into allocations.
> >
> > I add new Windows platform-specific shims for _aligned_malloc,
> > _aligned_realloc, and _aligned_free and wire them in for all users of
> > the allocator shims. I implement these routines on top of the Windows
> > Heap* API and leave uncommon _aligned_* shims to crash to ensure that
> > any future uses immediately surface why their use fails.
> >
> > Bug: 912500, 896019
> > Change-Id: Ieaa50b816ab277a6ad4b80ee8519027343fa9878
> > Reviewed-on: https://chromium-review.googlesource.com/c/1367485
> > Reviewed-by: danakj <danakj@chromium.org>
> > Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
> > Reviewed-by: Alexei Filippov <alph@chromium.org>
> > Reviewed-by: Erik Chen <erikchen@chromium.org>
> > Reviewed-by: Will Harris <wfh@chromium.org>
> > Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
> > Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#616106}
> 
> TBR=danakj@chromium.org,vitalybuka@chromium.org,alph@chromium.org,erikchen@chromium.org,wfh@chromium.org
> 
> Bug: 912500, 896019
> Change-Id: I05aeb4bc9ef2974f967a81c53cb7bbbcc6e955f3
> Reviewed-on: https://chromium-review.googlesource.com/c/1374959
> Reviewed-by: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
> Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#616171}

TBR=danakj@chromium.org,chrisha@chromium.org,primiano@chromium.org,alph@chromium.org,erikchen@chromium.org,wfh@chromium.org,siggi@chromium.org,vtsyrklevich@chromium.org,vitalybuka@chromium.org

Change-Id: Ic0c79f51b2b539f8daedf735ae1adfaa63ed5340
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 912500, 896019
Reviewed-on: https://chromium-review.googlesource.com/c/1375381
Reviewed-by: Elly Fong-Jones <ellyjones@chromium.org>
Commit-Queue: Elly Fong-Jones <ellyjones@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616293}
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/BUILD.gn
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/allocator_shim.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/allocator_shim.h
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/allocator_shim_default_dispatch_to_glibc.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/allocator_shim_default_dispatch_to_mac_zoned_malloc.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/allocator_shim_default_dispatch_to_tcmalloc.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/allocator_shim_default_dispatch_to_winheap.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/allocator_shim_override_ucrt_symbols_win.h
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/allocator_shim_unittest.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/winheap_stubs_win.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/allocator/winheap_stubs_win.h
[delete] https://crrev.com/5fdfc67b5dd544af91707d700ef46b5178275df9/base/allocator/winheap_stubs_win_unittest.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/debug/thread_heap_usage_tracker.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/base/sampling_heap_profiler/poisson_allocation_sampler.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/components/gwp_asan/client/sampling_allocator_shims.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/components/gwp_asan/client/sampling_allocator_shims_unittest.cc
[modify] https://crrev.com/2729d4f2727ea0bbe4639943fb61be94a5944d84/components/services/heap_profiling/public/cpp/allocator_shim.cc

Project Member

Comment 29 by bugdroid1@chromium.org, Dec 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cecda97aff81d3eed1b3522f82d40b7984558fe8

commit cecda97aff81d3eed1b3522f82d40b7984558fe8
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Thu Dec 13 20:28:31 2018

Reland "Reland "allocator: Add Windows _aligned_* shims""

This is a reland of 7eae85ba6448aa3cf1e14d6557e1f77a86c7e49f. The original
revert picked the wrong CL, the windows ASan build doesn't use the
allocator shims, instead it appears that the build failure was actually due
to a VS toolchain update as noted in  https://crbug.com/914947 

Original change's description:
> Reland "allocator: Add Windows _aligned_* shims"
>
> This is a reland of 0ba5ea124ec9de7d786cf6f4fc4ab2847c8d785e with a
> missing #if DCHECK_IS_ON() added.
>
> Original change's description:
> > allocator: Add Windows _aligned_* shims
> >
> > On Windows we don’t currently hook the _aligned_* allocation APIs, this
> > can cause issues because _aligned_realloc can call HeapSize and cause
> > GWP-ASan crashes similar to  bug 909720 . Unfortunately the
> > _aligned_realloc API is different enough that it can not be implemented
> > using the standard POSIX shims, in particular because _aligned_malloc
> > and _aligned_free don't return valid allocation addresses, they are
> > offsets into allocations.
> >
> > I add new Windows platform-specific shims for _aligned_malloc,
> > _aligned_realloc, and _aligned_free and wire them in for all users of
> > the allocator shims. I implement these routines on top of the Windows
> > Heap* API and leave uncommon _aligned_* shims to crash to ensure that
> > any future uses immediately surface why their use fails.
> >
> > Bug: 912500, 896019
> > Change-Id: Ieaa50b816ab277a6ad4b80ee8519027343fa9878
> > Reviewed-on: https://chromium-review.googlesource.com/c/1367485
> > Reviewed-by: danakj <danakj@chromium.org>
> > Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
> > Reviewed-by: Alexei Filippov <alph@chromium.org>
> > Reviewed-by: Erik Chen <erikchen@chromium.org>
> > Reviewed-by: Will Harris <wfh@chromium.org>
> > Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
> > Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#616106}
>
> TBR=danakj@chromium.org,vitalybuka@chromium.org,alph@chromium.org,erikchen@chromium.org,wfh@chromium.org
>
> Bug: 912500, 896019
> Change-Id: I05aeb4bc9ef2974f967a81c53cb7bbbcc6e955f3
> Reviewed-on: https://chromium-review.googlesource.com/c/1374959
> Reviewed-by: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
> Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#616171}

TBR=danakj@chromium.org,vitalybuka@chromium.org,alph@chromium.org,erikchen@chromium.org,wfh@chromium.org

Bug: 912500, 896019
Change-Id: Ia832ac212e91d328121b260b0f597fc002ccc3f5
Reviewed-on: https://chromium-review.googlesource.com/c/1376032
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#616417}
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/BUILD.gn
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/allocator_shim.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/allocator_shim.h
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/allocator_shim_default_dispatch_to_glibc.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/allocator_shim_default_dispatch_to_linker_wrapped_symbols.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/allocator_shim_default_dispatch_to_mac_zoned_malloc.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/allocator_shim_default_dispatch_to_tcmalloc.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/allocator_shim_default_dispatch_to_winheap.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/allocator_shim_override_ucrt_symbols_win.h
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/allocator_shim_unittest.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/winheap_stubs_win.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/winheap_stubs_win.h
[add] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/allocator/winheap_stubs_win_unittest.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/debug/thread_heap_usage_tracker.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/base/sampling_heap_profiler/poisson_allocation_sampler.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/components/gwp_asan/client/sampling_allocator_shims.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/components/gwp_asan/client/sampling_allocator_shims_unittest.cc
[modify] https://crrev.com/cecda97aff81d3eed1b3522f82d40b7984558fe8/components/services/heap_profiling/public/cpp/allocator_shim.cc

Project Member

Comment 30 by bugdroid1@chromium.org, Dec 17

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8f905913aa947d543b9e40c39b6c787b404f63d3

commit 8f905913aa947d543b9e40c39b6c787b404f63d3
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Mon Dec 17 18:07:48 2018

Fix gn check failure

First move the #include to an #if defined(OS_WIN) block since it's only
used on windows, and add nogncheck because //components/gwp_asan/client
is only included on windows but gn check doesn't understand #if blocks.

Bug: 896019
Change-Id: Ia3b4ba79fee76412a3d1ad6de4badbcd60724bd9
Reviewed-on: https://chromium-review.googlesource.com/c/1379193
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Greg Thompson <grt@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617158}
[modify] https://crrev.com/8f905913aa947d543b9e40c39b6c787b404f63d3/chrome/app/chrome_main_delegate.cc

Project Member

Comment 31 by bugdroid1@chromium.org, Dec 18

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b

commit dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Dec 18 18:04:01 2018

GWP-ASan: Add allocator quarantine

This change adds the ability to quarantine allocations to the GWP-ASan
allocator. It works by reserving space for total_pages pages, but only
allowing max_allocations <= total_pages concurrent allocations. This
means that there total_pages-max_allocations additional reserved pages
with stack traces saved that are in a ‘quarantine’ state.

This change is based on a change to tcmalloc by Matt Morehouse.

Bug: 896019
Change-Id: I2b5326ef720cafae389ef498e19a05dcd1ca8e47
Reviewed-on: https://chromium-review.googlesource.com/c/1373141
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#617559}
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/client/guarded_page_allocator.cc
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/client/guarded_page_allocator.h
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/client/guarded_page_allocator_unittest.cc
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/client/guarded_page_allocator_win.cc
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/client/gwp_asan.cc
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/client/sampling_allocator_shims.cc
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/client/sampling_allocator_shims.h
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/client/sampling_allocator_shims_unittest.cc
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/common/allocator_state.cc
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/common/allocator_state.h
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/common/allocator_state_unittest.cc
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/components/gwp_asan/crash_handler/crash_handler_unittest.cc
[modify] https://crrev.com/dc1a9a5e87d5b8a55fe9ff2a05899128f0f8ea6b/docs/gwp_asan.md

Project Member

Comment 32 by bugdroid1@chromium.org, Dec 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/96a7b37fe389ef830a15069d88cca18d6e1af705

commit 96a7b37fe389ef830a15069d88cca18d6e1af705
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Dec 21 21:51:58 2018

GWP-ASan: Fix possible UAF race

I'm debugging a crash that looks like it should be a UAF but doesn't
have a free stack, it made me take a look at this code and reorder when
we write the slot metadata.

Bug: 896019
Change-Id: If06889feb86c2ff51107631ad573f3727a9e0044
Reviewed-on: https://chromium-review.googlesource.com/c/1388651
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618616}
[modify] https://crrev.com/96a7b37fe389ef830a15069d88cca18d6e1af705/components/gwp_asan/client/guarded_page_allocator.cc

Project Member

Comment 33 by bugdroid1@chromium.org, Dec 21

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2f2eefbe8c7c85510fdbaa3e92dc14b8fc9d3eb7

commit 2f2eefbe8c7c85510fdbaa3e92dc14b8fc9d3eb7
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Dec 21 23:13:35 2018

GWP-ASan: Update default sampling frequency

My current experiment indicates this is the best default parameter to
use in terms of performance versus crashes.

Bug: 896019
Change-Id: I59028eba3bc994970d49b031e547a078f9702ba0
Reviewed-on: https://chromium-review.googlesource.com/c/1389020
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#618659}
[modify] https://crrev.com/2f2eefbe8c7c85510fdbaa3e92dc14b8fc9d3eb7/components/gwp_asan/client/gwp_asan.cc

Blockedon: 917804
Project Member

Comment 35 by bugdroid1@chromium.org, Dec 28

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f9c9065adfb3b13520ee329f2a6a477ff606dbd0

commit f9c9065adfb3b13520ee329f2a6a477ff606dbd0
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Dec 28 21:15:03 2018

GWP-ASan: Fix typos and outdated docs

The command line field trial parameters are outdated since the introduction of
the quarantine.

Bug: 896019
Change-Id: I4e1900d35580f3dd8ae3194e12ecf90000f7da24
Reviewed-on: https://chromium-review.googlesource.com/c/1390754
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#619192}
[modify] https://crrev.com/f9c9065adfb3b13520ee329f2a6a477ff606dbd0/components/gwp_asan/README.md
[modify] https://crrev.com/f9c9065adfb3b13520ee329f2a6a477ff606dbd0/docs/gwp_asan.md

Project Member

Comment 36 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9c33740e2875a4a60f45102e396ac34ef0d3d09a

commit 9c33740e2875a4a60f45102e396ac34ef0d3d09a
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Mon Jan 07 19:59:37 2019

GWP-ASan: Move overflow check into sampling path

Currently the presence of GWP-ASan could change how an integer overflow
in the calloc() parameters would be handled. This is probably not the
correct behavior, if we want to have consistent integer overflow checks
we need to implement them in the allocator layer, not here.

Bug: 896019
Change-Id: I709d822dc8dfcdbbefc6361d9c5c786bcd74854d
Reviewed-on: https://chromium-review.googlesource.com/c/1398294
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620437}
[modify] https://crrev.com/9c33740e2875a4a60f45102e396ac34ef0d3d09a/components/gwp_asan/client/sampling_allocator_shims.cc

Project Member

Comment 37 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4a45d566dbdda96197c3870a5a5de5706c37043c

commit 4a45d566dbdda96197c3870a5a5de5706c37043c
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Mon Jan 07 20:54:58 2019

GWP-ASan: Simplify test routine

Bug: 896019
Change-Id: I671c3436431ea612b32c4198d5788051c3eafe0a
Reviewed-on: https://chromium-review.googlesource.com/c/1388386
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620463}
[modify] https://crrev.com/4a45d566dbdda96197c3870a5a5de5706c37043c/components/gwp_asan/crash_handler/crash_handler_unittest.cc

Project Member

Comment 38 by bugdroid1@chromium.org, Jan 7

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/87c505975dce20361b62c10f3b8c1b11c1335e5e

commit 87c505975dce20361b62c10f3b8c1b11c1335e5e
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Mon Jan 07 21:22:32 2019

GWP-ASan: Use ring buffer to store free slots

The current method for finding a free slot is to pick a random starting
point in a bitmap and scan left or right. This has the disadvantage of
being O(n) AND relying on a system entropy source which could
potentially call into allocation methods while a lock is held. Instead,
store free slots in a ring buffer.

Bug: 896019, 917804
Change-Id: I514c6b5f65c4f29b61f90d0bf2c73b4d32211e99
Reviewed-on: https://chromium-review.googlesource.com/c/1395889
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620474}
[modify] https://crrev.com/87c505975dce20361b62c10f3b8c1b11c1335e5e/components/gwp_asan/client/guarded_page_allocator.cc
[modify] https://crrev.com/87c505975dce20361b62c10f3b8c1b11c1335e5e/components/gwp_asan/client/guarded_page_allocator.h

Project Member

Comment 39 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/cc792e6f0705b8fef1356c6c3ca7e34e08c32a85

commit cc792e6f0705b8fef1356c6c3ca7e34e08c32a85
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Jan 08 00:48:17 2019

GWP-ASan: Change GetGpa initialization

Currently the GuardedPageAllocator behind GetGpa() is initialized using
a function-local static. On a recent dev build this requires 5 memory
loads to accomplish and prevents inlining--changing the behavior
replaces the entire function with an inlined variable reference.

Bug: 896019
Change-Id: I7ad0f4077c70d40610ae3fdea884876e2bfdb50d
Reviewed-on: https://chromium-review.googlesource.com/c/1395979
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620555}
[modify] https://crrev.com/cc792e6f0705b8fef1356c6c3ca7e34e08c32a85/components/gwp_asan/client/guarded_page_allocator.h
[modify] https://crrev.com/cc792e6f0705b8fef1356c6c3ca7e34e08c32a85/components/gwp_asan/client/sampling_allocator_shims.cc

Project Member

Comment 40 by bugdroid1@chromium.org, Jan 8

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a2c9f59b2bbfc7495ea6520ff2145c6427892ac7

commit a2c9f59b2bbfc7495ea6520ff2145c6427892ac7
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Jan 08 18:07:28 2019

base/allocator: Fix missing _aligned_realloc check

_aligned_realloc(size=0) can return nullptr and should not cause a
possible failure. A unit test showed that it would currently not fail
anyway as WinCallNewHandler(0) returns false.

Bug: 912500, 896019
Change-Id: Ie3090e6b8d85cd581e9a1701882dcb35bde75fd2
Reviewed-on: https://chromium-review.googlesource.com/c/1396845
Reviewed-by: Will Harris <wfh@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620792}
[modify] https://crrev.com/a2c9f59b2bbfc7495ea6520ff2145c6427892ac7/base/allocator/allocator_shim.cc
[modify] https://crrev.com/a2c9f59b2bbfc7495ea6520ff2145c6427892ac7/base/allocator/allocator_shim_unittest.cc

Project Member

Comment 41 by bugdroid1@chromium.org, Jan 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8b711aa315bd41b0005de1eb5e3167490c1da171

commit 8b711aa315bd41b0005de1eb5e3167490c1da171
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Wed Jan 09 17:35:56 2019

GWP-ASan: Change sampling strategy

Currently GWP-ASan samples allocations when the counter in TLS reaches
zero. This heavily biases allocations towards the very first allocation
on thread start-up when the counter is set to zero by default. On
Windows the first allocation also happens to be long lived, limiting the
number of free slots and undersampling every other call site.

Change the sampling logic to fairly sample any given allocation,
including the first on thread start-up. This is possible by changing the
sampling logic to use a uniform distribution and forcing a sample to be
generated for new threads.

This has the side benefit of sidestepping an LLVM CodeGen bug and UB in
MSVC's implementation of std::poisson_distribution.

Bug: 896019, 919207
Change-Id: I3019ea36f71730a47be4847bf6ab3d114a75b0ce
Reviewed-on: https://chromium-review.googlesource.com/c/1400050
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621203}
[modify] https://crrev.com/8b711aa315bd41b0005de1eb5e3167490c1da171/components/gwp_asan/client/sampling_allocator_shims.cc
[modify] https://crrev.com/8b711aa315bd41b0005de1eb5e3167490c1da171/components/gwp_asan/client/sampling_allocator_shims_unittest.cc

Blockedon: 919207
Blockedon: 920440
Project Member

Comment 44 by bugdroid1@chromium.org, Jan 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/38a132967c0ebc3fa2a512a940c9789b5888efec

commit 38a132967c0ebc3fa2a512a940c9789b5888efec
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Jan 11 00:22:39 2019

Add GWP-ASan to field trial testing config.

Bug: 896019
Change-Id: I86c2b783518ab4f48268a9a993ea3053187fa4fc
Reviewed-on: https://chromium-review.googlesource.com/c/1388938
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Ilya Sherman <isherman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621822}
[modify] https://crrev.com/38a132967c0ebc3fa2a512a940c9789b5888efec/testing/variations/fieldtrial_testing_config.json

Project Member

Comment 45 by bugdroid1@chromium.org, Jan 11

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ad170133f41bfeda7531b88def96a85e7521dc9c

commit ad170133f41bfeda7531b88def96a85e7521dc9c
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Jan 11 00:55:09 2019

GWP-ASan: Eliminate duplicate test code

No functional change.

Bug: 896019
Change-Id: I095c9aebcc5cc511e7a7ac69a366b69cc4a350f6
Reviewed-on: https://chromium-review.googlesource.com/c/1405273
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#621837}
[modify] https://crrev.com/ad170133f41bfeda7531b88def96a85e7521dc9c/components/gwp_asan/crash_handler/crash_handler_unittest.cc

Project Member

Comment 46 by bugdroid1@chromium.org, Jan 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/bab4f10a0062cfda41acec7423696f3acba56851

commit bab4f10a0062cfda41acec7423696f3acba56851
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Sun Jan 13 06:21:48 2019

GWP-ASan: Report allocator region bounds

Recently while looking at crashes I have wished that I knew whether the
allocation I was looking at was the first/last allocation in the region.
Send the allocator bounds along in crash reports.

Bug: 896019
Change-Id: I1cfad6020f7a61292ba1c3faccf0700bcc425ab4
Reviewed-on: https://chromium-review.googlesource.com/c/1406294
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#622333}
[modify] https://crrev.com/bab4f10a0062cfda41acec7423696f3acba56851/components/gwp_asan/crash_handler/crash.proto
[modify] https://crrev.com/bab4f10a0062cfda41acec7423696f3acba56851/components/gwp_asan/crash_handler/crash_analyzer.cc
[modify] https://crrev.com/bab4f10a0062cfda41acec7423696f3acba56851/components/gwp_asan/crash_handler/crash_handler_unittest.cc

Project Member

Comment 47 by bugdroid1@chromium.org, Jan 17 (5 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c72ef12f9ff6132ce2368484883fa0a391807f43

commit c72ef12f9ff6132ce2368484883fa0a391807f43
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Thu Jan 17 21:23:01 2019

GWP-ASan: Optimize sampling using thread_local

After being assigned some performance regressions after enabling
GWP-ASan in the field trial testing config I benchmarked a number of
GWP-ASan configurations. I found that by turning down the sampling
frequency from the current value I could reduce the GWP-ASan overhead
but that it was still noticeable even with sampling values so high that
we only noticed the overhead of sampling in allocations (and performing
the range checks during frees though this cost is not noticeable.)

Currently sampling is performed by holding a counter in thread-local
storage and accessing TLS using system APIs. I found that using
thread_local to store the sampling counter I could reduce the overhead
of a micro-benchmark testing the sampling overhead by approximately
30-40%. ALWAYS_INLINE improves performance even further.

A bug on macOS has held up thread_local adoption in the C++11 allowed
features; however, I've disabled macOS support in order to get the
performance improvement on Windows and I'm working down the LLVM issue
blocking macOS adoption in parallel.

Bug: 922556, 920440, 896019
Change-Id: Ic35a0e44a25b27bcf3bf8a31234e9704f26b904b
Reviewed-on: https://chromium-review.googlesource.com/c/1416093
Auto-Submit: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623848}
[modify] https://crrev.com/c72ef12f9ff6132ce2368484883fa0a391807f43/components/gwp_asan/BUILD.gn
[modify] https://crrev.com/c72ef12f9ff6132ce2368484883fa0a391807f43/components/gwp_asan/client/BUILD.gn
[modify] https://crrev.com/c72ef12f9ff6132ce2368484883fa0a391807f43/components/gwp_asan/client/sampling_allocator_shims.cc
[delete] https://crrev.com/5bdea1a4780492ef2ae046f17b3ca9c64795711d/components/gwp_asan/client/sampling_allocator_shims_posix.h
[delete] https://crrev.com/5bdea1a4780492ef2ae046f17b3ca9c64795711d/components/gwp_asan/client/sampling_allocator_shims_win.h

Project Member

Comment 48 by bugdroid1@chromium.org, Jan 18 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9383cd3f0f2f36f9abfa38056ee9bb05cb438907

commit 9383cd3f0f2f36f9abfa38056ee9bb05cb438907
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Jan 18 19:40:14 2019

GWP-ASan: Remove call to VirtualProtect

Previously when I marked a page inaccessible I would first remove
read/write access and then decommit it (to save space used by the
quarantined pages.) I did this out of an excess of caution to ensure
that the page would actually be inaccessible as it was unclear to me if
MEM_DECOMMIT ensured that condition.

After testing and reading more I believe that MEM_DECOMMIT does
immediately make the page inaccessible. Removing the duplicate call to
VirtualProtect cuts 20% off a microbenchmark that runs a tight loop of
calls to GuardedPageAllocator::Allocate/Deallocate.

Bug: 896019
Change-Id: Iefa6b4ff65d99e8cf4b5c4fbe977a222f17ef0c3
Reviewed-on: https://chromium-review.googlesource.com/c/1422482
Auto-Submit: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624240}
[modify] https://crrev.com/9383cd3f0f2f36f9abfa38056ee9bb05cb438907/components/gwp_asan/client/guarded_page_allocator.h
[modify] https://crrev.com/9383cd3f0f2f36f9abfa38056ee9bb05cb438907/components/gwp_asan/client/guarded_page_allocator_win.cc

Project Member

Comment 49 by bugdroid1@chromium.org, Jan 18 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6b8849c12913ae09934a4f4cea658cd3c7088bed

commit 6b8849c12913ae09934a4f4cea658cd3c7088bed
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Jan 18 21:56:47 2019

GWP-ASan: Remove call to get entropy

We currently use a cryptographically secure entropy source to get 1 bit
of entropy to determine if we left- or right- align the allocation.
Instead, just use the low bit of the slot index to determine how to
align the allocation as the allocation->slot mapping should be random
anyhow.

Bug: 896019
Change-Id: I2574278aeab42ec66b57366eaaec912d4c0046e5
Reviewed-on: https://chromium-review.googlesource.com/c/1422479
Auto-Submit: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624307}
[modify] https://crrev.com/6b8849c12913ae09934a4f4cea658cd3c7088bed/components/gwp_asan/client/guarded_page_allocator.cc

Project Member

Comment 50 by bugdroid1@chromium.org, Jan 18 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ab1c623121fc3b4b55262ad6556fe730e1214656

commit ab1c623121fc3b4b55262ad6556fe730e1214656
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Jan 18 22:55:30 2019

GWP-ASan: Marginal stack trace improvement

Both of these functions are only called from one place and by inlining
them we remove a stack trace frame from every single GWP-ASan
allocation/deallocation that doesn't add any useful context.

Bug: 896019
Change-Id: I1a4fa413174b3a7fd292690ffc8f5a21141b2029
Reviewed-on: https://chromium-review.googlesource.com/c/1423201
Auto-Submit: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624332}
[modify] https://crrev.com/ab1c623121fc3b4b55262ad6556fe730e1214656/components/gwp_asan/client/guarded_page_allocator.h

Project Member

Comment 51 by bugdroid1@chromium.org, Jan 18 (4 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/67d131145b18fa93a24e4102d611fd7d3a4b0d6f

commit 67d131145b18fa93a24e4102d611fd7d3a4b0d6f
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Fri Jan 18 23:14:23 2019

GWP-ASan: Remove unnecessary check

The condition this is defending against is no longer possible with the
quarantine + ring buffer. It's not possible to free an allocation and
immediately allocate it again (unless you have no quarantine at all
which will cause other problems.)

Bug: 896019
Change-Id: Ic732ee90f6af1a334a4f99f4324c2cb05737551d
Reviewed-on: https://chromium-review.googlesource.com/c/1421683
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Auto-Submit: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624347}
[modify] https://crrev.com/67d131145b18fa93a24e4102d611fd7d3a4b0d6f/components/gwp_asan/client/guarded_page_allocator.cc
[modify] https://crrev.com/67d131145b18fa93a24e4102d611fd7d3a4b0d6f/components/gwp_asan/common/allocator_state.h

Project Member

Comment 52 by bugdroid1@chromium.org, Yesterday (24 hours ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6e6402ad86db251d6a4b4d3ba59d1fe81066212d

commit 6e6402ad86db251d6a4b4d3ba59d1fe81066212d
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Jan 22 07:50:20 2019

Link to GWP-ASan hotlist in documentation

Bug: 896019
Change-Id: I3dd89c82f62806f494fc750f5062034f9bffc3e3
Reviewed-on: https://chromium-review.googlesource.com/c/1423750
Auto-Submit: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vitaly Buka <vitalybuka@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624741}
[modify] https://crrev.com/6e6402ad86db251d6a4b4d3ba59d1fe81066212d/docs/gwp_asan.md

Project Member

Comment 53 by bugdroid, Today (9 hours ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d67de4bf7864713373d14216d34e86e4f9695b28

commit d67de4bf7864713373d14216d34e86e4f9695b28
Author: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Date: Tue Jan 22 23:25:14 2019

GWP-ASan: Allocate SlotMetadata array dynamically

Dynamically allocating SlotMetadata reduces the amount of memory
consumed in the current default configuration by ~6 kilobytes, or
about 13% of the GWP-ASan memory overhead. Furthermore, it sets up the
following change that eliminates the use of base::debug::StackTrace
and places the addresses directly in the SlotMetadata.

Bug: 896019, 921237, 923050
Change-Id: Ib60533821011a7aaeccc8278bc0ed7d747474a06
Reviewed-on: https://chromium-review.googlesource.com/c/1427440
Reviewed-by: Vitaly Buka <vitalybuka@chromium.org>
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Auto-Submit: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624983}
[modify] https://crrev.com/d67de4bf7864713373d14216d34e86e4f9695b28/components/gwp_asan/client/guarded_page_allocator.cc
[modify] https://crrev.com/d67de4bf7864713373d14216d34e86e4f9695b28/components/gwp_asan/client/guarded_page_allocator.h
[modify] https://crrev.com/d67de4bf7864713373d14216d34e86e4f9695b28/components/gwp_asan/common/allocator_state.cc
[modify] https://crrev.com/d67de4bf7864713373d14216d34e86e4f9695b28/components/gwp_asan/common/allocator_state.h
[modify] https://crrev.com/d67de4bf7864713373d14216d34e86e4f9695b28/components/gwp_asan/common/allocator_state_unittest.cc
[modify] https://crrev.com/d67de4bf7864713373d14216d34e86e4f9695b28/components/gwp_asan/crash_handler/crash_analyzer.cc
[modify] https://crrev.com/d67de4bf7864713373d14216d34e86e4f9695b28/components/gwp_asan/crash_handler/crash_analyzer.h
[modify] https://crrev.com/d67de4bf7864713373d14216d34e86e4f9695b28/tools/metrics/histograms/enums.xml

Sign in to add a comment