[PATCH] RISC-V: Raise error on unexpected ISA string at end.

[PATCH] RISC-V: Raise error on unexpected ISA string at end.

[Maxim Blinov]

This patch adds the same check that is present in binutils/bfd/elfxx-riscv.c.

Checks that we have reached the end of the string after all the
parsing routines have been run. Without this check, GCC silently
succeeds on erroneous input, and produces an assembly with a truncated
arch attribute.

tested with RUNTESTFLAGS="riscv.exp"

Thanks,
Maxim

gcc/ChangeLog:
2019-07-31  Maxim Blinov  <maxim.blinov@embecosm.com>

	* common/config/riscv/riscv-common.c: Check -march string ends
	with null.

gcc/testsuite/ChangeLog:
2019-07-31  Maxim Blinov  <maxim.blinov@embecosm.com>

	* gcc.target/riscv/attribute-10.c: New test.

---
 gcc/common/config/riscv/riscv-common.c        | 7 +++++++
 gcc/testsuite/gcc.target/riscv/attribute-10.c | 6 ++++++
 2 files changed, 13 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/attribute-10.c

diff --git a/gcc/common/config/riscv/riscv-common.c b/gcc/common/config/riscv/riscv-common.c
index eeb75717db0..64a309241da 100644
--- a/gcc/common/config/riscv/riscv-common.c
+++ b/gcc/common/config/riscv/riscv-common.c
@@ -513,6 +513,13 @@ riscv_subset_list::parse (const char *arch, location_t loc)
   if (p == NULL)
     goto fail;
 
+  if (*p != '\0')
+    {
+      error_at (loc, "-march=%s: unexpected ISA string at end: %<%s%>",
+               arch, p);
+      goto fail;
+    }
+
   return subset_list;
 
 fail:
diff --git a/gcc/testsuite/gcc.target/riscv/attribute-10.c b/gcc/testsuite/gcc.target/riscv/attribute-10.c
new file mode 100644
index 00000000000..dd817879a67
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/attribute-10.c
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv32im_s_sx_unexpectedstring -mabi=ilp32" } */
+int foo()
+{
+}
+/* { dg-error "unexpected ISA string at end:" "" { target { "riscv*-*-*" } } 0 } */
-- 
2.20.1

Re: [PATCH] RISC-V: Raise error on unexpected ISA string at end.

[Martin Sebor]

On 7/31/19 5:19 AM, Maxim Blinov wrote:
> This patch adds the same check that is present in binutils/bfd/elfxx-riscv.c.
> 
> Checks that we have reached the end of the string after all the
> parsing routines have been run. Without this check, GCC silently
> succeeds on erroneous input, and produces an assembly with a truncated
> arch attribute.
> 
> tested with RUNTESTFLAGS="riscv.exp"
> 
> Thanks,
> Maxim
> 
> gcc/ChangeLog:
> 2019-07-31  Maxim Blinov  <maxim.blinov@embecosm.com>
> 
> 	* common/config/riscv/riscv-common.c: Check -march string ends
> 	with null.
> 
> gcc/testsuite/ChangeLog:
> 2019-07-31  Maxim Blinov  <maxim.blinov@embecosm.com>
> 
> 	* gcc.target/riscv/attribute-10.c: New test.
> 
> ---
>   gcc/common/config/riscv/riscv-common.c        | 7 +++++++
>   gcc/testsuite/gcc.target/riscv/attribute-10.c | 6 ++++++
>   2 files changed, 13 insertions(+)
>   create mode 100644 gcc/testsuite/gcc.target/riscv/attribute-10.c
> 
> diff --git a/gcc/common/config/riscv/riscv-common.c b/gcc/common/config/riscv/riscv-common.c
> index eeb75717db0..64a309241da 100644
> --- a/gcc/common/config/riscv/riscv-common.c
> +++ b/gcc/common/config/riscv/riscv-common.c
> @@ -513,6 +513,13 @@ riscv_subset_list::parse (const char *arch, location_t loc)
>     if (p == NULL)
>       goto fail;
>   
> +  if (*p != '\0')
> +    {
> +      error_at (loc, "-march=%s: unexpected ISA string at end: %<%s%>",

I would expect the missing quotes around the option to trigger
a -Wformat-diag warning.  The %<%s%s> should also be flagged by
the same warning.  Changing the format string as follows should
avoid the warnings:

   error_at (loc, "%<-march=%s%>: unexpected ISA string at end: %qs"

Thanks
Martin

> +               arch, p);
> +      goto fail;
> +    }
> +
>     return subset_list;
>   
>   fail:
> diff --git a/gcc/testsuite/gcc.target/riscv/attribute-10.c b/gcc/testsuite/gcc.target/riscv/attribute-10.c
> new file mode 100644
> index 00000000000..dd817879a67
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/riscv/attribute-10.c
> @@ -0,0 +1,6 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -march=rv32im_s_sx_unexpectedstring -mabi=ilp32" } */
> +int foo()
> +{
> +}
> +/* { dg-error "unexpected ISA string at end:" "" { target { "riscv*-*-*" } } 0 } */
> 

[PATCH v2] RISC-V: Raise error on unexpected ISA string at end.

[Maxim Blinov]

Hi Martin, thanks for reviewing.

> I would expect the missing quotes around the option to trigger
> a -Wformat-diag warning.  The %<%s%s> should also be flagged by
> the same warning.  Changing the format string as follows should
> avoid the warnings:
> 
>   error_at (loc, "%<-march=%s%>: unexpected ISA string at end: %qs"

I've made the corresponding changes.
tested with RUNTESTFLAGS="riscv.exp"

Thanks,
Maxim

gcc/ChangeLog:
2019-07-31  Maxim Blinov  <maxim.blinov@embecosm.com>

	* common/config/riscv/riscv-common.c: Check -march string ends
	with null.

gcc/testsuite/ChangeLog:
2019-07-31  Maxim Blinov  <maxim.blinov@embecosm.com>

	* gcc.target/riscv/attribute-10.c: New test.

---
 gcc/common/config/riscv/riscv-common.c        | 7 +++++++
 gcc/testsuite/gcc.target/riscv/attribute-10.c | 6 ++++++
 2 files changed, 13 insertions(+)
 create mode 100644 gcc/testsuite/gcc.target/riscv/attribute-10.c

diff --git a/gcc/common/config/riscv/riscv-common.c b/gcc/common/config/riscv/riscv-common.c
index eeb75717db0..a16d6c5b448 100644
--- a/gcc/common/config/riscv/riscv-common.c
+++ b/gcc/common/config/riscv/riscv-common.c
@@ -513,6 +513,13 @@ riscv_subset_list::parse (const char *arch, location_t loc)
   if (p == NULL)
     goto fail;
 
+  if (*p != '\0')
+    {
+      error_at (loc, "%<-march=%s%>: unexpected ISA string at end: %qs",
+               arch, p);
+      goto fail;
+    }
+
   return subset_list;
 
 fail:
diff --git a/gcc/testsuite/gcc.target/riscv/attribute-10.c b/gcc/testsuite/gcc.target/riscv/attribute-10.c
new file mode 100644
index 00000000000..dd817879a67
--- /dev/null
+++ b/gcc/testsuite/gcc.target/riscv/attribute-10.c
@@ -0,0 +1,6 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -march=rv32im_s_sx_unexpectedstring -mabi=ilp32" } */
+int foo()
+{
+}
+/* { dg-error "unexpected ISA string at end:" "" { target { "riscv*-*-*" } } 0 } */
-- 
2.20.1

Re: [PATCH v2] RISC-V: Raise error on unexpected ISA string at end.

[Jim Wilson]

On Wed, Jul 31, 2019 at 9:41 AM Maxim Blinov <maxim.blinov@embecosm.com> wrote:
> gcc/ChangeLog:
> 2019-07-31  Maxim Blinov  <maxim.blinov@embecosm.com>
>
>         * common/config/riscv/riscv-common.c: Check -march string ends
>         with null.
>
> gcc/testsuite/ChangeLog:
> 2019-07-31  Maxim Blinov  <maxim.blinov@embecosm.com>
>
>         * gcc.target/riscv/attribute-10.c: New test.

Looks good.  I committed the revised patch.

Jim