Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alias settings_client internals to see it in a memory dump. #26654

Merged
merged 3 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 73 additions & 0 deletions chromium_src/base/debug/alias.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
/* Copyright (c) 2024 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_CHROMIUM_SRC_BASE_DEBUG_ALIAS_H_
#define BRAVE_CHROMIUM_SRC_BASE_DEBUG_ALIAS_H_

#include "base/containers/span.h"
#include "base/memory/raw_ptr_exclusion.h"
#include "base/memory/stack_allocated.h"
#include "base/ranges/algorithm.h"

#include "src/base/debug/alias.h" // IWYU pragma: export

namespace base::debug {

// StackObjectCopy creates a byte-for-byte copy of an object on the stack,
// allowing the object to be inspected in crash dumps even if the original
// object is optimized away by the compiler or is stored in protected memory.
// The copy maintains proper alignment and can be viewed as the original type in
// a debugger.
template <typename T>
class StackObjectCopy {
STACK_ALLOCATED();

public:
explicit StackObjectCopy(const T* original) {
if (original) {
// SAFETY: `original` is a valid pointer to a T object, and `kSize` is the
// size of T.
//
// Can't use `byte_span_from_ref` because `T` might be an abstract class.
auto original_object_memory = UNSAFE_BUFFERS(
make_span(reinterpret_cast<const uint8_t*>(original), kSize));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need make_span there. It can only be span I suspect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reported by reviewdog 🐶
[semgrep] Using reinterpret_cast against some data types may lead to undefined behaviour. In general, when needing to do these conversions, check how Chromium upstream does them. Most of the times a reinterpret_cast is wrong and there's no guarantee the compiler will generate the code that you thought it would.

Source: https://github.com/brave/security-action/blob/main/assets/semgrep_rules/client/reinterpret_cast.yaml


Cc @stoletheminerals @thypon @cdesouza-chromium

span(buffer_).copy_from(std::move(original_object_memory));
} else {
ranges::fill(buffer_, 0);
}
}

~StackObjectCopy() {
// Ensure the class isn't optimized away before destruction.
Alias(buffer_);
}

StackObjectCopy(const StackObjectCopy&) = delete;
StackObjectCopy& operator=(const StackObjectCopy&) = delete;

private:
constexpr static size_t kSize = sizeof(T);
constexpr static size_t kMinSize = 128;

alignas(T) uint8_t buffer_[kSize];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would that be enough to extract and store just vtable pointer?

RAW_PTR_EXCLUSION const T* const typed_view_ =
reinterpret_cast<const T*>(buffer_);
// Prevent the class from being optimized down to a register value.
const uint8_t padding_[kSize >= kMinSize ? 0 : kMinSize - kSize] = {};
};

} // namespace base::debug

// DEBUG_ALIAS_FOR_OBJECT creates a stack copy of an object and ensures it
// remains in memory for crash dumps. This is useful when you need to ensure an
// object's state is captured in crash reports, especially when the object might
// otherwise be optimized away or is not directly accessible.
//
// Usage: DEBUG_ALIAS_FOR_OBJECT(alias_name, pointer_to_object);
#define DEBUG_ALIAS_FOR_OBJECT(var_name, object) \
const ::base::debug::StackObjectCopy var_name(object); \
::base::debug::Alias(&var_name)

#endif // BRAVE_CHROMIUM_SRC_BASE_DEBUG_ALIAS_H_
3 changes: 3 additions & 0 deletions chromium_src/check_chromium_src_config.json5
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@
// These symbols found in associated paths will be ignored.
// Please, keep paths and symbols in alphabetical order.
"symbol_excludes": {
"base/debug/alias.h": [
"DEBUG_ALIAS_FOR_OBJECT",
],
"base/trace_event/builtin_categories.h": [
"BRAVE_INTERNAL_TRACE_LIST_BUILTIN_CATEGORIES",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ blink::WebContentSettingsClient* GetContentSettingsClientFor(
return worker_or_worklet->ContentSettingsClient();
}

base::debug::Alias(context);
DEBUG_ALIAS_FOR_OBJECT(context_alias, context);
NOTREACHED() << "Unhandled ExecutionContext type";
}

Expand Down Expand Up @@ -446,9 +446,9 @@ BraveFarblingLevel BraveSessionCache::GetBraveFarblingLevel(
GetContentSettingsClientFor(GetSupplementable())) {
auto shields_settings =
settings_client->GetBraveShieldsSettings(webcompat_content_settings);
// https://github.com/brave/brave-browser/issues/41724 debug.
// https://github.com/brave/brave-browser/issues/41889 debug.
if (!shields_settings) {
base::debug::Alias(settings_client);
DEBUG_ALIAS_FOR_OBJECT(settings_client_alias, settings_client);
cdesouza-chromium marked this conversation as resolved.
Show resolved Hide resolved
base::debug::DumpWithoutCrashing();
return default_shields_settings_->farbling_level;
}
Expand Down
Loading