[wasm] check trap handler consistency in debug builds; simplify structures

This CL includes validation code for the trap handler data structures in debug
mode to help catch issues like v8:6841 sooner in the future.

We also now eagerly initialize the free list pointers to make the logic of
finding the next free entry more obvious.

Bug: v8:5277
Change-Id: I13c3180c59b6152508c480e2042072a91e6ca977
Reviewed-on: https://chromium-review.googlesource.com/674128
Commit-Queue: Eric Holk <eholk@chromium.org>
Reviewed-by: Mircea Trofin <mtrofin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48186}

src/trap-handler/handler-outside.cc

@@ -33,6 +33,12 @@
 
 namespace {
 size_t gNextCodeObject = 0;
+
+#if defined(DEBUG)
+const bool kEnableDebug = true;
+#else
+const bool kEnableDebug = false;
+#endif
 }
 
 namespace v8 {
@@ -47,6 +53,66 @@ constexpr size_t HandlerDataSize(size_t num_protected_instructions) {
          num_protected_instructions * sizeof(ProtectedInstructionData);
 }
 
+namespace {
+template <typename = std::enable_if<kEnableDebug>>
+bool IsDisjoint(const CodeProtectionInfo* a, const CodeProtectionInfo* b) {
+  if (a == nullptr || b == nullptr) {
+    return true;
+  }
+
+  const auto a_base = reinterpret_cast<uintptr_t>(a->base);
+  const auto b_base = reinterpret_cast<uintptr_t>(b->base);
+
+  return a_base >= b_base + b->size || b_base >= a_base + a->size;
+}
+
+// Verify that the code range does not overlap any that have already been
+// registered.
+void VerifyCodeRangeIsDisjoint(const CodeProtectionInfo* code_info) {
+  for (size_t i = 0; i < gNumCodeObjects; ++i) {
+    DCHECK(IsDisjoint(code_info, gCodeObjects[i].code_info));
+  }
+}
+
+void ValidateCodeObjects() {
+  // Sanity-check the code objects
+  for (unsigned i = 0; i < gNumCodeObjects; ++i) {
+    const auto* data = gCodeObjects[i].code_info;
+
+    if (data == nullptr) continue;
+
+    // Do some sanity checks on the protected instruction data
+    for (unsigned i = 0; i < data->num_protected_instructions; ++i) {
+      DCHECK_GE(data->instructions[i].instr_offset, 0);
+      DCHECK_LT(data->instructions[i].instr_offset, data->size);
+      DCHECK_GE(data->instructions[i].landing_offset, 0);
+      DCHECK_LT(data->instructions[i].landing_offset, data->size);
+      DCHECK_GT(data->instructions[i].landing_offset,
+                data->instructions[i].instr_offset);
+    }
+  }
+
+  // Check the validity of the free list.
+  size_t free_count = 0;
+  for (size_t i = gNextCodeObject; i != gNumCodeObjects;
+       i = gCodeObjects[i].next_free) {
+    DCHECK_LT(i, gNumCodeObjects);
+    ++free_count;
+    // This check will fail if we encounter a cycle.
+    DCHECK_LE(free_count, gNumCodeObjects);
+  }
+
+  // Check that all free entries are reachable via the free list.
+  size_t free_count2 = 0;
+  for (size_t i = 0; i < gNumCodeObjects; ++i) {
+    if (gCodeObjects[i].code_info == nullptr) {
+      ++free_count2;
+    }
+  }
+  DCHECK_EQ(free_count, free_count2);
+}
+}  // namespace
+
 CodeProtectionInfo* CreateHandlerData(
     void* base, size_t size, size_t num_protected_instructions,
     ProtectedInstructionData* protected_instructions) {
@@ -91,6 +157,10 @@ int RegisterHandlerData(void* base, size_t size,
 
   MetadataLock lock;
 
+  if (kEnableDebug) {
+    VerifyCodeRangeIsDisjoint(data);
+  }
+
   size_t i = gNextCodeObject;
 
   // Explicitly convert std::numeric_limits<int>::max() to unsigned to avoid
@@ -125,24 +195,24 @@ int RegisterHandlerData(void* base, size_t size,
 
     memset(gCodeObjects + gNumCodeObjects, 0,
            sizeof(*gCodeObjects) * (new_size - gNumCodeObjects));
+    for (size_t j = gNumCodeObjects; j < new_size; ++j) {
+      gCodeObjects[j].next_free = j + 1;
+    }
     gNumCodeObjects = new_size;
   }
 
   DCHECK(gCodeObjects[i].code_info == nullptr);
 
   // Find out where the next entry should go.
-  if (gCodeObjects[i].next_free == 0) {
-    // if this is a fresh entry, use the next one.
-    gNextCodeObject = i + 1;
-    DCHECK(gNextCodeObject == gNumCodeObjects ||
-           (gCodeObjects[gNextCodeObject].code_info == nullptr &&
-            gCodeObjects[gNextCodeObject].next_free == 0));
-  } else {
-    gNextCodeObject = gCodeObjects[i].next_free - 1;
-  }
+  gNextCodeObject = gCodeObjects[i].next_free;
 
   if (i <= int_max) {
     gCodeObjects[i].code_info = data;
+
+    if (kEnableDebug) {
+      ValidateCodeObjects();
+    }
+
     return static_cast<int>(i);
   } else {
     return kInvalidIndex;
@@ -163,12 +233,16 @@ void ReleaseHandlerData(int index) {
     data = gCodeObjects[index].code_info;
     gCodeObjects[index].code_info = nullptr;
 
-    // +1 because we reserve {next_entry == 0} to indicate a fresh list entry.
-    gCodeObjects[index].next_free = gNextCodeObject + 1;
+    gCodeObjects[index].next_free = gNextCodeObject;
     gNextCodeObject = index;
+
+    if (kEnableDebug) {
+      ValidateCodeObjects();
+    }
   }
   // TODO(eholk): on debug builds, ensure there are no more copies in
   // the list.
+  DCHECK_NOT_NULL(data);  // make sure we're releasing legitimate handler data.
   free(data);
 }