From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id GUGSBRr6iWmOxzEAWB0awg (envelope-from ) for ; Mon, 09 Feb 2026 10:15:38 -0500 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=BIsIWX/W; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 09CC11E0BA; Mon, 09 Feb 2026 10:15:38 -0500 (EST) 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 C346F1E08D for ; Mon, 09 Feb 2026 10:15:36 -0500 (EST) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id 4DA0A4CF32B9 for ; Mon, 9 Feb 2026 15:15:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 4DA0A4CF32B9 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=BIsIWX/W 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 34BD04CF32BD for ; Mon, 9 Feb 2026 15:15:09 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 34BD04CF32BD 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 34BD04CF32BD Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1770650109; cv=none; b=I9zD0zFcumtojXXSsEwmR8rlRjeW/F7U3KeU7U0ux4xb3mpLCvnnLdXubMXmaz6AD+gsF7leWkDpWFwjeNDuM+VSS38q4G/g0ZUuEVl1VtMAr9QI16axnVfxjmtN164+ebGh/NLZBOSgizRdaainxL19SYJDqUWzx532I/VLxX8= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1770650109; c=relaxed/simple; bh=14C/a3eIdHbYy5Czxj0VpV30mczZqvwZIXaO7a70WUM=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=ZBjPN3fefLegW7ceRDyPMJNQqcQTAW6/PDUo7xEVDdXr7U0eKlQYK01MiNsZOy+ZO6iarINcRqQoWn4AEAvF7GHCrNswdjeNz946hBySelZfeKfqFU2xb4wKA+BdqDgFRPoaTfDJXn/uNXeeWXRH/PELIWHN/Ao9ddavYwSrYuQ= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 34BD04CF32BD DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1770650108; 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: in-reply-to:in-reply-to:references:references; bh=q2lQjwid9B+/h/1M+6umttidK8HhG8BrVS4gy5lT/40=; b=BIsIWX/WeGVx6hMMW45QLf1h5p6qgqTl6yQToRLoOKBjI19WaPzWlp1qE0tIpau0FKNV/V ZHRI/61mr8jj0dJDY1B7QJCcCL+rUn0RXX9xSTJE6ENl4LNTXjHA4Xc7+PC8M9rMVtxXUz vqBnRE8Y7G4kaywreHpaDDHeM3ddf5w= Received: from mail-wm1-f70.google.com (mail-wm1-f70.google.com [209.85.128.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-10-nEBim8RjPLaMk8HWdhcfbg-1; Mon, 09 Feb 2026 10:15:07 -0500 X-MC-Unique: nEBim8RjPLaMk8HWdhcfbg-1 X-Mimecast-MFC-AGG-ID: nEBim8RjPLaMk8HWdhcfbg_1770650106 Received: by mail-wm1-f70.google.com with SMTP id 5b1f17b1804b1-47ee432070aso28572105e9.1 for ; Mon, 09 Feb 2026 07:15:07 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770650106; x=1771254906; h=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=q2lQjwid9B+/h/1M+6umttidK8HhG8BrVS4gy5lT/40=; b=XcT6BJtULegtitUGf3OBHGRwA6+bAqtqi9lKpdtZDh5P9RFTaNOp8s3OwwTh7C0VNp yyc7RIlyrpVA9Ec30pajXhLn2liQHkdA09YMjVIBwRI6vNW5IfraaROgDDdVXYKdVVaq 2vp9a3+vznl3s87oDm6U7Ek/RpmgWg7falZUi7w9hWXVDnBi5VXaQsDgcxy7QEQ+OS7W Kq540HjA0f57RLr8sYEo4Vt7CNqbKaO7KxvjbI3PXJt+fXxyHyfX7XN+c/8gc/0h4q8Q awSbSfDKbgNmw+Dy90VEpYMl04P0kJsMugMpQX3xntgxwmWVt/fMrwVj7Ns45TEFoZHY tqYA== X-Gm-Message-State: AOJu0Yx57VDphc0mVfVTmyLDfmaiaOn699hxiKFoJiBzkcso4URApy/B F0KIwp/jji8RVSra5SEXno4Lq8kNkGzpNB5Icvx5xzK8ZGJEYj1mkZ7LQSuaXy6nF/kaedH3QHR 6VjBs0ySKQMPTBixr5hdMdzVbECd2GSxP/e6RwcdnXPsL/0IR2IUd2UCbXOuxdlEIcgB47fMPBA u4XCwpgNytvkvVqDMo6OD0tHeZ5BLRCKMd20JDkA2lgWbt4IU= X-Gm-Gg: AZuq6aJJxt+pgV5QLcyxf9U1VkoKSJzk+w1UgPE9ft2SgX5P0SAG6KFpEyo3qT4TJUF VYiXH//moVHp1GxmVctZ31qXFlDVAVrJ5QkJI/NAsENQqE3QXlBScELTHTZG3Aa+XYpdBj9/y4X rB+63UjI5sa9fO8dKhQPelxqffLEzsSAuYwKtzCPInat4b3nzgYrhTi56lRzkeiJXMFCkfb3LwH GC2Bk2Mjsjshpb2i0clh6CJSmMlKXauaJ+/HVBiNe/n00A8fFGsH/lpBmlpYDp954vcVLUWUIdu pX4OsXrE2AfYN6Fir+EGahLy+jdLlSd+H50LpK83uBvviPdLCC2jP7fsuRtp2E75Tu3BnRugr2p PWlvV X-Received: by 2002:a05:600c:3512:b0:459:db7b:988e with SMTP id 5b1f17b1804b1-483201e25c4mr179386965e9.13.1770650105868; Mon, 09 Feb 2026 07:15:05 -0800 (PST) X-Received: by 2002:a05:600c:3512:b0:459:db7b:988e with SMTP id 5b1f17b1804b1-483201e25c4mr179386155e9.13.1770650105079; Mon, 09 Feb 2026 07:15:05 -0800 (PST) Received: from localhost ([31.111.84.232]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-48323c0296dsm256722985e9.1.2026.02.09.07.15.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Feb 2026 07:15:04 -0800 (PST) From: Andrew Burgess To: gdb-patches@sourceware.org Cc: Guinevere Larsen , Fangrui Song Subject: Re: [PATCH v2] gdb: fix filename matching in skiplist_entry::do_skip_gfile_p In-Reply-To: <87ldh8dvvs.fsf@redhat.com> References: <20260203185528.946918-1-guinevere@redhat.com> <87ldh8dvvs.fsf@redhat.com> Date: Mon, 09 Feb 2026 15:15:03 +0000 Message-ID: <871piuc5jc.fsf@redhat.com> MIME-Version: 1.0 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: _ivCvfc769mK9ZMU2_0oeYj7mU1w1gf-8PFrh0QUzjw_1770650106 X-Mimecast-Originator: redhat.com Content-Type: text/plain 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 Andrew Burgess writes: > Guinevere Larsen writes: > >> GDB is incorrectly matching -gfile skip entries. This is a regression >> introduced by the following commit: >> >> commit 02646a4c561ec88491114b87950cbb827c7d614c >> Author: Fangrui Song >> Date: Sun Dec 29 14:57:44 2024 -0800 >> >> skip -gfile: call fnmatch without FNM_FILE_NAME >> >> The author made the reasonable, but unfortunately incorrect, assumption >> that glibc's fnmatch, and consequently gdb_filename_fnmatch, will return >> the integer equivalent of a boolean (that is, 0 if the filenames do *not* >> match, non-zero if they match), but that is incorrect. This made it so >> using `skip -gfile` would skip all functions except the ones that are >> meant to be skipped. This commit fixes that inverted logic. >> >> This incorrect matching masked a different bug, that we were treating >> all user-provided gfiles as absolute paths, and the gdb.base/skip.exp >> test happened to not catch it because the "skip.c" file is different >> enough to exit earlier in the comparisons. This was solved by making it >> so if the gfile is not absolute, the regex "*/" is prepended, allowing >> for any number of directories before the first user-provided one, but >> ensuring that the beginning of the directory must match what the user >> provided. >> >> The limitation of this approach is that, if a user has given the regex >> "a/c*.c", and the inferior contains both "/b/a/code.c" and >> "/c/a/code.c" and is running on the folder /b, functions from both may >> be skipped when it would be reasonable for the user to expect only >> /b/a/code.c to be skipped. This is done anyway because I believe the >> likelihood of something like this actually happening is incredibly low, >> and it was the behavior of GDB before the aforementioned commit so we'll >> just be returning to previous behavior. > > For what it's worth, I'm not convinced by the original patch. Sadly > there are no tests with the original patch, but based on the commit > message I think what they original wanted was, given a directory tree > like: > > /usr/include/first_header.h > /usr/include/dir1/some_header.h > /usr/include/dir2/other_header.h > > They can just: > > (gdb) skip -gfile /usr/include/*.h > > And skip all three headers. > > The problem I have with this is that I can no longer skip just the files > in one directory, so given: > > /foo/file_a.c > /foo/bar/file_b.c > > I can no longer do: > > (gdb) skip -gfile /foo/*.c > > And only skip 'file_a.c', the '*' here will always match both 'file_a.c' > and 'bar/file_b.c', which I think is unfortunate. > > I wonder if a better approach would be to add something like bash's '**' > globstar feature, which matches 0 or more sub-directories. In the > original case the user could then: > > (gdb) skip -gfile /usr/include/**/*.h > > This would match all three headers from the original example. And in > the second example: > > (gdb) skip -gfile /foo/*.c > > Would still only match 'file_a.c' as it currently does. If we wanted to > match both, then: > > (gdb) skip -gfile /foo/**/*.c > > Would do the trick. > > Unfortunately, fnmatch doesn't support '**' matching, so we'd have to > roll our own, but I think it would be worth it. > > Given the above, my vote would be to not merge this fix, but instead > revert commit 02646a4c561ec8849111, then we could make an attempt at > adding a '**' feature. > > We could merge this fix, but doing that would make the changed behaviour > that commit 02646a4c561ec8849111 tried to introduced actually work, at > which point people might start depending on it. Right now > 02646a4c561ec8849111 is broken, so revering it, even though it made it > into GDB 17, should be OK I think. > > Anyway, that's just my thoughts, not sure how others will feel. CC-ing Fangrui Song (original patch author). I'm planning to revert commit: commit 02646a4c561ec88491114b87950cbb827c7d614c Date: Sun Dec 29 14:57:44 2024 -0800 skip -gfile: call fnmatch without FNM_FILE_NAME In the gdb-17-branch and master branch on Wednesday. As the bug the Guinevere mentions, this patch has introduced a regression, and as I believe I have shown, this patch has changed GDB behaviour in a non-backward compatible way. I'll then take a look at implementing support for '**' as mentioned above as I believe this will solve the original issue in a backward compatible way. If any other global maintainer disagrees with this plan, speak out and I'll not go ahead with this. Thanks, Andrew > > Thanks, > Andrew > > > >> >> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33872 >> --- >> gdb/skip.c | 15 ++++++++++++++- >> gdb/testsuite/gdb.base/skip.exp | 15 +++++++++++++++ >> 2 files changed, 29 insertions(+), 1 deletion(-) >> >> diff --git a/gdb/skip.c b/gdb/skip.c >> index 1255c7d2152..5770d6251a8 100644 >> --- a/gdb/skip.c >> +++ b/gdb/skip.c >> @@ -548,7 +548,20 @@ skiplist_entry::do_skip_gfile_p (const symtab_and_line &function_sal) const >> /* Note: symtab_to_fullname caches its result, thus we don't have to. */ >> const char *fullname = symtab_to_fullname (function_sal.symtab); >> >> - result = gdb_filename_fnmatch (m_file.c_str (), fullname, FNM_NOESCAPE); >> + /* If the pattern does not correspond to an absolute path, add a glob >> + that accepts any leading path. While this is not technically >> + correct, it is very likely that a user wished to skip /a/b/code.c and >> + not skip /c/b/code.c, and both files are relevant to the project. >> + Plus, this is how GDB has always worked, so users are likely expecting >> + that behavior. */ >> + std::string glob = "*/"; >> + if (IS_ABSOLUTE_PATH (m_file.c_str ())) >> + glob = m_file; >> + else >> + glob += m_file; >> + >> + result = gdb_filename_fnmatch >> + (glob.c_str (), fullname, FNM_NOESCAPE) == 0; >> } >> >> if (debug_skip) >> diff --git a/gdb/testsuite/gdb.base/skip.exp b/gdb/testsuite/gdb.base/skip.exp >> index 4541e61ec00..7cec1fc7021 100644 >> --- a/gdb/testsuite/gdb.base/skip.exp >> +++ b/gdb/testsuite/gdb.base/skip.exp >> @@ -336,3 +336,18 @@ with_test_prefix "skip delete completion" { >> test_gdb_complete_none "skip delete a1" >> test_gdb_complete_none "skip delete 2-33" >> } >> + >> +# PR gdb/33872 was a regression that would happen when using >> +# skip -gfile and a full path had to be matched. The logic had >> +# been inverted and it wold skip all files but the one with the >> +# one we wanted to skip >> +with_test_prefix "PR gdb/33872" { >> + gdb_test "skip -gfi /unused/path/*" \ >> + "File\\(s\\) /unused/path/\\* will be skipped when stepping\." >> + >> + if {![runto_main]} { >> + return >> + } >> + >> + gdb_test "step" "bar \\\(\\\) at .*" >> +} >> >> base-commit: a6d46a04ea3e165c0ce2f11cd9d68bdcb81f4e4e >> -- >> 2.52.0