Don't unbox values for edges in heap snapshot

Summary:
D60798281 wrongly unboxed the array element when generating heap
snapshot edge to that element. It was a wrong assumption by looking at
JSObject, which also unboxes values when generating snapshot edges for
properties.

By unboxing, it means that the heap snapshot isn't representing exactly
how things are stored. With boxed doubles, the stored data is a pointer
rather than the number itself.

Reviewed By: neildhar

Differential Revision: D61638245

fbshipit-source-id: bc85ae37f86f29572b213a0e49496b72f471dd42

lib/VM/JSArray.cpp

@@ -52,7 +52,7 @@ void ArrayImpl::_snapshotAddEdgesImpl(
   for (uint32_t i = beginIndex; i < endIndex; i++) {
     const auto &elem = indexedStorage->at(gc.getPointerBase(), i - beginIndex);
     const llvh::Optional<HeapSnapshot::NodeID> elemID =
-        gc.getSnapshotID(elem.unboxToHV(gc.getPointerBase()));
+        gc.getSnapshotID(elem.toHV(gc.getPointerBase()));
     if (!elemID) {
       continue;
     }

lib/VM/JSObject.cpp

@@ -2471,7 +2471,7 @@ void JSObject::_snapshotAddEdgesImpl(GCCell *cell, GC &gc, HeapSnapshot &snap) {
         // Else, it's a user-visible property.
         HermesValue prop =
             getNamedSlotValueUnsafe(self, gc.getPointerBase(), desc.slot)
-                .unboxToHV(gc.getPointerBase());
+                .toHV(gc.getPointerBase());
         const llvh::Optional<HeapSnapshot::NodeID> idForProp =
             gc.getSnapshotID(prop);
         if (!idForProp) {

unittests/VMRuntime/HeapSnapshotTest.cpp

@@ -988,6 +988,9 @@ TEST_F(HeapSnapshotRuntimeTest, PropertyUpdatesTest) {
           FIRST_NAMED_PROPERTY_EDGE + 2));
   EXPECT_EQ(nodesAndEdges.second.size(), FIRST_NAMED_PROPERTY_EDGE + 2);
 
+// When Handle-SAN is enabled, we put all numbers on the heap, which changes
+// what the snapshot ID is.
+#ifndef HERMESVM_SANITIZE_HANDLES
   EXPECT_EQ(
       nodesAndEdges.second[FIRST_NAMED_PROPERTY_EDGE],
       Edge(
@@ -1001,6 +1004,7 @@ TEST_F(HeapSnapshotRuntimeTest, PropertyUpdatesTest) {
           HeapSnapshot::EdgeType::Property,
           "bar",
           runtime.getHeap().getIDTracker().getNumberID(200)));
+#endif
 }
 
 TEST_F(HeapSnapshotRuntimeTest, ArrayElementsCaptureNumeric) {
@@ -1047,24 +1051,32 @@ TEST_F(HeapSnapshotRuntimeTest, ArrayElementsCaptureNumeric) {
           arrayID,
           array->getAllocatedSize(),
           FIRST_NAMED_PROPERTY_EDGE + 6));
+// When Handle-SAN is enabled, we put all numbers on the heap, which changes
+// what the snapshot ID is.
+#ifndef HERMESVM_SANITIZE_HANDLES
   EXPECT_EQ(
       nodeAndEdges.second[FIRST_NAMED_PROPERTY_EDGE + 2],
       Edge(
           HeapSnapshot::EdgeType::Element,
           (1 << 20) + 1000,
           runtime.getHeap().getIDTracker().getNumberID(333)));
+#endif
   EXPECT_EQ(
       nodeAndEdges.second[FIRST_NAMED_PROPERTY_EDGE + 4],
       Edge(
           HeapSnapshot::EdgeType::Element,
           10,
           runtime.getHeap().getObjectID(firstElement.get())));
+// When Handle-SAN is enabled, we put all numbers on the heap, which changes
+// what the snapshot ID is.
+#ifndef HERMESVM_SANITIZE_HANDLES
   EXPECT_EQ(
       nodeAndEdges.second[FIRST_NAMED_PROPERTY_EDGE + 5],
       Edge(
           HeapSnapshot::EdgeType::Element,
           15,
           runtime.getHeap().getIDTracker().getNumberID(222)));
+#endif
 
   // Verify there are numeric nodes
   bool hasNumeric = false;
@@ -1131,26 +1143,34 @@ TEST_F(HeapSnapshotRuntimeTest, ArrayElementsNoNumeric) {
           array->getAllocatedSize(),
           FIRST_NAMED_PROPERTY_EDGE + 6));
 
+// When Handle-SAN is enabled, we put all numbers on the heap, which changes
+// what the snapshot ID is.
+#ifndef HERMESVM_SANITIZE_HANDLES
   EXPECT_EQ(
       nodeAndEdges.second[FIRST_NAMED_PROPERTY_EDGE + 2],
       Edge(
           HeapSnapshot::EdgeType::Element,
           (1 << 20) + 1000,
           GCBase::IDTracker::reserved(
               GCBase::IDTracker::ReservedObjectID::Number)));
+#endif
   EXPECT_EQ(
       nodeAndEdges.second[FIRST_NAMED_PROPERTY_EDGE + 4],
       Edge(
           HeapSnapshot::EdgeType::Element,
           10,
           runtime.getHeap().getObjectID(firstElement.get())));
+// When Handle-SAN is enabled, we put all numbers on the heap, which changes
+// what the snapshot ID is.
+#ifndef HERMESVM_SANITIZE_HANDLES
   EXPECT_EQ(
       nodeAndEdges.second[FIRST_NAMED_PROPERTY_EDGE + 5],
       Edge(
           HeapSnapshot::EdgeType::Element,
           15,
           GCBase::IDTracker::reserved(
               GCBase::IDTracker::ReservedObjectID::Number)));
+#endif
 
   // Verify there are no numeric nodes
   auto nodesIt = nodes.begin();