Issue doesn't repro if a --gtest_filter is specified which restricts things to just the FieldTrialListTests, either.

FWIW it looks like something (I think one of the FieldTrial tests) is leaving a FieldTrialList active with |overrides_| pointing out to freed stack or heap memory.

Not able to reproduce on Mac. Looking to resurrect my Windows build.

Actually, I was not passing the right flag on Mac. When I fixed that, I get other failures in base unit tests, with things like:

[----------] 1 test from SystemMetricsTest
[ RUN      ] SystemMetricsTest.LockedBytes
../../base/process/process_metrics_unittest.cc:86: Failure
Expected: (initial_locked_bytes + size * 1.5) > (new_locked_bytes), actual: 1.25829e+07 vs 16777216
[  FAILED  ] SystemMetricsTest.LockedBytes (10 ms)
[----------] 1 test from SystemMetricsTest (10 ms total)

And eventual crash here:

[----------] 1 test from SecurityTest
[ RUN      ] SecurityTest.NewOverflow
[15223:775:0915/164220.115664:256510574024538:FATAL:memory.cc(22)] Out of memory. size=0
0   libbase.dylib                       0x000000010e4bee0e base::debug::StackTrace::StackTrace(unsigned long) + 174
1   libbase.dylib                       0x000000010e4beecd base::debug::StackTrace::StackTrace(unsigned long) + 29
2   libbase.dylib                       0x000000010e4bd17c base::debug::StackTrace::StackTrace() + 28
3   libbase.dylib                       0x000000010e55ca9f logging::LogMessage::~LogMessage() + 479
4   libbase.dylib                       0x000000010e55a405 logging::LogMessage::~LogMessage() + 21
5   libbase.dylib                       0x000000010e67d9d2 base::(anonymous namespace)::OnNoMemory(unsigned long) + 226
6   libbase.dylib                       0x000000010e67d8e5 base::TerminateBecauseOutOfMemory(unsigned long) + 21
7   libbase.dylib                       0x000000010e67daad base::(anonymous namespace)::oom_killer_new() + 13
8   libc++abi.dylib                     0x00007fffacbb2e06 operator new(unsigned long) + 22
9   libc++abi.dylib                     0x00007fffacbb2eab operator new[](unsigned long, std::nothrow_t const&) + 11
10  base_unittests                      0x000000010bffa3e4 (anonymous namespace)::SecurityTest_NewOverflow_Test::TestBody() + 164
11  base_unittests                      0x000000010c84a49e void testing::internal::HandleSehExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 126
12  base_unittests                      0x000000010c825822 void testing::internal::HandleExceptionsInMethodIfSupported<testing::Test, void>(testing::Test*, void (testing::Test::*)(), char const*) + 114
13  base_unittests                      0x000000010c825756 testing::Test::Run() + 198

Anyway, that's orthogonal to this bug. For this one, running on Windows I can repro. However, when I specify gtest filter to run all the FieldTrialListTest's, it passes. So something from another test is likely leaking state.

Interestingly, the stack trace is about some ridiculous size being passed to a Pickle method.

Re #6: Yes, see comment #2. I dug into where the ridiculous size was coming from, and it appeared to be a use-after-free from the override's name string having been freed (IIUC) and presumably overwritten by something else.

+erikchen: Worth filing a bug for the LockedBytes test failure..?

Looks like the NewOverflow test is known not to be valid on Mac, FWIW (see https://cs.chromium.org/chromium/src/base/security_unittest.cc?gsn=OverflowTestsSoftExpectTrue&l=73).

For this bug, looks like the issue is FieldTrialParamTest.GetFieldTrialParamsByFeature ends up modifying the global FeatureList instance (rather than overriding it for testing), but using a testing FieldTrialList, so FeatureList ends up pointing at trials in the FieldTrialList that then get freed.

And this test in question happens to also use global FeatureList rather than creating one for testing.

I'll fix this on Monday. Thanks for the report!

Actually, it's a bit more subtle than that.

That test correctly uses ScopedFeatureList. However, the problem is the ScopedFeatureList dtor has this code:

  if (original_feature_list_) {
    FeatureList::ClearInstanceForTesting();
    FeatureList::RestoreInstanceForTesting(std::move(original_feature_list_));
  }

That means, if there was no FeatureList originally, it the FeatureList that was meant to be scoped doesn't get cleared and sticks around. So the one created by GetFieldTrialParamsByFeature ends up sticking around - while having invalid pointers to a FieldTrialList object that was destroyed.

Fix is to always clear the FeatureList, resetting back to null if that's how it was before. Question is how many tests rely on this behavior - I guess I'll find out on Monday when I look at fixing this.

Sent https://chromium-review.googlesource.com/c/chromium/src/+/671290 with the fix.

With that change, this issue is fixed, though there is a remaining issue on Windows when running with --single-process-tests - HighResolutionTimer.GetUsage which fails in the first EXPECT_EQ statement.

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

commit c49d1f40892bae0fa5ac10385ecb0e827409aa21
Author: Alexei Svitkine <asvitkine@chromium.org>
Date: Mon Sep 18 23:00:41 2017

Fix base_unittests crash on Windows with --single-process-tests.

The problem was caused by ScopedFeatureList not resetting the
FeatureList to null if it was that way prior to the scope. The
FeatureList that stuck around had references to FieldTrial objects
that were destroyed by a previous test and this caused a crash
when FieldTrialListTest.TestCopyFieldTrialStateToFlags went to use
them.

As part of the fix, also adds a ScopedFeatureList to that test
since it was previously relying on the left-over instance.

Finally, a lot of tests that used feature params were implicitly
relying on a FeatureList having been set (and leaked) by some
earlier test in the same batch. To keep these tests working
without modifying them all to have a ScopedFeatureList (when
they're not actually trying to test param override values), this
change updates FeatureList::GetFieldTrial() to have similar logic
to IsEnabled() when the FeatureList instance was not initialized.

With the new behavior, GetFieldTrial() - which is generally not
invoked directly but queried by the implementation of
base::GetFieldTrialParamValueByFeature() - will return a sane
default when FeatureList is not initialized. This way, it's
consistent with what we do for FeatureList::IsEnabled() - and in
both cases, they will set a bool that is checked to ensure that
once used that way, setting a FeatureList instance will be an
error.

BUG= 757651 

Change-Id: I79ce1ddcd000d1628c7396faee25ed848b1d8e1a
Reviewed-on: https://chromium-review.googlesource.com/671290
Reviewed-by: Wez <wez@chromium.org>
Commit-Queue: Alexei Svitkine <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502707}
[modify] https://crrev.com/c49d1f40892bae0fa5ac10385ecb0e827409aa21/base/feature_list.cc
[modify] https://crrev.com/c49d1f40892bae0fa5ac10385ecb0e827409aa21/base/metrics/field_trial_unittest.cc
[modify] https://crrev.com/c49d1f40892bae0fa5ac10385ecb0e827409aa21/base/test/scoped_feature_list.cc