Add constant splitting for user controlled constants in the full compiler

This is IA32 only for now.

Added a random value to each assembler instance (JIT cookie) to be used for constant splitting. Added safe versions of setting a register with an immediate value and for pushing an immediate value. Used these functions where user controlled immediate values could be emitted in the code stream. I also used it for immediates which are an argument number even though the number of formal arguments is currently limited to 16k.

I found no compares directly with user controlled constants.

I am not sure whether the test is that useful, but it might catch some changes missing constant splitting.

Review URL: http://codereview.chromium.org//7005031

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7868 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/assembler.cc b/src/assembler.cc
index 7a10e8e400abf8157cf753e7b026b089a4e23257..a75c94aa0d4570975be1367588806bad5043bf0d 100644 (file)
--- a/src/assembler.cc
+++ b/src/assembler.cc
@@ -73,6 +73,18 @@ const double DoubleConstant::nan = OS::nan_value();
 const double DoubleConstant::negative_infinity = -V8_INFINITY;
 const char* RelocInfo::kFillerCommentString = "DEOPTIMIZATION PADDING";
 
+// -----------------------------------------------------------------------------
+// Implementation of AssemblerBase
+
+AssemblerBase::AssemblerBase(Isolate* isolate)
+    : isolate_(isolate),
+      jit_cookie_(0) {
+  if (FLAG_mask_constants_with_cookie && isolate != NULL)  {
+    jit_cookie_ = V8::RandomPrivate(isolate);
+  }
+}
+
+
 // -----------------------------------------------------------------------------
 // Implementation of Label
 

diff --git a/src/assembler.h b/src/assembler.h
index 1f103d45b9b191566e0b2ce236b1c6b56bbe9a25..7a786bc077463029f596c8b0a5b08c717520d2c9 100644 (file)
--- a/src/assembler.h
+++ b/src/assembler.h
@@ -49,12 +49,14 @@ const unsigned kNoASTId = -1;
 
 class AssemblerBase: public Malloced {
  public:
-  explicit AssemblerBase(Isolate* isolate) : isolate_(isolate) {}
+  explicit AssemblerBase(Isolate* isolate);
 
   Isolate* isolate() const { return isolate_; }
+  int jit_cookie() { return jit_cookie_; }
 
  private:
   Isolate* isolate_;
+  int jit_cookie_;
 };
 
 // -----------------------------------------------------------------------------

diff --git a/src/ia32/assembler-ia32.h b/src/ia32/assembler-ia32.h
index 2137d0dc7b5de03dfd79462e56d568fd5e5f2a18..e933102cd967ffd763512e613024dbe893a6c530 100644 (file)
--- a/src/ia32/assembler-ia32.h
+++ b/src/ia32/assembler-ia32.h
@@ -279,6 +279,7 @@ class Immediate BASE_EMBEDDED {
   RelocInfo::Mode rmode_;
 
   friend class Assembler;
+  friend class MacroAssembler;
 };
 
 

diff --git a/src/ia32/full-codegen-ia32.cc b/src/ia32/full-codegen-ia32.cc
index aba82455f7e5774fd11ed67ddda82a0649dca027..59c3373866b9dfe413836a33151cd5547b08b07b 100644 (file)
--- a/src/ia32/full-codegen-ia32.cc
+++ b/src/ia32/full-codegen-ia32.cc
@@ -201,7 +201,7 @@ void FullCodeGenerator::Generate(CompilationInfo* info) {
     __ lea(edx,
            Operand(ebp, StandardFrameConstants::kCallerSPOffset + offset));
     __ push(edx);
-    __ push(Immediate(Smi::FromInt(scope()->num_parameters())));
+    __ SafePush(Immediate(Smi::FromInt(scope()->num_parameters())));
     // Arguments to ArgumentsAccessStub:
     //   function, receiver address, parameter count.
     // The stub will rewrite receiver and parameter count if the previous
@@ -390,13 +390,20 @@ void FullCodeGenerator::EffectContext::Plug(Handle<Object> lit) const {
 
 void FullCodeGenerator::AccumulatorValueContext::Plug(
     Handle<Object> lit) const {
-  __ Set(result_register(), Immediate(lit));
+  if (lit->IsSmi()) {
+    __ SafeSet(result_register(), Immediate(lit));
+  } else {
+    __ Set(result_register(), Immediate(lit));
+  }
 }
 
 
 void FullCodeGenerator::StackValueContext::Plug(Handle<Object> lit) const {
-  // Immediates can be pushed directly.
-  __ push(Immediate(lit));
+  if (lit->IsSmi()) {
+    __ SafePush(Immediate(lit));
+  } else {
+    __ push(Immediate(lit));
+  }
 }
 
 
@@ -739,7 +746,7 @@ void FullCodeGenerator::EmitDeclaration(Variable* variable,
       }
       ASSERT(prop->key()->AsLiteral() != NULL &&
              prop->key()->AsLiteral()->handle()->IsSmi());
-      __ Set(ecx, Immediate(prop->key()->AsLiteral()->handle()));
+      __ SafeSet(ecx, Immediate(prop->key()->AsLiteral()->handle()));
 
       Handle<Code> ic = is_strict_mode()
           ? isolate()->builtins()->KeyedStoreIC_Initialize_Strict()
@@ -1193,7 +1200,7 @@ void FullCodeGenerator::EmitDynamicLoadFromSlotFastCase(
           __ mov(edx,
                  ContextSlotOperandCheckExtensions(obj_proxy->var()->AsSlot(),
                                                    slow));
-          __ mov(eax, Immediate(key_literal->handle()));
+          __ SafeSet(eax, Immediate(key_literal->handle()));
           Handle<Code> ic =
               isolate()->builtins()->KeyedLoadIC_Initialize();
           EmitCallIC(ic, RelocInfo::CODE_TARGET, GetPropertyId(property));
@@ -1278,7 +1285,7 @@ void FullCodeGenerator::EmitVariableLoad(Variable* var) {
     ASSERT(key_literal->handle()->IsSmi());
 
     // Load the key.
-    __ mov(eax, Immediate(key_literal->handle()));
+    __ SafeSet(eax, Immediate(key_literal->handle()));
 
     // Do a keyed property load.
     Handle<Code> ic = isolate()->builtins()->KeyedLoadIC_Initialize();
@@ -1549,7 +1556,7 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) {
           MemOperand slot_operand =
               EmitSlotSearch(obj_proxy->var()->AsSlot(), ecx);
           __ push(slot_operand);
-          __ mov(eax, Immediate(property->key()->AsLiteral()->handle()));
+          __ SafeSet(eax, Immediate(property->key()->AsLiteral()->handle()));
         } else {
           VisitForStackValue(property->obj());
           VisitForAccumulatorValue(property->key());
@@ -1562,7 +1569,7 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) {
           MemOperand slot_operand =
               EmitSlotSearch(obj_proxy->var()->AsSlot(), ecx);
           __ push(slot_operand);
-          __ push(Immediate(property->key()->AsLiteral()->handle()));
+          __ SafePush(Immediate(property->key()->AsLiteral()->handle()));
         } else {
           VisitForStackValue(property->obj());
           VisitForStackValue(property->key());
@@ -1641,6 +1648,7 @@ void FullCodeGenerator::VisitAssignment(Assignment* expr) {
 void FullCodeGenerator::EmitNamedPropertyLoad(Property* prop) {
   SetSourcePosition(prop->position());
   Literal* key = prop->key()->AsLiteral();
+  ASSERT(!key->handle()->IsSmi());
   __ mov(ecx, Immediate(key->handle()));
   Handle<Code> ic = isolate()->builtins()->LoadIC_Initialize();
   EmitCallIC(ic, RelocInfo::CODE_TARGET, GetPropertyId(prop));
@@ -1807,7 +1815,7 @@ void FullCodeGenerator::EmitAssignment(Expression* expr, int bailout_ast_id) {
           EmitVariableLoad(prop->obj()->AsVariableProxy()->var());
         }
         __ mov(edx, eax);
-        __ Set(ecx, Immediate(prop->key()->AsLiteral()->handle()));
+        __ SafeSet(ecx, Immediate(prop->key()->AsLiteral()->handle()));
       } else {
         VisitForStackValue(prop->obj());
         VisitForAccumulatorValue(prop->key());
@@ -2320,7 +2328,7 @@ void FullCodeGenerator::VisitCallNew(CallNew* expr) {
   SetSourcePosition(expr->position());
 
   // Load function and argument count into edi and eax.
-  __ Set(eax, Immediate(arg_count));
+  __ SafeSet(eax, Immediate(arg_count));
   __ mov(edi, Operand(esp, arg_count * kPointerSize));
 
   Handle<Code> construct_builtin =
@@ -2660,7 +2668,7 @@ void FullCodeGenerator::EmitArguments(ZoneList<Expression*>* args) {
   // parameter count in eax.
   VisitForAccumulatorValue(args->at(0));
   __ mov(edx, eax);
-  __ mov(eax, Immediate(Smi::FromInt(scope()->num_parameters())));
+  __ SafeSet(eax, Immediate(Smi::FromInt(scope()->num_parameters())));
   ArgumentsAccessStub stub(ArgumentsAccessStub::READ_ELEMENT);
   __ CallStub(&stub);
   context()->Plug(eax);
@@ -2672,7 +2680,7 @@ void FullCodeGenerator::EmitArgumentsLength(ZoneList<Expression*>* args) {
 
   Label exit;
   // Get the number of formal parameters.
-  __ Set(eax, Immediate(Smi::FromInt(scope()->num_parameters())));
+  __ SafeSet(eax, Immediate(Smi::FromInt(scope()->num_parameters())));
 
   // Check if the calling frame is an arguments adaptor frame.
   __ mov(ebx, Operand(ebp, StandardFrameConstants::kCallerFPOffset));
@@ -3813,7 +3821,7 @@ void FullCodeGenerator::VisitCountOperation(CountOperation* expr) {
         MemOperand slot_operand =
             EmitSlotSearch(obj_proxy->var()->AsSlot(), ecx);
         __ push(slot_operand);
-        __ mov(eax, Immediate(prop->key()->AsLiteral()->handle()));
+        __ SafeSet(eax, Immediate(prop->key()->AsLiteral()->handle()));
       } else {
         VisitForStackValue(prop->obj());
         VisitForAccumulatorValue(prop->key());

diff --git a/src/ia32/macro-assembler-ia32.cc b/src/ia32/macro-assembler-ia32.cc
index 6fb16d831e114ebe40d4ac2d0b3a86d4c8cf1efd..d5eda7a0d7640be2bc25d2f5375316c91acdd006 100644 (file)
--- a/src/ia32/macro-assembler-ia32.cc
+++ b/src/ia32/macro-assembler-ia32.cc
@@ -191,7 +191,7 @@ void MacroAssembler::DebugBreak() {
 
 void MacroAssembler::Set(Register dst, const Immediate& x) {
   if (x.is_zero()) {
-    xor_(dst, Operand(dst));  // shorter than mov
+    xor_(dst, Operand(dst));  // Shorter than mov.
   } else {
     mov(dst, x);
   }
@@ -203,6 +203,33 @@ void MacroAssembler::Set(const Operand& dst, const Immediate& x) {
 }
 
 
+bool MacroAssembler::IsUnsafeImmediate(const Immediate& x) {
+  static const int kMaxImmediateBits = 17;
+  if (x.rmode_ != RelocInfo::NONE) return false;
+  return !is_intn(x.x_, kMaxImmediateBits);
+}
+
+
+void MacroAssembler::SafeSet(Register dst, const Immediate& x) {
+  if (IsUnsafeImmediate(x) && jit_cookie() != 0) {
+    Set(dst, Immediate(x.x_ ^ jit_cookie()));
+    xor_(dst, jit_cookie());
+  } else {
+    Set(dst, x);
+  }
+}
+
+
+void MacroAssembler::SafePush(const Immediate& x) {
+  if (IsUnsafeImmediate(x) && jit_cookie() != 0) {
+    push(Immediate(x.x_ ^ jit_cookie()));
+    xor_(Operand(esp, 0), Immediate(jit_cookie()));
+  } else {
+    push(x);
+  }
+}
+
+
 void MacroAssembler::CmpObjectType(Register heap_object,
                                    InstanceType type,
                                    Register map) {

diff --git a/src/ia32/macro-assembler-ia32.h b/src/ia32/macro-assembler-ia32.h
index 3ab1864694ac55f6cc4d8a66c185ed77642ab97f..f88d992fc7acc019e16d1dd524f1e59a4fe4c121 100644 (file)
--- a/src/ia32/macro-assembler-ia32.h
+++ b/src/ia32/macro-assembler-ia32.h
@@ -193,6 +193,11 @@ class MacroAssembler: public Assembler {
   void Set(Register dst, const Immediate& x);
   void Set(const Operand& dst, const Immediate& x);
 
+  // Support for constant splitting.
+  bool IsUnsafeImmediate(const Immediate& x);
+  void SafeSet(Register dst, const Immediate& x);
+  void SafePush(const Immediate& x);
+
   // Compare object type for heap object.
   // Incoming register is heap_object and outgoing register is map.
   void CmpObjectType(Register heap_object, InstanceType type, Register map);

diff --git a/test/cctest/test-compiler.cc b/test/cctest/test-compiler.cc
index d3dd9c6ff0e97b715604bedfc33026138ea46101..4c5f197bc649837efbf025e9a9f4987120e13f0d 100644 (file)
--- a/test/cctest/test-compiler.cc
+++ b/test/cctest/test-compiler.cc
@@ -31,6 +31,8 @@
 #include "v8.h"
 
 #include "compiler.h"
+#include "disasm.h"
+#include "disassembler.h"
 #include "execution.h"
 #include "factory.h"
 #include "platform.h"
@@ -348,3 +350,51 @@ TEST(GetScriptLineNumber) {
     CHECK_EQ(i, f->GetScriptLineNumber());
   }
 }
+
+
+#ifdef ENABLE_DISASSEMBLER
+static Handle<JSFunction> GetJSFunction(v8::Handle<v8::Object> obj,
+                                 const char* property_name) {
+  v8::Local<v8::Function> fun =
+      v8::Local<v8::Function>::Cast(obj->Get(v8_str(property_name)));
+  return v8::Utils::OpenHandle(*fun);
+}
+
+
+static void CheckCodeForUnsafeLiteral(Handle<JSFunction> f) {
+  // Create a disassembler with default name lookup.
+  disasm::NameConverter name_converter;
+  disasm::Disassembler d(name_converter);
+
+  if (f->code()->kind() == Code::FUNCTION) {
+    Address pc = f->code()->instruction_start();
+    int decode_size =
+        Min(f->code()->instruction_size(),
+            static_cast<int>(f->code()->stack_check_table_offset()));
+    Address end = pc + decode_size;
+
+    v8::internal::EmbeddedVector<char, 128> decode_buffer;
+    while (pc < end) {
+      pc += d.InstructionDecode(decode_buffer, pc);
+      CHECK(strstr(decode_buffer.start(), "mov eax,0x178c29c") == NULL);
+      CHECK(strstr(decode_buffer.start(), "push 0x178c29c") == NULL);
+      CHECK(strstr(decode_buffer.start(), "0x178c29c") == NULL);
+    }
+  }
+}
+
+
+TEST(SplitConstantsInFullCompiler) {
+  v8::HandleScope scope;
+  LocalContext env;
+
+  CompileRun("function f() { a = 12345678 }; f();");
+  CheckCodeForUnsafeLiteral(GetJSFunction(env->Global(), "f"));
+  CompileRun("function f(x) { a = 12345678 + x}; f(1);");
+  CheckCodeForUnsafeLiteral(GetJSFunction(env->Global(), "f"));
+  CompileRun("function f(x) { var arguments = 1; x += 12345678}; f(1);");
+  CheckCodeForUnsafeLiteral(GetJSFunction(env->Global(), "f"));
+  CompileRun("function f(x) { var arguments = 1; x = 12345678}; f(1);");
+  CheckCodeForUnsafeLiteral(GetJSFunction(env->Global(), "f"));
+}
+#endif

diff --git a/test/cctest/test-deoptimization.cc b/test/cctest/test-deoptimization.cc
index 5ab84f96cb087eefc367a569f45b35762b649495..056c98184537c9ff40f94cf15310ca0ed7554977 100644 (file)
--- a/test/cctest/test-deoptimization.cc
+++ b/test/cctest/test-deoptimization.cc
@@ -97,8 +97,8 @@ class AllowNativesSyntaxNoInlining {
 };
 
 
-Handle<JSFunction> GetJSFunction(v8::Handle<v8::Object> obj,
-                                 const char* property_name) {
+static Handle<JSFunction> GetJSFunction(v8::Handle<v8::Object> obj,
+                                        const char* property_name) {
   v8::Local<v8::Function> fun =
       v8::Local<v8::Function>::Cast(obj->Get(v8_str(property_name)));
   return v8::Utils::OpenHandle(*fun);