* [PATCH v2] aarch64: fix off-by-one in reading cpuinfo
@ 2022-10-06 9:28 Philipp Tomsich
2022-10-06 10:05 ` Richard Sandiford
0 siblings, 1 reply; 3+ messages in thread
From: Philipp Tomsich @ 2022-10-06 9:28 UTC (permalink / raw)
To: gcc-patches
Cc: Tamar Christina, Christoph Muellner, Richard Sandiford, Philipp Tomsich
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 <philipp.tomsich@vrull.eu>
---
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'));
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). */
--
2.34.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] aarch64: fix off-by-one in reading cpuinfo
2022-10-06 9:28 [PATCH v2] aarch64: fix off-by-one in reading cpuinfo Philipp Tomsich
@ 2022-10-06 10:05 ` Richard Sandiford
2022-10-06 10:37 ` Philipp Tomsich
0 siblings, 1 reply; 3+ messages in thread
From: Richard Sandiford @ 2022-10-06 10:05 UTC (permalink / raw)
To: Philipp Tomsich; +Cc: gcc-patches, Tamar Christina, Christoph Muellner
Philipp Tomsich <philipp.tomsich@vrull.eu> 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 <philipp.tomsich@vrull.eu>
>
> ---
>
> 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.
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). */
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] aarch64: fix off-by-one in reading cpuinfo
2022-10-06 10:05 ` Richard Sandiford
@ 2022-10-06 10:37 ` Philipp Tomsich
0 siblings, 0 replies; 3+ messages in thread
From: Philipp Tomsich @ 2022-10-06 10:37 UTC (permalink / raw)
To: Philipp Tomsich, gcc-patches, Tamar Christina,
Christoph Muellner, richard.sandiford
[-- Attachment #1: Type: text/plain, Size: 3906 bytes --]
On Thu, 6 Oct 2022 at 12:06, Richard Sandiford <richard.sandiford@arm.com>
wrote:
> Philipp Tomsich <philipp.tomsich@vrull.eu> 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 <philipp.tomsich@vrull.eu>
> >
> > ---
> >
> > 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). */
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-10-06 10:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-10-06 9:28 [PATCH v2] aarch64: fix off-by-one in reading cpuinfo Philipp Tomsich
2022-10-06 10:05 ` Richard Sandiford
2022-10-06 10:37 ` Philipp Tomsich
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).