Add debug info for field trial allocation crash

base/metrics/field_trial.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/metrics/field_trial.h"
 
 #include <algorithm>
 #include <utility>
 
 #include "base/base_switches.h"
@@ skipping 163 matching lines @@
 
 void AddForceFieldTrialsFlag(CommandLine* cmd_line) {
   std::string field_trial_states;
   FieldTrialList::AllStatesToString(&field_trial_states);
   if (!field_trial_states.empty()) {
     cmd_line->AppendSwitchASCII(switches::kForceFieldTrials,
                                 field_trial_states);
   }
 }
 
+// In order to debug the crash where two field trials with the same name but
+// different groups get added to the allocator, let's try to get the stack
+// frame where the second duplicate gets added here as well as the trial/group
+// names by going through the allocator and checking if a trial with the same
+// name already exists.
+void CheckAllocatorForTrial(FieldTrialList::FieldTrialAllocator* allocator,
+                            const std::string& trial_name,
+                            const std::string& group_name) {
+  FieldTrialList::FieldTrialAllocator::Iterator iter(allocator);
+  FieldTrial::FieldTrialRef ref;
+  while ((ref = iter.GetNextOfType(kFieldTrialType)) !=
+         FieldTrialList::FieldTrialAllocator::kReferenceNull) {
+    const FieldTrialEntry* entry =
+        allocator->GetAsObject<const FieldTrialEntry>(ref, kFieldTrialType);
+
+    StringPiece trial_name_to_check;
+    StringPiece group_name_to_check;
+    if (!entry->GetTrialAndGroupName(&trial_name_to_check,
+                                     &group_name_to_check)) {
+      CHECK(false);
+    }
+
+    std::string existing_trial_name = trial_name_to_check.as_string();

[brucedawson, 2016/11/21 22:07:00]

Why copy these to std::string objects? This requires allocating memory and I'm
not clear what it adds - why not strlcpy from the StringPiece objects?

[lawrencewu, 2016/11/22 15:17:54]

I think it's more readable this way, but yeah, since this method will be called
often we should probably focus more on resource usage. Fixed.

[brucedawson, 2016/11/22 17:55:24]

Thanks. Given the separate thread going on about the importance of reducing
allocation churn it seems particularly important to avoid unnecessary
allocations.

+    std::string existing_group_name = group_name_to_check.as_string();
+    if (existing_trial_name == trial_name) {
+      constexpr size_t buf_size = 100;
+
+      char trial_name_c_str[buf_size];
+      char group_name_c_str[buf_size];
+      char existing_group_name_c_str[buf_size];
+
+      strlcpy(trial_name_c_str, trial_name.c_str(), buf_size);
+      strlcpy(group_name_c_str, group_name.c_str(), buf_size);
+      strlcpy(existing_group_name_c_str, existing_group_name.c_str(), buf_size);
+
+      debug::Alias(trial_name_c_str);
+      debug::Alias(existing_group_name_c_str);
+      debug::Alias(group_name_c_str);

[brucedawson, 2016/11/21 22:07:00]

For consistency and to minimize confusion keep the same order in the
debug::Alias calls as for the declarations and strlcpy calls (i.e.; move
group_name_c_str up one).

[lawrencewu, 2016/11/22 15:17:54]

Done.

+      CHECK_EQ(existing_group_name, group_name);
+    }
+  }
+}
+
 #if defined(OS_WIN)
 HANDLE CreateReadOnlyHandle(FieldTrialList::FieldTrialAllocator* allocator) {
   HANDLE src = allocator->shared_memory()->handle().GetHandle();
   ProcessHandle process = GetCurrentProcess();
   DWORD access = SECTION_MAP_READ | SECTION_QUERY;
   HANDLE dst;
   if (!::DuplicateHandle(process, src, process, &dst, access, true, 0))
     return nullptr;
   return dst;
 }
@@ skipping 656 matching lines @@
       return false;
 
     // TODO(lawrencewu): Convert the API for CreateFieldTrial to take
     // StringPieces.
     FieldTrial* trial =
         CreateFieldTrial(trial_name.as_string(), group_name.as_string());
 
     // If we failed to create the field trial, crash with debug info.
     // TODO(665129): Remove this when the crash is resolved.
     if (!trial) {
+      constexpr size_t buf_size = 100;
+      char trial_name_c_str[buf_size] = {0};
+      char group_name_c_str[buf_size] = {0};
+      char existing_group_name_c_str[buf_size] = {0};
+
+      // Copy the names over to the stack.
       std::string trial_name_string = trial_name.as_string();
+      strlcpy(trial_name_c_str, trial_name_string.c_str(), buf_size);
       std::string group_name_string = group_name.as_string();
+      strlcpy(group_name_c_str, group_name_string.c_str(), buf_size);
+
+      // Alias the trial and group name.
+      debug::Alias(trial_name_c_str);
+      debug::Alias(group_name_c_str);
+
+      // Copy and alias the existing field trial name, if there is one.
       FieldTrial* existing_field_trial =
           FieldTrialList::Find(trial_name_string);
-      if (existing_field_trial)
-        debug::Alias(existing_field_trial->group_name_internal().c_str());
-      debug::Alias(trial_name_string.c_str());
-      debug::Alias(group_name_string.c_str());
+      if (existing_field_trial) {
+        std::string existing_group_name_string =
+            existing_field_trial->group_name_internal();
+        strlcpy(existing_group_name_c_str, group_name_string.c_str(), buf_size);
+        debug::Alias(existing_group_name_c_str);
+      }
       CHECK(!trial_name_string.empty());
       CHECK(!group_name_string.empty());
       CHECK_EQ(existing_field_trial->group_name_internal(),
                group_name.as_string());
       return false;
     }
 
     trial->ref_ = ref;
     if (entry->activated) {
       // Call |group()| to mark the trial as "used" and notify observers, if
@@ skipping 43 matching lines @@
 
   // Don't do anything if the allocator hasn't been instantiated yet.
   if (allocator == nullptr)
     return;
 
   // Or if the allocator is read only, which means we are in a child process and
   // shouldn't be writing to it.
   if (allocator->IsReadonly())
     return;
 
+  // TODO(665129): Remove this code once the bug has been fixed.
+  CheckAllocatorForTrial(allocator, field_trial->trial_name(),
+                         field_trial->group_name_internal());
+
   FieldTrial::State trial_state;
   if (!field_trial->GetStateWhileLocked(&trial_state))
     return;
 
   // Or if we've already added it. We must check after GetState since it can
   // also add to the allocator.
   if (field_trial->ref_)
     return;
 
   Pickle pickle;
@@ skipping 69 matching lines @@
     return;
   }
   AutoLock auto_lock(global_->lock_);
   CHECK(!global_->PreLockedFind(trial->trial_name())) << trial->trial_name();
   trial->AddRef();
   trial->SetTrialRegistered();
   global_->registered_[trial->trial_name()] = trial;
 }
 
 }  // namespace base