[v8-dev] Guard against undefined fields in global context. (issue 8917014)

Mon, 12 Dec 2011 11:11:19 -0800 

Reviewers: Vyacheslav Egorov,

Message:
Thank you for hunting down the bug.
There are other places that access the global contest list, but they (including 
visitors) seem to be safe.


http://codereview.chromium.org/8917014/diff/3/src/deoptimizer.cc
File src/deoptimizer.cc (right):

http://codereview.chromium.org/8917014/diff/3/src/deoptimizer.cc#newcode269
src/deoptimizer.cc:269: Object* global =
Context::cast(context)->get(Context::GLOBAL_INDEX);
Do we need check here? Is global object always defined?

Description:
Guard against undefined fields in global context.

BUG=v8:1860
TEST=
[email protected]

Please review this at http://codereview.chromium.org/8917014/

SVN Base: https://v8.googlecode.com/svn/branches/bleeding_edge

Affected files:
  M src/assembler.h
  M src/assembler.cc
  M src/deoptimizer.cc
  M src/heap.cc
  M src/incremental-marking.cc


Index: src/assembler.cc
diff --git a/src/assembler.cc b/src/assembler.cc
index 941f45c21132a5a9d09d929e12ed8d9d46c75333..b66f448407c8c1982bfc1336f32eb9c756f3e39e 100644 
--- a/src/assembler.cc
+++ b/src/assembler.cc
@@ -817,11 +817,6 @@ ExternalReference ExternalReference::compute_output_frames_function( 
 }
-ExternalReference ExternalReference::global_contexts_list(Isolate* isolate) { - return ExternalReference(isolate->heap()->global_contexts_list_address()); 
-}
-
-
ExternalReference ExternalReference::keyed_lookup_cache_keys(Isolate* isolate) { 
   return ExternalReference(isolate->keyed_lookup_cache()->keys_address());
 }
Index: src/assembler.h
diff --git a/src/assembler.h b/src/assembler.h
index 5c25768e6a0a60aead490b16c55cfed5a6892507..cec20fca073add3d9b77a7f4aa4d873b4df184d9 100644 
--- a/src/assembler.h
+++ b/src/assembler.h
@@ -590,7 +590,6 @@ class ExternalReference BASE_EMBEDDED {
   // Deoptimization support.
   static ExternalReference new_deoptimizer_function(Isolate* isolate);
static ExternalReference compute_output_frames_function(Isolate* isolate); 
-  static ExternalReference global_contexts_list(Isolate* isolate);

   // Static data in the keyed lookup cache.
   static ExternalReference keyed_lookup_cache_keys(Isolate* isolate);
Index: src/deoptimizer.cc
diff --git a/src/deoptimizer.cc b/src/deoptimizer.cc
index 108e547f2b95e6197a6d282bb432f7d06dde5ca2..b6df432322dfdb7c9ed11ba97cecd775cac7e9b4 100644 
--- a/src/deoptimizer.cc
+++ b/src/deoptimizer.cc
@@ -264,11 +264,14 @@ void Deoptimizer::VisitAllOptimizedFunctions(
   AssertNoAllocation no_allocation;

   // Run through the list of all global contexts and deoptimize.
-  Object* global = Isolate::Current()->heap()->global_contexts_list();
-  while (!global->IsUndefined()) {
- VisitAllOptimizedFunctionsForGlobalObject(Context::cast(global)->global(), 
-                                              visitor);
-    global = Context::cast(global)->get(Context::NEXT_CONTEXT_LINK);
+  Object* context = Isolate::Current()->heap()->global_contexts_list();
+  while (!context->IsUndefined()) {
+    Object* global = Context::cast(context)->get(Context::GLOBAL_INDEX);
+    if (!global->IsUndefined()) {
+      VisitAllOptimizedFunctionsForGlobalObject(JSObject::cast(global),
+                                                visitor);
+    }
+    context = Context::cast(context)->get(Context::NEXT_CONTEXT_LINK);
   }
 }

Index: src/heap.cc
diff --git a/src/heap.cc b/src/heap.cc
index bc7550ed9a63e3a8ff55a6ed815ea4b93d8b9f79..2a3439f306466c6850a98f49cb64b8ff3c1224f9 100644 
--- a/src/heap.cc
+++ b/src/heap.cc
@@ -648,7 +648,10 @@ void Heap::ClearJSFunctionResultCaches() {
     // Clear the caches:
     int length = caches->length();
     for (int i = 0; i < length; i++) {
-      JSFunctionResultCache::cast(caches->get(i))->Clear();
+      Object* cache = caches->get(i);
+      if (!cache->IsUndefined()) {
+        JSFunctionResultCache::cast(cache)->Clear();
+      }
     }
     // Get the next context:
     context = Context::cast(context)->get(Context::NEXT_CONTEXT_LINK);
@@ -665,7 +668,11 @@ void Heap::ClearNormalizedMapCaches() {

   Object* context = global_contexts_list_;
   while (!context->IsUndefined()) {
-    Context::cast(context)->normalized_map_cache()->Clear();
+    int index = Context::NORMALIZED_MAP_CACHE_INDEX;
+    Object* cache = Context::cast(context)->get(index);
+    if (!cache->IsUndefined()) {
+      NormalizedMapCache::cast(cache)->Clear();
+    }
     context = Context::cast(context)->get(Context::NEXT_CONTEXT_LINK);
   }
 }
Index: src/incremental-marking.cc
diff --git a/src/incremental-marking.cc b/src/incremental-marking.cc
index dd54c630733f9c74905dcd7928641e9f9e0fb2f7..5ef3a14372f39cf59827b313eb1d5e536f7287ab 100644 
--- a/src/incremental-marking.cc
+++ b/src/incremental-marking.cc
@@ -677,11 +677,15 @@ void IncrementalMarking::Hurry() {

   Object* context = heap_->global_contexts_list();
   while (!context->IsUndefined()) {
- NormalizedMapCache* cache = Context::cast(context)->normalized_map_cache(); 
-    MarkBit mark_bit = Marking::MarkBitFrom(cache);
-    if (Marking::IsGrey(mark_bit)) {
-      Marking::GreyToBlack(mark_bit);
-      MemoryChunk::IncrementLiveBytes(cache->address(), cache->Size());
+    int index = Context::NORMALIZED_MAP_CACHE_INDEX;
+    Object* cache_or_undefined = Context::cast(context)->get(index);
+    if (!cache_or_undefined->IsUndefined()) {
+ NormalizedMapCache* cache = NormalizedMapCache::cast(cache_or_undefined); 
+      MarkBit mark_bit = Marking::MarkBitFrom(cache);
+      if (Marking::IsGrey(mark_bit)) {
+        Marking::GreyToBlack(mark_bit);
+        MemoryChunk::IncrementLiveBytes(cache->address(), cache->Size());
+      }
     }
     context = Context::cast(context)->get(Context::NEXT_CONTEXT_LINK);
   }


--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to