From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id KL2bLPX042nxxSgAWB0awg (envelope-from ) for ; Sat, 18 Apr 2026 17:17:41 -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=fz15vrM5; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 86C701E0B1; Sat, 18 Apr 2026 17:17:41 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-3.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,RCVD_IN_VALIDITY_CERTIFIED_BLOCKED, RCVD_IN_VALIDITY_RPBL_BLOCKED,RCVD_IN_VALIDITY_SAFE_BLOCKED autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (vm01.sourceware.org [38.145.34.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 D26C21E04F for ; Sat, 18 Apr 2026 17:17:39 -0400 (EDT) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 933B84B920A5 for ; Sat, 18 Apr 2026 21:17:38 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 933B84B920A5 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=fz15vrM5 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by sourceware.org (Postfix) with ESMTP id 5F9A14B920A4 for ; Sat, 18 Apr 2026 21:16:49 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5F9A14B920A4 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 5F9A14B920A4 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.129.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1776547009; cv=none; b=u7yraxKLJ6+CzGu/srq4r1SUrQcrDnhc9vLHCwUC4QhBas1CTDw+5+H+1wlx7vm1eRMUj0xo4huGdGeIrXMqp3PcmhnVY1n75VeoKgjIS8MzBe4cELACDnNf1dwQby3nOx7Ea8rST/MqTJHmY5zIe02b/iai88n80hALpCxfY1U= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1776547009; c=relaxed/simple; bh=z3v/rbbCHrcG+lHwmMiONrXIuZe4LgGnzwGanc4v+Kk=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=HhkkE1M+sQIH60wval6EbKchpsWB6VLwQdI/6pp+qGoJ204FGrXLPTVd2MW1RabU7DARsZrLTGYaBc13E8KX+avF0wBVrI//KySrd9f6lEae/JSZbCiddJN9ysw9P4DuCOmsMsMleulzC0RRCHSk2PM+QGCRPqZqyNfGsQ6HK+I= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 5F9A14B920A4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1776547009; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=aW4cayXVtiYihLZ+R3s5bcFj0omOKojyvAfI4BzDnJA=; b=fz15vrM52Fpw1A7Vc1FIzqidJTUfGUpZZjCAyKEejGhN4B7fDnnh+sGWDX6dI6iVnHDK4Z MUGibDhauyKf1BlnSamj5I4QQgzHuabYpvOM7vjUxH9BpVlAldXGhgO0JRv3JsREwvKd+P +gzOV64bbz9pulU87354rKsSuHTFF1Y= Received: from mail-wm1-f69.google.com (mail-wm1-f69.google.com [209.85.128.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-619-6Hi7GWwuPoKCmrD-WApkdA-1; Sat, 18 Apr 2026 17:16:47 -0400 X-MC-Unique: 6Hi7GWwuPoKCmrD-WApkdA-1 X-Mimecast-MFC-AGG-ID: 6Hi7GWwuPoKCmrD-WApkdA_1776547006 Received: by mail-wm1-f69.google.com with SMTP id 5b1f17b1804b1-488d9e1e61aso14164945e9.0 for ; Sat, 18 Apr 2026 14:16:47 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1776547006; x=1777151806; h=content-transfer-encoding:mime-version:message-id:date:references :in-reply-to:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=nJqWeqURFRR++b8g5GE8AHy7yD3kANM3XZxWPxSoDP4=; b=Ex1Pw5ER84HOsgHyZ1G6MaosunNm0PInqivfXc9qsGTe/EKrysc1cxgmHBj9bQ8aQt WOx5KyjVDBi1MLoz1qdBelT6nythH1Ai93q4DxcAvrvlbBMqENtSgStramMpHoD9B+9G eniSa+EJjyolAjQ4C+3Pk7GEtOUuxUwqq4F+97zGmPCCqsXORRzVaF8bSv8cFhvPRYIA v+ldXLdvWb0Gf+0myiRvvF6Tam5UDVVSS5R0PRMlJeitRlOyg6ZsLiEE9WVXGg2gQ/nY /2NZBKH7fVOqYOzmWUR+MxGemd6BUZiingpBvsJi96IaO6zy2ZBnm6ertlTgnPE9tVTS hlVg== X-Forwarded-Encrypted: i=1; AFNElJ/T9WRd3cEjNY/m48NfxGaXr5BCNCiZo0et2kITmBoixSxLg/NMDwEI67LwwINR3U9X/FZgx0/WkLBraQ==@sourceware.org X-Gm-Message-State: AOJu0Yzwb4aSrRArjAreXnN68LEoV8oyqkbIyTkEdMlTxzTUVSvyUvvP 7rKKzZ/vtgX6j7Jw6gMc46KXBRhmLwQtXse/SlNn37FSHoRH6+lmNuEcP7FPlRtyJXCvmskLK6Z Jg8NkWij4NbvXqIju5nRXLCo1UZzsNzifTJK+ivP0PiDuUCspLg1kd16jGw9KnUI8cU4MqbY= X-Gm-Gg: AeBDiev96zBXm4beQSG/ZeoxQwvlrwRxTX2S0MR7Jag839t0cGfOd3SaShgIKDBleFc XN08kwdkhKgH4xQcxjj0rlR4QY+V6tyF8Pg2UhDkxzKEG+WJDQutWHcsVwefg56KawS5rRTbguo OT4/3BM/s07LlL4ltfWwXoJOP3znUGSoRoaB3zN4ngUtUw2Jzx3RogZXO120MTl6jvE7xc/logR Urh2Wdah0mo13mZH8K+1ukhKvAVEfcUUbuXVrfAa0DdfvsewFgqwX8sQMKSAqxUY4bcVam5dgxR UTWoLOsYMmJDVPBztPiv767HkCL3C1bL+wysGffE5I6AQBFZy/vnRJ7/kP9a/GGae4dbmsoZRTj McN3gz2LNBCLvB3Lu784Ic+fb0Mg= X-Received: by 2002:a05:600c:4fd1:b0:488:78f2:6b0 with SMTP id 5b1f17b1804b1-488fb78ede0mr110405755e9.29.1776547005988; Sat, 18 Apr 2026 14:16:45 -0700 (PDT) X-Received: by 2002:a05:600c:4fd1:b0:488:78f2:6b0 with SMTP id 5b1f17b1804b1-488fb78ede0mr110405555e9.29.1776547005487; Sat, 18 Apr 2026 14:16:45 -0700 (PDT) Received: from localhost ([31.111.84.232]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-488fc14a61asm145936825e9.15.2026.04.18.14.16.45 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 18 Apr 2026 14:16:45 -0700 (PDT) From: Andrew Burgess To: Alexandra =?utf-8?B?SMOhamtvdsOh?= , gdb-patches@sourceware.org Cc: ahajkova@redhat.com Subject: Re: [PATCH v3] gdb: Add source-tracking breakpoints feature In-Reply-To: <20260410084251.177366-1-ahajkova@redhat.com> References: <20260410084251.177366-1-ahajkova@redhat.com> Date: Sat, 18 Apr 2026 22:16:44 +0100 Message-ID: <871pgc3r5v.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: o2P-7Qr4T5O_FH1a3NpXwjWkpbHRGhxrdNj4pp_-SDo_1776547006 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable 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 Alexandra H=C3=A1jkov=C3=A1 writes: > The breakpoint_source structure stores captured source code lines > around a breakpoint location, along with a reference to the BFD > that was current when the source was captured. > > When source tracking is enabled (via 'set breakpoint source-tracking > enabled on'), GDB captures 3 lines of source context > (BREAKPOINT_SRC_CTX_LINES) along with the current BFD when a > breakpoint is first set. On executable reload (detected by comparing > BFDs), it searches within a 12-line window > (BREAKPOINT_SRC_CTX_LINES * BREAKPOINT_SRC_SEARCH_MULTIPLIER) for > the best match and adjusts the breakpoint location if needed. > > Tests added: > gdb.base/adjust_breakpoint.exp > gdb.base/adjust_breakpoint-missing-source.exp > gdb.base/source-tracking-inline.exp > > adjust_breakpoint.exp covers four scenarios: > - adjust the breakpoint when lines are deleted > - adjust the breakpoint when lines are inserted > - the tracked line disappears entirely > - verify the tracking can be disabled > > adjust_breakpoint-missing-source.exp covers the edge case where source > files are unavailable, verifying GDB falls back to non-tracking breakpoin= ts. > > source-tracking-inline.exp covers source tracking with inline functions. > > Add maintenance command to print tracked source code > Add documentation for the new source-tracking breakpoints feature. > > Limitations of the current implementation: > > Source tracking is not enabled for pending breakpoints that become > non-pending. When a breakpoint is created pending (e.g. with 'set > breakpoint pending on'), source context is not captured at creation > time since no symtab is available yet. When the breakpoint later > resolves to a location, re_set_default() only updates existing tracked > breakpoints and does not initiate tracking for newly resolved ones. > This could be fixed in the future by initiating source tracking in > re_set_default() when a breakpoint transitions from pending to > non-pending. > > Source tracking for ranged breakpoints is not currently supported. > Ranged breakpoints have a start and end location spec, and tracking > both independently raises questions about whether to preserve the > range length or track each end separately. For now, ranged > breakpoints will never be source-tracked. > @@ -13201,6 +13418,157 @@ code_breakpoint::location_spec_to_sals (locatio= n_spec *locspec, > return sals; > } > =20 > +/* Match BREAKPOINT_SRC_CTX_LINES lines of the initially stored source i= n a > + BREAKPOINT_SRC_CTX_LINES * BREAKPOINT_SRC_SEARCH_MULTIPLIER lines cur= rent > + source window. > + Returns new breakpoint line on success or -1 on failure. */ > +static int > +sliding_window_match (breakpoint_source *bp_source, > +=09=09 breakpoint_source *tmp_source) > +{ > + int bp_stored =3D bp_source->bp_line_stored; > + int tmp_size =3D (int) tmp_source->source_lines.size (); > + int bp_size =3D (int) bp_source->source_lines.size (); > + > + for (int i =3D 0; i < tmp_size; i++) > + { > + /* Look for the initial breakpoint line in a stored window. */ > + if (bp_source->source_lines[bp_stored] =3D=3D tmp_source->source_l= ines[i]) > +=09{ > +=09 /* Check if the stored lines before and after the breakpoint line > +=09 also match, to reduce false positives. */ > +=09 if (i > 0 && bp_stored > 0 > +=09 && (i + 1) < tmp_size > +=09 && (bp_stored + 1) < bp_size) > +=09 { > +=09 if ((bp_source->source_lines[bp_stored - 1] =3D=3D tmp_source->= source_lines[i - 1]) > +=09=09 && (bp_source->source_lines[bp_stored + 1] =3D=3D tmp_source->so= urce_lines[i + 1])) > +=09=09{ > +=09=09 return tmp_source->bp_line + i - tmp_source->bp_line_stored; > +=09=09} > +=09 } > +=09 else if (i =3D=3D 0 && (i + 1) < tmp_size > +=09=09 && (bp_stored + 1) < bp_size) > +=09 { > +=09 if (bp_source->source_lines[bp_stored + 1] =3D=3D tmp_source->s= ource_lines[i + 1]) > +=09=09{ > +=09=09 return tmp_source->bp_line + i - tmp_source->bp_line_stored; > +=09=09} > +=09 } > +=09 else if ((i + 1) =3D=3D tmp_size && i > 0 > +=09=09 && bp_stored > 0) > +=09 { > +=09 if (bp_source->source_lines[bp_stored - 1] =3D=3D tmp_source->s= ource_lines[i - 1]) > +=09=09{ > +=09=09 return tmp_source->bp_line + i - tmp_source->bp_line_stored; > +=09=09} > +=09 } I think there might be a case which isn't handled correctly here. If bp_stored =3D=3D 0 indicating that the breakpoint line _was_ the first line in the file, but i > 0 indicating that the line is no longer the first line, then I don't think any of these cases will hit. Also, the 'return' statements don't need the { ... }. > +=09} > + } > + > + return -1; > +} > + > +/* See breakpoint.h. */ > + > +void > +code_breakpoint::adjust_bp_for_source_tracking > + (program_space *filter_pspace, > + std::vector &expanded) > +{ > + if (expanded.empty () || expanded[0].symtab =3D=3D nullptr > + || !breakpoint_source_is_tracked (bp_source.get ())) > + return; We don't check `source_tracking_breakpoints` here, so if this feature is turned on, then a b/p is created with tracking, then the feature is turned off, the breakpoint will still adjust. I wonder if when the source-tracking feature is turned off, maybe we should convert all breakpoints to none source tracked by deleting the tracking information. If we did this then we wouldn't need to check the flag here as we would only see tracked breakpoints if the feature is currently on. > + > + struct compunit_symtab *cust =3D expanded[0].symtab->compunit (); > + if (cust =3D=3D nullptr || cust->objfile () =3D=3D nullptr) > + return; > + > + bfd *current_bfd =3D cust->objfile ()->obfd.get (); > + if (bp_source->source_bfd.get () =3D=3D current_bfd) > + return; > + > + /* BFD changed =E2=80=94 executable was reloaded. */ > + if (expanded.size () !=3D 1) > + { > + warning (_("Breakpoint %d now has multiple locations after reload,= " > +=09=09 "disabling source tracking."), number); > + bp_source.reset (); > + return; > + } > + > + /* If this fails then the location spec has changed since the > + breakpoint's source tracking was initially setup. */ > + gdb_assert (breakpoint_locspec_suitable_for_tracking (locspec.get ()))= ; > + > + int bp_stored =3D bp_source->bp_line_stored; > + std::string line; > + auto restore_styling =3D make_scoped_restore (&source_styling, false); > + if (!g_source_cache.get_source_lines (expanded[0].symtab, > +=09=09=09=09=09expanded[0].line, > +=09=09=09=09=09expanded[0].line, &line)) > + { > + /* Source is unreadable after reload =E2=80=94 drop tracking. */ > + bp_source.reset (); > + return; > + } > + > + if (line =3D=3D bp_source->source_lines[bp_stored]) > + { > + /* Line unchanged =E2=80=94 just refresh the capture with the new = BFD. */ > + bp_source =3D std::make_unique > +=09(breakpoint_source_capture (expanded, BREAKPOINT_SRC_CTX_LINES)); > + return; > + } > + > + breakpoint_source tmp_source > + =3D breakpoint_source_capture (expanded, > +=09=09=09=09 BREAKPOINT_SRC_CTX_LINES > +=09=09=09=09 * BREAKPOINT_SRC_SEARCH_MULTIPLIER); > + int new_bp_line =3D sliding_window_match (bp_source.get (), &tmp_sourc= e); > + if (new_bp_line =3D=3D -1) > + { > + warning (_("Breakpoint %d source code not found " > +=09=09 "after reload, keeping original location."), number); > + bp_source.reset (); > + return; > + } > + > + location_spec *spec =3D locspec.get (); > + std::string bp_string (spec->to_string ()); > + auto pos =3D bp_string.rfind (':'); > + if (pos =3D=3D std::string::npos) > + { > + warning (_("unable to update location spec for breakpoint %d, " > +=09=09 "disabling source tracking."), number); > + bp_source.reset (); > + return; > + } > + > + bp_string =3D bp_string.substr (0, pos + 1); > + bp_string.append (std::to_string (new_bp_line)); > + > + /* set_string only updates the cached display string, not the parsed > + spec_string that location_spec_to_sals actually uses. Replace the > + location spec entirely by re-parsing the updated string so the new > + line number takes effect. */ > + const char *new_spec_str =3D bp_string.c_str (); > + locspec =3D string_to_location_spec (&new_spec_str, current_language); > + spec =3D locspec.get (); > + > + int found; > + expanded =3D location_spec_to_sals (spec, filter_pspace, &found); > + if (found && new_bp_line !=3D bp_source->bp_line) > + { > + gdb_printf (_("Breakpoint %d adjusted from line %d to line %d.\n")= , > +=09=09 number, bp_source->bp_line, new_bp_line); > + notify_breakpoint_modified (this); > + } > + > + bp_source =3D std::make_unique > + (breakpoint_source_capture (expanded, BREAKPOINT_SRC_CTX_LINES)); > +} > + > /* The default re_set method, for typical hardware or software > breakpoints. Reevaluate the breakpoint and recreate its > locations. */ > @@ -13231,6 +13599,9 @@ code_breakpoint::re_set_default (struct program_s= pace *filter_pspace) > =20 > if (locspec_range_end !=3D nullptr) > =09{ > +=09 /* Ranged breakpoints are not currently tracked. */ > +=09 gdb_assert (!breakpoint_source_is_tracked (bp_source.get ())); > + > =09 std::vector sals_end > =09 =3D location_spec_to_sals (locspec_range_end.get (), > =09=09=09=09 filter_pspace, &found); > @@ -13239,6 +13610,8 @@ code_breakpoint::re_set_default (struct program_s= pace *filter_pspace) > =09} > } > =20 > + adjust_bp_for_source_tracking (filter_pspace, expanded); This can probably move in the preceding `if` block, if we don't pass through the `if` block then `expanded` will be empty. This isn't really a problem as adjust_bp_for_source_tracking has an early return if `expanded` is empty, but I think it makes more sense to move this call into the `if` block. > + > /* Update the locations for this breakpoint. For thread-specific > breakpoints this will remove any old locations that are for the wro= ng > program space -- this can happen if the user changes the thread of = a > @@ -82,6 +84,17 @@ enum remove_bp_reason > architecture. */ > =20 > #define=09BREAKPOINT_MAX=0916 > + > +/* Number of source lines to capture around a breakpoint for source trac= king. > + This context is used to match and relocate breakpoints when the execu= table > + is reloaded. The window is centered on the breakpoint line, capturin= g > + lines both before and after it. */ > +#define BREAKPOINT_SRC_CTX_LINES 3 > + > +/* Multiplier for the search window when looking for relocated breakpoin= ts. > + We search in (BREAKPOINT_SRC_CTX_LINES * BREAKPOINT_SRC_SEARCH_MULTIP= LIER) > + lines to find code that may have moved. */ > +#define BREAKPOINT_SRC_SEARCH_MULTIPLIER 4 These could probably move into breakpoint.c. Thanks, Andrew