Skip to content

Commit

Permalink
[PA] Make PA_BASE_*CHECK() not allocate memory.
Browse files Browse the repository at this point in the history 
To make PA_BASE_*CHECK() inside PartitionAlloc, make PA_BASE_*CHECK()
on-stack objects. This requires more stack size.
Looking at ThreadTest.StartWithOptions_StackSize, decided to make the
buffer size for logging 256u. (1024u causes the test's failure on
32bit bots)

This change also makes check.h depend on logging.h, but if check.h
includes logging.h, we will see "error: 'PA_LOG' macro redefined",
because chromeos/ash/components/multidevice/logging/logging.h
defines PA_LOG(). So split logging.h into logging.h and log_message.h,
and make check.h include log_message.h.

PS1 = copy logging.{h,cc} to log_message.{h,cc}

Change-Id: I65973b7482363cd6f77a6b7064cbb66fdd8a01e5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4806322
Reviewed-by: Yuki Shiino <yukishiino@chromium.org>
Reviewed-by: Kalvin Lee <kdlee@chromium.org>
Commit-Queue: Takashi Sakamoto <tasak@google.com>
Cr-Commit-Position: refs/heads/main@{#1188202}
  • Loading branch information
@tasak
tasak authored and Chromium LUCI CQ committed Aug 25, 2023
1 parent 0c5b1f5 commit 01bcc60
Show file tree
Hide file tree
Showing 8 changed files with 517 additions and 411 deletions.
2 changes: 2 additions & 0 deletions base/allocator/partition_allocator/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,6 +352,8 @@ source_set("allocator_base") {
"partition_alloc_base/export_template.h",
"partition_alloc_base/gtest_prod_util.h",
"partition_alloc_base/immediate_crash.h",
"partition_alloc_base/log_message.cc",
"partition_alloc_base/log_message.h",
"partition_alloc_base/logging.cc",
"partition_alloc_base/logging.h",
"partition_alloc_base/memory/page_size.h",
Expand Down
105 changes: 52 additions & 53 deletions base/allocator/partition_allocator/partition_alloc_base/check.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,77 +11,76 @@ namespace partition_alloc::internal::logging {
// TODO(1151236): Make CheckError not to allocate memory. So we can use
// CHECK() inside PartitionAllocator when PartitionAllocator-Everywhere is
// enabled. (Also need to modify LogMessage).
CheckError CheckError::Check(const char* file,
int line,
const char* condition) {
CheckError check_error(new LogMessage(file, line, LOGGING_FATAL));
check_error.stream() << "Check failed: " << condition << ". ";
return check_error;
}

CheckError CheckError::DCheck(const char* file,
int line,
const char* condition) {
CheckError check_error(new LogMessage(file, line, LOGGING_DCHECK));
check_error.stream() << "Check failed: " << condition << ". ";
return check_error;
CheckError::CheckError(const char* file,
int line,
LogSeverity severity,
const char* condition)
: log_message_(file, line, severity) {
log_message_.stream() << "Check failed: " << condition << ". ";
}

CheckError CheckError::PCheck(const char* file,
int line,
const char* condition) {
SystemErrorCode err_code = logging::GetLastSystemErrorCode();
#if BUILDFLAG(IS_WIN)
CheckError check_error(
new Win32ErrorLogMessage(file, line, LOGGING_FATAL, err_code));
#elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
CheckError check_error(
new ErrnoLogMessage(file, line, LOGGING_FATAL, err_code));
#endif
check_error.stream() << "Check failed: " << condition << ". ";
return check_error;
}
CheckError::CheckError(const char* file, int line, LogSeverity severity)
: log_message_(file, line, severity) {}

CheckError CheckError::PCheck(const char* file, int line) {
return PCheck(file, line, "");
CheckError::CheckError(const char* file,
int line,
LogSeverity severity,
const char* condition,
SystemErrorCode err_code)
: errno_log_message_(file, line, severity, err_code), has_errno(true) {
errno_log_message_.stream() << "Check failed: " << condition << ". ";
}

CheckError CheckError::DPCheck(const char* file,
int line,
const char* condition) {
SystemErrorCode err_code = logging::GetLastSystemErrorCode();
#if BUILDFLAG(IS_WIN)
CheckError check_error(
new Win32ErrorLogMessage(file, line, LOGGING_DCHECK, err_code));
#elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
CheckError check_error(
new ErrnoLogMessage(file, line, LOGGING_DCHECK, err_code));
#endif
check_error.stream() << "Check failed: " << condition << ". ";
return check_error;
}
check_error::Check::Check(const char* file, int line, const char* condition)
: CheckError(file, line, LOGGING_FATAL, condition) {}

CheckError CheckError::NotImplemented(const char* file,
int line,
const char* function) {
CheckError check_error(new LogMessage(file, line, LOGGING_ERROR));
check_error.stream() << "Not implemented reached in " << function;
return check_error;
check_error::DCheck::DCheck(const char* file, int line, const char* condition)
: CheckError(file, line, LOGGING_DCHECK, condition) {}

check_error::PCheck::PCheck(const char* file, int line, const char* condition)
: CheckError(file,
line,
LOGGING_FATAL,
condition,
logging::GetLastSystemErrorCode()) {}

check_error::PCheck::PCheck(const char* file, int line)
: PCheck(file, line, "") {}

check_error::DPCheck::DPCheck(const char* file, int line, const char* condition)
: CheckError(file,
line,
LOGGING_DCHECK,
condition,
logging::GetLastSystemErrorCode()) {}

check_error::NotImplemented::NotImplemented(const char* file,
int line,
const char* function)
: CheckError(file, line, LOGGING_ERROR) {
stream() << "Not implemented reached in " << function;
}

base::strings::CStringBuilder& CheckError::stream() {
return log_message_->stream();
return !has_errno ? log_message_.stream() : errno_log_message_.stream();
}

CheckError::~CheckError() {
// Note: This function ends up in crash stack traces. If its full name
// changes, the crash server's magic signature logic needs to be updated.
// See cl/306632920.
delete log_message_;
if (!has_errno) {
log_message_.~LogMessage();
} else {
#if BUILDFLAG(IS_WIN)
errno_log_message_.~Win32ErrorLogMessage();
#elif BUILDFLAG(IS_POSIX) || BUILDFLAG(IS_FUCHSIA)
errno_log_message_.~ErrnoLogMessage();
#endif // BUILDFLAG(IS_WIN)
}
}

CheckError::CheckError(LogMessage* log_message) : log_message_(log_message) {}

void RawCheckFailure(const char* message) {
RawLog(LOGGING_FATAL, message);
__builtin_unreachable();
Expand Down
125 changes: 81 additions & 44 deletions base/allocator/partition_allocator/partition_alloc_base/check.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include "base/allocator/partition_allocator/partition_alloc_base/component_export.h"
#include "base/allocator/partition_allocator/partition_alloc_base/debug/debugging_buildflags.h"
#include "base/allocator/partition_allocator/partition_alloc_base/immediate_crash.h"
#include "base/allocator/partition_allocator/partition_alloc_base/log_message.h"
#include "base/allocator/partition_allocator/partition_alloc_base/strings/cstring_builder.h"

#define PA_STRINGIFY_IMPL(s) #s
Expand Down Expand Up @@ -74,35 +75,71 @@ class LogMessage;
// Class used for raising a check error upon destruction.
class PA_COMPONENT_EXPORT(PARTITION_ALLOC) CheckError {
public:
static CheckError Check(const char* file, int line, const char* condition);

static CheckError DCheck(const char* file, int line, const char* condition);

static CheckError PCheck(const char* file, int line, const char* condition);
static CheckError PCheck(const char* file, int line);
// Stream for adding optional details to the error message.
base::strings::CStringBuilder& stream();
PA_NOMERGE ~CheckError();

static CheckError DPCheck(const char* file, int line, const char* condition);
protected:
CheckError(const char* file,
int line,
LogSeverity severity,
const char* condition);
CheckError(const char* file, int line, LogSeverity severity);
CheckError(const char* file,
int line,
LogSeverity severity,
const char* condition,
SystemErrorCode err_code);

union {
LogMessage log_message_;
#if BUILDFLAG(IS_WIN)
Win32ErrorLogMessage errno_log_message_;
#else
ErrnoLogMessage errno_log_message_;
#endif
};

// |has_errno| describes which union member is used, |log_message_| or
// |errno_log_message_|. If |has_errno| is true, CheckError initializes
// |errno_log_message_| at its constructor and destroys at its destructor.
// (This also means the CheckError is an instance of the parent class of
// PCheck or DPCheck.)
// If false, CheckError initializes and destroys |log_message_|.
const bool has_errno = false;
};

static CheckError NotImplemented(const char* file,
int line,
const char* function);
namespace check_error {

// Stream for adding optional details to the error message.
base::strings::CStringBuilder& stream();
// Class used for raising a check error upon destruction.
class PA_COMPONENT_EXPORT(PARTITION_ALLOC) Check : public CheckError {
public:
Check(const char* file, int line, const char* condition);
};

PA_NOMERGE ~CheckError();
class PA_COMPONENT_EXPORT(PARTITION_ALLOC) DCheck : public CheckError {
public:
DCheck(const char* file, int line, const char* condition);
};

CheckError(const CheckError& other) = delete;
CheckError& operator=(const CheckError& other) = delete;
CheckError(CheckError&& other) = default;
CheckError& operator=(CheckError&& other) = default;
class PA_COMPONENT_EXPORT(PARTITION_ALLOC) PCheck : public CheckError {
public:
PCheck(const char* file, int line, const char* condition);
PCheck(const char* file, int line);
};

private:
explicit CheckError(LogMessage* log_message);
class PA_COMPONENT_EXPORT(PARTITION_ALLOC) DPCheck : public CheckError {
public:
DPCheck(const char* file, int line, const char* condition);
};

LogMessage* log_message_;
class PA_COMPONENT_EXPORT(PARTITION_ALLOC) NotImplemented : public CheckError {
public:
NotImplemented(const char* file, int line, const char* function);
};

} // namespace check_error

#if defined(OFFICIAL_BUILD) && !defined(NDEBUG)
#error "Debug builds are not expected to be optimized as official builds."
#endif // defined(OFFICIAL_BUILD) && !defined(NDEBUG)
Expand All @@ -120,49 +157,49 @@ class PA_COMPONENT_EXPORT(PARTITION_ALLOC) CheckError {

#define PA_BASE_CHECK_WILL_STREAM() false

#define PA_BASE_PCHECK(condition) \
PA_LAZY_CHECK_STREAM( \
::partition_alloc::internal::logging::CheckError::PCheck(__FILE__, \
__LINE__) \
.stream(), \
#define PA_BASE_PCHECK(condition) \
PA_LAZY_CHECK_STREAM( \
::partition_alloc::internal::logging::check_error::PCheck(__FILE__, \
__LINE__) \
.stream(), \
PA_UNLIKELY(!(condition)))

#else

#define PA_BASE_CHECK(condition) \
PA_LAZY_CHECK_STREAM( \
::partition_alloc::internal::logging::CheckError::Check( \
__FILE__, __LINE__, #condition) \
.stream(), \
#define PA_BASE_CHECK(condition) \
PA_LAZY_CHECK_STREAM( \
::partition_alloc::internal::logging::check_error::Check( \
__FILE__, __LINE__, #condition) \
.stream(), \
!PA_ANALYZER_ASSUME_TRUE(condition))

#define PA_BASE_CHECK_WILL_STREAM() true

#define PA_BASE_PCHECK(condition) \
PA_LAZY_CHECK_STREAM( \
::partition_alloc::internal::logging::CheckError::PCheck( \
__FILE__, __LINE__, #condition) \
.stream(), \
#define PA_BASE_PCHECK(condition) \
PA_LAZY_CHECK_STREAM( \
::partition_alloc::internal::logging::check_error::PCheck( \
__FILE__, __LINE__, #condition) \
.stream(), \
!PA_ANALYZER_ASSUME_TRUE(condition))

#endif

#if BUILDFLAG(PA_DCHECK_IS_ON)

#define PA_BASE_DCHECK(condition) \
PA_LAZY_CHECK_STREAM( \
::partition_alloc::internal::logging::CheckError::DCheck( \
__FILE__, __LINE__, #condition) \
.stream(), \
!PA_ANALYZER_ASSUME_TRUE(condition))

#define PA_BASE_DPCHECK(condition) \
#define PA_BASE_DCHECK(condition) \
PA_LAZY_CHECK_STREAM( \
::partition_alloc::internal::logging::CheckError::DPCheck( \
::partition_alloc::internal::logging::check_error::DCheck( \
__FILE__, __LINE__, #condition) \
.stream(), \
!PA_ANALYZER_ASSUME_TRUE(condition))

#define PA_BASE_DPCHECK(condition) \
PA_LAZY_CHECK_STREAM( \
::partition_alloc::internal::logging::check_error::DPCheck( \
__FILE__, __LINE__, #condition) \
.stream(), \
!PA_ANALYZER_ASSUME_TRUE(condition))

#else

#define PA_BASE_DCHECK(condition) PA_EAT_CHECK_STREAM_PARAMS(!(condition))
Expand Down

0 comments on commit 01bcc60

Please sign in to comment.