Fix AddNewElemC bugs in HHIR

emitAddNewElemC didn't properly implement the specified
behavior for this opcode.  It assumed topC(1) was always an non-static
array with refcount 1 or 0.  This changes to interp the case when it's
not an array, and make the helper support static arrays.  (If it
matters for perf, we should engineer the bytecode spec to require this
as well, since the point of this opcode is initializing array literals
that can't be done as static arrays.)  In practice I think this is
only working because it always comes after a NewArray with a capacity
hint != 1, which means it's a newly allocated non-static array.
Ignoring the capacity hint would've broken it.

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

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));
 }

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

hphp/test/quick/asm_array_elem.hhas.expect

@@ -0,0 +1,4 @@
+array(1) {
+  [0]=>
+  string(3) "asd"
+}

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

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