public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: Ciaran Woodward <ciaranwoodward@xmos.com>
To: "gdb-patches@sourceware.org" <gdb-patches@sourceware.org>
Cc: Ciaran Woodward <ciaranwoodward@xmos.com>
Subject: [PATCH] Fix scoped_value_mark not working with empty value chain
Date: Mon, 22 May 2023 14:42:18 +0000	[thread overview]
Message-ID: <PAXPR09MB5583501A5BABF34867A616E5B9439@PAXPR09MB5583.eurprd09.prod.outlook.com> (raw)

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 | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/gdb/value.h b/gdb/value.h
index d042d816409..85a33abe7d7 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -1155,6 +1155,7 @@ class scoped_value_mark

   scoped_value_mark ()
     : m_value (value_mark ())
+    , m_freed (false)
   {
   }

@@ -1170,16 +1171,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;
 };

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

             reply	other threads:[~2023-05-22 14:42 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-22 14:42 Ciaran Woodward [this message]
2023-05-24 19:00 ` Tom Tromey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=PAXPR09MB5583501A5BABF34867A616E5B9439@PAXPR09MB5583.eurprd09.prod.outlook.com \
    --to=ciaranwoodward@xmos.com \
    --cc=gdb-patches@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).