Re: [PATCH] Fix DAP stackTrace through frames without debuginfo

[Tom Tromey <tromey@adacore.com>]

Tom> Sure.  Still a FAIL, but a different error.  Log attached.

Ok, I finally figured it out (I think) and it's a latent bug in
frames.py.  I have no idea how/why this ever worked.

Tom

commit 7190893e3563f7353a6ebbf130e7aa6a145c18a1
Author: Tom Tromey <tromey@adacore.com>
Date:   Thu Mar 9 07:47:29 2023 -0700

    Implement hash function for gdb.Frame
    
    Tom de Vries pointed out that one DAP test failed on Python 3.6
    because gdb.Frame is not hashable.  This error was mildly confusing,
    because I think the real issue was this:
    
    -    pair = (gdb.selected_thread().global_num, frame.level)
    
    Here, 'frame.level' is a bound method, not just a simple value.
    
    This patch adds a hash function to gdb.Frame, then fixes frames.py to
    use it directly.

diff --git a/gdb/frame-id.h b/gdb/frame-id.h
index 8ddf7d11408..53abbe203bf 100644
--- a/gdb/frame-id.h
+++ b/gdb/frame-id.h
@@ -112,6 +112,9 @@ struct frame_id
   /* Return a string representation of this frame id.  */
   std::string to_string () const;
 
+  /* A hash function for this frame_id.  */
+  hashval_t hash () const;
+
   /* Returns true when this frame_id and R identify the same
      frame.  */
   bool operator== (const frame_id &r) const;
diff --git a/gdb/frame.c b/gdb/frame.c
index 9b867b6cd9c..478bd881b63 100644
--- a/gdb/frame.c
+++ b/gdb/frame.c
@@ -228,26 +228,12 @@ frame_addr_hash (const void *ap)
 {
   const frame_info *frame = (const frame_info *) ap;
   const struct frame_id f_id = frame->this_id.value;
-  hashval_t hash = 0;
 
   gdb_assert (f_id.stack_status != FID_STACK_INVALID
 	      || f_id.code_addr_p
 	      || f_id.special_addr_p);
 
-  if (f_id.stack_status == FID_STACK_VALID)
-    hash = iterative_hash (&f_id.stack_addr,
-			   sizeof (f_id.stack_addr), hash);
-  if (f_id.code_addr_p)
-    hash = iterative_hash (&f_id.code_addr,
-			   sizeof (f_id.code_addr), hash);
-  if (f_id.special_addr_p)
-    hash = iterative_hash (&f_id.special_addr,
-			   sizeof (f_id.special_addr), hash);
-
-  char user_created_p = f_id.user_created_p;
-  hash = iterative_hash (&user_created_p, sizeof (user_created_p), hash);
-
-  return hash;
+  return f_id.hash ();
 }
 
 /* Internal equality function for the hash table.  This function
@@ -796,6 +782,29 @@ frame_id_artificial_p (frame_id l)
   return l.artificial_depth != 0;
 }
 
+/* See frame-id.h.  */
+
+hashval_t
+frame_id::hash () const
+{
+  hashval_t hash = 0;
+
+  if (stack_status == FID_STACK_VALID)
+    hash = iterative_hash (&stack_addr,
+			   sizeof (stack_addr), hash);
+  if (code_addr_p)
+    hash = iterative_hash (&code_addr,
+			   sizeof (code_addr), hash);
+  if (special_addr_p)
+    hash = iterative_hash (&special_addr,
+			   sizeof (special_addr), hash);
+
+  char user_created_p = user_created_p;
+  hash = iterative_hash (&user_created_p, sizeof (user_created_p), hash);
+
+  return hash;
+}
+
 bool
 frame_id::operator== (const frame_id &r) const
 {
diff --git a/gdb/python/lib/gdb/dap/frames.py b/gdb/python/lib/gdb/dap/frames.py
index 337bbedae0f..4b554f3d920 100644
--- a/gdb/python/lib/gdb/dap/frames.py
+++ b/gdb/python/lib/gdb/dap/frames.py
@@ -18,8 +18,8 @@ import gdb
 from .startup import in_gdb_thread
 
 
-# Map from frame (thread,level) pair to frame ID numbers.  Note we
-# can't use the frame itself here as it is not hashable.
+# Map from frame pair to the frame ID number that is passed back to
+# the client.
 _frame_ids = {}
 
 # Map from frame ID number back to frames.
@@ -42,12 +42,11 @@ gdb.events.cont.connect(_clear_frame_ids)
 def frame_id(frame):
     """Return the frame identifier for FRAME."""
     global _frame_ids, _id_to_frame
-    pair = (gdb.selected_thread().global_num, frame.level)
-    if pair not in _frame_ids:
+    if frame not in _frame_ids:
         id = len(_frame_ids)
-        _frame_ids[pair] = id
+        _frame_ids[frame] = id
         _id_to_frame[id] = frame
-    return _frame_ids[pair]
+    return _frame_ids[frame]
 
 
 @in_gdb_thread
diff --git a/gdb/python/py-frame.c b/gdb/python/py-frame.c
index ecd55d2e568..099f39ac44e 100644
--- a/gdb/python/py-frame.c
+++ b/gdb/python/py-frame.c
@@ -687,6 +687,23 @@ gdbpy_frame_stop_reason_string (PyObject *self, PyObject *args)
   return PyUnicode_Decode (str, strlen (str), host_charset (), NULL);
 }
 
+/* Implement the hash method.  */
+
+static Py_hash_t
+frapy_hash (PyObject *self)
+{
+  frame_object *self_frame = (frame_object *) self;
+
+  hashval_t hash = self_frame->frame_id.hash ();
+  hash <<= 1;
+  hash |= self_frame->frame_id_is_next;
+
+  Py_hash_t result = (Py_hash_t) hash;
+  if (result == -1)
+    result = 0;
+  return result;
+}
+
 /* Implements the equality comparison for Frame objects.
    All other comparison operators will throw a TypeError Python exception,
    as they aren't valid for frames.  */
@@ -816,7 +833,7 @@ PyTypeObject frame_object_type = {
   0,				  /* tp_as_number */
   0,				  /* tp_as_sequence */
   0,				  /* tp_as_mapping */
-  0,				  /* tp_hash  */
+  frapy_hash,			  /* tp_hash  */
   0,				  /* tp_call */
   frapy_str,			  /* tp_str */
   0,				  /* tp_getattro */