diff mbox series

[v2] Fix scoped_value_mark not working with empty value chain

Message ID PAXPR09MB5583F42E74A6511B0479FCD8B9469@PAXPR09MB5583.eurprd09.prod.outlook.com
State Committed
Headers
Series [v2] Fix scoped_value_mark not working with empty value chain |

Commit Message

Ciaran Woodward May 25, 2023, 11:14 a.m. UTC 
  The scoped_value_mark helper class was setting its internal
mark value to NULL to indicate that the value chain had already
been freed to mark.

However, value_mark() also returns NULL if the value chain is
empty at the time of call.

This lead to the situation that if the value chain was empty
at the time the scoped_value_mark was created, the class
would not correctly clean up the state when it was destroyed,
because it believed it had already been freed.

I noticed this because I was setting a watchpoint very early
in my debug session, and it was becoming a software watchpoint
rather than hardware. Running any command that called evaluate()
beforehand (such as 'x 0') would mean that a hardware watchpoint
was correctly used. After some careful examination of the
differences in execution, I noticed that values were being freed
later in the 'bad case', which lead me to notice the issue with
scoped_value_mark.
---
 gdb/value.h | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

--
2.25.1

Comments

Tom Tromey May 25, 2023, 6:59 p.m. UTC | #1 
>>>>> "Ciaran" == Ciaran Woodward <ciaranwoodward@xmos.com> writes:

Ciaran> The scoped_value_mark helper class was setting its internal
Ciaran> mark value to NULL to indicate that the value chain had already
Ciaran> been freed to mark.

...

Thanks.  I'm checking this in.

Tom
diff mbox series

Patch

diff --git a/gdb/value.h b/gdb/value.h
index a9c77a033ab..fe05ed1d4dd 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1170,16 +1170,17 @@  class scoped_value_mark
   /* Free the values currently on the value stack.  */
   void free_to_mark ()
   {
-    if (m_value != NULL)
+    if (!m_freed)
       {
-       value_free_to_mark (m_value);
-       m_value = NULL;
+        value_free_to_mark (m_value);
+        m_freed = true;
       }
   }

  private:

   const struct value *m_value;
+  bool m_freed = false;
 };

 extern struct value *value_cstring (const char *ptr, ssize_t len,