[runtime] Fully remove RUNTIME_ASSERT for good.

This fully deprecates all uses of the RUNTIME_ASSERT macro and removes
the macro and underlying logging function in question. All uses have
been replaces with CHECK macros which crash safely even in production.

It makes sure we discover abuse of runtime functions in the wild early
and also abort the process safely. Breaking assumptions in any runtime
function can no longer accidentally be caught by JavaScript.

R=yangguo@chromium.org
BUG=v8:5066

Review-Url: https://codereview.chromium.org/2132493002
Cr-Commit-Position: refs/heads/master@{#37704}

src/base/logging.cc

@@ -115,14 +115,3 @@ extern "C" void V8_Fatal(const char* file, int line, const char* format, ...) {
   fflush(stderr);
   v8::base::OS::Abort();
 }
-
-extern "C" void V8_RuntimeError(const char* file, int line,
-                                const char* message) {
-  fflush(stdout);
-  fflush(stderr);
-  v8::base::OS::PrintError("\n\n#\n# Runtime error in %s, line %d\n# ", file,
-                           line);
-  v8::base::OS::PrintError("\n# %s\n", message);
-  v8::base::DumpBacktrace();
-  fflush(stderr);
-}

src/base/logging.h

@@ -15,9 +15,6 @@
 extern "C" PRINTF_FORMAT(3, 4) V8_NORETURN
     void V8_Fatal(const char* file, int line, const char* format, ...);
 
-extern "C" void V8_RuntimeError(const char* file, int line,
-                                const char* message);
-
 // The FATAL, UNREACHABLE and UNIMPLEMENTED macros are useful during
 // development, but they should not be relied on in the final product.
 #ifdef DEBUG

src/debug/mirrors.js

@@ -2342,6 +2342,7 @@ ScriptMirror.prototype.source = function() {
 
 
 ScriptMirror.prototype.setSource = function(source) {
+  if (!IS_STRING(source)) throw MakeError(kDebugger, "Source is not a string");
   %DebugSetScriptSource(this.script_, source);
 };
 

src/runtime/runtime-strings.cc

@@ -292,11 +292,13 @@ RUNTIME_FUNCTION(Runtime_SubString) {
     CONVERT_SMI_ARG_CHECKED(to_number, 2);
     start = from_number;
     end = to_number;
-  } else {
+  } else if (args[1]->IsNumber() && args[2]->IsNumber()) {
     CONVERT_DOUBLE_ARG_CHECKED(from_number, 1);
     CONVERT_DOUBLE_ARG_CHECKED(to_number, 2);
     start = FastD2IChecked(from_number);
     end = FastD2IChecked(to_number);
+  } else {
+    return isolate->ThrowIllegalOperation();
   }
   // The following condition is intentionally robust because the SubStringStub
   // delegates here and we test this in cctest/test-strings/RobustSubStringStub.

src/runtime/runtime-utils.h

@@ -11,117 +11,83 @@
 namespace v8 {
 namespace internal {
 
-#ifdef DEBUG
-
-#define RUNTIME_ASSERT(value)                      \
-  do {                                             \
-    if (!(value)) {                                \
-      V8_RuntimeError(__FILE__, __LINE__, #value); \
-      return isolate->ThrowIllegalOperation();     \
-    }                                              \
-  } while (0)
-
-#else
-
-#define RUNTIME_ASSERT(value)                  \
-  do {                                         \
-    if (!(value)) {                            \
-      return isolate->ThrowIllegalOperation(); \
-    }                                          \
-  } while (0)
-
-#endif
-
 // Cast the given object to a value of the specified type and store
 // it in a variable with the given name.  If the object is not of the
-// expected type call IllegalOperation and return.
+// expected type we crash safely.
 #define CONVERT_ARG_CHECKED(Type, name, index) \
-  RUNTIME_ASSERT(args[index]->Is##Type());     \
+  CHECK(args[index]->Is##Type());              \
   Type* name = Type::cast(args[index]);
 
 #define CONVERT_ARG_HANDLE_CHECKED(Type, name, index) \
-  RUNTIME_ASSERT(args[index]->Is##Type());            \
+  CHECK(args[index]->Is##Type());                     \
   Handle<Type> name = args.at<Type>(index);
 
 #define CONVERT_NUMBER_ARG_HANDLE_CHECKED(name, index) \
-  RUNTIME_ASSERT(args[index]->IsNumber());             \
+  CHECK(args[index]->IsNumber());                      \
   Handle<Object> name = args.at<Object>(index);
 
 // Cast the given object to a boolean and store it in a variable with
-// the given name.  If the object is not a boolean call IllegalOperation
-// and return.
+// the given name.  If the object is not a boolean we crash safely.
 #define CONVERT_BOOLEAN_ARG_CHECKED(name, index) \
-  RUNTIME_ASSERT(args[index]->IsBoolean());      \
+  CHECK(args[index]->IsBoolean());               \
   bool name = args[index]->IsTrue(isolate);
 
 // Cast the given argument to a Smi and store its value in an int variable
-// with the given name.  If the argument is not a Smi call IllegalOperation
-// and return.
+// with the given name.  If the argument is not a Smi we crash safely.
 #define CONVERT_SMI_ARG_CHECKED(name, index) \
-  RUNTIME_ASSERT(args[index]->IsSmi());      \
+  CHECK(args[index]->IsSmi());               \
   int name = args.smi_at(index);
 
 // Cast the given argument to a double and store it in a variable with
 // the given name.  If the argument is not a number (as opposed to
-// the number not-a-number) call IllegalOperation and return.
+// the number not-a-number) we crash safely.
 #define CONVERT_DOUBLE_ARG_CHECKED(name, index) \
-  RUNTIME_ASSERT(args[index]->IsNumber());      \
+  CHECK(args[index]->IsNumber());               \
   double name = args.number_at(index);
 
-
 // Cast the given argument to a size_t and store its value in a variable with
-// the given name.  If the argument is not a size_t call IllegalOperation and
-// return.
+// the given name.  If the argument is not a size_t we crash safely.
 #define CONVERT_SIZE_ARG_CHECKED(name, index)            \
-  RUNTIME_ASSERT(args[index]->IsNumber());               \
+  CHECK(args[index]->IsNumber());                        \
   Handle<Object> name##_object = args.at<Object>(index); \
   size_t name = 0;                                       \
-  RUNTIME_ASSERT(TryNumberToSize(isolate, *name##_object, &name));
-
+  CHECK(TryNumberToSize(isolate, *name##_object, &name));
 
 // Call the specified converter on the object *comand store the result in
 // a variable of the specified type with the given name.  If the
-// object is not a Number call IllegalOperation and return.
+// object is not a Number we crash safely.
 #define CONVERT_NUMBER_CHECKED(type, name, Type, obj) \
-  RUNTIME_ASSERT(obj->IsNumber());                    \
+  CHECK(obj->IsNumber());                             \
   type name = NumberTo##Type(obj);
 
-
 // Cast the given argument to PropertyDetails and store its value in a
-// variable with the given name.  If the argument is not a Smi call
-// IllegalOperation and return.
+// variable with the given name.  If the argument is not a Smi we crash safely.
 #define CONVERT_PROPERTY_DETAILS_CHECKED(name, index) \
-  RUNTIME_ASSERT(args[index]->IsSmi());               \
+  CHECK(args[index]->IsSmi());                        \
   PropertyDetails name = PropertyDetails(Smi::cast(args[index]));
 
-
 // Assert that the given argument has a valid value for a LanguageMode
 // and store it in a LanguageMode variable with the given name.
-#define CONVERT_LANGUAGE_MODE_ARG_CHECKED(name, index)        \
-  RUNTIME_ASSERT(args[index]->IsSmi());                       \
-  RUNTIME_ASSERT(is_valid_language_mode(args.smi_at(index))); \
+#define CONVERT_LANGUAGE_MODE_ARG_CHECKED(name, index) \
+  CHECK(args[index]->IsSmi());                         \
+  CHECK(is_valid_language_mode(args.smi_at(index)));   \
   LanguageMode name = static_cast<LanguageMode>(args.smi_at(index));
 
-
 // Assert that the given argument is a number within the Int32 range
-// and convert it to int32_t.  If the argument is not an Int32 call
-// IllegalOperation and return.
+// and convert it to int32_t.  If the argument is not an Int32 we crash safely.
 #define CONVERT_INT32_ARG_CHECKED(name, index) \
-  RUNTIME_ASSERT(args[index]->IsNumber());     \
+  CHECK(args[index]->IsNumber());              \
   int32_t name = 0;                            \
-  RUNTIME_ASSERT(args[index]->ToInt32(&name));
-
+  CHECK(args[index]->ToInt32(&name));
 
 // Cast the given argument to PropertyAttributes and store its value in a
-// variable with the given name.  If the argument is not a Smi call or the
-// enum value is out of range, call IllegalOperation and return.
-#define CONVERT_PROPERTY_ATTRIBUTES_CHECKED(name, index)                   \
-  RUNTIME_ASSERT(args[index]->IsSmi());                                    \
-  RUNTIME_ASSERT(                                                          \
-      (args.smi_at(index) & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) == 0); \
+// variable with the given name.  If the argument is not a Smi or the
+// enum value is out of range, we crash safely.
+#define CONVERT_PROPERTY_ATTRIBUTES_CHECKED(name, index)                     \
+  CHECK(args[index]->IsSmi());                                               \
+  CHECK((args.smi_at(index) & ~(READ_ONLY | DONT_ENUM | DONT_DELETE)) == 0); \
   PropertyAttributes name = static_cast<PropertyAttributes>(args.smi_at(index));
 
-
 // A mechanism to return a pair of Object pointers in registers (if possible).
 // How this is achieved is calling convention-dependent.
 // All currently supported x86 compiles uses calling conventions that are cdecl

test/mjsunit/es6/reflect-define-property.js

@@ -441,28 +441,6 @@ try {
 }
 
 
-// Test runtime calls to DefineAccessorPropertyUnchecked - make sure we don't
-// crash.
-try {
-  %DefineAccessorPropertyUnchecked(0, 0, 0, 0, 0);
-} catch (e) {
-  assertTrue(/illegal access/.test(e));
-}
-
-try {
-  %DefineAccessorPropertyUnchecked(null, null, null, null, null);
-} catch (e) {
-  assertTrue(/illegal access/.test(e));
-}
-
-// Defining properties null should fail even when we have
-// other allowed values
-try {
-  %DefineAccessorPropertyUnchecked(null, 'foo', func, null, 0);
-} catch (e) {
-  assertTrue(/illegal access/.test(e));
-}
-
 // Test that all possible differences in step 6 in DefineOwnProperty are
 // exercised, i.e., any difference in the given property descriptor and the
 // existing properties should not return true, but throw an error if the

test/mjsunit/object-define-property.js

@@ -467,28 +467,6 @@ try {
 }
 
 
-// Test runtime calls to DefineAccessorPropertyUnchecked - make sure we don't
-// crash.
-try {
-  %DefineAccessorPropertyUnchecked(0, 0, 0, 0, 0);
-} catch (e) {
-  assertTrue(/illegal access/.test(e));
-}
-
-try {
-  %DefineAccessorPropertyUnchecked(null, null, null, null, null);
-} catch (e) {
-  assertTrue(/illegal access/.test(e));
-}
-
-// Defining properties null should fail even when we have
-// other allowed values
-try {
-  %DefineAccessorPropertyUnchecked(null, 'foo', func, null, 0);
-} catch (e) {
-  assertTrue(/illegal access/.test(e));
-}
-
 // Test that all possible differences in step 6 in DefineOwnProperty are
 // exercised, i.e., any difference in the given property descriptor and the
 // existing properties should not return true, but throw an error if the