From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-f47.google.com (mail-wm1-f47.google.com [209.85.128.47]) by sourceware.org (Postfix) with ESMTPS id D57CB3858D38 for ; Fri, 12 Apr 2024 17:02:36 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D57CB3858D38 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 D57CB3858D38 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=209.85.128.47 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712941367; cv=none; b=nh5XJexDqTRUiI2ay+WbmVjAkQwtvXPbhwBeoOKSSKeRS8d3RoQX2oCXnBpsbuWZd+0pOprWQSTH6YRa5yCxqVbO6J6MZgd7D3SnQScxYRT5XfDQ6j7OevAEx92JAD1rGS8qezgMvk30HAYrrL10T4gzhvuDHdON6DFUyUlYKc0= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1712941367; c=relaxed/simple; bh=nKMEH4N2ZioXUu+W5dZHRp/P6sdRwdxFnEAZkVPiMjE=; h=Message-ID:Date:MIME-Version:Subject:To:From; b=oZQiYizgVNmW/b5Kfn0afoKsbE2VUqmQHwDzu4sZtYXgxsBeJu+AqzNZxNM0Nj9ozTy7hpLbAEPOec0/tEM4hMifNwNu+3odSmCGhOh16aVhhdmJWnQ5jx16E6v6A+HpAId636ZJ0zsSWXJryERo3h7IALNhM2eBs/APa7ZoWEs= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-wm1-f47.google.com with SMTP id 5b1f17b1804b1-41819c27ea3so482995e9.0 for ; Fri, 12 Apr 2024 10:02:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712941355; x=1713546155; h=content-transfer-encoding:in-reply-to:content-language:from :references:to:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=Nd6C/0QuaYFU5FFLumq0/3ylj3Mryrvw0faAPx4ZxIw=; b=jfeDZLlPCpiCjkQEajs6wmUtEiloNEqTBo2ZMTsapNzgE/G5X2wr2/6+5dGzb/zzvX x4OOzUyWfRYSPGLYpNAOMhxY4sGn9XG+7tvTBd5g5laBrc2s2Kt7Ft2wkiS5V54uFtUg /H7z5IpbUR0YGMNU8voKC8WJobP1LSvhhkYxo19NfWNx9qTnxL5uq7Ci75Ezbu20ZTOB 8AmfBzAtccLjrqH2x/X/Nmi3cZ1Z1+du0FAoakbXbN83kZBogyC7zsmBTztcVvjGTg4l q4L5CC8GKMcMuJQyJdMZp5qnlU4W/MdgAHSnv7vZZ3EjWuiCcwVE74RzNGs5zf/c9ROL nRCw== X-Forwarded-Encrypted: i=1; AJvYcCUFgSFWfhSYqIHQoJD2vLhFI8xWCaQzUfeEsEcP4Z7/oTuS0XEgm4j2eYZNvLS9FoAYXjR7fUYthmgfQvgRfH5jFHLTK5vROH6MbA== X-Gm-Message-State: AOJu0YwWMS7tYrmXKh3JhWVDsrAEWMSNJ8gb93ZQp0dBYL9ek8i0X/aS dxcTtcxlMvvEno1B8dm65HeKW3d7vVUFreOrMzNAWHJv4KDvtB8NsHijyC+J X-Google-Smtp-Source: AGHT+IEDP1jVO4UFHNo2R29agEkMMG4lEA2RMTJrMmASn3F+C6whDYExkTMvDgdinM7o5cWJAcjtwg== X-Received: by 2002:a05:600c:4f0f:b0:414:1351:8662 with SMTP id l15-20020a05600c4f0f00b0041413518662mr2789664wmq.12.1712941355177; Fri, 12 Apr 2024 10:02:35 -0700 (PDT) Received: from ?IPV6:2001:8a0:f93d:b900:ae52:9c19:14d0:4b50? ([2001:8a0:f93d:b900:ae52:9c19:14d0:4b50]) by smtp.gmail.com with ESMTPSA id v13-20020a05600c444d00b0041663450a4asm9343869wmn.45.2024.04.12.10.02.34 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 12 Apr 2024 10:02:34 -0700 (PDT) Message-ID: <2846f8ac-bd84-4641-85ad-c585cd49d37b@palves.net> Date: Fri, 12 Apr 2024 18:02:31 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] gdb/linux-nat: Fix mem access ptrace fallback (PR threads/31579) To: Andrew Burgess , gdb-patches@sourceware.org References: <20240411113941.204268-1-pedro@palves.net> <8734rq5zy2.fsf@redhat.com> From: Pedro Alves Content-Language: en-US In-Reply-To: <8734rq5zy2.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-11.1 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,HEADER_FROM_DIFFERENT_DOMAINS,KAM_DMARC_STATUS,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_PASS,TXREP autolearn=ham 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 2024-04-12 17:05, Andrew Burgess wrote: > Pedro Alves writes: > >> >> diff --git a/gdb/linux-nat.c b/gdb/linux-nat.c >> index 2602e1f240d..d2b9aea724d 100644 >> --- a/gdb/linux-nat.c >> +++ b/gdb/linux-nat.c > >> @@ -2199,6 +2199,21 @@ mark_lwp_dead (lwp_info *lp, int status) >> lp->stopped = 1; >> } >> >> +/* Return true if LP is dead, with a pending exit/signalled event. */ >> + >> +static bool >> +is_lwp_marked_dead (lwp_info *lp) >> +{ >> + switch (lp->waitstatus.kind ()) >> + { >> + case TARGET_WAITKIND_EXITED: >> + case TARGET_WAITKIND_THREAD_EXITED: >> + case TARGET_WAITKIND_SIGNALLED: >> + return true; >> + } >> + return false; >> +} > > I wonder if this would be better as a method on waitstatus? There's at > least one place in infrun.c (in handle_one) where we check for these > three statuses, and there's a few places where we check > TARGET_WAITKIND_EXITED and TARGET_WAITKIND_SIGNALLED ... and I suspect > we either _should_ be checking for TARGET_WAITKIND_THREAD_EXITED, or it > wouldn't matter if we did. > > Not that I'm suggesting you should look at all those ... just I'm > wondering if it would be better to make is_lwp_marked_dead a waitstatus > method now, and we can then clean up other users later? > I've named the function is_lwp_marked_dead because we have the corresponding mark_lwp_dead just above. If we add a method to waitstatus, we'd naturally want to call it something else, like ... I don't know, it's hard, maybe ws.is_exit_like() or some such. I think we'd still want the lwp function wrapping that, as the fact that we mark is in the pending status field is a bit of a lower level implementation detail than the semantics of the is_lwp_marked_dead function. > Consider this an optional suggestion, we can always make this a method > later if needed. Thanks, seems worth it of exploration, but I'll take the option and go with what I have in this patch. > >> + >> /* Wait for LP to stop. Returns the wait status, or 0 if the LWP has >> exited. */ >> >> @@ -3879,6 +3894,20 @@ linux_proc_xfer_memory_partial (int pid, gdb_byte *readbuf, >> const gdb_byte *writebuf, ULONGEST offset, >> LONGEST len, ULONGEST *xfered_len); >> >> +/* Look for an LWP of PID that we know is ptrace-stopped. Returns >> + NULL if none is found. */ > > s/NULL/nullptr/ ? Personally, I prefer writing NULL in comments than nullptr, because saying (I mean, imagine reading the comment aloud) "null ptr" is kind of awkward, in my mind. It see it more as NULL meaning "the null pointer value", i.e., the word "null" uppercased, than literally the NULL macro. I've seen other patches from others using NULL in comments so I thought it was other's preference as well. > > Approved-By: Andrew Burgess Thanks.