From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-yw1-x1131.google.com (mail-yw1-x1131.google.com [IPv6:2607:f8b0:4864:20::1131]) by sourceware.org (Postfix) with ESMTPS id 372063854164 for ; Thu, 6 Oct 2022 10:37:41 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 372063854164 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=vrull.eu Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=vrull.eu Received: by mail-yw1-x1131.google.com with SMTP id 00721157ae682-358bf076f1fso13705607b3.9 for ; Thu, 06 Oct 2022 03:37:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=vrull.eu; s=google; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :from:to:cc:subject:date; bh=0lvepRsJra9BUsOQ5QAtSuD70Pi6pGQ77crk1O0NJvY=; b=cL3vzHapbsYYR34+d/KoLzep8o5hgNFWXNHaNS9U0kub/AKtAbtfL8GfU6qI1sjeOI /tpVKWZfKFq1QWIHKAVwQi/MQFAhxRUXbNEVtHgHAfsMMrkNV4hONp0JdPB18kwkjACs 3EDdXrepj1Z31bHXofz5wbu/GdbWdESq3M6nNCiFH7UKGTh9ighWlP3pG39LJgHMP9YE 4xLdkWfAsJOYBjPD25MxhFh4LO+FpYGAMsXJJhqUH3LhoJZd4o0gaLi6PmTBKSJ1uo+v tS+iRWQDRc7lHvBT9yXyWMxGBuhyKnv9WZcCzZtVy1DzWHW72gQWaFaSdpzHrah8wwRT oP2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:subject:message-id:date:from:in-reply-to:references:mime-version :x-gm-message-state:from:to:cc:subject:date; bh=0lvepRsJra9BUsOQ5QAtSuD70Pi6pGQ77crk1O0NJvY=; b=PNSLoHog6rMTFRGbZupdtsuJp8pHpTnGouz18icV/RpCLDVZ631RSVyYO7MWXuwvou EzQSNsar04YO69kyMCl+IS9DlLitRk239GRR0uyawbgMqpZfq+gmr9C3ApjxdELTU53+ ZCOBKT/IJ8ehU/tZdmNxV8GGSv/G07uCgK6FZvhsb5nPYm5x3uSfrekPyCXQnZK9Pxaf G+c/C1bvpRKylGer8kB9Bk+HfP6VJfAS2wH/QCqA4W1ydnDLztY5ktugA+eGXxwlwh4g nRRNQ7ymM5rpgMxldjwXhgTdmQXAPi7yBID6ewCUxJhjyM2X3ELlwc9Cwvum4VQlU4Jr 6Ewg== X-Gm-Message-State: ACrzQf2AdZttX8Iqu/Oun2heJxfCojATi2jtdBEV7bQn0HfX9fxW/jK8 inRH3okd97KkTpPZ7uvpBdxiYQ77yg3BiOBsYUQYMAU0JYi2fg== X-Google-Smtp-Source: AMsMyM4Trn3SKTeyJ6vO7331NFE26dbCiLQQpIcgTI4GuxNBtX2wDVw4r3xM1K4xcJujg4ad0GL5bN3f+iYgpRr0Suc= X-Received: by 2002:a81:1908:0:b0:33c:7394:9ee1 with SMTP id 8-20020a811908000000b0033c73949ee1mr3859957ywz.408.1665052660455; Thu, 06 Oct 2022 03:37:40 -0700 (PDT) MIME-Version: 1.0 References: <20221006092840.607374-1-philipp.tomsich@vrull.eu> In-Reply-To: From: Philipp Tomsich Date: Thu, 6 Oct 2022 12:37:29 +0200 Message-ID: Subject: Re: [PATCH v2] aarch64: fix off-by-one in reading cpuinfo To: Philipp Tomsich , gcc-patches@gcc.gnu.org, Tamar Christina , Christoph Muellner , richard.sandiford@arm.com Content-Type: multipart/alternative; boundary="0000000000005bf1ce05ea5b4801" X-Spam-Status: No, score=-8.5 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,HTML_MESSAGE,JMQ_SPF_NEUTRAL,KAM_SHORT,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: --0000000000005bf1ce05ea5b4801 Content-Type: text/plain; charset="UTF-8" On Thu, 6 Oct 2022 at 12:06, Richard Sandiford wrote: > Philipp Tomsich writes: > > Fixes: 341573406b39 > > > > Don't subtract one from the result of strnlen() when trying to point > > to the first character after the current string. This issue would > > cause individual characters (where the 128 byte buffers are stitched > > together) to be lost. > > > > gcc/ChangeLog: > > > > * config/aarch64/driver-aarch64.cc (readline): Fix off-by-one. > > > > gcc/testsuite/ChangeLog: > > > > * gcc.target/aarch64/cpunative/info_18: New test. > > * gcc.target/aarch64/cpunative/native_cpu_18.c: New test. > > > > Signed-off-by: Philipp Tomsich > > > > --- > > > > Changes in v2: > > - Add a a regression test (as per review comment). > > > > gcc/config/aarch64/driver-aarch64.cc | 4 ++-- > > .../gcc.target/aarch64/cpunative/info_18 | 8 ++++++++ > > .../gcc.target/aarch64/cpunative/native_cpu_18.c | 15 +++++++++++++++ > > 3 files changed, 25 insertions(+), 2 deletions(-) > > create mode 100644 gcc/testsuite/gcc.target/aarch64/cpunative/info_18 > > create mode 100644 > gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_18.c > > > > diff --git a/gcc/config/aarch64/driver-aarch64.cc > b/gcc/config/aarch64/driver-aarch64.cc > > index 52ff537908e..48250e68034 100644 > > --- a/gcc/config/aarch64/driver-aarch64.cc > > +++ b/gcc/config/aarch64/driver-aarch64.cc > > @@ -203,9 +203,9 @@ readline (FILE *f) > > return std::string (); > > /* If we're not at the end of the line then override the > > \0 added by fgets. */ > > - last = strnlen (buf, size) - 1; > > + last = strnlen (buf, size); > > } > > - while (!feof (f) && buf[last] != '\n'); > > + while (!feof (f) && (last > 0 && buf[last - 1] != '\n')); > > Very minor, but: I think the normal GCC style would be to avoid the > extra (...). > > OK with that change, thanks. OK for backports too after a settling period. > Applied to master (with that change). Thanks! I'll backport around the end of this month, if no new issues are caused by this change. Philipp. > > Richard > > > > > std::string result (buf); > > free (buf); > > diff --git a/gcc/testsuite/gcc.target/aarch64/cpunative/info_18 > b/gcc/testsuite/gcc.target/aarch64/cpunative/info_18 > > new file mode 100644 > > index 00000000000..25061a4abe8 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/aarch64/cpunative/info_18 > > @@ -0,0 +1,8 @@ > > +processor : 0 > > +BogoMIPS : 2000.00 > > +Features : fp asimd evtstrm aes pmull sha1 sha2 crc32 atomics fphp > asimdhp cpuid asimdrdm jscvt fcma lrcpc dcpop sha3 asimddp sha512 asimdfhm > dit uscat ilrcpc flagm ssbs sb dcpodp flagm2 frint i8mm bf16 rng ecv > > +CPU implementer : 0xc0 > > +CPU architecture: 8 > > +CPU variant : 0x0 > > +CPU part : 0xac3 > > +CPU revision : 0 > > diff --git a/gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_18.c > b/gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_18.c > > new file mode 100644 > > index 00000000000..b5f0a3005f5 > > --- /dev/null > > +++ b/gcc/testsuite/gcc.target/aarch64/cpunative/native_cpu_18.c > > @@ -0,0 +1,15 @@ > > +/* { dg-do compile { target { { aarch64*-*-linux*} && native } } } */ > > +/* { dg-set-compiler-env-var GCC_CPUINFO > "$srcdir/gcc.target/aarch64/cpunative/info_18" } */ > > +/* { dg-additional-options "-mcpu=native" } */ > > + > > +int main() > > +{ > > + return 0; > > +} > > + > > +/* { dg-final { scan-assembler {\.arch > armv8.6-a\+crc\+fp16\+aes\+sha3\+rng} } } */ > > + > > +/* Test one where the boundary of buffer size would overwrite the last > > + character read when stitching the fgets-calls together. With the > > + test data provided, this would truncate the 'sha512' into 'ha512' > > + (dropping the 'sha3' feature). */ > --0000000000005bf1ce05ea5b4801--