[SyzyAsan] Enable Crashpad reporter as a 50/50 experiment.

syzygy/agent/asan/runtime.cc

Index: syzygy/agent/asan/runtime.cc
diff --git a/syzygy/agent/asan/runtime.cc b/syzygy/agent/asan/runtime.cc
index bd851fb2179d9d31842e50a5dc89d1da12678361..30a7045386ff7c5d2706bf39ccadf473a65d96c2 100644
--- a/syzygy/agent/asan/runtime.cc
+++ b/syzygy/agent/asan/runtime.cc
@@ -387,20 +387,35 @@ void LaunchMessageBox(const base::StringPiece& message) {
   ::MessageBoxA(nullptr, message.data(), nullptr, MB_OK | MB_ICONEXCLAMATION);
 }
 
-std::unique_ptr<ReporterInterface> CreateCrashReporter(AsanLogger* logger) {
+AsanFeatureSet GenerateRandomFeatureSet() {
+  AsanFeatureSet enabled_features =
+      static_cast<AsanFeatureSet>(base::RandGenerator(ASAN_FEATURE_MAX));
+  DCHECK_LT(enabled_features, ASAN_FEATURE_MAX);
+  enabled_features &= kAsanValidFeatures;
+  return enabled_features;
+}
+
+std::unique_ptr<ReporterInterface> CreateCrashReporter(
+    AsanLogger* logger, AsanFeatureSet* feature_set) {
+  DCHECK_NE(static_cast<AsanLogger*>(nullptr), logger);
+  DCHECK_NE(static_cast<AsanFeatureSet*>(nullptr), feature_set);
   std::unique_ptr<ReporterInterface> reporter;
 
+  bool created_crashpad = false;
+
   // First try to grab the preferred crash reporter, overridden by the
   // environment.
   std::unique_ptr<base::Environment> env(base::Environment::Create());
   std::string reporter_name;
   if (env->GetVar("SYZYASAN_CRASH_REPORTER", &reporter_name)) {
-    if (reporter_name == "crashpad")
+    if (reporter_name == "crashpad") {
       reporter.reset(reporters::CrashpadReporter::Create().release());
-    else if (reporter_name == "kasko")
+      created_crashpad = (reporter.get() != nullptr);
+    } else if (reporter_name == "kasko") {
       reporter.reset(reporters::KaskoReporter::Create().release());
-    else if (reporter_name == "breakpad")
+    } else if (reporter_name == "breakpad") {
       reporter.reset(reporters::BreakpadReporter::Create().release());
+    }
 
     if (reporter.get() != nullptr) {
       logger->Write(base::StringPrintf(
@@ -415,9 +430,12 @@ std::unique_ptr<ReporterInterface> CreateCrashReporter(AsanLogger* logger) {
   // No crash reporter was explicitly specified, or it was unable to be
   // initialized. Try all of them, starting with the most recent.
 
-  // Don't try grabbing a Crashpad reporter by default, as we're not yet
-  // ready to ship this.
-  // TODO(chrisha): Add Crashpad support behind a feature flag.
+  // Only create a crashpad reporter if the feature was enabled.
+  if (reporter.get() == nullptr &&
+      (*feature_set & ASAN_FEATURE_ENABLE_CRASHPAD)) {
+    reporter.reset(reporters::CrashpadReporter::Create().release());
+    created_crashpad = (reporter.get() != nullptr);
+  }
 
   // Try to initialize a Kasko crash reporter.
   if (reporter.get() == nullptr)
@@ -427,6 +445,15 @@ std::unique_ptr<ReporterInterface> CreateCrashReporter(AsanLogger* logger) {
   if (reporter.get() == nullptr)
     reporter.reset(reporters::BreakpadReporter::Create().release());
 
+  // Ensure the feature flag (which will end up being reported as an
+  // experiment group in instrumented Chromen) reflects whether or not a
+  // crashpad reporter is in use.
+  if (created_crashpad) {
+    *feature_set |= ASAN_FEATURE_ENABLE_CRASHPAD;
+  } else {
+    *feature_set &= ~ASAN_FEATURE_ENABLE_CRASHPAD;
+  }
+
   return reporter;
 }
 
@@ -483,19 +510,26 @@ bool AsanRuntime::SetUp(const std::wstring& flags_command_line) {
     return false;
   WindowsHeapAdapter::SetUp(heap_manager_.get());
 
-  if (params_.feature_randomization) {
-    AsanFeatureSet feature_set = GenerateRandomFeatureSet();
-    PropagateFeatureSet(feature_set);
+  AsanFeatureSet feature_set = 0;
+  if (params_.feature_randomization)
+    GenerateRandomFeatureSet();

[Sigurður Ásgeirsson, 2016/05/18 14:24:51]

uh??

[Sigurður Ásgeirsson, 2016/05/18 14:26:16]

That is to say - this looks like a noop to me?

[chrisha, 2016/05/18 14:32:57]

Indeed it is. Cut and paste error.

Also, as Seb pointed out, I totally spaced on writing the unittest. Ignore this
for now :)

+
+  // Creating the crash reporter may override a randomized feature, so do this
+  // before propagating paramters. The name 'disable_breakpad_reporting' is
+  // legacy; this actually means to disable all external crash reporting
+  // integration.
+  if (!params_.disable_breakpad_reporting) {
+    crash_reporter_.reset(
+        CreateCrashReporter(logger(), &feature_set).release());
   }
 
+  // Propagate randomized features if they're enabled.
+  if (params_.feature_randomization)
+    PropagateFeatureSet(feature_set);
+
   // Propagates the flags values to the different modules.
   PropagateParams();
 
-  // The name 'disable_breakpad_reporting' is legacy; this actually means to
-  // disable all external crash reporting integration.
-  if (!params_.disable_breakpad_reporting)
-    crash_reporter_.reset(CreateCrashReporter(logger()).release());
-
   // Set up the appropriate error handler depending on whether or not
   // we successfully found a crash reporter.
   if (crash_reporter_.get() != nullptr) {
@@ -961,14 +995,6 @@ void AsanRuntime::LogAsanErrorInfo(AsanErrorInfo* error_info) {
   }
 }
 
-AsanFeatureSet AsanRuntime::GenerateRandomFeatureSet() {
-  AsanFeatureSet enabled_features =
-      static_cast<AsanFeatureSet>(base::RandGenerator(ASAN_FEATURE_MAX));
-  DCHECK_LT(enabled_features, ASAN_FEATURE_MAX);
-  enabled_features &= kAsanValidFeatures;
-  return enabled_features;
-}
-
 void AsanRuntime::PropagateFeatureSet(AsanFeatureSet feature_set) {
   DCHECK_EQ(0U, feature_set & ~kAsanValidFeatures);
   heap_manager_->enable_page_protections_ =