* Re: [PATCH] Prevent downgrading of hardware watchpoints if possible
2026-01-09 19:48 ` [PATCH] Prevent downgrading of hardware watchpoints if possible Hannes Domani
@ 2026-06-26 14:58 ` Hannes Domani
2026-06-29 20:39 ` Guinevere Larsen
2026-07-03 3:18 ` Thiago Jung Bauermann
2 siblings, 0 replies; 6+ messages in thread
From: Hannes Domani @ 2026-06-26 14:58 UTC (permalink / raw)
To: gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Prevent downgrading of hardware watchpoints if possible
2026-01-09 19:48 ` [PATCH] Prevent downgrading of hardware watchpoints if possible Hannes Domani
2026-06-26 14:58 ` Hannes Domani
@ 2026-06-29 20:39 ` Guinevere Larsen
2026-07-02 15:03 ` Hannes Domani
2026-07-03 3:18 ` Thiago Jung Bauermann
2 siblings, 1 reply; 6+ messages in thread
From: Guinevere Larsen @ 2026-06-29 20:39 UTC (permalink / raw)
To: Hannes Domani, gdb-patches
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)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Prevent downgrading of hardware watchpoints if possible
2026-06-29 20:39 ` Guinevere Larsen
@ 2026-07-02 15:03 ` Hannes Domani
0 siblings, 0 replies; 6+ messages in thread
From: Hannes Domani @ 2026-07-02 15:03 UTC (permalink / raw)
To: gdb-patches, Guinevere Larsen
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Prevent downgrading of hardware watchpoints if possible
2026-01-09 19:48 ` [PATCH] Prevent downgrading of hardware watchpoints if possible Hannes Domani
2026-06-26 14:58 ` Hannes Domani
2026-06-29 20:39 ` Guinevere Larsen
@ 2026-07-03 3:18 ` Thiago Jung Bauermann
2026-07-03 19:18 ` Hannes Domani
2 siblings, 1 reply; 6+ messages in thread
From: Thiago Jung Bauermann @ 2026-07-03 3:18 UTC (permalink / raw)
To: Hannes Domani; +Cc: gdb-patches
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)
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Prevent downgrading of hardware watchpoints if possible
2026-07-03 3:18 ` Thiago Jung Bauermann
@ 2026-07-03 19:18 ` Hannes Domani
0 siblings, 0 replies; 6+ messages in thread
From: Hannes Domani @ 2026-07-03 19:18 UTC (permalink / raw)
To: Thiago Jung Bauermann; +Cc: gdb-patches
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
^ permalink raw reply [flat|nested] 6+ messages in thread