Fix representation issue in FastArrayPushStub

camillobruni · Commit bot · commit 9478356ed307 · 2016-04-07T12:46:08.000Z
Pushing undefined onto a FAST_DOUBLE_ARRAY does not enforce the right representation checks. BUG=chromuim:599089 LOG=n Review URL: https://codereview.chromium.org/1868973002 Cr-Commit-Position: refs/heads/master@{#35332}
diff --git a/src/code-stubs-hydrogen.cc b/src/code-stubs-hydrogen.cc
@@ -721,9 +721,15 @@ HValue* CodeStubGraphBuilderBase::BuildPushElement(HValue* object, HValue* argc,
     {
       HInstruction* argument =
           Add<HAccessArgumentsAt>(argument_elements, argc, key);
-      Representation r = IsFastSmiElementsKind(kind) ? Representation::Smi()
-                                                     : Representation::Double();
-      AddUncasted<HForceRepresentation>(argument, r);
+      IfBuilder can_store(this);
+      can_store.IfNot<HIsSmiAndBranch>(argument);
+      if (IsFastDoubleElementsKind(kind)) {
+        can_store.And();
+        can_store.IfNot<HCompareMap>(argument,
+                                     isolate()->factory()->heap_number_map());
+      }
+      can_store.ThenDeopt(Deoptimizer::kFastArrayPushFailed);
+      can_store.End();
     }
     builder.EndBody();
   }
diff --git a/test/mjsunit/regress/regress-599089-array-push.js b/test/mjsunit/regress/regress-599089-array-push.js
@@ -0,0 +1,10 @@
+// Copyright 2016 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+
+var array = [1.2, 1.2];
+array.length = 0;
+array.push(undefined);
+assertEquals(1, array.length);
+assertEquals([undefined], array);

Original file line number	Diff line number	Diff line change
`@@ -721,9 +721,15 @@ HValue* CodeStubGraphBuilderBase::BuildPushElement(HValue* object, HValue* argc,`
`721`	`721`	`{`
`722`	`722`	`HInstruction* argument =`
`723`	`723`	`Add<HAccessArgumentsAt>(argument_elements, argc, key);`
`724`		`- Representation r = IsFastSmiElementsKind(kind) ? Representation::Smi()`
`725`		`- : Representation::Double();`
`726`		`- AddUncasted<HForceRepresentation>(argument, r);`
	`724`	`+ IfBuilder can_store(this);`
	`725`	`+ can_store.IfNot<HIsSmiAndBranch>(argument);`
	`726`	`+ if (IsFastDoubleElementsKind(kind)) {`
	`727`	`+ can_store.And();`
	`728`	`+ can_store.IfNot<HCompareMap>(argument,`
	`729`	`+ isolate()->factory()->heap_number_map());`
	`730`	`+ }`
	`731`	`+ can_store.ThenDeopt(Deoptimizer::kFastArrayPushFailed);`
	`732`	`+ can_store.End();`
`727`	`733`	`}`
`728`	`734`	`builder.EndBody();`
`729`	`735`	`}`