Improved function entry hook coverage

src/api.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 5129 matching lines @@
   i::V8::SetEntropySource(source);
 }
 
 
 void v8::V8::SetReturnAddressLocationResolver(
       ReturnAddressLocationResolver return_address_resolver) {
   i::V8::SetReturnAddressLocationResolver(return_address_resolver);
 }
 
 
 bool v8::V8::SetFunctionEntryHook(FunctionEntryHook entry_hook) {

[danno, 2013/06/13 15:33:44]

API calls should take an Isolate parameter if you need it. To deprecate the
other API, you might need to have both the Isolate and non-isolate version for a
while, the non-isolate version calling the isolate version with
"i::Isolate::Current();"

[Sigurður Ásgeirsson, 2013/06/19 20:51:51]

Done.

-  return i::ProfileEntryHookStub::SetFunctionEntryHook(entry_hook);
+  i::Isolate* isolate = i::Isolate::Current();
+
+  // We don't allow switching from one hook to another, as hooks don't stack.
+  // This way a second user can at least detect that there's a problem.
+  if (entry_hook != NULL && isolate->HasFunctionEntryHook())

[danno, 2013/06/13 15:33:44]

Seems like this should be a ASSERT(entry_hook != NULL); and a if
(isolate->HasFunctionEntryHook()) {

[Sigurður Ásgeirsson, 2013/06/19 20:51:51]

Done.

+    return false;
+
+  isolate->SetFunctionEntryHook(entry_hook);

[danno, 2013/06/13 15:33:44]

It would be great to introduce some checks to make sure that this routine is
called at a time that is acceptable. It must be invoked on an Isolate before the
first code object is generated, otherwise you will get strange effects. Is there
any way to police that? Or perhaps introduce a new isolate constructor that
takes the hook to make sure that it's always set from the very beginning?
Something like that might make this functionality harder to abuse.

[Sigurður Ásgeirsson, 2013/06/19 20:51:51]

Done.

+  return true;
 }
 
 
 void v8::V8::SetJitCodeEventHandler(
     JitCodeEventOptions options, JitCodeEventHandler event_handler) {
   i::Isolate* isolate = i::Isolate::Current();
   // Ensure that logging is initialized for our isolate.
   isolate->InitializeLoggingAndCounters();
   isolate->logger()->SetCodeEventHandler(options, event_handler);
 }
@@ skipping 2834 matching lines @@
 
   v->VisitPointers(blocks_.first(), first_block_limit_);
 
   for (int i = 1; i < blocks_.length(); i++) {
     v->VisitPointers(blocks_[i], &blocks_[i][kHandleBlockSize]);
   }
 }
 
 
 } }  // namespace v8::internal