From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f50.google.com (mail-wr1-f50.google.com [209.85.221.50]) by sourceware.org (Postfix) with ESMTPS id 417913858D33 for ; Wed, 22 Feb 2023 15:20:59 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 417913858D33 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f50.google.com with SMTP id 6so7705370wrb.11 for ; Wed, 22 Feb 2023 07:20:59 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:content-language:in-reply-to:mime-version :user-agent:date:message-id:from:references:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=5BbEbVYtWeTFvDVQfmOf/jgEMwyyTnnGJoJ0G9eIUrg=; b=lycZSlIQugOPbBdHzm5zHfKBa78oX0zhylcBJe10xkM5p+AAL3EwZdvLHyzfFxPAqa o1PW0rZBc0H7IqX5eEEnWNXAwoeZrCX0ilzwLE8LM6Pqej2HXwYkHyZvfLnCMS1MUt7B L2fRj+RBVz6pIdqxLTYzYNdxtdQZHdzAnmKsfPJgv4iBw0JzslPNFK0TNW/3xYN48y0J 0BtF/Zl1vrfO/K55/VAXGMqBfQ99vgMw5vMCnMKiyM3J3aF7p4VqDCZLytXjnTQfc6qz SEPgSXyU7gI8b96L6kgXJVtzurV+NMTMoXGfCo8qxnWtMvkP6YXLmrdMqKYdYPDhjjzn 1EJA== X-Gm-Message-State: AO0yUKWSQrXE9Bna634Qc3GKipQYByLo1q9Fqd/eYhS4PVHl48PDJPul mizoYYzUSXtHQG9ncQ+QTccUjYCQnZeikA== X-Google-Smtp-Source: AK7set8kjQmQB953Ycv/ODH5TZIl+Z3b3WTaKxvHltNvcc/UyTeo4hyCspQfTkasOIGFGF1Tw/0HCg== X-Received: by 2002:a5d:6052:0:b0:2bd:f5bd:5482 with SMTP id j18-20020a5d6052000000b002bdf5bd5482mr8941032wrt.28.1677079257852; Wed, 22 Feb 2023 07:20:57 -0800 (PST) Received: from ?IPv6:2001:8a0:f92b:9e00::1fe? ([2001:8a0:f92b:9e00::1fe]) by smtp.gmail.com with ESMTPSA id k17-20020a5d6291000000b002c70c99db74sm1628237wru.86.2023.02.22.07.20.57 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 22 Feb 2023 07:20:57 -0800 (PST) Subject: Re: [PATCH 8/8] gdb: fix mi breakpoint-deleted notifications for thread-specific b/p To: Andrew Burgess , gdb-patches@sourceware.org References: <4737168144b9f25ca181b669f3b5f1b6e7e53b14.1676901929.git.aburgess@redhat.com> From: Pedro Alves Message-ID: <927d43eb-eb59-7512-4924-cd6bde2685fd@palves.net> Date: Wed, 22 Feb 2023 15:20:56 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.1 MIME-Version: 1.0 In-Reply-To: <4737168144b9f25ca181b669f3b5f1b6e7e53b14.1676901929.git.aburgess@redhat.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hi! On 2023-02-20 2:13 p.m., Andrew Burgess via Gdb-patches wrote: > My proposed solution is this; in remove_threaded_breakpoints, instead > of setting the disposition to disp_del_at_next_stop and setting the > number to zero, we now just call delete_breakpoint directly. > > The notification will now be sent out immediately; as soon as the > thread exits. > > As the number has not changed when delete_breakpoint is called, the > notification will have the correct number. > > And as the breakpoint is immediately removed from the breakpoint list, > we no longer need to worry about 'maint info breakpoints' trying to > print the thread-id for an exited thread. > > My only concern is that calling delete_breakpoint directly seems so > obvious that I wonder why the original patch (that added > remove_threaded_breakpoints) didn't take this approach. This code was > added in commit 49fa26b0411d, but the commit message offers no clues > to why this approach was taken, and the original email thread offers > no insights either[2]. There are no test regressions after making > this change, so I'm hopeful that this is going to be fine. > > [2] https://sourceware.org/pipermail/gdb-patches/2013-September/106493.html I can't be absolutely sure there weren't other reasons, but I think that was because back then we couldn't access inferior memory if the current thread was running or had exited, when native debugging on GNU/Linux. One of the original versions of the step-over-thread-exit series had that problem as well, which was what lead to changing gdb to always access memory via /proc/pid/mem instead of ptrace, eliminating that class of problems. > > The Complication > ---------------- > > Of course, it couldn't be that simple. It never is. :-) > + # The output has arrived! Check how we did. There are other bugs > + # that come into play here which change which output we'll see. > + if {$mode eq "separate" && !$is_remote} { > + gdb_assert { $saw_mi_thread_exited && $saw_mi_bp_deleted \ > + && !$saw_cli_thread_exited \ > + && !$saw_cli_bp_deleted } \ > + $gdb_test_name > + } else { > + if { $saw_mi_thread_exited && $saw_mi_bp_deleted \ > + && $saw_cli_thread_exited \ > + && $saw_cli_bp_deleted } { > + kpass "gdb/30129" $gdb_test_name > + } elseif { $saw_mi_thread_exited && $saw_mi_bp_deleted \ > + && !$saw_cli_thread_exited \ > + && $saw_cli_bp_deleted } { > + kfail "gdb/30129" $gdb_test_name > + } else { > + fail "$gdb_test_name (XXX)" I guess you meant to remove the XX or replace it with something else? Otherwise LGTM. Approved-By: Pedro Alves