src: expose ListNode<T>::prev_ on postmortem metadata

[legendecas]

Make ListNode<T> postmortem easier to find last items in the queue.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

[addaleax]

Does this come from some other PR? I feel like I’ve seen this before 😄

[legendecas]

@addaleax Yes, I extracted them from #29207, since there are no longer failures for #29207 to pass the test without this improvement.

[bnoordhuis]

While this approach isn't wrong, you could also export the offset of the prev_ field and then use that:

diff --git a/src/node_postmortem_metadata.cc b/src/node_postmortem_metadata.cc
index 05800f79b0..ccb347e951 100644
--- a/src/node_postmortem_metadata.cc
+++ b/src/node_postmortem_metadata.cc
@@ -27,9 +27,11 @@
     HandleWrap::handle_wrap_queue_)                                           \
   V(Environment_HandleWrapQueue, head_, ListNode_HandleWrap,                  \
     Environment::HandleWrapQueue::head_)                                      \
+  V(ListNode_HandleWrap, prev_, uintptr_t, ListNode<HandleWrap>::prev_)       \
   V(ListNode_HandleWrap, next_, uintptr_t, ListNode<HandleWrap>::next_)       \
   V(Environment_ReqWrapQueue, head_, ListNode_ReqWrapQueue,                   \
     Environment::ReqWrapQueue::head_)                                         \
+  V(ListNode_ReqWrap, prev_, uintptr_t, ListNode<ReqWrapBase>::prev_)         \
   V(ListNode_ReqWrap, next_, uintptr_t, ListNode<ReqWrapBase>::next_)
 
 extern "C" {

And then:

diff --git a/test/cctest/test_node_postmortem_metadata.cc b/test/cctest/test_node_postmortem_metadata.cc
index 79b766939b..0eac0240cd 100644
--- a/test/cctest/test_node_postmortem_metadata.cc
+++ b/test/cctest/test_node_postmortem_metadata.cc
@@ -19,8 +19,10 @@ extern uintptr_t
 extern uintptr_t
     nodedbg_offset_Environment__req_wrap_queue___Environment_ReqWrapQueue;
 extern uintptr_t nodedbg_offset_ExternalString__data__uintptr_t;
+extern uintptr_t nodedbg_offset_ListNode_ReqWrap__prev___uintptr_t;
 extern uintptr_t nodedbg_offset_ListNode_ReqWrap__next___uintptr_t;
 extern uintptr_t nodedbg_offset_ReqWrap__req_wrap_queue___ListNode_ReqWrapQueue;
+extern uintptr_t nodedbg_offset_ListNode_HandleWrap__prev___uintptr_t;
 extern uintptr_t nodedbg_offset_ListNode_HandleWrap__next___uintptr_t;
 extern uintptr_t
     nodedbg_offset_Environment_ReqWrapQueue__head___ListNode_ReqWrapQueue;
@@ -143,12 +145,12 @@ TEST_F(DebugSymbolsTest, HandleWrapList) {
   auto queue = reinterpret_cast<uintptr_t>((*env)->handle_wrap_queue());
   auto head = queue +
       nodedbg_offset_Environment_HandleWrapQueue__head___ListNode_HandleWrap;
-  auto next =
-      head + nodedbg_offset_ListNode_HandleWrap__next___uintptr_t;
-  next = *reinterpret_cast<uintptr_t*>(next);
+  auto prev =
+      head + nodedbg_offset_ListNode_HandleWrap__prev___uintptr_t;
+  prev = *reinterpret_cast<uintptr_t*>(prev);
 
   auto expected = reinterpret_cast<uintptr_t>(&obj);
-  auto calculated = next -
+  auto calculated = prev -
       nodedbg_offset_HandleWrap__handle_wrap_queue___ListNode_HandleWrap;
   EXPECT_EQ(expected, calculated);
 
@@ -177,13 +179,13 @@ TEST_F(DebugSymbolsTest, ReqWrapList) {
   auto queue = reinterpret_cast<uintptr_t>((*env)->req_wrap_queue());
   auto head = queue +
       nodedbg_offset_Environment_ReqWrapQueue__head___ListNode_ReqWrapQueue;
-  auto next =
+  auto prev =
       head + nodedbg_offset_ListNode_ReqWrap__next___uintptr_t;
-  next = *reinterpret_cast<uintptr_t*>(next);
+  prev = *reinterpret_cast<uintptr_t*>(prev);
 
   auto expected = reinterpret_cast<uintptr_t>(&obj);
   auto calculated =
-      next - nodedbg_offset_ReqWrap__req_wrap_queue___ListNode_ReqWrapQueue;
+      prev - nodedbg_offset_ReqWrap__req_wrap_queue___ListNode_ReqWrapQueue;
   EXPECT_EQ(expected, calculated);
 
   obj.Dispatched();

If nothing else, the logic should probably also be updated for the ReqWrap list.

[legendecas]

@bnoordhuis That would be much simpler.

I've updated the patch to also make sure coverage on both metadata (the next and prev).

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/26116/

[Trott]

Landed in 45efe67