diff --git a/hphp/runtime/base/array/hphp_array.cpp b/hphp/runtime/base/array/hphp_array.cpp index 6a0a92d0e..32663560d 100644 --- a/hphp/runtime/base/array/hphp_array.cpp +++ b/hphp/runtime/base/array/hphp_array.cpp @@ -1423,11 +1423,13 @@ ArrayData* HphpArray::append(CVarRef v, bool copy) { */ static NEVER_INLINE ArrayData* genericAddNewElemC(ArrayData* a, TypedValue value) { - assert(a->getCount() <= 1); - ArrayData* UNUSED r = a->append(tvAsCVarRef(&value), false); + ArrayData* r = a->append(tvAsCVarRef(&value), a->getCount() != 1); + if (UNLIKELY(r != a)) { + r->incRefCount(); + decRefArr(a); + } tvRefcountedDecRef(value); - assert(r == a); - return a; + return r; } /* @@ -1436,12 +1438,13 @@ ArrayData* genericAddNewElemC(ArrayData* a, TypedValue value) { * hphp_array.h. */ ArrayData* HphpArray::AddNewElemC(ArrayData* a, TypedValue value) { - assert(a->getCount() <= 1 && value.m_type != KindOfRef); + assert(value.m_type != KindOfRef); HphpArray* h; ElmInd* ei; int64_t k; if (LIKELY(a->isHphpArray()) && ((h = (HphpArray*)a), LIKELY(h->m_pos >= 0)) && + LIKELY(h->getCount() <= 1) && LIKELY(!h->isFull()) && ((k = h->m_nextKI), LIKELY(k >= 0)) && ((ei = &h->m_hash[k & h->m_tableMask]), LIKELY(!validElmInd(*ei)))) { diff --git a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp index cd1b802d8..ee0c69c1c 100644 --- a/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp +++ b/hphp/runtime/vm/translator/hopt/hhbctranslator.cpp @@ -467,9 +467,12 @@ void HhbcTranslator::emitAddElemC() { } void HhbcTranslator::emitAddNewElemC() { - // TODO(#2437059): this is broken - SSATmp* val = popC(); - SSATmp* arr = popC(); + if (!topC(1)->isA(Type::Arr)) { + return emitInterpOneOrPunt(Type::Arr, 2, 0); + } + + auto const val = popC(); + auto const arr = popC(); // The AddNewElem helper decrefs its args, so don't decref pop'ed values. push(gen(AddNewElem, arr, val)); } diff --git a/hphp/test/quick/asm_array_elem.hhas b/hphp/test/quick/asm_array_elem.hhas new file mode 100644 index 000000000..7e20e46f5 --- /dev/null +++ b/hphp/test/quick/asm_array_elem.hhas @@ -0,0 +1,24 @@ + +.main { + FPushFuncD 0 "main" + FCall 0 + PopR + Int 1 + RetC +} + +.function main() { + NewArray + String "asd" + AddNewElemC + SetL $foo + PopC + + FPushFuncD 1 "var_dump" + FPassL 0 $foo + FCall 1 + PopR + + Int 1 + RetC +} diff --git a/hphp/test/quick/asm_array_elem.hhas.expect b/hphp/test/quick/asm_array_elem.hhas.expect new file mode 100644 index 000000000..feeb8ee9e --- /dev/null +++ b/hphp/test/quick/asm_array_elem.hhas.expect @@ -0,0 +1,4 @@ +array(1) { + [0]=> + string(3) "asd" +} diff --git a/hphp/test/quick/asm_bad_array_elem.hhas b/hphp/test/quick/asm_bad_array_elem.hhas new file mode 100644 index 000000000..970424ced --- /dev/null +++ b/hphp/test/quick/asm_bad_array_elem.hhas @@ -0,0 +1,18 @@ + +.main { + FPushFuncD 0 "main" + FCall 0 + PopR + Int 1 + RetC +} + +.function main() { + String "asd" + String "asd" + AddNewElemC + Print + PopC + Int 1 + RetC +} diff --git a/hphp/test/quick/asm_bad_array_elem.hhas.expectf b/hphp/test/quick/asm_bad_array_elem.hhas.expectf new file mode 100644 index 000000000..8dc4bd167 --- /dev/null +++ b/hphp/test/quick/asm_bad_array_elem.hhas.expectf @@ -0,0 +1 @@ +HipHop Fatal error: AddNewElemC: $2 must be an array in %s on line %s