[v8-dev] Re: The Array function must be looked up in the native context. (issue 184383003)

Fri, 28 Feb 2014 02:40:28 -0800 

Reviewers: dcarney,

Message:
Committed patchset #2 manually as r19597 (tree was closed).

Description:
The Array function must be looked up in the native context.
Platforms x64, a64, arm and mips had the bug that the array function was looked up in the global context instead of the native context. Fix this, restoring a weakened assert in hydrogen along the way (by the fix for crbug 347528, which 
helped find this case).

[email protected]

Committed: https://code.google.com/p/v8/source/detail?r=19597

Please review this at https://codereview.chromium.org/184383003/

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

Affected files (+14, -80 lines):
  M src/a64/code-stubs-a64.cc
  M src/a64/macro-assembler-a64.h
  M src/a64/macro-assembler-a64.cc
  M src/arm/code-stubs-arm.cc
  M src/arm/macro-assembler-arm.h
  M src/arm/macro-assembler-arm.cc
  M src/hydrogen.cc
  M src/ia32/code-stubs-ia32.cc
  M src/ia32/macro-assembler-ia32.h
  M src/ia32/macro-assembler-ia32.cc
  M src/mips/code-stubs-mips.cc
  M src/mips/macro-assembler-mips.h
  M src/mips/macro-assembler-mips.cc
  M src/x64/code-stubs-x64.cc
  M src/x64/macro-assembler-x64.h
  M src/x64/macro-assembler-x64.cc


Index: src/a64/code-stubs-a64.cc
diff --git a/src/a64/code-stubs-a64.cc b/src/a64/code-stubs-a64.cc
index b8f5809fb86ed29f21e27b87af2d4202ad6033ee..89bc51cb60c675187d770396bfb59d237649ea9b 100644 
--- a/src/a64/code-stubs-a64.cc
+++ b/src/a64/code-stubs-a64.cc
@@ -3290,7 +3290,7 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { 
   __ JumpIfNotRoot(x5, Heap::kAllocationSiteMapRootIndex, &miss);

   // Make sure the function is the Array() function
-  __ LoadArrayFunction(x4);
+  __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, x4);
   __ Cmp(x1, x4);
   __ B(ne, &megamorphic);
   __ B(&done);
@@ -3312,7 +3312,7 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { 
   // indicate the ElementsKind if function is the Array constructor.
   __ Bind(&initialize);
   // Make sure the function is the Array() function
-  __ LoadArrayFunction(x4);
+  __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, x4);
   __ Cmp(x1, x4);
   __ B(ne, &not_array_function);

Index: src/a64/macro-assembler-a64.cc
diff --git a/src/a64/macro-assembler-a64.cc b/src/a64/macro-assembler-a64.cc
index 5b979c2b31ed496463018ce49d4c69ac73ceabb2..e1a8755f4fc24d25993d1a56aa3b1834e47b1972 100644 
--- a/src/a64/macro-assembler-a64.cc
+++ b/src/a64/macro-assembler-a64.cc
@@ -4712,17 +4712,6 @@ void MacroAssembler::LoadTransitionedArrayMapConditional( 
 }


-void MacroAssembler::LoadArrayFunction(Register function) {
-  // Load the global or builtins object from the current context.
-  Ldr(function, GlobalObjectMemOperand());
-  // Load the global context from the global or builtins object.
-  Ldr(function,
-      FieldMemOperand(function, GlobalObject::kGlobalContextOffset));
-  // Load the array function from the native context.
- Ldr(function, ContextMemOperand(function, Context::ARRAY_FUNCTION_INDEX)); 
-}
-
-
 void MacroAssembler::LoadGlobalFunction(int index, Register function) {
   // Load the global or builtins object from the current context.
   Ldr(function, GlobalObjectMemOperand());
Index: src/a64/macro-assembler-a64.h
diff --git a/src/a64/macro-assembler-a64.h b/src/a64/macro-assembler-a64.h
index 153e4d0367674edfb7412a48918117f7e9ec6661..09d667bbf0d4f0dd9d109d7ef33e13db6b655919 100644 
--- a/src/a64/macro-assembler-a64.h
+++ b/src/a64/macro-assembler-a64.h
@@ -1888,7 +1888,6 @@ class MacroAssembler : public Assembler {
       Register scratch,
       Label* no_map_match);

-  void LoadArrayFunction(Register function);
   void LoadGlobalFunction(int index, Register function);
// Load the initial map from the global function. The registers function and 
Index: src/arm/code-stubs-arm.cc
diff --git a/src/arm/code-stubs-arm.cc b/src/arm/code-stubs-arm.cc
index b3a3acf3e76a36a2eccdf9aaaca9958fdbbce0e8..ac46fe8c12e8f5c668827cb7caa1bce5ed8aa56c 100644 
--- a/src/arm/code-stubs-arm.cc
+++ b/src/arm/code-stubs-arm.cc
@@ -3037,7 +3037,7 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { 
   __ b(ne, &miss);

   // Make sure the function is the Array() function
-  __ LoadArrayFunction(r4);
+  __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, r4);
   __ cmp(r1, r4);
   __ b(ne, &megamorphic);
   __ jmp(&done);
@@ -3060,7 +3060,7 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { 
   // indicate the ElementsKind if function is the Array constructor.
   __ bind(&initialize);
   // Make sure the function is the Array() function
-  __ LoadArrayFunction(r4);
+  __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, r4);
   __ cmp(r1, r4);
   __ b(ne, &not_array_function);

Index: src/arm/macro-assembler-arm.cc
diff --git a/src/arm/macro-assembler-arm.cc b/src/arm/macro-assembler-arm.cc
index 58ce0a8545ceeabc6f0528de919ea2c49afcfd69..d705c90bd2edf62c953e2ea39449ada558b277e6 100644 
--- a/src/arm/macro-assembler-arm.cc
+++ b/src/arm/macro-assembler-arm.cc
@@ -2902,19 +2902,6 @@ void MacroAssembler::LoadGlobalFunction(int index, Register function) { 
 }


-void MacroAssembler::LoadArrayFunction(Register function) {
-  // Load the global or builtins object from the current context.
-  ldr(function,
-      MemOperand(cp, Context::SlotOffset(Context::GLOBAL_OBJECT_INDEX)));
-  // Load the global context from the global or builtins object.
-  ldr(function,
-      FieldMemOperand(function, GlobalObject::kGlobalContextOffset));
-  // Load the array function from the native context.
-  ldr(function,
- MemOperand(function, Context::SlotOffset(Context::ARRAY_FUNCTION_INDEX))); 
-}
-
-
 void MacroAssembler::LoadGlobalFunctionInitialMap(Register function,
                                                   Register map,
                                                   Register scratch) {
Index: src/arm/macro-assembler-arm.h
diff --git a/src/arm/macro-assembler-arm.h b/src/arm/macro-assembler-arm.h
index 67707fa311ec2c780a016247caa7ffd9fcf2a971..0eaf4dc9b10fcb5e7d4c8272b375a6585cb83f66 100644 
--- a/src/arm/macro-assembler-arm.h
+++ b/src/arm/macro-assembler-arm.h
@@ -571,7 +571,6 @@ class MacroAssembler: public Assembler {
       Label* no_map_match);

   void LoadGlobalFunction(int index, Register function);
-  void LoadArrayFunction(Register function);

   // Load the initial map from the global function. The registers
   // function and map can be the same, function is then overwritten.
Index: src/hydrogen.cc
diff --git a/src/hydrogen.cc b/src/hydrogen.cc
index 2f3df9e22a6b2b212c904b2e2198ec2f2fe47b1c..55d19e05b264910d35bbc57202d55e7e46be7aa2 100644 
--- a/src/hydrogen.cc
+++ b/src/hydrogen.cc
@@ -8225,15 +8225,14 @@ static bool IsAllocationInlineable(Handle<JSFunction> constructor) { 


 bool HOptimizedGraphBuilder::IsCallNewArrayInlineable(CallNew* expr) {
-  Handle<AllocationSite> site = expr->allocation_site();
-  if (site.is_null()) return false;
-
   Handle<JSFunction> caller = current_info()->closure();
   Handle<JSFunction> target(isolate()->native_context()->array_function(),
                             isolate());
   int argument_count = expr->arguments()->length();
// We should have the function plus array arguments on the environment stack. 
   ASSERT(environment()->length() >= (argument_count + 1));
+  Handle<AllocationSite> site = expr->allocation_site();
+  ASSERT(!site.is_null());

   bool inline_ok = false;
   if (site->CanInlineCall()) {
Index: src/ia32/code-stubs-ia32.cc
diff --git a/src/ia32/code-stubs-ia32.cc b/src/ia32/code-stubs-ia32.cc
index 78824195df456e2787c6be0f12e76c77ec56e125..2ef5a87da26eba19a5c4f9069bd90ebf8fef9633 100644 
--- a/src/ia32/code-stubs-ia32.cc
+++ b/src/ia32/code-stubs-ia32.cc
@@ -2352,11 +2352,9 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { 
   __ cmp(FieldOperand(ecx, 0), Immediate(allocation_site_map));
   __ j(not_equal, &miss);

-  // Load the global or builtins object from the current context
-  __ LoadGlobalContext(ecx);
   // Make sure the function is the Array() function
-  __ cmp(edi, Operand(ecx,
-                      Context::SlotOffset(Context::ARRAY_FUNCTION_INDEX)));
+  __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, ecx);
+  __ cmp(edi, ecx);
   __ j(not_equal, &megamorphic);
   __ jmp(&done, Label::kFar);
@@ -2377,10 +2375,9 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { 
   // An uninitialized cache is patched with the function or sentinel to
   // indicate the ElementsKind if function is the Array constructor.
   __ bind(&initialize);
-  __ LoadGlobalContext(ecx);
   // Make sure the function is the Array() function
-  __ cmp(edi, Operand(ecx,
-                      Context::SlotOffset(Context::ARRAY_FUNCTION_INDEX)));
+  __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, ecx);
+  __ cmp(edi, ecx);
   __ j(not_equal, &not_array_function);

   // The target function is the Array constructor,
Index: src/ia32/macro-assembler-ia32.cc
diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc index 2f463981824d71347a689f6f46248251feacf534..cc0f392f185212826144898536c1945fef233d7f 100644 
--- a/src/ia32/macro-assembler-ia32.cc
+++ b/src/ia32/macro-assembler-ia32.cc
@@ -2689,16 +2689,6 @@ void MacroAssembler::LoadTransitionedArrayMapConditional( 
 }


-void MacroAssembler::LoadGlobalContext(Register global_context) {
-  // Load the global or builtins object from the current context.
-  mov(global_context,
-      Operand(esi, Context::SlotOffset(Context::GLOBAL_OBJECT_INDEX)));
-  // Load the native context from the global or builtins object.
-  mov(global_context,
-      FieldOperand(global_context, GlobalObject::kNativeContextOffset));
-}
-
-
 void MacroAssembler::LoadGlobalFunction(int index, Register function) {
   // Load the global or builtins object from the current context.
   mov(function,
Index: src/ia32/macro-assembler-ia32.h
diff --git a/src/ia32/macro-assembler-ia32.h b/src/ia32/macro-assembler-ia32.h index 456845881f88daa6aff0d70912d5aa99dff3ddda..6b0573ca6eb443cbe4b47d35a71efffa90954793 100644 
--- a/src/ia32/macro-assembler-ia32.h
+++ b/src/ia32/macro-assembler-ia32.h
@@ -262,8 +262,6 @@ class MacroAssembler: public Assembler {
       Register scratch,
       Label* no_map_match);

-  void LoadGlobalContext(Register global_context);
-
   // Load the global function with the given index.
   void LoadGlobalFunction(int index, Register function);

Index: src/mips/code-stubs-mips.cc
diff --git a/src/mips/code-stubs-mips.cc b/src/mips/code-stubs-mips.cc
index 4b3107fec70c7fb78a9b72bb440406b8590a4554..dba129ac9a1c3b3c6993638240e39ebc338dfd17 100644 
--- a/src/mips/code-stubs-mips.cc
+++ b/src/mips/code-stubs-mips.cc
@@ -3184,7 +3184,7 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { 
   __ Branch(&miss, ne, t1, Operand(at));

   // Make sure the function is the Array() function
-  __ LoadArrayFunction(t0);
+  __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, t0);
   __ Branch(&megamorphic, ne, a1, Operand(t0));
   __ jmp(&done);
@@ -3207,7 +3207,7 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { 
   // indicate the ElementsKind if function is the Array constructor.
   __ bind(&initialize);
   // Make sure the function is the Array() function
-  __ LoadArrayFunction(t0);
+  __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, t0);
   __ Branch(&not_array_function, ne, a1, Operand(t0));

   // The target function is the Array constructor.
Index: src/mips/macro-assembler-mips.cc
diff --git a/src/mips/macro-assembler-mips.cc b/src/mips/macro-assembler-mips.cc index ddf926a765c4b55190911e78a689ba6930740b3b..1c22a89e156bec424701803d27d26e0d8baeeae1 100644 
--- a/src/mips/macro-assembler-mips.cc
+++ b/src/mips/macro-assembler-mips.cc
@@ -4442,19 +4442,6 @@ void MacroAssembler::LoadGlobalFunction(int index, Register function) { 
 }


-void MacroAssembler::LoadArrayFunction(Register function) {
-  // Load the global or builtins object from the current context.
-  lw(function,
-     MemOperand(cp, Context::SlotOffset(Context::GLOBAL_OBJECT_INDEX)));
-  // Load the global context from the global or builtins object.
-  lw(function,
-     FieldMemOperand(function, GlobalObject::kGlobalContextOffset));
-  // Load the array function from the native context.
-  lw(function,
- MemOperand(function, Context::SlotOffset(Context::ARRAY_FUNCTION_INDEX))); 
-}
-
-
 void MacroAssembler::LoadGlobalFunctionInitialMap(Register function,
                                                   Register map,
                                                   Register scratch) {
Index: src/mips/macro-assembler-mips.h
diff --git a/src/mips/macro-assembler-mips.h b/src/mips/macro-assembler-mips.h index 45bfcb38d28adc5125a9c908bbdeb256953c5709..1b64f25689d3c8d1d220bf38d1bbc5105ac9f79f 100644 
--- a/src/mips/macro-assembler-mips.h
+++ b/src/mips/macro-assembler-mips.h
@@ -872,7 +872,6 @@ class MacroAssembler: public Assembler {
       Label* no_map_match);

   void LoadGlobalFunction(int index, Register function);
-  void LoadArrayFunction(Register function);

   // Load the initial map from the global function. The registers
   // function and map can be the same, function is then overwritten.
Index: src/x64/code-stubs-x64.cc
diff --git a/src/x64/code-stubs-x64.cc b/src/x64/code-stubs-x64.cc
index 8c5a430014de2658879369f53378c2dd251dac05..9cf02fafd9473b5fd0f7d7ed23fa3bf524a573bf 100644 
--- a/src/x64/code-stubs-x64.cc
+++ b/src/x64/code-stubs-x64.cc
@@ -2194,7 +2194,7 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { 
   __ j(not_equal, &miss);

   // Make sure the function is the Array() function
-  __ LoadArrayFunction(rcx);
+  __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, rcx);
   __ cmpq(rdi, rcx);
   __ j(not_equal, &megamorphic);
   __ jmp(&done);
@@ -2216,7 +2216,7 @@ static void GenerateRecordCallTarget(MacroAssembler* masm) { 
   // indicate the ElementsKind if function is the Array constructor.
   __ bind(&initialize);
   // Make sure the function is the Array() function
-  __ LoadArrayFunction(rcx);
+  __ LoadGlobalFunction(Context::ARRAY_FUNCTION_INDEX, rcx);
   __ cmpq(rdi, rcx);
   __ j(not_equal, &not_array_function);

Index: src/x64/macro-assembler-x64.cc
diff --git a/src/x64/macro-assembler-x64.cc b/src/x64/macro-assembler-x64.cc
index 5988b7d2a69ba058fac9abbed05ee5b943be0252..95f17247a975896330620d443d7069d154018194 100644 
--- a/src/x64/macro-assembler-x64.cc
+++ b/src/x64/macro-assembler-x64.cc
@@ -4547,15 +4547,6 @@ void MacroAssembler::LoadGlobalFunction(int index, Register function) { 
 }


-void MacroAssembler::LoadArrayFunction(Register function) {
-  movp(function,
-       Operand(rsi, Context::SlotOffset(Context::GLOBAL_OBJECT_INDEX)));
- movp(function, FieldOperand(function, GlobalObject::kGlobalContextOffset)); 
-  movp(function,
- Operand(function, Context::SlotOffset(Context::ARRAY_FUNCTION_INDEX))); 
-}
-
-
 void MacroAssembler::LoadGlobalFunctionInitialMap(Register function,
                                                   Register map) {
   // Load the initial map.  The global functions all have initial maps.
Index: src/x64/macro-assembler-x64.h
diff --git a/src/x64/macro-assembler-x64.h b/src/x64/macro-assembler-x64.h
index 5efa2d921270874206fe8674e008222aa9bf4f63..63cf9c5bcc10c4a8371d84d616daafb5f5e3c070 100644 
--- a/src/x64/macro-assembler-x64.h
+++ b/src/x64/macro-assembler-x64.h
@@ -1236,7 +1236,6 @@ class MacroAssembler: public Assembler {

   // Load the global function with the given index.
   void LoadGlobalFunction(int index, Register function);
-  void LoadArrayFunction(Register function);

   // Load the initial map from the global function. The registers
   // function and map can be the same.


--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group. 
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to