Commit 088be1e6 authored by Vlad Tsyrklevich's avatar Vlad Tsyrklevich Committed by Commit Bot

GWP-ASan: Increase allocations on canary/dev

In r624120 I reduced the number of allocations used by GWP-ASan from
MaxAllocations/TotalPages 32/128 to 6/20. These are good parameters for
widespread use to limit the amount of memory/perf regression on stable;
however, it makes it much less likely GWP-ASan will catch newly
introduced bugs on canary/dev. Add a multiplier by which to increase the
GWP-ASan parameters on canary/dev to ensure we maintain good coverage of
new code.

Bug: 896019
Change-Id: I1ce59c25878ef0c066e04a0dedeba1f3aa97ec0d
Reviewed-on: https://chromium-review.googlesource.com/c/1429240
Commit-Queue: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Reviewed-by: default avatarVitaly Buka <vitalybuka@chromium.org>
Reviewed-by: default avatarKen Rockot <rockot@google.com>
Auto-Submit: Vlad Tsyrklevich <vtsyrklevich@chromium.org>
Cr-Commit-Position: refs/heads/master@{#626105}
parent c3ce84e0
......@@ -530,7 +530,10 @@ bool ChromeMainDelegate::ShouldCreateFeatureList() {
void ChromeMainDelegate::PostFieldTrialInitialization() {
#if defined(OS_WIN)
gwp_asan::EnableForMalloc();
version_info::Channel channel = chrome::GetChannel();
bool is_canary_dev = (channel == version_info::Channel::CANARY ||
channel == version_info::Channel::DEV);
gwp_asan::EnableForMalloc(is_canary_dev);
#endif
}
......
......@@ -4,12 +4,15 @@
#include "components/gwp_asan/client/gwp_asan.h"
#include <algorithm>
#include <limits>
#include "base/debug/crash_logging.h"
#include "base/feature_list.h"
#include "base/logging.h"
#include "base/macros.h"
#include "base/metrics/field_trial_params.h"
#include "base/numerics/safe_conversions.h"
#include "base/numerics/safe_math.h"
#include "base/rand_util.h"
#include "base/strings/stringprintf.h"
#include "components/gwp_asan/client/guarded_page_allocator.h"
......@@ -34,13 +37,20 @@ const base::FeatureParam<int> kAllocationSamplingParam{
const base::FeatureParam<double> kProcessSamplingParam{
&kGwpAsan, "ProcessSamplingProbability", 1.0};
bool EnableForMalloc() {
// The multiplier to increase MaxAllocations/TotalPages on canary/dev builds.
const base::FeatureParam<int> kCanaryDevMultiplierParam{
&kGwpAsan, "CanaryDevMultiplier", 5};
bool EnableForMalloc(bool is_canary_dev) {
if (!base::FeatureList::IsEnabled(kGwpAsan))
return false;
static_assert(AllocatorState::kGpaMaxPages <= std::numeric_limits<int>::max(),
"kGpaMaxPages out of range");
constexpr int kMaxPages = static_cast<int>(AllocatorState::kGpaMaxPages);
int total_pages = kTotalPagesParam.Get();
if (total_pages < 1 ||
total_pages > base::checked_cast<int>(AllocatorState::kGpaMaxPages)) {
if (total_pages < 1 || total_pages > kMaxPages) {
DLOG(ERROR) << "GWP-ASan TotalPages is out-of-range: " << total_pages;
return false;
}
......@@ -67,6 +77,31 @@ bool EnableForMalloc() {
return false;
}
int multiplier = 1;
if (is_canary_dev)
multiplier = kCanaryDevMultiplierParam.Get();
if (multiplier < 1 || multiplier > kMaxPages) {
DLOG(ERROR) << "GWP-ASan CanaryDevMultiplier is out-of-range: "
<< multiplier;
return false;
}
base::CheckedNumeric<int> total_pages_mult = total_pages;
total_pages_mult *= multiplier;
base::CheckedNumeric<int> max_allocations_mult = max_allocations;
max_allocations_mult *= multiplier;
if (!total_pages_mult.IsValid() || !max_allocations_mult.IsValid()) {
DLOG(ERROR) << "GWP-ASan multiplier caused out-of-range multiply: "
<< multiplier;
return false;
}
total_pages = std::min<int>(total_pages_mult.ValueOrDie(), kMaxPages);
max_allocations =
std::min<int>(max_allocations_mult.ValueOrDie(), total_pages);
if (base::RandDouble() >= process_sampling_probability)
return false;
......@@ -77,8 +112,8 @@ bool EnableForMalloc() {
} // namespace
} // namespace internal
void EnableForMalloc() {
static bool init_once = internal::EnableForMalloc();
void EnableForMalloc(bool is_canary_dev) {
static bool init_once = internal::EnableForMalloc(is_canary_dev);
ignore_result(init_once);
}
......
......@@ -10,8 +10,9 @@
namespace gwp_asan {
// Enable GWP-ASan for the current process. This should only be called once per
// process. This can not be disabled once it has been enabled.
GWP_ASAN_EXPORT void EnableForMalloc();
// process. This can not be disabled once it has been enabled. The caller should
// indicate whether this build is a canary or dev build.
GWP_ASAN_EXPORT void EnableForMalloc(bool is_canary_dev);
} // namespace gwp_asan
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment