Add CHECK and friends from Chromium.
Replace FATAL_ERROR_IF with the more familiar (to Chromium developers)
CHECK and DCHECK. The full Chromium implementation is fairly elaborate
but I copied enough to get us most of the benefits. I believe the main
missing component is a more advanced stack dump. For this bit I relied
on the V8 implementation.
There are a few minor modifications from the Chromium original:
- The FatalMessage class is specialized for logging fatal error
messages and aborting. Chromium uses the general LogMessage class,
which we could consider moving towards in the future.
- NOTIMPLEMENTED() and NOTREACHED() have been removed, partly because
I don't want to rely on our logging.h until base/ and system_wrappers/
are consolidated.
- FATAL() replaces LOG(FATAL).
Minor modifications from V8's stack dump:
- If parsing of a stack trace symbol fails, just print the unparsed
symbol. (I noticed this happened on Mac.)
- Use __GLIBCXX__ and __UCLIBC__. This is from examining the backtrace
use in Chromium.
UNREACHABLE() has been removed because its behavior is different than
Chromium's NOTREACHED(), which is bound to cause confusion. The few uses
were replaced with FATAL(), matching the previous behavior.
Add a NO_RETURN macro, allowing us to remove unreachable return
statements following a CHECK/FATAL.
TESTED=the addition of dummy CHECK, DCHECK, CHECK_EQ and FATAL did the
did the right things. Stack traces work on Mac, but I don't get symbols
on Linux.
R=henrik.lundin@webrtc.org, kwiberg@webrtc.org, tommi@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/22449004
git-svn-id: http://webrtc.googlecode.com/svn/trunk/webrtc@7003 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/base/checks.cc b/base/checks.cc
index 0f67c76..99d8877 100644
--- a/base/checks.cc
+++ b/base/checks.cc
@@ -8,27 +8,122 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include <stdio.h>
-#include <stdlib.h>
-#include <stdarg.h>
+// Most of this was borrowed (with minor modifications) from V8's and Chromium's
+// src/base/logging.cc.
+
+// Use the C++ version to provide __GLIBCXX__.
+#include <cstdio>
+
+#if defined(__GLIBCXX__) && !defined(__UCLIBC__)
+#include <cxxabi.h>
+#include <execinfo.h>
+#endif
+
+#if defined(ANDROID)
+#define LOG_TAG "rtc"
+#include <android/log.h> // NOLINT
+#endif
+
+#include <iostream>
#include "webrtc/base/checks.h"
+#include "webrtc/base/common.h"
#include "webrtc/base/logging.h"
+#if defined(_MSC_VER)
+// Warning C4722: destructor never returns, potential memory leak.
+// FatalMessage's dtor very intentionally aborts.
+#pragma warning(disable:4722)
+#endif
+
namespace rtc {
-void Fatal(const char* file, int line, const char* format, ...) {
- char msg[256];
+void VPrintError(const char* format, va_list args) {
+#if defined(WEBRTC_ANDROID)
+ __android_log_vprint(ANDROID_LOG_ERROR, LOG_TAG, format, args);
+#else
+ vfprintf(stderr, format, args);
+#endif
+}
- va_list arguments;
- va_start(arguments, format);
- vsnprintf(msg, sizeof(msg), format, arguments);
- va_end(arguments);
+void PrintError(const char* format, ...) {
+ va_list args;
+ va_start(args, format);
+ VPrintError(format, args);
+ va_end(args);
+}
- LOG(LS_ERROR) << "\n\n#\n# Fatal error in " << file
- << ", line " << line << "\n# " << msg
- << "\n#\n";
+// TODO(ajm): This works on Mac (although the parsing fails) but I don't seem
+// to get usable symbols on Linux. This is copied from V8. Chromium has a more
+// advanced stace trace system; also more difficult to copy.
+void DumpBacktrace() {
+#if defined(__GLIBCXX__) && !defined(__UCLIBC__)
+ void* trace[100];
+ int size = backtrace(trace, ARRAY_SIZE(trace));
+ char** symbols = backtrace_symbols(trace, size);
+ PrintError("\n==== C stack trace ===============================\n\n");
+ if (size == 0) {
+ PrintError("(empty)\n");
+ } else if (symbols == NULL) {
+ PrintError("(no symbols)\n");
+ } else {
+ for (int i = 1; i < size; ++i) {
+ char mangled[201];
+ if (sscanf(symbols[i], "%*[^(]%*[(]%200[^)+]", mangled) == 1) { // NOLINT
+ PrintError("%2d: ", i);
+ int status;
+ size_t length;
+ char* demangled = abi::__cxa_demangle(mangled, NULL, &length, &status);
+ PrintError("%s\n", demangled != NULL ? demangled : mangled);
+ free(demangled);
+ } else {
+ // If parsing failed, at least print the unparsed symbol.
+ PrintError("%s\n", symbols[i]);
+ }
+ }
+ }
+ free(symbols);
+#endif
+}
+
+FatalMessage::FatalMessage(const char* file, int line) {
+ Init(file, line);
+}
+
+FatalMessage::FatalMessage(const char* file, int line, std::string* result) {
+ Init(file, line);
+ stream_ << "Check failed: " << *result << std::endl << "# ";
+ delete result;
+}
+
+NO_RETURN FatalMessage::~FatalMessage() {
+ fflush(stdout);
+ fflush(stderr);
+ stream_ << std::endl << "#" << std::endl;
+ PrintError(stream_.str().c_str());
+ DumpBacktrace();
+ fflush(stderr);
abort();
}
+void FatalMessage::Init(const char* file, int line) {
+ stream_ << std::endl << std::endl << "#" << std::endl << "# Fatal error in "
+ << file << ", line " << line << std::endl << "# ";
+}
+
+// MSVC doesn't like complex extern templates and DLLs.
+#if !defined(COMPILER_MSVC)
+// Explicit instantiations for commonly used comparisons.
+template std::string* MakeCheckOpString<int, int>(
+ const int&, const int&, const char* names);
+template std::string* MakeCheckOpString<unsigned long, unsigned long>(
+ const unsigned long&, const unsigned long&, const char* names);
+template std::string* MakeCheckOpString<unsigned long, unsigned int>(
+ const unsigned long&, const unsigned int&, const char* names);
+template std::string* MakeCheckOpString<unsigned int, unsigned long>(
+ const unsigned int&, const unsigned long&, const char* names);
+template std::string* MakeCheckOpString<std::string, std::string>(
+ const std::string&, const std::string&, const char* name);
+#endif
+
} // namespace rtc
diff --git a/base/checks.h b/base/checks.h
index b85b50a..9f8122e 100644
--- a/base/checks.h
+++ b/base/checks.h
@@ -8,30 +8,181 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-// This module contains some basic debugging facilities.
-// Originally comes from shared/commandlineflags/checks.h
-
#ifndef WEBRTC_BASE_CHECKS_H_
#define WEBRTC_BASE_CHECKS_H_
+#include <stdint.h>
+#include <string.h>
+
+#include <sstream>
+
+#include "webrtc/base/logging.h"
+#include "webrtc/typedefs.h"
+
+// The macros here print a message to stderr and abort under various
+// conditions. All will accept additional stream messages. For example:
+// DCHECK_EQ(foo, bar) << "I'm printed when foo != bar.";
+//
+// - CHECK(x) is an assertion that x is always true, and that if it isn't, it's
+// better to terminate the process than to continue. During development, the
+// reason that it's better to terminate might simply be that the error
+// handling code isn't in place yet; in production, the reason might be that
+// the author of the code truly believes that x will always be true, but that
+// she recognizes that if she is wrong, abrupt and unpleasant process
+// termination is still better than carrying on with the assumption violated.
+//
+// - DCHECK(x) is the same as CHECK(x)---an assertion that x is always
+// true---except that x will only be evaluated in debug builds; in production
+// builds, x is simply assumed to be true. This is useful if evaluating x is
+// expensive and the expected cost of failing to detect the violated
+// assumption is acceptable. You should not handle cases where a production
+// build fails to spot a violated condition, even those that would result in
+// crashes. If the code needs to cope with the error, make it cope, but don't
+// call DCHECK; if the condition really can't occur, but you'd sleep better
+// at night knowing that the process will suicide instead of carrying on in
+// case you were wrong, use CHECK instead of DCHECK.
+//
+// - CHECK_EQ, _NE, _GT, ..., and DCHECK_EQ, _NE, _GT, ... are specialized
+// variants of CHECK and DCHECK that print prettier messages if the condition
+// doesn't hold. Prefer them to raw CHECK and DCHECK.
+//
+// - FATAL() aborts unconditionally.
+//
+// TODO(ajm): Ideally, this would be combined with webrtc/base/logging.h.
+
namespace rtc {
-// Prints an error message to stderr and aborts execution.
-void Fatal(const char* file, int line, const char* format, ...);
+// Helper macro which avoids evaluating the arguments to a stream if
+// the condition doesn't hold.
+#define LAZY_STREAM(stream, condition) \
+ !(condition) ? (void) 0 : rtc::LogMessageVoidify() & (stream)
+
+// The actual stream used isn't important.
+#define EAT_STREAM_PARAMETERS \
+ true ? (void) 0 : rtc::LogMessageVoidify() & rtc::FatalMessage("", 0).stream()
+
+// CHECK dies with a fatal error if condition is not true. It is *not*
+// controlled by NDEBUG, so the check will be executed regardless of
+// compilation mode.
+//
+// We make sure CHECK et al. always evaluates their arguments, as
+// doing CHECK(FunctionWithSideEffect()) is a common idiom.
+#define CHECK(condition) \
+ LAZY_STREAM(rtc::FatalMessage(__FILE__, __LINE__).stream(), !(condition)) \
+ << "Check failed: " #condition << std::endl << "# "
+
+// Helper macro for binary operators.
+// Don't use this macro directly in your code, use CHECK_EQ et al below.
+//
+// TODO(akalin): Rewrite this so that constructs like if (...)
+// CHECK_EQ(...) else { ... } work properly.
+#define CHECK_OP(name, op, val1, val2) \
+ if (std::string* _result = \
+ rtc::Check##name##Impl((val1), (val2), \
+ #val1 " " #op " " #val2)) \
+ rtc::FatalMessage(__FILE__, __LINE__, _result).stream()
+
+// Build the error message string. This is separate from the "Impl"
+// function template because it is not performance critical and so can
+// be out of line, while the "Impl" code should be inline. Caller
+// takes ownership of the returned string.
+template<class t1, class t2>
+std::string* MakeCheckOpString(const t1& v1, const t2& v2, const char* names) {
+ std::ostringstream ss;
+ ss << names << " (" << v1 << " vs. " << v2 << ")";
+ std::string* msg = new std::string(ss.str());
+ return msg;
+}
+
+// MSVC doesn't like complex extern templates and DLLs.
+#if !defined(COMPILER_MSVC)
+// Commonly used instantiations of MakeCheckOpString<>. Explicitly instantiated
+// in logging.cc.
+extern template std::string* MakeCheckOpString<int, int>(
+ const int&, const int&, const char* names);
+extern template
+std::string* MakeCheckOpString<unsigned long, unsigned long>(
+ const unsigned long&, const unsigned long&, const char* names);
+extern template
+std::string* MakeCheckOpString<unsigned long, unsigned int>(
+ const unsigned long&, const unsigned int&, const char* names);
+extern template
+std::string* MakeCheckOpString<unsigned int, unsigned long>(
+ const unsigned int&, const unsigned long&, const char* names);
+extern template
+std::string* MakeCheckOpString<std::string, std::string>(
+ const std::string&, const std::string&, const char* name);
+#endif
+
+// Helper functions for CHECK_OP macro.
+// The (int, int) specialization works around the issue that the compiler
+// will not instantiate the template version of the function on values of
+// unnamed enum type - see comment below.
+#define DEFINE_CHECK_OP_IMPL(name, op) \
+ template <class t1, class t2> \
+ inline std::string* Check##name##Impl(const t1& v1, const t2& v2, \
+ const char* names) { \
+ if (v1 op v2) return NULL; \
+ else return MakeCheckOpString(v1, v2, names); \
+ } \
+ inline std::string* Check##name##Impl(int v1, int v2, const char* names) { \
+ if (v1 op v2) return NULL; \
+ else return MakeCheckOpString(v1, v2, names); \
+ }
+DEFINE_CHECK_OP_IMPL(EQ, ==)
+DEFINE_CHECK_OP_IMPL(NE, !=)
+DEFINE_CHECK_OP_IMPL(LE, <=)
+DEFINE_CHECK_OP_IMPL(LT, < )
+DEFINE_CHECK_OP_IMPL(GE, >=)
+DEFINE_CHECK_OP_IMPL(GT, > )
+#undef DEFINE_CHECK_OP_IMPL
+
+#define CHECK_EQ(val1, val2) CHECK_OP(EQ, ==, val1, val2)
+#define CHECK_NE(val1, val2) CHECK_OP(NE, !=, val1, val2)
+#define CHECK_LE(val1, val2) CHECK_OP(LE, <=, val1, val2)
+#define CHECK_LT(val1, val2) CHECK_OP(LT, < , val1, val2)
+#define CHECK_GE(val1, val2) CHECK_OP(GE, >=, val1, val2)
+#define CHECK_GT(val1, val2) CHECK_OP(GT, > , val1, val2)
+
+// The DCHECK macro is equivalent to CHECK except that it only generates code in
+// debug builds.
+#if !defined(NDEBUG)
+#define DCHECK(condition) CHECK(condition)
+#define DCHECK_EQ(v1, v2) CHECK_EQ(v1, v2)
+#define DCHECK_NE(v1, v2) CHECK_NE(v1, v2)
+#define DCHECK_GE(v1, v2) CHECK_GE(v1, v2)
+#define DCHECK_LT(v1, v2) CHECK_LT(v1, v2)
+#define DCHECK_LE(v1, v2) CHECK_LE(v1, v2)
+#else
+#define DCHECK(condition) EAT_STREAM_PARAMETERS
+#define DCHECK_EQ(v1, v2) EAT_STREAM_PARAMETERS
+#define DCHECK_NE(v1, v2) EAT_STREAM_PARAMETERS
+#define DCHECK_GE(v1, v2) EAT_STREAM_PARAMETERS
+#define DCHECK_LT(v1, v2) EAT_STREAM_PARAMETERS
+#define DCHECK_LE(v1, v2) EAT_STREAM_PARAMETERS
+#endif
+
+#define FATAL() rtc::FatalMessage(__FILE__, __LINE__).stream()
+// TODO(ajm): Consider adding NOTIMPLEMENTED and NOTREACHED macros when
+// base/logging.h and system_wrappers/logging.h are consolidated such that we
+// can match the Chromium behavior.
+
+// Like a stripped-down LogMessage from logging.h, except that it aborts.
+class FatalMessage {
+ public:
+ FatalMessage(const char* file, int line);
+ // Used for CHECK_EQ(), etc. Takes ownership of the given string.
+ FatalMessage(const char* file, int line, std::string* result);
+ NO_RETURN ~FatalMessage();
+
+ std::ostream& stream() { return stream_; }
+
+ private:
+ void Init(const char* file, int line);
+
+ std::ostringstream stream_;
+};
} // namespace rtc
-// Trigger a fatal error (which aborts the process and prints an error
-// message). FATAL_ERROR_IF may seem a lot like assert, but there's a crucial
-// difference: it's always "on". This means that it can be used to check for
-// regular errors that could actually happen, not just programming errors that
-// supposedly can't happen---but triggering a fatal error will kill the process
-// in an ugly way, so it's not suitable for catching errors that might happen
-// in production.
-#define FATAL_ERROR(msg) do { rtc::Fatal(__FILE__, __LINE__, msg); } while (0)
-#define FATAL_ERROR_IF(x) do { if (x) FATAL_ERROR("check failed"); } while (0)
-
-// The UNREACHABLE macro is very useful during development.
-#define UNREACHABLE() FATAL_ERROR("unreachable code")
-
#endif // WEBRTC_BASE_CHECKS_H_
diff --git a/base/flags.cc b/base/flags.cc
index fe7a334..a5e1c45 100644
--- a/base/flags.cc
+++ b/base/flags.cc
@@ -56,7 +56,7 @@
variable_->s = default_.s;
return;
}
- UNREACHABLE();
+ FATAL() << "unreachable code";
}
@@ -67,8 +67,7 @@
case Flag::FLOAT: return "float";
case Flag::STRING: return "string";
}
- UNREACHABLE();
- return NULL;
+ FATAL() << "unreachable code";
}
@@ -87,7 +86,7 @@
printf("%s", p->s);
return;
}
- UNREACHABLE();
+ FATAL() << "unreachable code";
}
@@ -164,8 +163,7 @@
if (*arg == '=') {
// make a copy so we can NUL-terminate flag name
int n = static_cast<int>(arg - *name);
- if (n >= buffer_size)
- Fatal(__FILE__, __LINE__, "CHECK(%s) failed", "n < buffer_size");
+ CHECK_LT(n, buffer_size);
memcpy(buffer, *name, n * sizeof(char));
buffer[n] = '\0';
*name = buffer;
@@ -259,8 +257,7 @@
void FlagList::Register(Flag* flag) {
assert(flag != NULL && strlen(flag->name()) > 0);
- if (Lookup(flag->name()) != NULL)
- Fatal(flag->file(), 0, "flag %s declared twice", flag->name());
+ CHECK(!Lookup(flag->name())) << "flag " << flag->name() << " declared twice";
flag->next_ = list_;
list_ = flag;
}
@@ -294,6 +291,6 @@
delete[] argv_;
}
-#endif // WEBRTC_WIN
+#endif // WEBRTC_WIN
} // namespace rtc
diff --git a/base/opensslidentity.cc b/base/opensslidentity.cc
index 915680c..30ac6e2 100644
--- a/base/opensslidentity.cc
+++ b/base/opensslidentity.cc
@@ -253,13 +253,11 @@
std::string OpenSSLCertificate::ToPEMString() const {
BIO* bio = BIO_new(BIO_s_mem());
if (!bio) {
- UNREACHABLE();
- return std::string();
+ FATAL() << "unreachable code";
}
if (!PEM_write_bio_X509(bio, x509_)) {
BIO_free(bio);
- UNREACHABLE();
- return std::string();
+ FATAL() << "unreachable code";
}
BIO_write(bio, "\0", 1);
char* buffer;
@@ -276,13 +274,11 @@
// Calculates the DER representation of the certificate, from scratch.
BIO* bio = BIO_new(BIO_s_mem());
if (!bio) {
- UNREACHABLE();
- return;
+ FATAL() << "unreachable code";
}
if (!i2d_X509_bio(bio, x509_)) {
BIO_free(bio);
- UNREACHABLE();
- return;
+ FATAL() << "unreachable code";
}
char* data;
size_t length = BIO_get_mem_data(bio, &data);
diff --git a/common_audio/wav_writer.cc b/common_audio/wav_writer.cc
index f3b8118..30a220c 100644
--- a/common_audio/wav_writer.cc
+++ b/common_audio/wav_writer.cc
@@ -29,17 +29,17 @@
num_channels_(num_channels),
num_samples_(0),
file_handle_(fopen(filename.c_str(), "wb")) {
- FATAL_ERROR_IF(!CheckWavParameters(num_channels_,
- sample_rate_,
- kWavFormat,
- kBytesPerSample,
- num_samples_));
- FATAL_ERROR_IF(!file_handle_);
+ CHECK(file_handle_);
+ CHECK(CheckWavParameters(num_channels_,
+ sample_rate_,
+ kWavFormat,
+ kBytesPerSample,
+ num_samples_));
// Write a blank placeholder header, since we need to know the total number
// of samples before we can fill in the real data.
static const uint8_t blank_header[kWavHeaderSize] = {0};
- FATAL_ERROR_IF(fwrite(blank_header, kWavHeaderSize, 1, file_handle_) != 1);
+ CHECK_EQ(1u, fwrite(blank_header, kWavHeaderSize, 1, file_handle_));
}
WavFile::~WavFile() {
@@ -52,15 +52,15 @@
#endif
const size_t written =
fwrite(samples, sizeof(*samples), num_samples, file_handle_);
- FATAL_ERROR_IF(written != num_samples);
+ CHECK_EQ(num_samples, written);
num_samples_ += static_cast<uint32_t>(written);
- FATAL_ERROR_IF(written > std::numeric_limits<uint32_t>::max() ||
- num_samples_ < written); // detect uint32_t overflow
- FATAL_ERROR_IF(!CheckWavParameters(num_channels_,
- sample_rate_,
- kWavFormat,
- kBytesPerSample,
- num_samples_));
+ CHECK(written <= std::numeric_limits<uint32_t>::max() ||
+ num_samples_ >= written); // detect uint32_t overflow
+ CHECK(CheckWavParameters(num_channels_,
+ sample_rate_,
+ kWavFormat,
+ kBytesPerSample,
+ num_samples_));
}
void WavFile::WriteSamples(const float* samples, size_t num_samples) {
@@ -74,12 +74,12 @@
}
void WavFile::Close() {
- FATAL_ERROR_IF(fseek(file_handle_, 0, SEEK_SET) != 0);
+ CHECK_EQ(0, fseek(file_handle_, 0, SEEK_SET));
uint8_t header[kWavHeaderSize];
WriteWavHeader(header, num_channels_, sample_rate_, kWavFormat,
kBytesPerSample, num_samples_);
- FATAL_ERROR_IF(fwrite(header, kWavHeaderSize, 1, file_handle_) != 1);
- FATAL_ERROR_IF(fclose(file_handle_) != 0);
+ CHECK_EQ(1u, fwrite(header, kWavHeaderSize, 1, file_handle_));
+ CHECK_EQ(0, fclose(file_handle_));
file_handle_ = NULL;
}
diff --git a/common_audio/wav_writer.h b/common_audio/wav_writer.h
index e058265..45bcbac 100644
--- a/common_audio/wav_writer.h
+++ b/common_audio/wav_writer.h
@@ -20,7 +20,7 @@
namespace webrtc {
// Simple C++ class for writing 16-bit PCM WAV files. All error handling is
-// by calls to FATAL_ERROR(), making it unsuitable for anything but debug code.
+// by calls to CHECK(), making it unsuitable for anything but debug code.
class WavFile {
public:
// Open a new WAV file for writing.
diff --git a/modules/audio_coding/main/acm2/acm_send_test.cc b/modules/audio_coding/main/acm2/acm_send_test.cc
index 67cc9b2..cbe2ae5 100644
--- a/modules/audio_coding/main/acm2/acm_send_test.cc
+++ b/modules/audio_coding/main/acm2/acm_send_test.cc
@@ -50,8 +50,8 @@
int channels,
int payload_type,
int frame_size_samples) {
- FATAL_ERROR_IF(AudioCodingModule::Codec(
- payload_name, &codec_, sampling_freq_hz, channels) != 0);
+ CHECK_EQ(0, AudioCodingModule::Codec(payload_name, &codec_, sampling_freq_hz,
+ channels));
codec_.pltype = payload_type;
codec_.pacsize = frame_size_samples;
codec_registered_ = (acm_->RegisterSendCodec(codec_) == 0);
@@ -73,9 +73,8 @@
// Insert audio and process until one packet is produced.
while (clock_.TimeInMilliseconds() < test_duration_ms_) {
clock_.AdvanceTimeMilliseconds(kBlockSizeMs);
- FATAL_ERROR_IF(
- !audio_source_->Read(input_block_size_samples_, input_frame_.data_));
- FATAL_ERROR_IF(acm_->Add10MsData(input_frame_) != 0);
+ CHECK(audio_source_->Read(input_block_size_samples_, input_frame_.data_));
+ CHECK_EQ(0, acm_->Add10MsData(input_frame_));
input_frame_.timestamp_ += input_block_size_samples_;
int32_t encoded_bytes = acm_->Process();
if (encoded_bytes > 0) {
diff --git a/typedefs.h b/typedefs.h
index 38cda51..871f04e 100644
--- a/typedefs.h
+++ b/typedefs.h
@@ -120,4 +120,13 @@
#endif
#endif
+// Annotate a function that will not return control flow to the caller.
+#if defined(_MSC_VER)
+#define NO_RETURN __declspec(noreturn)
+#elif defined(__GNUC__)
+#define NO_RETURN __attribute__((noreturn))
+#else
+#define NO_RETURN
+#endif
+
#endif // WEBRTC_TYPEDEFS_H_
