Skip to content

GitLab

T
tangled
Commit 56cfeac9 authored by Brian White's avatar Brian White Committed by Commit Bot
Browse files
Options

Remove Freed Object Tracker. 

The root cause of the Android "core trampler" has been found.  This
reverts the tracker used to record what objects were previously
allocated in the memory block used by the object being overwritten.

While the general "freed_object_tracker" module is generic and could
be re-used should similar tramplers appear, it is best not to leave
code in the build that is not actually being used.  Feel free to
ressurect this code in the future if there is a need for it.

TBR=asvitkine
(on vacation)

Bug: 744734
Change-Id: I4f8224253365ac6c5e08f12401104593bf090588
Reviewed-on: https://chromium-review.googlesource.com/688043
Commit-Queue: Brian White <bcwhite@chromium.org>
Reviewed-by: default avatarPrimiano Tucci <primiano@chromium.org>
Reviewed-by: default avatarMark Mentovai <mark@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505371}
parent a8b85626
Hide whitespace changes
Inline Side-by-side
Showing with 0 additions and 343 deletions
base/BUILD.gn
View file @ 56cfeac9
......@@ -924,8 +924,6 @@ component("base") {
"trace_event/common/trace_event_common.h",
"trace_event/event_name_filter.cc",
"trace_event/event_name_filter.h",
"trace_event/freed_object_tracker.cc",
"trace_event/freed_object_tracker.h",
"trace_event/heap_profiler.h",
"trace_event/heap_profiler_allocation_context.cc",
"trace_event/heap_profiler_allocation_context.h",
......@@ -2199,7 +2197,6 @@ test("base_unittests") {
"tools_sanity_unittest.cc",
"trace_event/blame_context_unittest.cc",
"trace_event/event_name_filter_unittest.cc",
"trace_event/freed_object_tracker_unittest.cc",
"trace_event/heap_profiler_allocation_context_tracker_unittest.cc",
"trace_event/heap_profiler_allocation_register_unittest.cc",
"trace_event/heap_profiler_heap_dump_writer_unittest.cc",
......
base/metrics/histogram.cc
View file @ 56cfeac9
......@@ -35,11 +35,6 @@
#include "base/values.h"
#include "build/build_config.h"
#if !defined(OS_NACL) // crbug/744734
#include "base/trace_event/freed_object_tracker.h"
#include "base/trace_event/heap_profiler_allocation_context.h"
#endif
namespace base {
namespace {
......@@ -591,45 +586,6 @@ bool Histogram::ValidateHistogramContents(bool crash_if_invalid,
std::string debug_string = base::StringPrintf(
"%s/%" PRIu32 "#%d", histogram_name().c_str(), bad_fields, identifier);
#if !defined(OS_NACL)
// Bad dummy field: look up the previous object at "this" address.
// Bad name string: look up the previous object at its "data" address.
base::trace_event::AllocationRegister::Allocation allocation;
if (((bad_fields & (1 << kDummyField)) != 0 &&
base::trace_event::FreedObjectTracker::GetInstance()->Get(
this, &allocation)) ||
((bad_fields & (1 << kHistogramNameField)) != 0 &&
base::trace_event::FreedObjectTracker::GetInstance()->Get(
histogram_name().data(), &allocation))) {
debug_string.reserve(10000);
if (allocation.context.type_name) {
debug_string += " (";
debug_string += allocation.context.type_name;
debug_string += ")";
}
for (int i = allocation.context.backtrace.frame_count - 1; i >= 0; --i) {
base::trace_event::StackFrame& frame =
allocation.context.backtrace.frames[i];
switch (frame.type) {
case base::trace_event::StackFrame::Type::TRACE_EVENT_NAME:
debug_string += " EV=\"";
debug_string += static_cast<const char*>(frame.value);
debug_string += "\"";
break;
case base::trace_event::StackFrame::Type::THREAD_NAME:
debug_string += " T=\"";
debug_string += static_cast<const char*>(frame.value);
debug_string += "\"";
break;
case base::trace_event::StackFrame::Type::PROGRAM_COUNTER:
debug_string += " PC:";
debug_string += StringPrintf(
"%08" PRIXPTR, reinterpret_cast<uintptr_t>(frame.value));
break;
}
}
} else {
debug_string += " [no previous allocation]";
}
base::debug::ScopedCrashKey crash_key("bad_histogram", debug_string);
#endif
CHECK(false) << debug_string;
......
base/trace_event/freed_object_tracker.cc deleted 100644 → 0
View file @ a8b85626
// Copyright 2017 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/trace_event/freed_object_tracker.h"
#include "base/allocator/allocator_shim.h"
#include "base/allocator/features.h"
#include "base/lazy_instance.h"
#include "base/logging.h"
#include "base/trace_event/heap_profiler_allocation_context.h"
#include "base/trace_event/heap_profiler_allocation_context_tracker.h"
#include "build/build_config.h"
namespace base {
namespace trace_event {
namespace {
#if BUILDFLAG(USE_ALLOCATOR_SHIM)
using base::allocator::AllocatorDispatch;
void* HookAlloc(const AllocatorDispatch* self, size_t size, void* context) {
const AllocatorDispatch* const next = self->next;
return next->alloc_function(next, size, context);
}
void* HookZeroInitAlloc(const AllocatorDispatch* self,
size_t n,
size_t size,
void* context) {
const AllocatorDispatch* const next = self->next;
return next->alloc_zero_initialized_function(next, n, size, context);
}
void* HookAllocAligned(const AllocatorDispatch* self,
size_t alignment,
size_t size,
void* context) {
const AllocatorDispatch* const next = self->next;
return next->alloc_aligned_function(next, alignment, size, context);
}
void* HookRealloc(const AllocatorDispatch* self,
void* address,
size_t size,
void* context) {
const AllocatorDispatch* const next = self->next;
void* new_addr = next->realloc_function(next, address, size, context);
if (size == 0 || new_addr != address)
FreedObjectTracker::GetInstance()->RemoveAllocation(address);
return new_addr;
}
void HookFree(const AllocatorDispatch* self, void* address, void* context) {
if (address)
FreedObjectTracker::GetInstance()->RemoveAllocation(address);
const AllocatorDispatch* const next = self->next;
next->free_function(next, address, context);
}
size_t HookGetSizeEstimate(const AllocatorDispatch* self,
void* address,
void* context) {
const AllocatorDispatch* const next = self->next;
return next->get_size_estimate_function(next, address, context);
}
unsigned HookBatchMalloc(const AllocatorDispatch* self,
size_t size,
void** results,
unsigned num_requested,
void* context) {
const AllocatorDispatch* const next = self->next;
return next->batch_malloc_function(next, size, results, num_requested,
context);
}
void HookBatchFree(const AllocatorDispatch* self,
void** to_be_freed,
unsigned num_to_be_freed,
void* context) {
for (unsigned i = 0; i < num_to_be_freed; ++i) {
FreedObjectTracker::GetInstance()->RemoveAllocation(to_be_freed[i]);
}
const AllocatorDispatch* const next = self->next;
next->batch_free_function(next, to_be_freed, num_to_be_freed, context);
}
void HookFreeDefiniteSize(const AllocatorDispatch* self,
void* address,
size_t size,
void* context) {
if (address)
FreedObjectTracker::GetInstance()->RemoveAllocation(address);
const AllocatorDispatch* const next = self->next;
next->free_definite_size_function(next, address, size, context);
}
AllocatorDispatch g_allocator_hooks = {
&HookAlloc, /* alloc_function */
&HookZeroInitAlloc, /* alloc_zero_initialized_function */
&HookAllocAligned, /* alloc_aligned_function */
&HookRealloc, /* realloc_function */
&HookFree, /* free_function */
&HookGetSizeEstimate, /* get_size_estimate_function */
&HookBatchMalloc, /* batch_malloc_function */
&HookBatchFree, /* batch_free_function */
&HookFreeDefiniteSize, /* free_definite_size_function */
nullptr, /* next */
};
#endif // BUILDFLAG(USE_ALLOCATOR_SHIM)
} // namespace
LazyInstance<FreedObjectTracker>::Leaky g_freed_object_tracker;
// static
FreedObjectTracker* FreedObjectTracker::GetInstance() {
return g_freed_object_tracker.Pointer();
}
FreedObjectTracker::FreedObjectTracker() {}
FreedObjectTracker::~FreedObjectTracker() {}
void FreedObjectTracker::Enable() {
#if BUILDFLAG(USE_ALLOCATOR_SHIM)
allocation_register_.SetEnabled();
allocator::InsertAllocatorDispatch(&g_allocator_hooks);
#if !DCHECK_IS_ON()
// This causes a NOTREACHED exception in OnThreadExitInternal trying to call
// a destructor but it works fine when DCHECK isn't enabled.
base::trace_event::AllocationContextTracker::SetCaptureMode(
base::trace_event::AllocationContextTracker::CaptureMode::NATIVE_STACK);
#endif
#endif
}
void FreedObjectTracker::DisableForTesting() {
#if BUILDFLAG(USE_ALLOCATOR_SHIM)
allocation_register_.SetDisabled();
allocator::RemoveAllocatorDispatchForTesting(&g_allocator_hooks);
#endif
}
bool FreedObjectTracker::Get(
const void* address,
AllocationRegister::Allocation* out_allocation) const {
if (!allocation_register_.is_enabled())
return false;
return allocation_register_.Get(address, out_allocation);
}
void FreedObjectTracker::RemoveAllocation(void* address) {
if (!allocation_register_.is_enabled())
return;
auto* tracker = AllocationContextTracker::GetInstanceForCurrentThread();
if (!tracker)
return;
AllocationContext context;
if (!tracker->GetContextSnapshot(&context))
return;
// Original size is unknown. 0 won't be recorded.
allocation_register_.Insert(address, 1, context);
}
} // namespace trace_event
} // namespace base
base/trace_event/freed_object_tracker.h deleted 100644 → 0
View file @ a8b85626
// Copyright 2017 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.
#ifndef BASE_METRICS_FREED_OBJECT_TRACKER_H_
#define BASE_METRICS_FREED_OBJECT_TRACKER_H_
#include "base/macros.h"
#include "base/trace_event/heap_profiler_allocation_register.h"
#include "base/trace_event/sharded_allocation_register.h"
namespace base {
template <typename T>
struct LazyInstanceTraitsBase;
namespace trace_event {
// Records what objects were freed so use-after-free knows where to look. This
// only tracks the last object freed from a given memory location so could be
// incorrect if too much time passes between being freed and being accessed.
// This is most accurate with BUILDFLAG(CAN_UNWIND_WITH_FRAME_POINTERS) set
// by 64-bit builds or "enable_profiling=true".
class BASE_EXPORT FreedObjectTracker {
public:
static FreedObjectTracker* GetInstance();
void Enable();
void DisableForTesting();
// Gets information about an allocation. The |address| has to be an exact
// match for the original allocation; no searching is done for a larger
// allocation that encompasses that address. Returns false if no match
// is found.
bool Get(const void* address,
AllocationRegister::Allocation* out_allocation) const;
// This is public because it's called from the anonymous namespace.
void RemoveAllocation(void* address);
private:
friend struct base::LazyInstanceTraitsBase<FreedObjectTracker>;
FreedObjectTracker();
~FreedObjectTracker();
ShardedAllocationRegister allocation_register_;
DISALLOW_COPY_AND_ASSIGN(FreedObjectTracker);
};
} // namespace trace_event
} // namespace base
#endif // BASE_METRICS_FREED_OBJECT_TRACKER_H_
base/trace_event/freed_object_tracker_unittest.cc deleted 100644 → 0
View file @ a8b85626
// Copyright 2017 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/trace_event/freed_object_tracker.h"
#include <memory>
#include "base/allocator/allocator_shim.h"
#include "base/allocator/features.h"
#include "build/build_config.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace base {
namespace trace_event {
class FreedObjectTrackerTest : public testing::Test {
public:
FreedObjectTrackerTest() {}
~FreedObjectTrackerTest() override {}
// A 24-byte class for testing.
struct Size24 {
char bytes[24];
};
void SetUp() override {
#if defined(OS_MACOSX) && BUILDFLAG(USE_ALLOCATOR_SHIM)
allocator::InitializeAllocatorShim();
#endif
FreedObjectTracker::GetInstance()->Enable();
}
void TearDown() override {
FreedObjectTracker::GetInstance()->DisableForTesting();
}
};
#if BUILDFLAG(USE_ALLOCATOR_SHIM)
#define MAYBE_Get Get
#else
#define MAYBE_Get DISABLED_Get
#endif
TEST_F(FreedObjectTrackerTest, MAYBE_Get) {
FreedObjectTracker* tracker = FreedObjectTracker::GetInstance();
AllocationRegister::Allocation allocation;
std::unique_ptr<Size24> obj = std::make_unique<Size24>();
Size24* objptr = obj.get();
ASSERT_FALSE(tracker->Get(objptr, &allocation));
obj.reset();
ASSERT_TRUE(tracker->Get(objptr, &allocation));
EXPECT_EQ(objptr, allocation.address);
EXPECT_LT(0U, allocation.context.backtrace.frame_count);
}
} // namespace trace_event
} // namespace base
chrome/browser/chrome_browser_field_trials.cc
View file @ 56cfeac9
......@@ -19,7 +19,6 @@
#include "base/strings/string_util.h"
#include "base/task_scheduler/post_task.h"
#include "base/time/time.h"
#include "base/trace_event/freed_object_tracker.h"
#include "build/build_config.h"
#include "chrome/browser/metrics/chrome_metrics_service_client.h"
#include "chrome/browser/metrics/chrome_metrics_services_manager_client.h"
......@@ -39,9 +38,6 @@
namespace {
const base::Feature kFreedObjectTrackerFeature{
"FreedObjectTracker", base::FEATURE_DISABLED_BY_DEFAULT};
// Creating a "spare" file for persistent metrics involves a lot of I/O and
// isn't important so delay the operation for a while after startup.
#if defined(OS_ANDROID)
......@@ -242,9 +238,5 @@ void ChromeBrowserFieldTrials::InstantiateDynamicTrials() {
// Persistent histograms must be enabled as soon as possible.
InstantiatePersistentHistograms();
// Enable use-after-free information gathering.
if (base::FeatureList::IsEnabled(kFreedObjectTrackerFeature))
base::trace_event::FreedObjectTracker::GetInstance()->Enable();
tracing::SetupBackgroundTracingFieldTrial();
}
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