From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f44.google.com (mail-wm1-f44.google.com [209.85.128.44]) by sourceware.org (Postfix) with ESMTPS id 7839B385840F for ; Mon, 13 Nov 2023 14:05:44 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7839B385840F Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7839B385840F Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.44 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699884354; cv=none; b=KaCmeSrcNlju2BYDPwmd5TSnrpnmPpWuqdl9eR5zQUlE2E3p+cxOGa1+RubtwG+kCO3IfTVPReQkt+5gOOfvMifwSxJ/WU1fAcfkg6zoqTEQVz6M7NKTJBMvJUNo6+Manb3wYNi9V1WOBXTi/0YhZHyRpsuyd01/DGL0dXg3LjQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1699884354; c=relaxed/simple; bh=aNS/hclfijj5VhXlpbxkd49WH2Qf3BNiBHu4DfTSf9k=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=ACa91suzmOm28Rl19moBTVxh5bfVpychg/4JCqPJ+fkswYLMfsc/mnEQFefbEtzKoDnRB4ahsFvW0hlPygPdrDof06LWPjEioNONfeFcffM2Z2qGaPCBP0b/+CgnreHBI9jJsuE8EA6BaalnG1wJadZwRwdJ+wLDD3G4zCERNwQ= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-f44.google.com with SMTP id 5b1f17b1804b1-408382da7f0so37087705e9.0 for ; Mon, 13 Nov 2023 06:05:44 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1699884343; x=1700489143; h=content-transfer-encoding:in-reply-to:from:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=zJbHP7ASlLBswKRzNb4AFD+OjHgEs3ciwlIqWYqOE78=; b=uHpIj9v8JZrtgIpVjNnQXA828A2jGDtW23tDleFROL3OIZcyVGVSqHSP/ZOqgJyZ4F TNT14sDA+RRuSzPaKtwuwiIQ1niPgtL6WAhhSwIopvAsJakui2g/+LBpJuwsNU1jyznn 4yoNewgc9HAniqAI8AC1GemscGGDXYgLv7/fI8tsy45ZWSVnWvktgTPtQnMcmDDJ0LwH AINm/JQq8Sc8Y2hk4u2HFbkKmrKexNsYUtjcyQp6KXPKocgmKvctMaE+o/Ax0cuyRqmi q4l2aRFp09EUvW4pr2kLDBeC67Ds0kIk8urHQqi2kspufT4gDwJ68SSZTIIJJrh9ZcLl e93A== X-Gm-Message-State: AOJu0Yz8ibd6KGuNUp4rDZ+uxcto+E6Q66BIx8W7QYOByJ7WRtkPiW2U xfQ2tQhbB6lxmUgQc8Q2DlBLcWgox4M= X-Google-Smtp-Source: AGHT+IEf1h61t9/pGZSHct46KwSzTK9KzX2Wll4+q+s57A8j42lrPVfbF8VCLl4wQrwD1SpyolLMLg== X-Received: by 2002:a05:6000:401e:b0:323:2563:be13 with SMTP id cp30-20020a056000401e00b003232563be13mr4148580wrb.52.1699884343142; Mon, 13 Nov 2023 06:05:43 -0800 (PST) Received: from ?IPV6:2001:8a0:f91e:1a00:8060:1e54:fb28:9635? ([2001:8a0:f91e:1a00:8060:1e54:fb28:9635]) by smtp.gmail.com with ESMTPSA id w7-20020adff9c7000000b0032fbd0c7d04sm5525593wrr.55.2023.11.13.06.05.42 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 13 Nov 2023 06:05:42 -0800 (PST) Message-ID: <26bf07ae-ae1a-40b8-88f8-a4b147416d67@palves.net> Date: Mon, 13 Nov 2023 14:05:42 +0000 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH 04/31] Step over clone syscall w/ breakpoint, TARGET_WAITKIND_THREAD_CLONED Content-Language: en-US To: Andrew Burgess , gdb-patches@sourceware.org References: <20221212203101.1034916-1-pedro@palves.net> <20221212203101.1034916-5-pedro@palves.net> <87bkm9qw4i.fsf@redhat.com> <87fs9yccni.fsf@redhat.com> From: Pedro Alves In-Reply-To: <87fs9yccni.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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: On 2023-03-21 16:06, Andrew Burgess wrote: > Pedro Alves writes: > >> On 2023-02-04 3:38 p.m., Andrew Burgess wrote: >> The solution implemented here is to model clone on fork/vfork, and >> introduce a new TARGET_WAITKIND_THREAD_CLONED event. This event is >> similar to TARGET_WAITKIND_FORKED and TARGET_WAITKIND_VFORKED, except >> that we end up with a new thread in the same process, instead of a new >> thread of a new process. Like FORKED and VFORKED, THREAD_CLONED >> waitstatuses have a child_ptid property, and the child is held stopped >> until GDB explicitly resumes it. This addresses the in-line stepping >> case (issues #1 and #2). >> >> The infrun code that handles displaced stepping fixup for the child >> after a fork/vfork event is thus reused for THREAD_CLONE, with some >> minimal conditions added, addressing the displaced stepping case >> (issue #3). >> >> The native Linux backend is adjusted to unconditionally report >> TARGET_WAITKIND_THREAD_CLONED events to the core. >> >> Following the follow_fork model in core GDB, we introduce a >> target_follow_clone target method, which is responsible for making the >> new clone child visible to the rest of GDB. >> >> Subsequent patches will add clone events support to the remote >> protocol and gdbserver. >> >> A testcase will be added by a later patch. > > What's the motivation for splitting the implementation from the > associated tests? It's just that while developing this, I kept reordering patches, while fixing different aspects, different modes (displaced step, in-line step), different targets (native, remote). It was so much easier to keep the testcase in a separate patch, and fix it / adjust it as I went along. Keeping the testcase separate also allowed for keeping you listed as the main git commit author for that patch. I've now merged the testcase into this patch, and added a note giving you credit (along with git tag, of course). The test fails for remote at this point, so I added a "require" call to skip it on anything other than native at this point. > > Though your implementation may be different (better) than mine, the > original test I wrote[1] still passes just fine with this commit. > > When digging back through old patches it's much nicer to easier to find > the GDB changes and the tests in a single commit IMHO. Do feel free to > take the tests from that commit, if that helps at all (though I notice I > left some debug logging in which would need cleaning up, and the test > fails when using a remote target, but that's easily fixed by skipping > the test in that case). > > [1] https://sourceware.org/pipermail/gdb-patches/2021-June/180520.html > I guess you noticed later while reading the rest of the series, but I did already take the tests from that commit, and preserved them as patch #13 in this series: https://inbox.sourceware.org/gdb-patches/8735331856.fsf@redhat.com/ I had kept the "New in vX" notes in there so you could more easily tell what had changed from yours. (I've now dropped them in the new version.) > I've looked through the code and it all looks good to me. > > Reviewed-By: Andrew Burgess > Thanks!