From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 9EDbD5GQhGkQuikAWB0awg (envelope-from ) for ; Thu, 05 Feb 2026 07:44:01 -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=F8UQ74Pt; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 2DB221E08D; Thu, 05 Feb 2026 07:44:01 -0500 (EST) X-Spam-Checker-Version: SpamAssassin 4.0.1 (2024-03-25) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-2.6 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,RDNS_NONE autolearn=ham autolearn_force=no version=4.0.1 Received: from vm01.sourceware.org (unknown [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 2B8581E08D for ; Thu, 05 Feb 2026 07:44:00 -0500 (EST) Received: from vm01.sourceware.org (localhost [127.0.0.1]) by sourceware.org (Postfix) with ESMTP id A01B44BA2E18 for ; Thu, 5 Feb 2026 12:43:53 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org A01B44BA2E18 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=F8UQ74Pt 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 853064BA2E08 for ; Thu, 5 Feb 2026 12:43:26 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 853064BA2E08 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 853064BA2E08 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=1770295406; cv=none; b=IICLzZA/hevm7TECyo2aPU/kk5Zjt3p2gZAWt6+9euBHd8Bke4OfnPNi3dOTRGnnkq0+Bqk2bgkcjB62DtstYv0+0qPmb17fBydE39MiPYVvglhQcEBc1gXpvXixZSJigwHxZ8e8g8y3DkJ48/lSFyYyJoV/7InMotICk+tZ818= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1770295406; c=relaxed/simple; bh=f3lf8GBv5HJVozzBXbsK2jNiegQNfgtCxm/1u9nhyS4=; h=DKIM-Signature:Message-ID:Date:MIME-Version:Subject:To:From; b=mIEYA0OsJLgA1whIgf4Xtf3xSDZuSgHbrP8eJhpOoQ87vs2dJfZkMvPn0n8J7cobX062gfdeS8fglnnvIwcQA4JZeMu2V9XbX1bDYVDPTd/1D29ufnOcd0KAdgHVJCAA7fN0/b6qbzHPgOQzMem82ENfHyzD2Peao5hQl8WQMV0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 853064BA2E08 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1770295406; 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=2Rf/eVI8FZfC/qTzB6vr5Tb6FIPHIIcXjNXopFg1+LY=; b=F8UQ74Ptd/rX+Sl2cZvbsuwASdGWzoAyxfOPgHR7ANc2C8NOTk0tn9vIHdfqZU0JwCpZ4Q CqwlWzMY+CBvAfpizAxQeY0QF1J1+k1MRYtn9Oa8eiKuYUH6bLoyuw9zcHfnPROZo6QNLB qcKtUK8XWls+x9YmiyksHNi+jrtApYw= Received: from mail-dl1-f71.google.com (mail-dl1-f71.google.com [74.125.82.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-627-2hRtMoEdPkq6xopTWKjbWQ-1; Thu, 05 Feb 2026 07:43:25 -0500 X-MC-Unique: 2hRtMoEdPkq6xopTWKjbWQ-1 X-Mimecast-MFC-AGG-ID: 2hRtMoEdPkq6xopTWKjbWQ_1770295404 Received: by mail-dl1-f71.google.com with SMTP id a92af1059eb24-11b94abc09dso1154550c88.1 for ; Thu, 05 Feb 2026 04:43:24 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1770295404; x=1770900204; 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=2Rf/eVI8FZfC/qTzB6vr5Tb6FIPHIIcXjNXopFg1+LY=; b=JK5AuypS+ffqKQnUgT9lht10rXFRvniedTF/xmc5XS5k0Cn2T8Hr20y8pgY2hNBfQX YAYMI0lZ2/U9vSIknjkoec2M4g13Bcy/u7YsNNDak0QiT5KJC/35aeEAZMd0V3YjF76r L3oEP25K4UP/Zwh6RFFpXx1KQjzNB/40faKtukRfehgFfN0CCJjKE6ZdfxTeO6Dps96o qERJH7uk3pq4jbpA/qJe4iLftWlGRHq9UtF3Gz0Z/F0XiQt7jvUVhT13NlAYivV6jg7e lQmQQdNiPySj5Kz6WYrNSYf1gCcoZgWmBAHi2JGpycyGLw/6hB1izMRj6ftC7xVtzrIo 0DUg== X-Forwarded-Encrypted: i=1; AJvYcCVmLGRL4cWk9SL81MmcFJ2lY42Dc0zKr7pxB8XIM1IoJTapT4q28hpapIWv0svEiNyn9UM+vstZDlGCbQ==@sourceware.org X-Gm-Message-State: AOJu0Ywgm5BfhySrnovX3PDYQ9HgJNLreP05yYeK5sZvjU2Mh97em84i eAeodxqo0IuM6iT2ORzxzw9K+ZPzpSddttIGSKXZF//y1iKdt/HPkZ+2pwF9sDRqt8F9NvLpj/A 6DvaZ9djhOiGqcZzUmWcYhUChmtiYMBnv3RyIqlpUwohrqk2fjMIzYrQ6Z9c7cb4= X-Gm-Gg: AZuq6aK3EJ2elnheCj7O7frc3av2qZHjY5EAC0j3sd3HRHBAJrUwHdpfXCabppOeIZT kXRoqIQnWawITZ0xZxwqmPB15YURqb65KTa59TQ41dKMOq9E6FMDJDCjc6WmRJYYcla9PEjU0k5 Gajh67kEe2Av5c/lACzjd75k1M3TiOgExpSyjjT17XRAbSuSgHPLQItwHMnFI8l4SPK52krkPf+ ssU5M3Fmcc791gP4bTp0d8qVaqo44hh44RaE/BeJD5KyNSpr0L95aCMG+KZd6ngOcFFUuki43ss 1Lz2GMow2DUb7xYkABD4OduSF0B9PmeLAk9LSLNW9Ej7iPU00q5EP9U1c66KkUGSEHzC8iWT/g5 TlNxQAa/BvNVNZY64STge6g== X-Received: by 2002:a05:7022:426:b0:121:72bb:3cd7 with SMTP id a92af1059eb24-126f4771ce0mr2463994c88.7.1770295403739; Thu, 05 Feb 2026 04:43:23 -0800 (PST) X-Received: by 2002:a05:7022:426:b0:121:72bb:3cd7 with SMTP id a92af1059eb24-126f4771ce0mr2463980c88.7.1770295403151; Thu, 05 Feb 2026 04:43:23 -0800 (PST) Received: from ?IPV6:2804:14d:8084:a5b1::1000? ([2804:14d:8084:a5b1::1000]) by smtp.gmail.com with ESMTPSA id a92af1059eb24-126f4e0f98asm4563846c88.5.2026.02.05.04.43.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 05 Feb 2026 04:43:22 -0800 (PST) Message-ID: <07a6ac05-0f88-4d4c-8d0a-d664d55e1733@redhat.com> Date: Thu, 5 Feb 2026 09:43:20 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] gdb: fix filename matching in skiplist_entry::do_skip_gfile_p To: Andrew Burgess , gdb-patches@sourceware.org References: <20260203185528.946918-1-guinevere@redhat.com> <87ldh8dvvs.fsf@redhat.com> From: Guinevere Larsen In-Reply-To: <87ldh8dvvs.fsf@redhat.com> X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: VDj6gOLGJWh7El9JkM5r8ld-HDBZbRqCpmL-4SfLoJo_1770295404 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 2/4/26 6:47 PM, Andrew Burgess wrote: > 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. I decided to look for the original patch thread, to ping the relevant people and get their opinions, but here it is: https://inbox.sourceware.org/gdb-patches/20241230185639.370189-1-maskray@sourceware.org/ It seems like the patch was never fully approved, Eli just approved the documentation changes and the contributor must have misunderstood that for a full approval... Making the case for reverting 02646a4c561ec8849111 even stronger -- Cheers, Guinevere Larsen It/she > > 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