[PATCH] Prevent downgrading of hardware watchpoints if possible

[PATCH] Prevent downgrading of hardware watchpoints if possible

[Hannes Domani]

The lazy flag of a value can tell us if the value contents are available.
But this could be either because it was simply never actually accessed,
or it tried to be accessed, and failed.
The latter are interesting for watchpoints, the former are not.

Currently it only uses lazy values if they are at the head of the value
chain, but this is fragile logic, and degrades some watchpoints to
software watchpoints where it's actually not necessary.

So this adds a new value flag 'm_fetch_lazy_failed' which tells if the
watchpoint expression actually tried to access the value and failed.
And this is used instead of the value chain location to tell if a lazy
value should be used as a watchpoint location.

Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27423
---
 gdb/breakpoint.c                              | 11 ++--
 .../gdb.base/watchpoint-hw-no-degradation.c   | 51 +++++++++++++++++++
 .../gdb.base/watchpoint-hw-no-degradation.exp | 38 ++++++++++++++
 gdb/value.c                                   |  3 ++
 gdb/value.h                                   | 12 ++++-
 5 files changed, 108 insertions(+), 7 deletions(-)
 create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
 create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index af4de248ab6..13d95325604 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -2293,11 +2293,10 @@ update_watchpoint (struct watchpoint *b, bool reparse)
 
 	  /* If it's a memory location, and GDB actually needed
 	     its contents to evaluate the expression, then we
-	     must watch it.  If the first value returned is
-	     still lazy, that means an error occurred reading it;
+	     must watch it.  If an error occurred reading it,
 	     watch it anyway in case it becomes readable.  */
 	  if (v->lval () == lval_memory
-	      && (v == val_chain[0] || ! v->lazy ()))
+	      && (! v->lazy () || v->fetch_lazy_failed ()))
 	    {
 	      struct type *vtype = check_typedef (v->type ());
 
@@ -10716,12 +10715,12 @@ can_use_hardware_watchpoint (const std::vector<value_ref_ptr> &vals)
 
       if (v->lval () == lval_memory)
 	{
-	  if (v != head && v->lazy ())
+	  if (v->lazy () && ! v->fetch_lazy_failed ())
 	    /* A lazy memory lvalue in the chain is one that GDB never
 	       needed to fetch; we either just used its address (e.g.,
 	       `a' in `a.b') or we never needed it at all (e.g., `a'
-	       in `a,b').  This doesn't apply to HEAD; if that is
-	       lazy then it was not readable, but watch it anyway.  */
+	       in `a,b').  If it failed to fetch a lazy value, then it
+	       was not readable, so watch it in this case as well.  */
 	    ;
 	  else
 	    {
diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
new file mode 100644
index 00000000000..cfff2a44f90
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
@@ -0,0 +1,51 @@
+/* This testcase is part of GDB, the GNU debugger.
+
+   Copyright 2026 Free Software Foundation, Inc.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+*/
+
+#include <sys/mman.h>
+#include <unistd.h>
+#include <stdio.h>
+
+int
+main (void)
+{
+  size_t len = sysconf(_SC_PAGESIZE);
+
+  /* Map and unmap memory block to get address.  */
+  void *p = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON|MAP_PRIVATE, -1, 0);
+  if (p == MAP_FAILED)
+    {
+      perror ("mmap");
+      return 1;
+    }
+  munmap (p, len);
+
+  /* Now memory block at address P is inaccessible.
+     Remap block at same address, so it becomes accessible again.  */
+  p = mmap (p, len, PROT_READ|PROT_WRITE,
+	    MAP_ANON|MAP_PRIVATE|MAP_FIXED, -1, 0);
+  if (p == MAP_FAILED)
+    {
+      perror ("mmap");
+      return 1;
+    }
+
+  *(int *) p = 1;
+
+  return 0;
+}
diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
new file mode 100644
index 00000000000..94ef335c77f
--- /dev/null
+++ b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
@@ -0,0 +1,38 @@
+# Copyright 2009-2026 Free Software Foundation, Inc.
+
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Test if watchpoint doesn't degrade to a software watchpoint if part of
+# expression isn't accessible at time of watchpoint creation.
+
+require allow_hw_watchpoint_access_tests
+
+standard_testfile
+
+if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
+    return -1
+}
+
+if {![runto_main]} {
+    return -1
+}
+
+gdb_breakpoint [gdb_get_line_number "mmap (p, len"]
+gdb_continue_to_breakpoint "mmap"
+
+gdb_test "eval \"watch *(int *)%p == 0\",p" \
+	 "Hardware watchpoint $decimal: .*"
+
+gdb_test "continue" \
+	 "Old value = <unreadable>.*New value = 0.*"
diff --git a/gdb/value.c b/gdb/value.c
index a52d4a6742c..c076617d102 100644
--- a/gdb/value.c
+++ b/gdb/value.c
@@ -1546,6 +1546,7 @@ value::copy () const
   val->m_bitpos = m_bitpos;
   val->m_bitsize = m_bitsize;
   val->m_lazy = m_lazy;
+  val->m_fetch_lazy_failed = m_fetch_lazy_failed;
   val->m_embedded_offset = embedded_offset ();
   val->m_pointed_to_offset = m_pointed_to_offset;
   val->m_modifiable = m_modifiable;
@@ -4122,6 +4123,8 @@ value::fetch_lazy ()
      value.  */
   gdb_assert (m_optimized_out.empty ());
   gdb_assert (m_unavailable.empty ());
+  /* Will be reset with set_lazy() at the end if successful.  */
+  m_fetch_lazy_failed = true;
   if (m_is_zero)
     {
       /* Nothing.  */
diff --git a/gdb/value.h b/gdb/value.h
index 8dc3192f637..77919082fe3 100644
--- a/gdb/value.h
+++ b/gdb/value.h
@@ -138,6 +138,7 @@ struct value
       m_stack (false),
       m_is_zero (false),
       m_in_history (false),
+      m_fetch_lazy_failed (false),
       m_type (type_),
       m_enclosing_type (type_)
   {
@@ -279,7 +280,13 @@ struct value
   { return m_lazy; }
 
   void set_lazy (bool val)
-  { m_lazy = val; }
+  {
+    m_lazy = val;
+    m_fetch_lazy_failed = false;
+  }
+
+  bool fetch_lazy_failed () const
+  { return m_fetch_lazy_failed; }
 
   /* If a value represents a C++ object, then the `type' field gives the
      object's compile-time type.  If the object actually belongs to some
@@ -681,6 +688,9 @@ struct value
   /* True if this a value recorded in value history; false otherwise.  */
   bool m_in_history : 1;
 
+  /* True if fetch_lazy() did not finish sucessfully.  */
+  bool m_fetch_lazy_failed : 1;
+
   /* Location of value (if lval).  */
   union
   {
-- 
2.52.0

Re: [PATCH] Prevent downgrading of hardware watchpoints if possible

[Hannes Domani]

 Late ping, because I forgot about this patch myself.

Am Freitag, 9. Januar 2026 um 20:50:29 MEZ hat Hannes Domani <ssbssa@yahoo.de> Folgendes geschrieben:

> The lazy flag of a value can tell us if the value contents are available.
> But this could be either because it was simply never actually accessed,
> or it tried to be accessed, and failed.
> The latter are interesting for watchpoints, the former are not.
> 
> Currently it only uses lazy values if they are at the head of the value
> chain, but this is fragile logic, and degrades some watchpoints to
> software watchpoints where it's actually not necessary.
> 
> So this adds a new value flag 'm_fetch_lazy_failed' which tells if the
> watchpoint expression actually tried to access the value and failed.
> And this is used instead of the value chain location to tell if a lazy
> value should be used as a watchpoint location.
> 
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27423
> ---
> gdb/breakpoint.c                              | 11 ++--
> .../gdb.base/watchpoint-hw-no-degradation.c  | 51 +++++++++++++++++++
> .../gdb.base/watchpoint-hw-no-degradation.exp | 38 ++++++++++++++
> gdb/value.c                                  |  3 ++
> gdb/value.h                                  | 12 ++++-
> 5 files changed, 108 insertions(+), 7 deletions(-)
> create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
> create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> 
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index af4de248ab6..13d95325604 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -2293,11 +2293,10 @@ update_watchpoint (struct watchpoint *b, bool reparse)
> 
>       /* If it's a memory location, and GDB actually needed
>         its contents to evaluate the expression, then we
> -        must watch it.  If the first value returned is
> -        still lazy, that means an error occurred reading it;
> +        must watch it.  If an error occurred reading it,
>         watch it anyway in case it becomes readable.  */
>       if (v->lval () == lval_memory
> -          && (v == val_chain[0] || ! v->lazy ()))
> +          && (! v->lazy () || v->fetch_lazy_failed ()))
>         {
>           struct type *vtype = check_typedef (v->type ());
> 
> @@ -10716,12 +10715,12 @@ can_use_hardware_watchpoint (const std::vector<value_ref_ptr> &vals)
> 
>       if (v->lval () == lval_memory)
>     {
> -      if (v != head && v->lazy ())
> +      if (v->lazy () && ! v->fetch_lazy_failed ())
>         /* A lazy memory lvalue in the chain is one that GDB never
>           needed to fetch; we either just used its address (e.g.,
>           `a' in `a.b') or we never needed it at all (e.g., `a'
> -          in `a,b').  This doesn't apply to HEAD; if that is
> -          lazy then it was not readable, but watch it anyway.  */
> +          in `a,b').  If it failed to fetch a lazy value, then it
> +          was not readable, so watch it in this case as well.  */
>         ;
>       else
>         {
> diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
> new file mode 100644
> index 00000000000..cfff2a44f90
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
> @@ -0,0 +1,51 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +  Copyright 2026 Free Software Foundation, Inc.
> +
> +  This program is free software; you can redistribute it and/or modify
> +  it under the terms of the GNU General Public License as published by
> +  the Free Software Foundation; either version 3 of the License, or
> +  (at your option) any later version.
> +
> +  This program is distributed in the hope that it will be useful,
> +  but WITHOUT ANY WARRANTY; without even the implied warranty of
> +  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +  GNU General Public License for more details.
> +
> +  You should have received a copy of the GNU General Public License
> +  along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +*/
> +
> +#include <sys/mman.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +
> +int
> +main (void)
> +{
> +  size_t len = sysconf(_SC_PAGESIZE);
> +
> +  /* Map and unmap memory block to get address.  */
> +  void *p = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON|MAP_PRIVATE, -1, 0);
> +  if (p == MAP_FAILED)
> +    {
> +      perror ("mmap");
> +      return 1;
> +    }
> +  munmap (p, len);
> +
> +  /* Now memory block at address P is inaccessible.
> +    Remap block at same address, so it becomes accessible again.  */
> +  p = mmap (p, len, PROT_READ|PROT_WRITE,
> +        MAP_ANON|MAP_PRIVATE|MAP_FIXED, -1, 0);
> +  if (p == MAP_FAILED)
> +    {
> +      perror ("mmap");
> +      return 1;
> +    }
> +
> +  *(int *) p = 1;
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> new file mode 100644
> index 00000000000..94ef335c77f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> @@ -0,0 +1,38 @@
> +# Copyright 2009-2026 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test if watchpoint doesn't degrade to a software watchpoint if part of
> +# expression isn't accessible at time of watchpoint creation.
> +
> +require allow_hw_watchpoint_access_tests
> +
> +standard_testfile
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> +    return -1
> +}
> +
> +if {![runto_main]} {
> +    return -1
> +}
> +
> +gdb_breakpoint [gdb_get_line_number "mmap (p, len"]
> +gdb_continue_to_breakpoint "mmap"
> +
> +gdb_test "eval \"watch *(int *)%p == 0\",p" \
> +    "Hardware watchpoint $decimal: .*"
> +
> +gdb_test "continue" \
> +    "Old value = <unreadable>.*New value = 0.*"
> diff --git a/gdb/value.c b/gdb/value.c
> index a52d4a6742c..c076617d102 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -1546,6 +1546,7 @@ value::copy () const
>   val->m_bitpos = m_bitpos;
>   val->m_bitsize = m_bitsize;
>   val->m_lazy = m_lazy;
> +  val->m_fetch_lazy_failed = m_fetch_lazy_failed;
>   val->m_embedded_offset = embedded_offset ();
>   val->m_pointed_to_offset = m_pointed_to_offset;
>   val->m_modifiable = m_modifiable;
> @@ -4122,6 +4123,8 @@ value::fetch_lazy ()
>       value.  */
>   gdb_assert (m_optimized_out.empty ());
>   gdb_assert (m_unavailable.empty ());
> +  /* Will be reset with set_lazy() at the end if successful.  */
> +  m_fetch_lazy_failed = true;
>   if (m_is_zero)
>     {
>       /* Nothing.  */
> diff --git a/gdb/value.h b/gdb/value.h
> index 8dc3192f637..77919082fe3 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -138,6 +138,7 @@ struct value
>       m_stack (false),
>       m_is_zero (false),
>       m_in_history (false),
> +      m_fetch_lazy_failed (false),
>       m_type (type_),
>       m_enclosing_type (type_)
>   {
> @@ -279,7 +280,13 @@ struct value
>   { return m_lazy; }
> 
>   void set_lazy (bool val)
> -  { m_lazy = val; }
> +  {
> +    m_lazy = val;
> +    m_fetch_lazy_failed = false;
> +  }
> +
> +  bool fetch_lazy_failed () const
> +  { return m_fetch_lazy_failed; }
> 
>   /* If a value represents a C++ object, then the `type' field gives the
>       object's compile-time type.  If the object actually belongs to some
> @@ -681,6 +688,9 @@ struct value
>   /* True if this a value recorded in value history; false otherwise.  */
>   bool m_in_history : 1;
> 
> +  /* True if fetch_lazy() did not finish sucessfully.  */
> +  bool m_fetch_lazy_failed : 1;
> +
>   /* Location of value (if lval).  */
>   union
>   {
> -- 
> 2.52.0

Re: [PATCH] Prevent downgrading of hardware watchpoints if possible

[Guinevere Larsen]

On 1/9/26 4:48 PM, Hannes Domani wrote:
> The lazy flag of a value can tell us if the value contents are available.
> But this could be either because it was simply never actually accessed,
> or it tried to be accessed, and failed.
> The latter are interesting for watchpoints, the former are not.
>
> Currently it only uses lazy values if they are at the head of the value
> chain, but this is fragile logic, and degrades some watchpoints to
> software watchpoints where it's actually not necessary.
>
> So this adds a new value flag 'm_fetch_lazy_failed' which tells if the
> watchpoint expression actually tried to access the value and failed.
> And this is used instead of the value chain location to tell if a lazy
> value should be used as a watchpoint location.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27423
> ---

Hi Hannes!

I took a look over this test, and I can't really review the technical 
parts, but I tested locally and the test does cause the bug before the 
change, and the code changes do solve the test, so I'm happy to add my 
tested tag.

Tested-By: Guinevere Larsen <guinevere@redhat.com>

That said, I have one question and the pre-commit.exp test did point out 
an issue, inlined.

>   gdb/breakpoint.c                              | 11 ++--
>   .../gdb.base/watchpoint-hw-no-degradation.c   | 51 +++++++++++++++++++
>   .../gdb.base/watchpoint-hw-no-degradation.exp | 38 ++++++++++++++
>   gdb/value.c                                   |  3 ++
>   gdb/value.h                                   | 12 ++++-
>   5 files changed, 108 insertions(+), 7 deletions(-)
>   create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
>   create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index af4de248ab6..13d95325604 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -2293,11 +2293,10 @@ update_watchpoint (struct watchpoint *b, bool reparse)
>   
>   	  /* If it's a memory location, and GDB actually needed
>   	     its contents to evaluate the expression, then we
> -	     must watch it.  If the first value returned is
> -	     still lazy, that means an error occurred reading it;
> +	     must watch it.  If an error occurred reading it,
>   	     watch it anyway in case it becomes readable.  */
>   	  if (v->lval () == lval_memory
> -	      && (v == val_chain[0] || ! v->lazy ()))
> +	      && (! v->lazy () || v->fetch_lazy_failed ()))
>   	    {
>   	      struct type *vtype = check_typedef (v->type ());
>   
> @@ -10716,12 +10715,12 @@ can_use_hardware_watchpoint (const std::vector<value_ref_ptr> &vals)
>   
>         if (v->lval () == lval_memory)
>   	{
> -	  if (v != head && v->lazy ())
> +	  if (v->lazy () && ! v->fetch_lazy_failed ())
>   	    /* A lazy memory lvalue in the chain is one that GDB never
>   	       needed to fetch; we either just used its address (e.g.,
>   	       `a' in `a.b') or we never needed it at all (e.g., `a'
> -	       in `a,b').  This doesn't apply to HEAD; if that is
> -	       lazy then it was not readable, but watch it anyway.  */
> +	       in `a,b').  If it failed to fetch a lazy value, then it
> +	       was not readable, so watch it in this case as well.  */
>   	    ;
>   	  else
>   	    {
> diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
> new file mode 100644
> index 00000000000..cfff2a44f90
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
> @@ -0,0 +1,51 @@
> +/* This testcase is part of GDB, the GNU debugger.
> +
> +   Copyright 2026 Free Software Foundation, Inc.
> +
> +   This program is free software; you can redistribute it and/or modify
> +   it under the terms of the GNU General Public License as published by
> +   the Free Software Foundation; either version 3 of the License, or
> +   (at your option) any later version.
> +
> +   This program is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +   GNU General Public License for more details.
> +
> +   You should have received a copy of the GNU General Public License
> +   along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +*/
> +
> +#include <sys/mman.h>
> +#include <unistd.h>
> +#include <stdio.h>
> +
> +int
> +main (void)
> +{
> +  size_t len = sysconf(_SC_PAGESIZE);
> +
> +  /* Map and unmap memory block to get address.  */
> +  void *p = mmap (0, len, PROT_READ|PROT_WRITE, MAP_ANON|MAP_PRIVATE, -1, 0);
> +  if (p == MAP_FAILED)
> +    {
> +      perror ("mmap");
> +      return 1;
> +    }
> +  munmap (p, len);
> +
> +  /* Now memory block at address P is inaccessible.
> +     Remap block at same address, so it becomes accessible again.  */
> +  p = mmap (p, len, PROT_READ|PROT_WRITE,
> +	    MAP_ANON|MAP_PRIVATE|MAP_FIXED, -1, 0);
> +  if (p == MAP_FAILED)
> +    {
> +      perror ("mmap");
> +      return 1;
> +    }
> +
> +  *(int *) p = 1;
> +
> +  return 0;
> +}
> diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> new file mode 100644
> index 00000000000..94ef335c77f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> @@ -0,0 +1,38 @@
> +# Copyright 2009-2026 Free Software Foundation, Inc.
Shouldn't the copyright year just be 2026?
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test if watchpoint doesn't degrade to a software watchpoint if part of
> +# expression isn't accessible at time of watchpoint creation.
> +
> +require allow_hw_watchpoint_access_tests
> +
> +standard_testfile
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> +    return -1
> +}
> +
> +if {![runto_main]} {
> +    return -1
> +}
> +
> +gdb_breakpoint [gdb_get_line_number "mmap (p, len"]
> +gdb_continue_to_breakpoint "mmap"
> +
> +gdb_test "eval \"watch *(int *)%p == 0\",p" \
> +	 "Hardware watchpoint $decimal: .*"
> +
> +gdb_test "continue" \
> +	 "Old value = <unreadable>.*New value = 0.*"
> diff --git a/gdb/value.c b/gdb/value.c
> index a52d4a6742c..c076617d102 100644
> --- a/gdb/value.c
> +++ b/gdb/value.c
> @@ -1546,6 +1546,7 @@ value::copy () const
>     val->m_bitpos = m_bitpos;
>     val->m_bitsize = m_bitsize;
>     val->m_lazy = m_lazy;
> +  val->m_fetch_lazy_failed = m_fetch_lazy_failed;
>     val->m_embedded_offset = embedded_offset ();
>     val->m_pointed_to_offset = m_pointed_to_offset;
>     val->m_modifiable = m_modifiable;
> @@ -4122,6 +4123,8 @@ value::fetch_lazy ()
>        value.  */
>     gdb_assert (m_optimized_out.empty ());
>     gdb_assert (m_unavailable.empty ());
> +  /* Will be reset with set_lazy() at the end if successful.  */
> +  m_fetch_lazy_failed = true;
>     if (m_is_zero)
>       {
>         /* Nothing.  */
> diff --git a/gdb/value.h b/gdb/value.h
> index 8dc3192f637..77919082fe3 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -138,6 +138,7 @@ struct value
>         m_stack (false),
>         m_is_zero (false),
>         m_in_history (false),
> +      m_fetch_lazy_failed (false),
>         m_type (type_),
>         m_enclosing_type (type_)
>     {
> @@ -279,7 +280,13 @@ struct value
>     { return m_lazy; }
>   
>     void set_lazy (bool val)
> -  { m_lazy = val; }
> +  {
> +    m_lazy = val;
> +    m_fetch_lazy_failed = false;
> +  }
> +
> +  bool fetch_lazy_failed () const
> +  { return m_fetch_lazy_failed; }
>   
>     /* If a value represents a C++ object, then the `type' field gives the
>        object's compile-time type.  If the object actually belongs to some
> @@ -681,6 +688,9 @@ struct value
>     /* True if this a value recorded in value history; false otherwise.  */
>     bool m_in_history : 1;
>   
> +  /* True if fetch_lazy() did not finish sucessfully.  */
sucessfully ==> successfully
> +  bool m_fetch_lazy_failed : 1;
> +
>     /* Location of value (if lval).  */
>     union
>     {


-- 
Cheers,
Guinevere Larsen
it/its
she/her (deprecated)

Re: [PATCH] Prevent downgrading of hardware watchpoints if possible

[Hannes Domani]

 Am Montag, 29. Juni 2026 um 22:39:21 MESZ hat Guinevere Larsen <guinevere@redhat.com> Folgendes geschrieben:

> On 1/9/26 4:48 PM, Hannes Domani wrote:
> > The lazy flag of a value can tell us if the value contents are available.
> > But this could be either because it was simply never actually accessed,
> > or it tried to be accessed, and failed.
> > The latter are interesting for watchpoints, the former are not.
> >
> > Currently it only uses lazy values if they are at the head of the value
> > chain, but this is fragile logic, and degrades some watchpoints to
> > software watchpoints where it's actually not necessary.
> >
> > So this adds a new value flag 'm_fetch_lazy_failed' which tells if the
> > watchpoint expression actually tried to access the value and failed.
> > And this is used instead of the value chain location to tell if a lazy
> > value should be used as a watchpoint location.
> >
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27423
> > ---
> 
> Hi Hannes!
> 
> I took a look over this test, and I can't really review the technical
> parts, but I tested locally and the test does cause the bug before the
> change, and the code changes do solve the test, so I'm happy to add my
> tested tag.
> 
> Tested-By: Guinevere Larsen <guinevere@redhat.com>
> 
> That said, I have one question and the pre-commit.exp test did point out
> an issue, inlined.


> > diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> > new file mode 100644
> > index 00000000000..94ef335c77f
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> > @@ -0,0 +1,38 @@
> > +# Copyright 2009-2026 Free Software Foundation, Inc.
> Shouldn't the copyright year just be 2026?

Yes.


> > +
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 3 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > +
> > +# Test if watchpoint doesn't degrade to a software watchpoint if part of
> > +# expression isn't accessible at time of watchpoint creation.
> > +
> > +require allow_hw_watchpoint_access_tests
> > +
> > +standard_testfile
> > +
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> > +    return -1
> > +}
> > +
> > +if {![runto_main]} {
> > +    return -1
> > +}
> > +
> > +gdb_breakpoint [gdb_get_line_number "mmap (p, len"]
> > +gdb_continue_to_breakpoint "mmap"
> > +
> > +gdb_test "eval \"watch *(int *)%p == 0\",p" \
> > +    "Hardware watchpoint $decimal: .*"
> > +
> > +gdb_test "continue" \
> > +    "Old value = <unreadable>.*New value = 0.*"
> > diff --git a/gdb/value.c b/gdb/value.c
> > index a52d4a6742c..c076617d102 100644
> > diff --git a/gdb/value.h b/gdb/value.h
> > index 8dc3192f637..77919082fe3 100644
> > --- a/gdb/value.h
> > +++ b/gdb/value.h
> > @@ -138,6 +138,7 @@ struct value
> >        m_stack (false),
> >        m_is_zero (false),
> >        m_in_history (false),
> > +      m_fetch_lazy_failed (false),
> >        m_type (type_),
> >        m_enclosing_type (type_)
> >    {
> > @@ -279,7 +280,13 @@ struct value
> >    { return m_lazy; }
> > 
> >    void set_lazy (bool val)
> > -  { m_lazy = val; }
> > +  {
> > +    m_lazy = val;
> > +    m_fetch_lazy_failed = false;
> > +  }
> > +
> > +  bool fetch_lazy_failed () const
> > +  { return m_fetch_lazy_failed; }
> > 
> >    /* If a value represents a C++ object, then the `type' field gives the
> >        object's compile-time type.  If the object actually belongs to some
> > @@ -681,6 +688,9 @@ struct value
> >    /* True if this a value recorded in value history; false otherwise.  */
> >    bool m_in_history : 1;
> > 
> > +  /* True if fetch_lazy() did not finish sucessfully.  */
> sucessfully ==> successfully

Fixed locally as well.

Since then I've enabled pre-commit as well, so I shouldn't miss these
problems any more.


Hannes

Re: [PATCH] Prevent downgrading of hardware watchpoints if possible

[Thiago Jung Bauermann]

Hannes Domani <ssbssa@yahoo.de> writes:

> The lazy flag of a value can tell us if the value contents are available.
> But this could be either because it was simply never actually accessed,
> or it tried to be accessed, and failed.
> The latter are interesting for watchpoints, the former are not.
>
> Currently it only uses lazy values if they are at the head of the value
> chain, but this is fragile logic, and degrades some watchpoints to
> software watchpoints where it's actually not necessary.
>
> So this adds a new value flag 'm_fetch_lazy_failed' which tells if the
> watchpoint expression actually tried to access the value and failed.
> And this is used instead of the value chain location to tell if a lazy
> value should be used as a watchpoint location.
>
> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27423
> ---
>  gdb/breakpoint.c                              | 11 ++--
>  .../gdb.base/watchpoint-hw-no-degradation.c   | 51 +++++++++++++++++++
>  .../gdb.base/watchpoint-hw-no-degradation.exp | 38 ++++++++++++++
>  gdb/value.c                                   |  3 ++
>  gdb/value.h                                   | 12 ++++-
>  5 files changed, 108 insertions(+), 7 deletions(-)
>  create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
>  create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp

I like the solution, thanks!

Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>

Just one nit in the testcase:

> diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> new file mode 100644
> index 00000000000..94ef335c77f
> --- /dev/null
> +++ b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> @@ -0,0 +1,38 @@
> +# Copyright 2009-2026 Free Software Foundation, Inc.
> +
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 3 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +
> +# Test if watchpoint doesn't degrade to a software watchpoint if part of
> +# expression isn't accessible at time of watchpoint creation.
> +
> +require allow_hw_watchpoint_access_tests
> +
> +standard_testfile
> +
> +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> +    return -1
> +}
> +
> +if {![runto_main]} {
> +    return -1
> +}

There's no need to return a value from the top-level. Please change the
returns above to just "return".

> +
> +gdb_breakpoint [gdb_get_line_number "mmap (p, len"]
> +gdb_continue_to_breakpoint "mmap"
> +
> +gdb_test "eval \"watch *(int *)%p == 0\",p" \
> +	 "Hardware watchpoint $decimal: .*"
> +
> +gdb_test "continue" \
> +	 "Old value = <unreadable>.*New value = 0.*"
⋮
> diff --git a/gdb/value.h b/gdb/value.h
> index 8dc3192f637..77919082fe3 100644
> --- a/gdb/value.h
> +++ b/gdb/value.h
> @@ -138,6 +138,7 @@ struct value
>        m_stack (false),
>        m_is_zero (false),
>        m_in_history (false),
> +      m_fetch_lazy_failed (false),
>        m_type (type_),
>        m_enclosing_type (type_)
>    {
> @@ -279,7 +280,13 @@ struct value
>    { return m_lazy; }
>  
>    void set_lazy (bool val)
> -  { m_lazy = val; }
> +  {
> +    m_lazy = val;
> +    m_fetch_lazy_failed = false;
> +  }
> +
> +  bool fetch_lazy_failed () const
> +  { return m_fetch_lazy_failed; }
>  
>    /* If a value represents a C++ object, then the `type' field gives the
>       object's compile-time type.  If the object actually belongs to some
> @@ -681,6 +688,9 @@ struct value
>    /* True if this a value recorded in value history; false otherwise.  */
>    bool m_in_history : 1;
>  
> +  /* True if fetch_lazy() did not finish sucessfully.  */
> +  bool m_fetch_lazy_failed : 1;
> +
>    /* Location of value (if lval).  */
>    union
>    {

As a side note, because of this:

  /* Note that the fields in this structure are arranged to save a bit
     of memory.  */

  struct value
  {

I decided to check:

(top-gdb) ptype /o value
/* offset      |    size */  type = struct value {
                             private:
/*      0      |       4 */    lval_type m_lval;
/*      4: 0   |       1 */    bool m_modifiable : 1;
/*      4: 1   |       1 */    bool m_lazy : 1;
/*      4: 2   |       1 */    bool m_initialized : 1;
/*      4: 3   |       1 */    bool m_stack : 1;
/*      4: 4   |       1 */    bool m_is_zero : 1;
/*      4: 5   |       1 */    bool m_in_history : 1;
/*      4: 6   |       1 */    bool m_fetch_lazy_failed : 1;
/* XXX  1-bit hole       */
/* XXX  3-byte hole      */
/*      8      |      40 */    union {
/*                     8 */        CORE_ADDR address;

This patch doesn't change the size of struct value.

-- 
Thiago
(he/him)

Re: [PATCH] Prevent downgrading of hardware watchpoints if possible

[Hannes Domani]

 Am Freitag, 3. Juli 2026 um 05:18:19 MESZ hat Thiago Jung Bauermann <thiago.bauermann@linaro.org> Folgendes geschrieben:

> Hannes Domani <ssbssa@yahoo.de> writes:
> 
> > The lazy flag of a value can tell us if the value contents are available.
> > But this could be either because it was simply never actually accessed,
> > or it tried to be accessed, and failed.
> > The latter are interesting for watchpoints, the former are not.
> >
> > Currently it only uses lazy values if they are at the head of the value
> > chain, but this is fragile logic, and degrades some watchpoints to
> > software watchpoints where it's actually not necessary.
> >
> > So this adds a new value flag 'm_fetch_lazy_failed' which tells if the
> > watchpoint expression actually tried to access the value and failed.
> > And this is used instead of the value chain location to tell if a lazy
> > value should be used as a watchpoint location.
> >
> > Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=27423
> > ---
> >  gdb/breakpoint.c                              | 11 ++--
> >  .../gdb.base/watchpoint-hw-no-degradation.c  | 51 +++++++++++++++++++
> >  .../gdb.base/watchpoint-hw-no-degradation.exp | 38 ++++++++++++++
> >  gdb/value.c                                  |  3 ++
> >  gdb/value.h                                  | 12 ++++-
> >  5 files changed, 108 insertions(+), 7 deletions(-)
> >  create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.c
> >  create mode 100644 gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> 
> I like the solution, thanks!
> 
> Reviewed-by: Thiago Jung Bauermann <thiago.bauermann@linaro.org>
> 
> Just one nit in the testcase:
> 
> > diff --git a/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> > new file mode 100644
> > index 00000000000..94ef335c77f
> > --- /dev/null
> > +++ b/gdb/testsuite/gdb.base/watchpoint-hw-no-degradation.exp
> > @@ -0,0 +1,38 @@
> > +# Copyright 2009-2026 Free Software Foundation, Inc.
> > +
> > +# This program is free software; you can redistribute it and/or modify
> > +# it under the terms of the GNU General Public License as published by
> > +# the Free Software Foundation; either version 3 of the License, or
> > +# (at your option) any later version.
> > +#
> > +# This program is distributed in the hope that it will be useful,
> > +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> > +# GNU General Public License for more details.
> > +#
> > +# You should have received a copy of the GNU General Public License
> > +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> > +
> > +# Test if watchpoint doesn't degrade to a software watchpoint if part of
> > +# expression isn't accessible at time of watchpoint creation.
> > +
> > +require allow_hw_watchpoint_access_tests
> > +
> > +standard_testfile
> > +
> > +if {[prepare_for_testing "failed to prepare" $testfile $srcfile debug]} {
> > +    return -1
> > +}
> > +
> > +if {![runto_main]} {
> > +    return -1
> > +}
> 
> There's no need to return a value from the top-level. Please change the
> returns above to just "return".

I've made these changes.


Hannes