This patch is the propagation version of https://codereview.chromium.org/10824032 patch

test/cctest/test-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 11590 matching lines @@
   v8::internal::Isolate::Current()->compilation_cache()->Clear();
 
   // Verify that entry hooking is now disabled.
   RunLoopInNewEnv();
   CHECK_EQ(0u, bar_count);
   CHECK_EQ(0u, foo_count);
 }
 
 
 static i::HashMap* code_map = NULL;
+static i::HashMap* jitcode_line_info = NULL;
 static int saw_bar = 0;
 static int move_events = 0;
 
 
 static bool FunctionNameIs(const char* expected,
                            const v8::JitCodeEvent* event) {
   // Log lines for functions are of the general form:
   // "LazyCompile:<type><function_name>", where the type is one of
   // "*", "~" or "".
   static const char kPreamble[] = "LazyCompile:";
@@ skipping 19 matching lines @@
   if (tail_len != expected_len)
     return false;
 
   return strncmp(tail, expected, expected_len) == 0;
 }
 
 
 static void event_handler(const v8::JitCodeEvent* event) {
   CHECK(event != NULL);
   CHECK(code_map != NULL);
+  CHECK(jitcode_line_info != NULL);
 
   switch (event->type) {
     case v8::JitCodeEvent::CODE_ADDED: {
         CHECK(event->code_start != NULL);
         CHECK_NE(0, static_cast<int>(event->code_len));
         CHECK(event->name.str != NULL);
         i::HashMap::Entry* entry =
             code_map->Lookup(event->code_start,
                              i::ComputePointerHash(event->code_start),
                              true);
@@ skipping 28 matching lines @@
           CHECK(entry != NULL);
           entry->value = reinterpret_cast<void*>(event->code_len);
         }
       }
       break;
 
     case v8::JitCodeEvent::CODE_REMOVED:
       // Object/code removal events are currently not dispatched from the GC.
       CHECK(false);
       break;
+
+    // For CODE_START_LINE_INFO_RECORDING event, we will create one
+    // JITCodeLineInfo data structure pointed by event->user_dat. We record it

[danno, 2013/01/10 16:47:33]

nit: user_data

+    // in jitcode_line_info.
+    case v8::JitCodeEvent::CODE_START_LINE_INFO_RECORDING: {
+        CHECK(event->user_data != NULL);
+        i::HashMap::Entry* entry =
+            jitcode_line_info->Lookup(event->user_data,
+                             i::ComputePointerHash(event->user_data),

[danno, 2013/01/10 16:47:33]

nit: style, please use Google C++ coding guidelines (parameter alignment)

+                             true);
+        entry->value = reinterpret_cast<void*>(event->user_data);
+      }
+      break;
+    // For these two events, we will check whether the event->user_data
+    // data structure is created before during CODE_START_LINE_INFO_RECORDING
+    // event. And delete it in CODE_END_LINE_INFO_RECORDING event handling.
+    case v8::JitCodeEvent::CODE_END_LINE_INFO_RECORDING: {
+        CHECK(event->user_data != NULL);
+        uint32_t hash = i::ComputePointerHash(event->user_data);
+        i::HashMap::Entry* entry =
+            jitcode_line_info->Lookup(event->user_data, hash, false);
+        CHECK(entry != NULL);
+        delete reinterpret_cast<i::JITCodeLineInfo*>(event->user_data);

[danno, 2013/01/10 16:47:33]

YIKES! Disposing of a void* that is allocated within V8 should _never_ be the
responsibility of a client of the V8 API. It exposes an implementation detail
that the client should not know about. 

The lifecycle of a data structure should either be completely controlled by V8
or completely by the event handler, not mixed. As mentioned above, I believe the
right thing to do is make JITCodeLineInfo a VTune-specific thing and make it
completely managed by the event handler. In the tests, you don't have to have a
full-blown JITCodeLineInfo, but rather you should just verify that the user_data
is passed through correctly and that the right ODE_ADD_LINE_POS_INFO events get
sent. You don't need a JITCodeLineInfo for that.

[chunyang.dai, 2013/01/18 10:22:44]

I am sorry I misused it. I moved the memory management to the event_handler.

+      }
+      break;
+
+    case v8::JitCodeEvent::CODE_ADD_LINE_POS_INFO: {
+        CHECK(event->user_data != NULL);
+        uint32_t hash = i::ComputePointerHash(event->user_data);
+        i::HashMap::Entry* entry =
+            jitcode_line_info->Lookup(event->user_data, hash, false);
+        CHECK(entry != NULL);
+      }
+      break;
+
     default:
       // Impossible event.
       CHECK(false);
       break;
   }
 }
 
 
 static bool MatchPointers(void* key1, void* key2) {
   return key1 == key2;
@@ skipping 13 matching lines @@
 
   // Run this test in a new isolate to make sure we don't
   // have remnants of state from other code.
   v8::Isolate* isolate = v8::Isolate::New();
   isolate->Enter();
 
   {
     i::HashMap code(MatchPointers);
     code_map = &code;
 
+    i::HashMap lineinfo(MatchPointers);
+    jitcode_line_info = &lineinfo;
+
     saw_bar = 0;
     move_events = 0;
 
     i::FLAG_stress_compaction = true;
     V8::SetJitCodeEventHandler(v8::kJitCodeEventDefault, event_handler);
 
     v8::HandleScope scope;
     // Generate new code objects sparsely distributed across several
     // different fragmented code-space pages.
     const int kIterations = 10;
@@ skipping 12 matching lines @@
       ISOLATE->compilation_cache()->Clear();
     }
 
     // Force code movement.
     HEAP->CollectAllAvailableGarbage("TestSetJitCodeEventHandler");
 
     CHECK_LE(kIterations, saw_bar);
     CHECK_NE(0, move_events);
 
     code_map = NULL;
+    jitcode_line_info = NULL;
     V8::SetJitCodeEventHandler(v8::kJitCodeEventDefault, NULL);
   }
 
   isolate->Exit();
   isolate->Dispose();
 
   // Do this in a new isolate.
   isolate = v8::Isolate::New();
   isolate->Enter();
 
   // Verify that we get callbacks for existing code objects when we
   // request enumeration of existing code.
   {
     v8::HandleScope scope;
     LocalContext env;
     CompileRun(script);
 
     // Now get code through initial iteration.
     i::HashMap code(MatchPointers);
     code_map = &code;
 
+    i::HashMap lineinfo(MatchPointers);
+    jitcode_line_info = &lineinfo;
+
     V8::SetJitCodeEventHandler(v8::kJitCodeEventEnumExisting, event_handler);
     V8::SetJitCodeEventHandler(v8::kJitCodeEventDefault, NULL);
 
     code_map = NULL;
+    jitcode_line_info = NULL;
 
     // We expect that we got some events. Note that if we could get code removal
     // notifications, we could compare two collections, one created by listening
     // from the time of creation of an isolate, and the other by subscribing
     // with EnumExisting.
     CHECK_NE(0, code.occupancy());
   }
 
   isolate->Exit();
   isolate->Dispose();
@@ skipping 6391 matching lines @@
 
   i::Semaphore* sem_;
   volatile int sem_value_;
 };
 
 
 THREADED_TEST(SemaphoreInterruption) {
   ThreadInterruptTest().RunTest();
 }
 #endif  // WIN32