Discard CHECK failure messages for official builds to save binary size

FATAL() calls with more than one argument are preserved.

The rest of chrome does this as well. Stack traces and minidumps should
be sufficient for analyzing the reason for crashes.

This saves 110kb for Android arm32.

Bug: chromium:958807
Change-Id: I88a1ec82f1ed7bd5e7dbccf6d645d5584f16de82
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1598159
Commit-Queue: Andrew Grieve <agrieve@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Ross McIlroy <rmcilroy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61426}

src/base/logging.cc

@@ -53,7 +53,12 @@ void PrettyPrintChar(std::ostream& os, int ch) {
 }
 
 void DefaultDcheckHandler(const char* file, int line, const char* message) {
+#ifdef DEBUG
   V8_Fatal(file, line, "Debug check failed: %s.", message);
+#else
+  // This case happens only for unit tests.
+  V8_Fatal("Debug check failed: %s.", message);
+#endif
 }
 
 }  // namespace
@@ -144,7 +149,13 @@ class FailureMessage {
 
 }  // namespace
 
+#ifdef DEBUG
 void V8_Fatal(const char* file, int line, const char* format, ...) {
+#else
+void V8_Fatal(const char* format, ...) {
+  const char* file = "";
+  int line = 0;
+#endif
   va_list arguments;
   va_start(arguments, format);
   // Format the error message into a stack object for later retrieveal by the
@@ -171,6 +182,15 @@ void V8_Fatal(const char* file, int line, const char* format, ...) {
   v8::base::OS::Abort();
 }
 
+#if !defined(DEBUG) && defined(OFFICIAL_BUILD)
+void V8_FatalNoContext() {
+  v8::base::OS::PrintError("V8 CHECK or FATAL\n");
+  if (v8::base::g_print_stack_trace) v8::base::g_print_stack_trace();
+  fflush(stderr);
+  v8::base::OS::Abort();
+}
+#endif
+
 void V8_Dcheck(const char* file, int line, const char* message) {
   v8::base::g_dcheck_function(file, line, message);
 }

src/base/logging.h

@@ -14,17 +14,40 @@
 #include "src/base/compiler-specific.h"
 #include "src/base/template-utils.h"
 
-[[noreturn]] PRINTF_FORMAT(3, 4) V8_BASE_EXPORT V8_NOINLINE
-    void V8_Fatal(const char* file, int line, const char* format, ...);
-
 V8_BASE_EXPORT V8_NOINLINE void V8_Dcheck(const char* file, int line,
                                           const char* message);
 
 #ifdef DEBUG
+// In debug, include file, line, and full error message for all
+// FATAL() calls.
+[[noreturn]] PRINTF_FORMAT(3, 4) V8_BASE_EXPORT V8_NOINLINE
+    void V8_Fatal(const char* file, int line, const char* format, ...);
 #define FATAL(...) V8_Fatal(__FILE__, __LINE__, __VA_ARGS__)
+
+#elif !defined(OFFICIAL_BUILD)
+// In non-official release, include full error message, but drop file & line
+// numbers. It saves binary size to drop the |file| & |line| as opposed to just
+// passing in "", 0 for them.
+[[noreturn]] PRINTF_FORMAT(1, 2) V8_BASE_EXPORT V8_NOINLINE
+    void V8_Fatal(const char* format, ...);
+#define FATAL V8_Fatal
+
 #else
-#define FATAL(...) V8_Fatal("", 0, __VA_ARGS__)
+// In official builds, include only messages that contain parameters because
+// single-message errors can always be derived from stack traces.
+[[noreturn]] V8_BASE_EXPORT V8_NOINLINE void V8_FatalNoContext();
+[[noreturn]] PRINTF_FORMAT(1, 2) V8_BASE_EXPORT V8_NOINLINE
+    void V8_Fatal(const char* format, ...);
+// FATAL(msg) -> V8_FatalNoContext()
+// FATAL(msg, ...) -> V8_Fatal()
+#define FATAL_HELPER(_7, _6, _5, _4, _3, _2, _1, _0, ...) _0
+#define FATAL_DISCARD_ARG(arg) V8_FatalNoContext()
+#define FATAL(...)                                                            \
+  FATAL_HELPER(__VA_ARGS__, V8_Fatal, V8_Fatal, V8_Fatal, V8_Fatal, V8_Fatal, \
+               V8_Fatal, V8_Fatal, FATAL_DISCARD_ARG)                         \
+  (__VA_ARGS__)
 #endif
+
 #define UNIMPLEMENTED() FATAL("unimplemented code")
 #define UNREACHABLE() FATAL("unreachable code")
 
@@ -38,6 +61,14 @@ V8_BASE_EXPORT void SetPrintStackTrace(void (*print_stack_trace_)());
 V8_BASE_EXPORT void SetDcheckFunction(void (*dcheck_Function)(const char*, int,
                                                               const char*));
 
+// In official builds, assume all check failures can be debugged given just the
+// stack trace.
+#if !defined(DEBUG) && defined(OFFICIAL_BUILD)
+#define CHECK_FAILED_HANDLER(message) FATAL("ignored")
+#else
+#define CHECK_FAILED_HANDLER(message) FATAL("Check failed: %s.", message)
+#endif
+
 // CHECK dies with a fatal error if condition is not true.  It is *not*
 // controlled by DEBUG, so the check will be executed regardless of
 // compilation mode.
@@ -47,7 +78,7 @@ V8_BASE_EXPORT void SetDcheckFunction(void (*dcheck_Function)(const char*, int,
 #define CHECK_WITH_MSG(condition, message) \
   do {                                     \
     if (V8_UNLIKELY(!(condition))) {       \
-      FATAL("Check failed: %s.", message); \
+      CHECK_FAILED_HANDLER(message);       \
     }                                      \
   } while (false)
 #define CHECK(condition) CHECK_WITH_MSG(condition, #condition)

src/compiler/code-assembler.cc

@@ -1917,10 +1917,10 @@ Address CheckObjectType(Address raw_value, Address raw_type,
   }
   std::stringstream value_description;
   value->Print(value_description);
-  V8_Fatal(__FILE__, __LINE__,
-           "Type cast failed in %s\n"
-           "  Expected %s but found %s",
-           location->ToAsciiArray(), expected, value_description.str().c_str());
+  FATAL(
+      "Type cast failed in %s\n"
+      "  Expected %s but found %s",
+      location->ToAsciiArray(), expected, value_description.str().c_str());
 #else
   UNREACHABLE();
 #endif

src/wasm/function-body-decoder-impl.h

@@ -1452,8 +1452,8 @@ class WasmDecoder : public Decoder {
         V8_FALLTHROUGH;
       }
       default:
-        V8_Fatal(__FILE__, __LINE__, "unimplemented opcode: %x (%s)", opcode,
-                 WasmOpcodes::OpcodeName(opcode));
+        FATAL("unimplemented opcode: %x (%s)", opcode,
+              WasmOpcodes::OpcodeName(opcode));
         return {0, 0};
     }
 #undef DECLARE_OPCODE_CASE

test/cctest/compiler/test-code-generator.cc

@@ -796,8 +796,8 @@ class TestEnvironment : public HandleAndZoneScope {
                         layout_[i]);
         std::ostringstream actual_str;
         PrintStateValue(actual_str, main_isolate(), actual_value, layout_[i]);
-        V8_Fatal(__FILE__, __LINE__, "Expected: '%s' but got '%s'",
-                 expected_str.str().c_str(), actual_str.str().c_str());
+        FATAL("Expected: '%s' but got '%s'", expected_str.str().c_str(),
+              actual_str.str().c_str());
       }
     }
   }

test/cctest/test-compiler.cc

@@ -670,17 +670,16 @@ TEST(CompileFunctionInContextScriptOrigin) {
 }
 
 void TestCompileFunctionInContextToStringImpl() {
-#define CHECK_NOT_CAUGHT(__local_context__, try_catch, __op__)                \
-  do {                                                                        \
-    const char* op = (__op__);                                                \
-    v8::Local<v8::Context> context = (__local_context__);                     \
-    if (try_catch.HasCaught()) {                                              \
-      v8::String::Utf8Value error(                                            \
-          CcTest::isolate(),                                                  \
-          try_catch.Exception()->ToString(context).ToLocalChecked());         \
-      V8_Fatal(__FILE__, __LINE__,                                            \
-               "Unexpected exception thrown during %s:\n\t%s\n", op, *error); \
-    }                                                                         \
+#define CHECK_NOT_CAUGHT(__local_context__, try_catch, __op__)             \
+  do {                                                                     \
+    const char* op = (__op__);                                             \
+    v8::Local<v8::Context> context = (__local_context__);                  \
+    if (try_catch.HasCaught()) {                                           \
+      v8::String::Utf8Value error(                                         \
+          CcTest::isolate(),                                               \
+          try_catch.Exception()->ToString(context).ToLocalChecked());      \
+      FATAL("Unexpected exception thrown during %s:\n\t%s\n", op, *error); \
+    }                                                                      \
   } while (false)
 
   {  // NOLINT

test/cctest/test-disasm-arm.cc

@@ -140,9 +140,9 @@ bool DisassembleAndCompare(byte* begin, UseRegex use_regex,
 
 // Verify that all invocations of the COMPARE macro passed successfully.
 // Exit with a failure if at least one of the tests failed.
-#define VERIFY_RUN() \
-if (failure) { \
-    V8_Fatal(__FILE__, __LINE__, "ARM Disassembler tests failed.\n"); \
+#define VERIFY_RUN()                           \
+  if (failure) {                               \
+    FATAL("ARM Disassembler tests failed.\n"); \
   }
 
 // clang-format off

test/cctest/test-disasm-mips.cc

@@ -94,9 +94,9 @@ bool DisassembleAndCompare(byte* pc, const char* compare_string) {
 
 // Verify that all invocations of the COMPARE macro passed successfully.
 // Exit with a failure if at least one of the tests failed.
-#define VERIFY_RUN() \
-if (failure) { \
-    V8_Fatal(__FILE__, __LINE__, "MIPS Disassembler tests failed.\n"); \
+#define VERIFY_RUN()                            \
+  if (failure) {                                \
+    FATAL("MIPS Disassembler tests failed.\n"); \
   }
 
 #define COMPARE_PC_REL_COMPACT(asm_, compare_string, offset)                   \

test/cctest/test-disasm-mips64.cc

@@ -94,9 +94,9 @@ bool DisassembleAndCompare(byte* pc, const char* compare_string) {
 
 // Verify that all invocations of the COMPARE macro passed successfully.
 // Exit with a failure if at least one of the tests failed.
-#define VERIFY_RUN() \
-if (failure) { \
-    V8_Fatal(__FILE__, __LINE__, "MIPS Disassembler tests failed.\n"); \
+#define VERIFY_RUN()                            \
+  if (failure) {                                \
+    FATAL("MIPS Disassembler tests failed.\n"); \
   }
 
 #define COMPARE_PC_REL_COMPACT(asm_, compare_string, offset)                   \

test/cctest/test-disasm-ppc.cc

@@ -90,9 +90,9 @@ bool DisassembleAndCompare(byte* pc, const char* compare_string) {
 
 // Verify that all invocations of the COMPARE macro passed successfully.
 // Exit with a failure if at least one of the tests failed.
-#define VERIFY_RUN()                                                  \
-  if (failure) {                                                      \
-    V8_Fatal(__FILE__, __LINE__, "PPC Disassembler tests failed.\n"); \
+#define VERIFY_RUN()                           \
+  if (failure) {                               \
+    FATAL("PPC Disassembler tests failed.\n"); \
   }
 
 TEST(DisasmPPC) {

test/cctest/test-disasm-s390.cc

@@ -88,9 +88,9 @@ bool DisassembleAndCompare(byte* pc, const char* compare_string) {
 
 // Verify that all invocations of the COMPARE macro passed successfully.
 // Exit with a failure if at least one of the tests failed.
-#define VERIFY_RUN()                                                   \
-  if (failure) {                                                       \
-    V8_Fatal(__FILE__, __LINE__, "S390 Disassembler tests failed.\n"); \
+#define VERIFY_RUN()                            \
+  if (failure) {                                \
+    FATAL("S390 Disassembler tests failed.\n"); \
   }
 
 TEST(TwoBytes) {

test/cctest/test-log.cc

@@ -181,9 +181,8 @@ class ScopedLoggerInitializer {
             printf("%s\n", log_.at(i).c_str());
           }
           printf("%zu\n", current);
-          V8_Fatal(__FILE__, __LINE__, "%s, ... %p apperead twice:\n    %s",
-                   search_term.c_str(), reinterpret_cast<void*>(address),
-                   current_line.c_str());
+          FATAL("%s, ... %p apperead twice:\n    %s", search_term.c_str(),
+                reinterpret_cast<void*>(address), current_line.c_str());
         }
       }
       map.insert({address, current_line});
@@ -941,22 +940,22 @@ void ValidateMapDetailsLogging(v8::Isolate* isolate,
     if (map_create_addresses.find(address) == map_create_addresses.end()) {
       // logger->PrintLog();
       i::Map::cast(obj)->Print();
-      V8_Fatal(__FILE__, __LINE__,
-               "Map (%p, #%zu) creation not logged during startup with "
-               "--trace-maps!"
-               "\n# Expected Log Line: map-create, ... %p",
-               reinterpret_cast<void*>(obj->ptr()), i,
-               reinterpret_cast<void*>(obj->ptr()));
+      FATAL(
+          "Map (%p, #%zu) creation not logged during startup with "
+          "--trace-maps!"
+          "\n# Expected Log Line: map-create, ... %p",
+          reinterpret_cast<void*>(obj->ptr()), i,
+          reinterpret_cast<void*>(obj->ptr()));
     } else if (map_details_addresses.find(address) ==
                map_details_addresses.end()) {
       // logger->PrintLog();
       i::Map::cast(obj)->Print();
-      V8_Fatal(__FILE__, __LINE__,
-               "Map (%p, #%zu) details not logged during startup with "
-               "--trace-maps!"
-               "\n# Expected Log Line: map-details, ... %p",
-               reinterpret_cast<void*>(obj->ptr()), i,
-               reinterpret_cast<void*>(obj->ptr()));
+      FATAL(
+          "Map (%p, #%zu) details not logged during startup with "
+          "--trace-maps!"
+          "\n# Expected Log Line: map-details, ... %p",
+          reinterpret_cast<void*>(obj->ptr()), i,
+          reinterpret_cast<void*>(obj->ptr()));
     }
   }
 }

test/cctest/wasm/test-wasm-stack.cc

@@ -20,16 +20,15 @@ using v8::Utils;
 
 namespace {
 
-#define CHECK_CSTREQ(exp, found)                                           \
-  do {                                                                     \
-    const char* exp_ = (exp);                                              \
-    const char* found_ = (found);                                          \
-    DCHECK_NOT_NULL(exp);                                                  \
-    if (V8_UNLIKELY(found_ == nullptr || strcmp(exp_, found_) != 0)) {     \
-      V8_Fatal(__FILE__, __LINE__,                                         \
-               "Check failed: (%s) != (%s) ('%s' vs '%s').", #exp, #found, \
-               exp_, found_ ? found_ : "<null>");                          \
-    }                                                                      \
+#define CHECK_CSTREQ(exp, found)                                              \
+  do {                                                                        \
+    const char* exp_ = (exp);                                                 \
+    const char* found_ = (found);                                             \
+    DCHECK_NOT_NULL(exp);                                                     \
+    if (V8_UNLIKELY(found_ == nullptr || strcmp(exp_, found_) != 0)) {        \
+      FATAL("Check failed: (%s) != (%s) ('%s' vs '%s').", #exp, #found, exp_, \
+            found_ ? found_ : "<null>");                                      \
+    }                                                                         \
   } while (false)
 
 void PrintStackTrace(v8::Isolate* isolate, v8::Local<v8::StackTrace> stack) {

test/cctest/wasm/test-wasm-trap-position.cc

@@ -21,16 +21,15 @@ using v8::Utils;
 
 namespace {
 
-#define CHECK_CSTREQ(exp, found)                                           \
-  do {                                                                     \
-    const char* exp_ = (exp);                                              \
-    const char* found_ = (found);                                          \
-    DCHECK_NOT_NULL(exp);                                                  \
-    if (V8_UNLIKELY(found_ == nullptr || strcmp(exp_, found_) != 0)) {     \
-      V8_Fatal(__FILE__, __LINE__,                                         \
-               "Check failed: (%s) != (%s) ('%s' vs '%s').", #exp, #found, \
-               exp_, found_ ? found_ : "<null>");                          \
-    }                                                                      \
+#define CHECK_CSTREQ(exp, found)                                              \
+  do {                                                                        \
+    const char* exp_ = (exp);                                                 \
+    const char* found_ = (found);                                             \
+    DCHECK_NOT_NULL(exp);                                                     \
+    if (V8_UNLIKELY(found_ == nullptr || strcmp(exp_, found_) != 0)) {        \
+      FATAL("Check failed: (%s) != (%s) ('%s' vs '%s').", #exp, #found, exp_, \
+            found_ ? found_ : "<null>");                                      \
+    }                                                                         \
   } while (false)
 
 struct ExceptionInfo {

test/fuzzer/regexp-builtins.cc

@@ -374,8 +374,7 @@ void CompileRunAndVerify(FuzzerArgs* args, const std::string& source) {
     uint32_t hash = StringHasher::HashSequentialString(
         args->input_data, static_cast<int>(args->input_length),
         kRegExpBuiltinsFuzzerHashSeed);
-    V8_Fatal(__FILE__, __LINE__,
-             "!ResultAreIdentical(args); RegExpBuiltinsFuzzerHash=%x", hash);
+    FATAL("!ResultAreIdentical(args); RegExpBuiltinsFuzzerHash=%x", hash);
   }
 }