[debug] additional checks for built-ins calling runtime functions.

R=jgruber@chromium.org
BUG=v8:5821

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

src/debug/debug-evaluate.cc

@@ -260,58 +260,80 @@ namespace {
 
 bool IntrinsicHasNoSideEffect(Runtime::FunctionId id) {
 // Use macro to include both inlined and non-inlined version of an intrinsic.
-#define INTRINSIC_WHITELIST(V)      \
-  /* Conversions */                 \
-  V(ToInteger)                      \
-  V(ToObject)                       \
-  V(ToString)                       \
-  V(ToLength)                       \
-  V(ToNumber)                       \
-  /* Type checks */                 \
-  V(IsJSReceiver)                   \
-  V(IsSmi)                          \
-  V(IsArray)                        \
-  V(IsFunction)                     \
-  V(IsDate)                         \
-  V(IsJSProxy)                      \
-  V(IsRegExp)                       \
-  V(IsTypedArray)                   \
-  V(ClassOf)                        \
-  /* Loads */                       \
-  V(LoadLookupSlotForCall)          \
-  /* Arrays */                      \
-  V(ArraySpeciesConstructor)        \
-  V(NormalizeElements)              \
-  V(GetArrayKeys)                   \
-  V(HasComplexElements)             \
-  V(EstimateNumberOfElements)       \
-  /* Errors */                      \
-  V(ReThrow)                        \
-  V(ThrowReferenceError)            \
-  V(ThrowSymbolIteratorInvalid)     \
-  V(ThrowIteratorResultNotAnObject) \
-  V(NewTypeError)                   \
-  /* Strings */                     \
-  V(StringCharCodeAt)               \
-  V(StringIndexOf)                  \
-  V(StringReplaceOneCharWithString) \
-  V(SubString)                      \
-  V(RegExpInternalReplace)          \
-  /* Literals */                    \
-  V(CreateArrayLiteral)             \
-  V(CreateObjectLiteral)            \
-  V(CreateRegExpLiteral)            \
-  /* Collections */                 \
-  V(JSCollectionGetTable)           \
-  V(FixedArrayGet)                  \
-  V(StringGetRawHashField)          \
-  V(GenericHash)                    \
-  V(MapIteratorInitialize)          \
-  V(MapInitialize)                  \
-  /* Misc. */                       \
-  V(ForInPrepare)                   \
-  V(Call)                           \
-  V(MaxSmi)                         \
+#define INTRINSIC_WHITELIST(V)       \
+  /* Conversions */                  \
+  V(ToInteger)                       \
+  V(ToObject)                        \
+  V(ToString)                        \
+  V(ToLength)                        \
+  V(ToNumber)                        \
+  /* Type checks */                  \
+  V(IsJSReceiver)                    \
+  V(IsSmi)                           \
+  V(IsArray)                         \
+  V(IsFunction)                      \
+  V(IsDate)                          \
+  V(IsJSProxy)                       \
+  V(IsRegExp)                        \
+  V(IsTypedArray)                    \
+  V(ClassOf)                         \
+  /* Loads */                        \
+  V(LoadLookupSlotForCall)           \
+  /* Arrays */                       \
+  V(ArraySpeciesConstructor)         \
+  V(NormalizeElements)               \
+  V(GetArrayKeys)                    \
+  V(HasComplexElements)              \
+  V(EstimateNumberOfElements)        \
+  /* Errors */                       \
+  V(ReThrow)                         \
+  V(ThrowReferenceError)             \
+  V(ThrowSymbolIteratorInvalid)      \
+  V(ThrowIteratorResultNotAnObject)  \
+  V(NewTypeError)                    \
+  /* Strings */                      \
+  V(StringCharCodeAt)                \
+  V(StringIndexOf)                   \
+  V(StringReplaceOneCharWithString)  \
+  V(SubString)                       \
+  V(RegExpInternalReplace)           \
+  /* Literals */                     \
+  V(CreateArrayLiteral)              \
+  V(CreateObjectLiteral)             \
+  V(CreateRegExpLiteral)             \
+  /* Collections */                  \
+  V(JSCollectionGetTable)            \
+  V(FixedArrayGet)                   \
+  V(StringGetRawHashField)           \
+  V(GenericHash)                     \
+  V(MapIteratorInitialize)           \
+  V(MapInitialize)                   \
+  /* Called from builtins */         \
+  V(StringParseFloat)                \
+  V(StringParseInt)                  \
+  V(StringCharCodeAtRT)              \
+  V(StringIndexOfUnchecked)          \
+  V(SymbolDescriptiveString)         \
+  V(GenerateRandomNumbers)           \
+  V(ExternalStringGetChar)           \
+  V(GlobalPrint)                     \
+  V(AllocateInNewSpace)              \
+  V(AllocateSeqOneByteString)        \
+  V(AllocateSeqTwoByteString)        \
+  V(ObjectCreate)                    \
+  V(ObjectHasOwnProperty)            \
+  V(ArrayIndexOf)                    \
+  V(ArrayIncludes_Slow)              \
+  V(ArrayIsArray)                    \
+  V(ThrowTypeError)                  \
+  V(ThrowCalledOnNullOrUndefined)    \
+  V(ThrowIncompatibleMethodReceiver) \
+  V(ThrowInvalidHint)                \
+  V(ThrowNotDateError)               \
+  /* Misc. */                        \
+  V(ForInPrepare)                    \
+  V(Call)                            \
+  V(MaxSmi)                          \
   V(HasInPrototypeChain)
 
 #define CASE(Name)       \
@@ -649,6 +671,22 @@ bool DebugEvaluate::FunctionHasNoSideEffect(Handle<SharedFunctionInfo> info) {
     int builtin_index = info->code()->builtin_index();
     if (builtin_index >= 0 && builtin_index < Builtins::builtin_count &&
         BuiltinHasNoSideEffect(static_cast<Builtins::Name>(builtin_index))) {
+#ifdef DEBUG
+      int mode = RelocInfo::ModeMask(RelocInfo::EXTERNAL_REFERENCE);
+      bool failed = false;
+      for (RelocIterator it(info->code(), mode); !it.done(); it.next()) {
+        RelocInfo* rinfo = it.rinfo();
+        Address address = rinfo->target_external_reference();
+        const Runtime::Function* function = Runtime::FunctionForEntry(address);
+        if (function == nullptr) continue;
+        if (!IntrinsicHasNoSideEffect(function->function_id)) {
+          PrintF("Whitelisted builtin %s calls non-whitelisted intrinsic %s\n",
+                 Builtins::name(builtin_index), function->name);
+          failed = true;
+        }
+        CHECK(!failed);
+      }
+#endif  // DEBUG
       return true;
     }
   }

test/cctest/test-debug.cc

@@ -6712,3 +6712,29 @@ TEST(DebugGetPossibleBreakpointsReturnLocations) {
     CHECK(returns_count == 1);
   }
 }
+
+TEST(DebugEvaluateNoSideEffect) {
+  LocalContext env;
+  i::Isolate* isolate = CcTest::i_isolate();
+  i::HandleScope scope(isolate);
+  i::List<i::Handle<i::JSFunction>> list;
+  {
+    i::HeapIterator iterator(isolate->heap());
+    while (i::HeapObject* obj = iterator.next()) {
+      if (!obj->IsJSFunction()) continue;
+      i::JSFunction* fun = i::JSFunction::cast(obj);
+      list.Add(i::Handle<i::JSFunction>(fun));
+    }
+  }
+
+  // Perform side effect check on all built-in functions. The side effect check
+  // itself contains additional sanity checks.
+  for (i::Handle<i::JSFunction> fun : list) {
+    bool failed = false;
+    {
+      i::NoSideEffectScope scope(isolate, true);
+      failed = !isolate->debug()->PerformSideEffectCheck(fun);
+    }
+    if (failed) isolate->clear_pending_exception();
+  }
+}