Commit bb172d1e authored by Brian Sheedy's avatar Brian Sheedy Committed by Commit Bot

Revert "Add ModuleCache API contract checks"

This reverts commit affe1977.

Reason for revert: Causing flaky crashes on Mac, see
https://bugs.chromium.org/p/chromium/issues/detail?id=1085222#c28.

Original change's description:
> Add ModuleCache API contract checks
>
> Adds checks to ensure early detection of API contract violations that
> would lead to use-after-frees.
>
> Bug: 1127466
> Change-Id: Ic73b30f115f5206572a427ac44674ff6a9b9f249
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2425523
> Commit-Queue: Mike Wittman <wittman@chromium.org>
> Reviewed-by: Etienne Pierre-Doray <etiennep@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#809987}

TBR=wittman@chromium.org,etiennep@chromium.org

Change-Id: Iae2cd6197448b8f54e8ba6918979c1b6f43157b5
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1127466, 1085222
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2429867Reviewed-by: default avatarBrian Sheedy <bsheedy@chromium.org>
Commit-Queue: Brian Sheedy <bsheedy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#810291}
parent 2e94ca8d
......@@ -39,11 +39,8 @@ const ModuleCache::Module* ModuleCache::GetModuleForAddress(uintptr_t address) {
std::unique_ptr<const Module> new_module = CreateModuleForAddress(address);
if (!new_module)
return nullptr;
const auto result = native_modules_.insert(std::move(new_module));
// Ensure that the new module was inserted an isn't equivalent to an existing
// module.
DCHECK(result.second);
return result.first->get();
const auto loc = native_modules_.insert(std::move(new_module));
return loc.first->get();
}
std::vector<const ModuleCache::Module*> ModuleCache::GetModules() const {
......@@ -90,27 +87,12 @@ void ModuleCache::UpdateNonNativeModules(
// Insert the modules to be added. This operation is O((m + a) + a*log(a))
// where m is the number of current modules and a is the number of modules to
// be added.
const size_t prior_non_native_modules_size = non_native_modules_.size();
non_native_modules_.insert(std::make_move_iterator(new_modules.begin()),
std::make_move_iterator(new_modules.end()));
// Every module in |new_modules| should have been moved into
// |non_native_modules_|. This guards against use-after-frees if |new_modules|
// were to contain any modules equivalent to what's already in
// |non_native_modules_|, in which case the module would remain in
// |new_modules| and be deleted on return from the function. While this
// scenario would be a violation of the API contract, it would present a
// difficult-to-track-down crash scenario.
CHECK_EQ(prior_non_native_modules_size + new_modules.size(),
non_native_modules_.size());
}
void ModuleCache::AddCustomNativeModule(std::unique_ptr<const Module> module) {
const bool was_inserted = native_modules_.insert(std::move(module)).second;
// |module| should have been inserted into |native_modules_|, indicating that
// there was no equivalent module already present. While this scenario would
// be a violation of the API contract, it would present a
// difficult-to-track-down crash scenario.
CHECK(was_inserted);
native_modules_.insert(std::move(module));
}
const ModuleCache::Module* ModuleCache::GetExistingModuleForAddress(
......
......@@ -88,10 +88,7 @@ class BASE_EXPORT ModuleCache {
// specifically they no longer participate in the GetModuleForAddress()
// lookup. They continue to exist for the lifetime of the ModuleCache,
// however, so that existing references to them remain valid. Modules in
// |new_modules| are added to the set of active non-native modules. Modules in
// |new_modules| may not overlap with any non-native Modules already present
// in ModuleCache, unless those modules are provided in |defunct_modules| in
// the same call.
// |new_modules| are added to the set of active non-native modules.
void UpdateNonNativeModules(
const std::vector<const Module*>& defunct_modules,
std::vector<std::unique_ptr<const Module>> new_modules);
......@@ -99,8 +96,6 @@ class BASE_EXPORT ModuleCache {
// Adds a custom native module to the cache. This is intended to support
// native modules that require custom handling. In general, native modules
// will be found and added automatically when invoking GetModuleForAddress().
// |module| may not overlap with any native Modules already present in
// ModuleCache.
void AddCustomNativeModule(std::unique_ptr<const Module> module);
// Gets the module containing |address| if one already exists, or nullptr
......
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