From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id KMaeCRbYQmrgRR4AWB0awg (envelope-from ) for ; Mon, 29 Jun 2026 16:39:50 -0400 Authentication-Results: simark.ca; dkim=pass (1024-bit key; unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Qk21xnap; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 147431E098; Mon, 29 Jun 2026 16:39:50 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-6.4 required=5.0 tests=ARC_SIGNED,ARC_VALID,BAYES_00, DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [IPv6:2620:52:6:3111::32]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange x25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id 071441E024 for ; Mon, 29 Jun 2026 16:39:49 -0400 (EDT) Received: from vm01.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id C045A4BA23FF for ; Mon, 29 Jun 2026 20:39:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org C045A4BA23FF Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Qk21xnap Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 76E8B4BA23DC for ; Mon, 29 Jun 2026 20:39:20 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 76E8B4BA23DC Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 76E8B4BA23DC Authentication-Results: sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1782765560; cv=none; b=Al5MvIPHFv0KlYJLN1TDTMmoGsNdzeP9J58bTmtd3JOyumT1/sEfs7hhcpgILX0ZUcDvcVMzNyVRwwrrBMo064HS+ghK+OGtbOnlmGDKYRFvfdmJIVgUFEfnFjAGT81D5YLNL2n7eLVpwI2pCw11JROKwpRP6C/EPPlGu1kQNvc= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1782765560; c=relaxed/simple; bh=6mwlFNyK6q75ssE+x5DTAopgQcEqh74mD6LQjMGq6PY=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=sld8+FxpVEDPIW423T6OkjTD6fIPNN87VaEsvqCf3F1SLACQ8RO0pXuZ2xuZ408dX32+48zW965IR4efIHEABguKt99pYO/CnatblK0eEX2fS17wWZ/d+TQkKCWlbEd+n3pT0WiU5sCeYU2ckOUMbZ4WIJlKf+TBMzjfnrWemZY= ARC-Authentication-Results: i=1; sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=Qk21xnap DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 76E8B4BA23DC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1782765560; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=G+5SbsUjpeZsa+ia29EJgErzm/c16piGqE2sh1quczg=; b=Qk21xnap2Y2LzGQl/HMQZ3WrM0gudPFgTRupyUPIKs43bXbqM06T53fHHZGyidFAloeMf/ YiHJu/kPIS4I1utsKu34UDTos8iQNtZ7DrriaPg+96eT5RNe1xtLz8Nb4OoqYoddtiS66n EjE8l+1fgyOw3r5JGX1N1oUUZGB8AyQ= Received: from mail-vs1-f72.google.com (mail-vs1-f72.google.com [209.85.217.72]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-630-Zouafh8xOeCKbA7IfeSjRQ-1; Mon, 29 Jun 2026 16:39:18 -0400 X-MC-Unique: Zouafh8xOeCKbA7IfeSjRQ-1 X-Mimecast-MFC-AGG-ID: Zouafh8xOeCKbA7IfeSjRQ_1782765558 Received: by mail-vs1-f72.google.com with SMTP id ada2fe7eead31-739b4a4301dso925719137.0 for ; Mon, 29 Jun 2026 13:39:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1782765558; x=1783370358; h=content-transfer-encoding:in-reply-to:from:content-language :references:to:subject:user-agent:mime-version:date:message-id :x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=G+5SbsUjpeZsa+ia29EJgErzm/c16piGqE2sh1quczg=; b=IGPpBe0oCvUbiVSiL9BKszFbpW/fqq/ydIGQIqjexrNoNQE9rwPyl1/FIZkzWOB3P7 5Zs4a3rknhtr5W8ssPIp7VlYUaGgIiWJHqGU6oY2msC35G9gxS/RkI6JPgg8fH+xWbMZ z9f8m0KnFwLvuN3hUAWDYpT8Y8mKdgl9nf8bfN1V8iYZ0mqxNJC1jlH4plC4t/mH+Rqq hc7hnCalehk87yDPpYXzakVY2tl7HI6mqpUMyJsdOa+kvnYSIe5XcMgCO1W8JAqOASdC KW5eSlJCd5ST0bamr6pRgCkPbtFVNqGjnFJQRLuZtrIA8HyGAhEVEu7uydrBSLyAmpKF GLLw== X-Forwarded-Encrypted: i=1; AHgh+RqgwEtoy/mitu/lSQyvBbDV7O0QDmnKf29kha8oOTsDSslu0B7OY+XrEoNyK6Rj57+O5X8MyQav6+c9Dw==@sourceware.org X-Gm-Message-State: AOJu0YyjnpOEeNqZFguwhR0XTJl90SCcl3Hz/Gn2GzpVre59w2FNOsJg qKpmyAKlQNXogIZ/DdcjVCiJr+uGE3ow8jBqA0RkMgv/8ZDUglADuGSE/M9IVFEMnmR7Kgnpqrs ZI5w+lG3KRHucfC+vn5xHUo8ti3TrDX+ca1q7RjrgGd/F4dPKE0uoZyZlDjFjlIE= X-Gm-Gg: AfdE7ckY2S/a/QwK/ftP48A8tod0oB69ZJRBxn5MG9wXv//sDW0OJ4/ch1n1qMi6YoI XQS2qihR8OWIattr6L4tqPTWqdswhR00iasDuNfEp49vzubu+M95lvPJxoUnSvityJBjKdsRUbR YgHvxkFm4XwgVN5uq0s8+nbegnr6UpLBMwOc3sVXzsng2KPkWtaN3VtZ1CV5K/e914t4R/vB0qQ bHc0Eyv+KckQWIM433/WW62TJacIW+/LIny/pDLMAnjUwr6cciE/x8mxaIpi1MtXzdYWlWXwoWw YkG8CYgde9jWsarpbtOIZji+I0O+JJViVB1Bdo5U2oVh8z376TsQYJg+yHcItVOgoIYL1JeHfMU 4E7KcCBEJHnbdlmbLKuYE X-Received: by 2002:a05:6102:951:b0:730:db02:1d08 with SMTP id ada2fe7eead31-73a366e9e71mr835810137.10.1782765558231; Mon, 29 Jun 2026 13:39:18 -0700 (PDT) X-Received: by 2002:a05:6102:951:b0:730:db02:1d08 with SMTP id ada2fe7eead31-73a366e9e71mr835795137.10.1782765557612; Mon, 29 Jun 2026 13:39:17 -0700 (PDT) Received: from ?IPV6:2804:14d:8084:993e::75d? ([2804:14d:8084:993e::75d]) by smtp.gmail.com with ESMTPSA id ada2fe7eead31-73a821a7c40sm51841137.6.2026.06.29.13.39.15 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 29 Jun 2026 13:39:16 -0700 (PDT) Message-ID: <7703d72c-faec-4792-84c9-3ed71fad4024@redhat.com> Date: Mon, 29 Jun 2026 17:39:13 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] Prevent downgrading of hardware watchpoints if possible To: Hannes Domani , gdb-patches@sourceware.org References: <20260109194839.1598134-1-ssbssa.ref@yahoo.de> <20260109194839.1598134-1-ssbssa@yahoo.de> From: Guinevere Larsen In-Reply-To: <20260109194839.1598134-1-ssbssa@yahoo.de> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: t2mo2Phs5V8QVKsTBHNal6ytXilBCX3ndxwr0ELgSLc_1782765558 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces~public-inbox=simark.ca@sourceware.org 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 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 &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 . > + > +*/ > + > +#include > +#include > +#include > + > +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 . > + > +# 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 = .*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)