Revision: 17891
Author:   [email protected]
Date:     Tue Nov 19 22:23:41 2013 UTC
Log:      MIPS: Fixed crashes exposed though fuzzing.

Port r17886 (e2fb3ed)

Original commit message:
The %_OneByteSeqStringSetChar intrinsic expects its arguments to be checked before being called for efficiency reasons, but the fuzzer provided no such checks. Now the intrinsic is robust to bad input if FLAG_debug_code is set.

[email protected], [email protected]
TEST=test/mjsunit/regress/regress-320948.js
BUG=chromium:320948
LOG=Y

Review URL: https://codereview.chromium.org/68793008

Patch from Balazs Kilvady <[email protected]>.
http://code.google.com/p/v8/source/detail?r=17891

Modified:
 /branches/bleeding_edge/src/mips/full-codegen-mips.cc
 /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc
 /branches/bleeding_edge/src/mips/lithium-mips.cc
 /branches/bleeding_edge/src/mips/lithium-mips.h
 /branches/bleeding_edge/src/mips/macro-assembler-mips.cc
 /branches/bleeding_edge/src/mips/macro-assembler-mips.h

=======================================
--- /branches/bleeding_edge/src/mips/full-codegen-mips.cc Tue Nov 19 12:59:09 2013 UTC +++ /branches/bleeding_edge/src/mips/full-codegen-mips.cc Tue Nov 19 22:23:41 2013 UTC
@@ -3514,29 +3514,6 @@
   __ bind(&done);
   context()->Plug(v0);
 }
-
-
-void FullCodeGenerator::EmitSeqStringSetCharCheck(Register string,
-                                                  Register index,
-                                                  Register value,
-                                                  uint32_t encoding_mask) {
-  __ And(at, index, Operand(kSmiTagMask));
-  __ Check(eq, kNonSmiIndex, at, Operand(zero_reg));
-  __ And(at, value, Operand(kSmiTagMask));
-  __ Check(eq, kNonSmiValue, at, Operand(zero_reg));
-
-  __ lw(at, FieldMemOperand(string, String::kLengthOffset));
-  __ Check(lt, kIndexIsTooLarge, index, Operand(at));
-
-  __ Check(ge, kIndexIsNegative, index, Operand(zero_reg));
-
-  __ lw(at, FieldMemOperand(string, HeapObject::kMapOffset));
-  __ lbu(at, FieldMemOperand(at, Map::kInstanceTypeOffset));
-
-  __ And(at, at, Operand(kStringRepresentationMask | kStringEncodingMask));
-  __ Subu(at, at, Operand(encoding_mask));
-  __ Check(eq, kUnexpectedStringType, at, Operand(zero_reg));
-}


 void FullCodeGenerator::EmitOneByteSeqStringSetChar(CallRuntime* expr) {
@@ -3553,8 +3530,16 @@
   __ Pop(index, value);

   if (FLAG_debug_code) {
+    __ And(at, value, Operand(kSmiTagMask));
+    __ ThrowIf(ne, kNonSmiValue, at, Operand(zero_reg));
+    __ And(at, index, Operand(kSmiTagMask));
+    __ ThrowIf(ne, kNonSmiIndex, at, Operand(zero_reg));
+    __ SmiUntag(index, index);
static const uint32_t one_byte_seq_type = kSeqStringTag | kOneByteStringTag;
-    EmitSeqStringSetCharCheck(string, index, value, one_byte_seq_type);
+    Register scratch = t5;
+    __ EmitSeqStringSetCharCheck(
+        string, index, value, scratch, one_byte_seq_type);
+    __ SmiTag(index, index);
   }

   __ SmiUntag(value, value);
@@ -3582,8 +3567,16 @@
   __ Pop(index, value);

   if (FLAG_debug_code) {
+    __ And(at, value, Operand(kSmiTagMask));
+    __ ThrowIf(ne, kNonSmiValue, at, Operand(zero_reg));
+    __ And(at, index, Operand(kSmiTagMask));
+    __ ThrowIf(ne, kNonSmiIndex, at, Operand(zero_reg));
+    __ SmiUntag(index, index);
static const uint32_t two_byte_seq_type = kSeqStringTag | kTwoByteStringTag;
-    EmitSeqStringSetCharCheck(string, index, value, two_byte_seq_type);
+    Register scratch = t5;
+    __ EmitSeqStringSetCharCheck(
+        string, index, value, scratch, two_byte_seq_type);
+    __ SmiTag(index, index);
   }

   __ SmiUntag(value, value);
=======================================
--- /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc Fri Nov 15 17:34:34 2013 UTC +++ /branches/bleeding_edge/src/mips/lithium-codegen-mips.cc Tue Nov 19 22:23:41 2013 UTC
@@ -1820,16 +1820,13 @@

   if (FLAG_debug_code) {
     Register scratch = scratch0();
-    __ lw(scratch, FieldMemOperand(string, HeapObject::kMapOffset));
-    __ lbu(scratch, FieldMemOperand(scratch, Map::kInstanceTypeOffset));
-
-    __ And(scratch, scratch,
-           Operand(kStringRepresentationMask | kStringEncodingMask));
+    Register index = ToRegister(instr->index());
static const uint32_t one_byte_seq_type = kSeqStringTag | kOneByteStringTag; static const uint32_t two_byte_seq_type = kSeqStringTag | kTwoByteStringTag;
-    __ Subu(at, scratch, Operand(encoding == String::ONE_BYTE_ENCODING
-                                ? one_byte_seq_type : two_byte_seq_type));
-    __ Check(eq, kUnexpectedStringType, at, Operand(zero_reg));
+    int encoding_mask =
+        instr->hydrogen()->encoding() == String::ONE_BYTE_ENCODING
+        ? one_byte_seq_type : two_byte_seq_type;
+ __ EmitSeqStringSetCharCheck(string, index, value, scratch, encoding_mask);
   }

MemOperand operand = BuildSeqStringOperand(string, instr->index(), encoding);
=======================================
--- /branches/bleeding_edge/src/mips/lithium-mips.cc Tue Nov 19 02:26:10 2013 UTC +++ /branches/bleeding_edge/src/mips/lithium-mips.cc Tue Nov 19 22:23:41 2013 UTC
@@ -1826,10 +1826,13 @@


 LInstruction* LChunkBuilder::DoSeqStringSetChar(HSeqStringSetChar* instr) {
-  LOperand* string = UseRegister(instr->string());
-  LOperand* index = UseRegisterOrConstant(instr->index());
-  LOperand* value = UseRegister(instr->value());
-  return new(zone()) LSeqStringSetChar(string, index, value);
+  LOperand* string = UseRegisterAtStart(instr->string());
+  LOperand* index = FLAG_debug_code
+      ? UseRegisterAtStart(instr->index())
+      : UseRegisterOrConstantAtStart(instr->index());
+  LOperand* value = UseRegisterAtStart(instr->value());
+ LOperand* context = FLAG_debug_code ? UseFixed(instr->context(), cp) : NULL;
+  return new(zone()) LSeqStringSetChar(context, string, index, value);
 }


=======================================
--- /branches/bleeding_edge/src/mips/lithium-mips.h Tue Nov 12 19:05:38 2013 UTC +++ /branches/bleeding_edge/src/mips/lithium-mips.h Tue Nov 19 22:23:41 2013 UTC
@@ -1381,19 +1381,21 @@
 };


-class LSeqStringSetChar V8_FINAL : public LTemplateInstruction<1, 3, 0> {
+class LSeqStringSetChar V8_FINAL : public LTemplateInstruction<1, 4, 0> {
  public:
-  LSeqStringSetChar(LOperand* string,
+  LSeqStringSetChar(LOperand* context,
+                    LOperand* string,
                     LOperand* index,
                     LOperand* value) {
-    inputs_[0] = string;
-    inputs_[1] = index;
-    inputs_[2] = value;
+    inputs_[0] = context;
+    inputs_[1] = string;
+    inputs_[2] = index;
+    inputs_[3] = value;
   }

-  LOperand* string() { return inputs_[0]; }
-  LOperand* index() { return inputs_[1]; }
-  LOperand* value() { return inputs_[2]; }
+  LOperand* string() { return inputs_[1]; }
+  LOperand* index() { return inputs_[2]; }
+  LOperand* value() { return inputs_[3]; }

   DECLARE_CONCRETE_INSTRUCTION(SeqStringSetChar, "seq-string-set-char")
   DECLARE_HYDROGEN_ACCESSOR(SeqStringSetChar)
=======================================
--- /branches/bleeding_edge/src/mips/macro-assembler-mips.cc Mon Nov 18 20:51:30 2013 UTC +++ /branches/bleeding_edge/src/mips/macro-assembler-mips.cc Tue Nov 19 22:23:41 2013 UTC
@@ -5049,6 +5049,44 @@
   stack_passed_words += kCArgSlotCount;
   return stack_passed_words;
 }
+
+
+void MacroAssembler::EmitSeqStringSetCharCheck(Register string,
+                                               Register index,
+                                               Register value,
+                                               Register scratch,
+                                               uint32_t encoding_mask) {
+  Label is_object;
+  And(at, string, Operand(kSmiTagMask));
+  ThrowIf(eq, kNonObject, at, Operand(zero_reg));
+
+  lw(at, FieldMemOperand(string, HeapObject::kMapOffset));
+  lbu(at, FieldMemOperand(at, Map::kInstanceTypeOffset));
+
+  andi(at, at, kStringRepresentationMask | kStringEncodingMask);
+  li(scratch, Operand(encoding_mask));
+  ThrowIf(ne, kUnexpectedStringType, at, Operand(scratch));
+
+ // The index is assumed to be untagged coming in, tag it to compare with the + // string length without using a temp register, it is restored at the end of
+  // this function.
+  Label index_tag_ok, index_tag_bad;
+  // On ARM TrySmiTag is used here.
+  AdduAndCheckForOverflow(index, index, index, scratch);
+  BranchOnOverflow(&index_tag_bad, scratch);
+  Branch(&index_tag_ok);
+  bind(&index_tag_bad);
+  Throw(kIndexIsTooLarge);
+  bind(&index_tag_ok);
+
+  lw(at, FieldMemOperand(string, String::kLengthOffset));
+  ThrowIf(ge, kIndexIsTooLarge, index, Operand(at));
+
+  li(at, Operand(Smi::FromInt(0)));
+  ThrowIf(lt, kIndexIsNegative, index, Operand(at));
+
+  SmiUntag(index, index);
+}


 void MacroAssembler::PrepareCallCFunction(int num_reg_arguments,
@@ -5429,6 +5467,57 @@

   bind(&done);
 }
+
+
+void MacroAssembler::Throw(BailoutReason reason) {
+  Label throw_start;
+  bind(&throw_start);
+#ifdef DEBUG
+  const char* msg = GetBailoutReason(reason);
+  if (msg != NULL) {
+    RecordComment("Throw message: ");
+    RecordComment(msg);
+  }
+#endif
+
+  li(a0, Operand(Smi::FromInt(reason)));
+  push(a0);
+  // Disable stub call restrictions to always allow calls to throw.
+  if (!has_frame_) {
+    // We don't actually want to generate a pile of code for this, so just
+    // claim there is a stack frame, without generating one.
+    FrameScope scope(this, StackFrame::NONE);
+    CallRuntime(Runtime::kThrowMessage, 1);
+  } else {
+    CallRuntime(Runtime::kThrowMessage, 1);
+  }
+  // will not return here
+  if (is_trampoline_pool_blocked()) {
+    // If the calling code cares throw the exact number of
+    // instructions generated, we insert padding here to keep the size
+    // of the ThrowMessage macro constant.
+    // Currently in debug mode with debug_code enabled the number of
+    // generated instructions is 14, so we use this as a maximum value.
+    static const int kExpectedThrowMessageInstructions = 14;
+    int throw_instructions = InstructionsGeneratedSince(&throw_start);
+    ASSERT(throw_instructions <= kExpectedThrowMessageInstructions);
+    while (throw_instructions++ < kExpectedThrowMessageInstructions) {
+      nop();
+    }
+  }
+}
+
+
+void MacroAssembler::ThrowIf(Condition cc,
+                             BailoutReason reason,
+                             Register rs,
+                             Operand rt) {
+  Label L;
+  Branch(&L, NegateCondition(cc), rs, rt);
+  Throw(reason);
+  // will not return here
+  bind(&L);
+}


 void MacroAssembler::LoadInstanceDescriptors(Register map,
=======================================
--- /branches/bleeding_edge/src/mips/macro-assembler-mips.h Fri Nov 8 19:35:37 2013 UTC +++ /branches/bleeding_edge/src/mips/macro-assembler-mips.h Tue Nov 19 22:23:41 2013 UTC
@@ -967,6 +967,12 @@
   // handler chain.
   void ThrowUncatchable(Register value);

+  // Throw a message string as an exception.
+  void Throw(BailoutReason reason);
+
+  // Throw a message string as an exception if a condition is not true.
+ void ThrowIf(Condition cc, BailoutReason reason, Register rs, Operand rt);
+
   // Copies a fixed number of fields of heap objects from src to dst.
void CopyFields(Register dst, Register src, RegList temps, int field_count);

@@ -1452,6 +1458,12 @@

   void JumpIfNotUniqueName(Register reg, Label* not_unique_name);

+  void EmitSeqStringSetCharCheck(Register string,
+                                 Register index,
+                                 Register value,
+                                 Register scratch,
+                                 uint32_t encoding_mask);
+
   // Test that both first and second are sequential ASCII strings.
   // Assume that they are non-smis.
   void JumpIfNonSmisNotBothSequentialAsciiStrings(Register first,

--
--
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