From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-oi1-x22f.google.com (mail-oi1-x22f.google.com [IPv6:2607:f8b0:4864:20::22f]) by sourceware.org (Postfix) with ESMTPS id 53EF7385B182 for ; Tue, 29 Nov 2022 03:17:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 53EF7385B182 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-oi1-x22f.google.com with SMTP id r76so13820605oie.13 for ; Mon, 28 Nov 2022 19:17:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=content-transfer-encoding:mime-version:message-id:date:in-reply-to :subject:cc:to:from:user-agent:references:from:to:cc:subject:date :message-id:reply-to; bh=R2j9h5fnY0syBj/LRgu41EMdNoks3emc44ImbNvfJdk=; b=Z+a1t+MYUjryaIxrh5tRHyrVDD3KmlK2xhmEEn1wi9FCSDmsvWqwakU7sBzSddEnsl T5UShnxJws8SnCN15wLbsWGQM0jVyYczwAWL7RAd/dHynmI/mnPwqiXj5OzYBUqEMjGX jTL8tQQlobO7ULKldubiCKrL0uJjUhSZH4172EDrMzZDn57wcIqjetdio7T7zAM5zTA9 kg8udx9hzUJ5a2iX+lDxrNSLXOCvagXxKiqURvOWtFWuOC3InhTniMgHIL4kMJFUbAk3 8KM2h6GC1oCTf+pLR0SgCqOTlBjp/08MHr+Fv186cIceRB9hmywByk3wRGzAKhApyWmu WtSQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:message-id:date:in-reply-to :subject:cc:to:from:user-agent:references:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=R2j9h5fnY0syBj/LRgu41EMdNoks3emc44ImbNvfJdk=; b=p8L0Sy16+vQ+1f4xB8z+A2HYlEl2+ghsPy4avHTfYBQfphdVWk2vZNEGqTtXGtvox1 BZLN+Xr7sgmyBX3NEOxGrAfIti7iEd6qTaaTx2DAw6b9FmsNPj8UQivYQWREa6Z7SdaH TBFQHxFaWxbVSdiiae5pp7IFq9euyU3yMj0KGmn7Ao1z4dHKY6ytg5J3BjQfwQLkuodE xUystbVDvFABceKc/Tq5djJu4rXSFi+v22AUlPh4CORIYj2o3IU2z8udBm7Glo0lyRNT SBvG4j1uAZmwvtKgOl7Y9aQ4zxRmSHK0t6g9ehBLyX7M9q5ubiM2Tt0nPrJgfvZQk/Xh c+OA== X-Gm-Message-State: ANoB5pm7YZfVlFg42JX9dAhoODqUYGEEtszup5HpdpVnhIhbwXhKGrtQ CA4xKWQCdMJkFCsMFUfriAdt9Q== X-Google-Smtp-Source: AA0mqf69+1NIxrDnx2UbRKLuSW6T3Q6Ma+lV4YdZgB4z8YSJswotELKPtiet+9eW1Mha/xnn6vtdHQ== X-Received: by 2002:aca:5dc4:0:b0:35a:6b1a:531 with SMTP id r187-20020aca5dc4000000b0035a6b1a0531mr25727788oib.153.1669691829653; Mon, 28 Nov 2022 19:17:09 -0800 (PST) Received: from localhost ([2804:14d:7e39:8470:201c:ea26:41ce:5eb]) by smtp.gmail.com with ESMTPSA id r75-20020acaa84e000000b00354b0850fb6sm5069802oie.33.2022.11.28.19.17.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Nov 2022 19:17:09 -0800 (PST) References: <20221126020452.1686509-1-thiago.bauermann@linaro.org> <20221126020452.1686509-3-thiago.bauermann@linaro.org> <43979fb5-a40b-9dd8-f4e6-bde0a598e505@simark.ca> User-agent: mu4e 1.8.11; emacs 28.2 From: Thiago Jung Bauermann To: Simon Marchi Cc: gdb-patches@sourceware.org, Luis Machado Subject: Re: [PATCH v2 2/6] gdbserver: Add PID parameter to linux_get_auxv and linux_get_hwcap In-reply-to: <43979fb5-a40b-9dd8-f4e6-bde0a598e505@simark.ca> Date: Tue, 29 Nov 2022 03:17:06 +0000 Message-ID: <87y1ruh2kt.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-11.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,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: Simon Marchi writes: > On 11/25/22 21:04, Thiago Jung Bauermann via Gdb-patches wrote: >> This patch doesn't change gdbserver behaviour, but after later changes a= re >> made it avoids a null pointer dereference when HWCAP needs to be obtained >> for a specific process while current_thread is nullptr. >>=20 >> Fixing linux_read_auxv, linux_get_hwcap and linux_get_hwcap2 to take a P= ID >> parameter seems more correct than setting current_thread in one particul= ar >> code path. >>=20 >> Changes are propagated to allow passing the new parameter through the ca= ll >> chain. > > Thanks, this is ok, with the following nits fixed: > > Approved-By: Simon Marchi > > ... unless Luis is strongly against it. Thanks! v3 will have the nits fixed and also change uses of =E2=80=9Cpid_of (thread= )=E2=80=9D to =E2=80=9Cthread->id.pid ()=E2=80=9D. >> --- >> 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 | 2 +- >> gdbserver/target.cc | 4 ++-- >> gdbserver/target.h | 2 +- >> 11 files changed, 28 insertions(+), 30 deletions(-) >>=20 >> diff --git a/gdbserver/linux-aarch64-low.cc b/gdbserver/linux-aarch64-lo= w.cc >> index db5086962612..a6ed68f93029 100644 >> --- a/gdbserver/linux-aarch64-low.cc >> +++ b/gdbserver/linux-aarch64-low.cc >> @@ -823,12 +823,13 @@ aarch64_target::low_arch_setup () >> if (is_elf64) >> { >> struct aarch64_features features; >> + int pid =3D pid_of (current_thread); >>=20=20 >> features.vq =3D aarch64_sve_get_vq (tid); >> /* A-profile PAC is 64-bit only. */ >> - features.pauth =3D linux_get_hwcap (8) & AARCH64_HWCAP_PACA; >> + features.pauth =3D (linux_get_hwcap (pid, 8) & AARCH64_HWCAP_PACA= ); > > Avoid adding the outer parenthesis. Ah, that's a remnant from an older version of the code where this line was longer and I added the parentheses to fix indentation. Fixed for v3. >> diff --git a/gdbserver/target.h b/gdbserver/target.h >> index 18ab969dda70..fe7a80b645bc 100644 >> --- a/gdbserver/target.h >> +++ b/gdbserver/target.h >> @@ -175,7 +175,7 @@ class process_stratum_target >> /* Read auxiliary vector data from the inferior process. >>=20=20 >> Read LEN bytes at OFFSET into a buffer at MYADDR. */ >> - virtual int read_auxv (CORE_ADDR offset, unsigned char *myaddr, >> + virtual int read_auxv (int pid, CORE_ADDR offset, unsigned char *myad= dr, > > Please update the doc of the method to replace "from the inferior > process" to from "process with pid PID". Indeed, thanks for spotting it. Fixed for v3. --=20 Thiago