From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) by sourceware.org (Postfix) with ESMTPS id 7DEE13858D1E for ; Mon, 6 Feb 2023 19:54:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7DEE13858D1E 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-f54.google.com with SMTP id d14so11482941wrr.9 for ; Mon, 06 Feb 2023 11:54:22 -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:cc:to:subject :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=v6JwXw4TFrrtkpzKow+l2ieE0qnhHsDxdyejCfPrPcU=; b=5vWQ5Z7nfXV3vOozIZEZPZLTG8j5LjxAHPUrk0JdGTRI+gFVUPk9/cBvCyATe5MHE7 sw4aMTOz2CXRCQbOH61gj6xavJvqWbk8jMNagsyH6GiV7fPOU4Yi7AJcSpH3OSqTKQ3Z G+SsyfI9hvOg+caHTlEkSOGxP4J2yKdk5DHtIerHyKgQ3BzvgnMhep5CN56Q55Aq6EGN TVZn5Uv9rxQeX1v1WNACEtcV6tiBD3M2crf9Eu6n7FBFvAvgExiYrS7HJOut3jJFtOKx s6Hy5iEauklJVvSpS5rjFQbR6PXo5fq+v7Mi+9NqkVVdVxMgb7hSxMHvQDgXCfLA/56r VmXA== X-Gm-Message-State: AO0yUKXJVb3c02kWWcRTFoy6l8cEuTR6uk+jJhZB892Ho8M6KfNotNon i4sdQv1zIi3UvS3FTwS8BmCMa0BFol3fcA== X-Google-Smtp-Source: AK7set/SKLQXyysgeseBkWsxJuojEale9+aTC9evfG61JkHYs5YQNncaZTzdziO5Tsijmm9tJVRrmg== X-Received: by 2002:adf:e786:0:b0:2c3:e4bf:3bfa with SMTP id n6-20020adfe786000000b002c3e4bf3bfamr567983wrm.39.1675713261328; Mon, 06 Feb 2023 11:54:21 -0800 (PST) Received: from ?IPv6:2001:8a0:f92b:9e00::1fe? ([2001:8a0:f92b:9e00::1fe]) by smtp.gmail.com with ESMTPSA id e23-20020a5d5957000000b002366e3f1497sm9583907wri.6.2023.02.06.11.54.20 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 06 Feb 2023 11:54:21 -0800 (PST) Subject: Re: [PATCH v3 2/8] gdbserver: Add PID parameter to linux_get_auxv and linux_get_hwcap To: Thiago Jung Bauermann , gdb-patches@sourceware.org Cc: Simon Marchi References: <20230130044518.3322695-1-thiago.bauermann@linaro.org> <20230130044518.3322695-3-thiago.bauermann@linaro.org> From: Pedro Alves Message-ID: Date: Mon, 6 Feb 2023 19:54:19 +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: <20230130044518.3322695-3-thiago.bauermann@linaro.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-10.7 required=5.0 tests=BAYES_00,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM,GIT_PATCH_0,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=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 2023-01-30 4:45 a.m., Thiago Jung Bauermann via Gdb-patches wrote: > This patch doesn't change gdbserver behaviour, That's not actually true -- before the patch, we use the lwpid of the current thread, while after the patch we always use the pid of the process. Here: > int > -linux_process_target::read_auxv (CORE_ADDR offset, unsigned char *myaddr, > - unsigned int len) > +linux_process_target::read_auxv (int pid, CORE_ADDR offset, > + unsigned char *myaddr, unsigned int len) > { > char filename[PATH_MAX]; > int fd, n; > - int pid = lwpid_of (current_thread); > > xsnprintf (filename, sizeof filename, "/proc/%d/auxv", pid); > AFAICT by playing with debugging the gdb.threads/leader-exit.exp program, you can't read /proc/PID/auxv if the leader thread has exited (is zombie). Maybe that ends up not mattering in practice, not sure, but it is a behavior change. > but after later changes are > made it avoids a null pointer dereference when HWCAP needs to be obtained > for a specific process while current_thread is nullptr. > > Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a PID > parameter seems more correct than setting current_thread in one particular > code path. > > Changes are propagated to allow passing the new parameter through the call > chain. > > Approved-By: Simon Marchi > --- > gdbserver/linux-aarch64-low.cc | 7 ++++--- > gdbserver/linux-arm-low.cc | 2 +- > gdbserver/linux-low.cc | 18 +++++++++--------- > gdbserver/linux-low.h | 9 ++++----- > gdbserver/linux-ppc-low.cc | 6 +++--- > gdbserver/linux-s390-low.cc | 2 +- > gdbserver/netbsd-low.cc | 4 +--- > gdbserver/netbsd-low.h | 2 +- > gdbserver/server.cc | 3 ++- > gdbserver/target.cc | 4 ++-- > gdbserver/target.h | 4 ++-- > 11 files changed, 30 insertions(+), 31 deletions(-) > > diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-low.cc > index 3c09e086afee..2ed6e95562c5 100644 > --- a/gdbserver/linux-aarch64-low.cc > +++ b/gdbserver/linux-aarch64-low.cc > @@ -846,12 +846,13 @@ aarch64_target::low_arch_setup () > if (is_elf64) > { > struct aarch64_features features; > + int pid = current_thread->id.pid (); You can also write: current_process ()->pid which is a more to the point. Also, sometimes there's a current process, but not a current thread, so when we don't actually need a thread, current_process() is better. Pedro Alves