[PATCH] pr 65702 - error out for invalid register asms earlier

[PATCH] pr 65702 - error out for invalid register asms earlier

[tbsaunde+gcc]

From: Trevor Saunders <tbsaunde+gcc@tbsaunde.org>

Hi,

$subject.  To avoid regressions I kept the checks when generating rtl, but I
believe its impossible for those to trigger now and we can remove the checks.

bootstrapped + regtested on x86_64-linux-gnu, ok?

Trev

gcc/c/ChangeLog:

2016-01-25  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* c-decl.c (finish_decl): Check if asm register is valid.

gcc/ChangeLog:

2016-01-25  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* varasm.c (register_asmspec_ok_p): New function.
	(make_decl_rtl): Adjust.
	* varasm.h (register_asmspec_ok_p): New prototype.

gcc/cp/ChangeLog:

2016-01-25  Trevor Saunders  <tbsaunde+gcc@tbsaunde.org>

	* decl.c (make_rtl_for_nonlocal_decl): Check if register asm is
	valid.
---
 gcc/c/c-decl.c                                |   8 +-
 gcc/cp/decl.c                                 |   4 +-
 gcc/testsuite/g++.dg/torture/register-asm-1.C |  14 +++
 gcc/testsuite/gcc.dg/reg-vol-struct-1.c       |   2 +-
 gcc/testsuite/gcc.dg/torture/register-asm-1.c |  12 +++
 gcc/varasm.c                                  | 150 +++++++++++++++-----------
 gcc/varasm.h                                  |   3 +
 7 files changed, 129 insertions(+), 64 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/torture/register-asm-1.C
 create mode 100644 gcc/testsuite/gcc.dg/torture/register-asm-1.c

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 1ec6042..9257f35 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -4867,7 +4867,9 @@ finish_decl (tree decl, location_t init_loc, tree init,
 	       when a tentative file-scope definition is seen.
 	       But at end of compilation, do output code for them.  */
 	    DECL_DEFER_OUTPUT (decl) = 1;
-	  if (asmspec && C_DECL_REGISTER (decl))
+	  if (asmspec
+	      && C_DECL_REGISTER (decl)
+	      && register_asmspec_ok_p (decl, asmspec, DECL_MODE (decl)))
 	    DECL_HARD_REGISTER (decl) = 1;
 	  rest_of_decl_compilation (decl, true, 0);
 	}
@@ -4878,7 +4880,9 @@ finish_decl (tree decl, location_t init_loc, tree init,
 	     in a particular register.  */
 	  if (asmspec && C_DECL_REGISTER (decl))
 	    {
-	      DECL_HARD_REGISTER (decl) = 1;
+	      if (register_asmspec_ok_p (decl, asmspec, DECL_MODE (decl)))
+		DECL_HARD_REGISTER (decl) = 1;
+
 	      /* This cannot be done for a structure with volatile
 		 fields, on which DECL_REGISTER will have been
 		 reset.  */
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index f4604b6..6d130bd 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -6201,7 +6201,9 @@ make_rtl_for_nonlocal_decl (tree decl, tree init, const char* asmspec)
       /* The `register' keyword, when used together with an
 	 asm-specification, indicates that the variable should be
 	 placed in a particular register.  */
-      if (VAR_P (decl) && DECL_REGISTER (decl))
+      if (VAR_P (decl)
+	  && DECL_REGISTER (decl)
+	  && register_asmspec_ok_p (decl, asmspec, DECL_MODE (decl)))
 	{
 	  set_user_assembler_name (decl, asmspec);
 	  DECL_HARD_REGISTER (decl) = 1;
diff --git a/gcc/testsuite/g++.dg/torture/register-asm-1.C b/gcc/testsuite/g++.dg/torture/register-asm-1.C
new file mode 100644
index 0000000..b5cfc84
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/register-asm-1.C
@@ -0,0 +1,14 @@
+/* { dg-do compile } */
+
+class A {
+	  int m_fn1() const;
+};
+int a[1];
+int b;
+int A::m_fn1() const {
+	register int c asm(""); // { dg-error "invalid register name for 'c'" }
+	while (b)
+		if (a[5])
+			c = b;
+	return c;
+}
diff --git a/gcc/testsuite/gcc.dg/reg-vol-struct-1.c b/gcc/testsuite/gcc.dg/reg-vol-struct-1.c
index b885f91..e67c7a2 100644
--- a/gcc/testsuite/gcc.dg/reg-vol-struct-1.c
+++ b/gcc/testsuite/gcc.dg/reg-vol-struct-1.c
@@ -12,7 +12,7 @@ f (void)
 {
   register struct S a;
   register struct S b[2];
-  register struct S c __asm__("nosuchreg"); /* { dg-error "object with volatile field" "explicit reg name" } */
+  register struct S c __asm__("nosuchreg"); /* { dg-error "invalid register name for 'c'|cannot put object with volatile field into register" } */
   &a; /* { dg-error "address of register" "explicit address" } */
   b; /* { dg-error "address of register" "implicit address" } */
 }
diff --git a/gcc/testsuite/gcc.dg/torture/register-asm-1.c b/gcc/testsuite/gcc.dg/torture/register-asm-1.c
new file mode 100644
index 0000000..1949f62
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/register-asm-1.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+
+   int a[1], b;
+int
+foo ()
+{
+    register int c asm (""); /* { dg-error "invalid register name for 'c'" } */
+      while (b)
+	    if (a[5])
+	      c = b;
+    return c;
+}
diff --git a/gcc/varasm.c b/gcc/varasm.c
index 3a3573e..7e3aebc9 100644
--- a/gcc/varasm.c
+++ b/gcc/varasm.c
@@ -1270,6 +1270,55 @@ ultimate_transparent_alias_target (tree *alias)
   return target;
 }
 
+/* Check that a register asm spec is correct, and compatibility with
+   machine_mode mode.  If reg_number is nonnull and the spec is valid then set
+   it to the register number designated by the spec.  */
+
+bool
+register_asmspec_ok_p (tree decl, const char *asmspec, machine_mode mode,
+		       int *reg_number)
+{
+  int reg = decode_reg_name (asmspec);
+  if (reg == -1)
+    {
+      error ("register name not specified for %q+D", decl);
+      return false;
+    }
+  else if (reg < 0)
+    {
+      error ("invalid register name for %q+D", decl);
+      return false;
+    }
+  else if (mode == BLKmode)
+    {
+      error ("data type of %q+D isn%'t suitable for a register", decl);
+    return false;
+    }
+  else if (!in_hard_reg_set_p (accessible_reg_set, mode, reg))
+    {
+      error ("the register specified for %q+D cannot be accessed"
+	     " by the current target", decl);
+      return false;
+    }
+  else if (!in_hard_reg_set_p (operand_reg_set, mode, reg))
+    {
+      error ("the register specified for %q+D is not general enough"
+	     " to be used as a register variable", decl);
+      return false;
+    }
+  else if (!HARD_REGNO_MODE_OK (reg, mode))
+    {
+      error ("register specified for %q+D isn%'t suitable for data type",
+	     decl);
+      return false;
+    }
+
+  if (reg_number)
+    *reg_number = reg;
+
+  return true;
+}
+
 /* Create the DECL_RTL for a VAR_DECL or FUNCTION_DECL.  DECL should
    have static storage duration.  In other words, it should not be an
    automatic variable, including PARM_DECLs.
@@ -1283,7 +1332,6 @@ void
 make_decl_rtl (tree decl)
 {
   const char *name = 0;
-  int reg_number;
   tree id;
   rtx x;
 
@@ -1358,73 +1406,55 @@ make_decl_rtl (tree decl)
     {
       const char *asmspec = name+1;
       machine_mode mode = DECL_MODE (decl);
-      reg_number = decode_reg_name (asmspec);
-      /* First detect errors in declaring global registers.  */
-      if (reg_number == -1)
-	error ("register name not specified for %q+D", decl);
-      else if (reg_number < 0)
-	error ("invalid register name for %q+D", decl);
-      else if (mode == BLKmode)
-	error ("data type of %q+D isn%'t suitable for a register",
-	       decl);
-      else if (!in_hard_reg_set_p (accessible_reg_set, mode, reg_number))
-	error ("the register specified for %q+D cannot be accessed"
-	       " by the current target", decl);
-      else if (!in_hard_reg_set_p (operand_reg_set, mode, reg_number))
-	error ("the register specified for %q+D is not general enough"
-	       " to be used as a register variable", decl);
-      else if (!HARD_REGNO_MODE_OK (reg_number, mode))
-	error ("register specified for %q+D isn%'t suitable for data type",
-               decl);
-      /* Now handle properly declared static register variables.  */
-      else
+      int reg_number;
+      if (!register_asmspec_ok_p (decl, asmspec, mode, &reg_number))
 	{
-	  int nregs;
+	  /* Avoid internal errors from invalid register
+	     specifications.  */
+	  SET_DECL_ASSEMBLER_NAME (decl, NULL_TREE);
+	  DECL_HARD_REGISTER (decl) = 0;
+	  /* Also avoid SSA inconsistencies by pretending this is an external
+	     decl now.  */
+	  DECL_EXTERNAL (decl) = 1;
+	  return;
+	}
 
-	  if (DECL_INITIAL (decl) != 0 && TREE_STATIC (decl))
-	    {
-	      DECL_INITIAL (decl) = 0;
-	      error ("global register variable has initial value");
-	    }
-	  if (TREE_THIS_VOLATILE (decl))
-	    warning (OPT_Wvolatile_register_var,
-		     "optimization may eliminate reads and/or "
-		     "writes to register variables");
 
-	  /* If the user specified one of the eliminables registers here,
-	     e.g., FRAME_POINTER_REGNUM, we don't want to get this variable
-	     confused with that register and be eliminated.  This usage is
-	     somewhat suspect...  */
+      if (DECL_INITIAL (decl) != 0 && TREE_STATIC (decl))
+	{
+	  DECL_INITIAL (decl) = 0;
+	  error ("global register variable has initial value");
+	}
+      if (TREE_THIS_VOLATILE (decl))
+	warning (OPT_Wvolatile_register_var,
+		 "optimization may eliminate reads and/or "
+		 "writes to register variables");
 
-	  SET_DECL_RTL (decl, gen_raw_REG (mode, reg_number));
-	  ORIGINAL_REGNO (DECL_RTL (decl)) = reg_number;
-	  REG_USERVAR_P (DECL_RTL (decl)) = 1;
+      /* If the user specified one of the eliminables registers here,
+	 e.g., FRAME_POINTER_REGNUM, we don't want to get this variable
+	 confused with that register and be eliminated.  This usage is
+	 somewhat suspect...  */
 
-	  if (TREE_STATIC (decl))
-	    {
-	      /* Make this register global, so not usable for anything
-		 else.  */
+      SET_DECL_RTL (decl, gen_raw_REG (mode, reg_number));
+      ORIGINAL_REGNO (DECL_RTL (decl)) = reg_number;
+      REG_USERVAR_P (DECL_RTL (decl)) = 1;
+
+      if (TREE_STATIC (decl))
+	{
+	  /* Make this register global, so not usable for anything
+	     else.  */
 #ifdef ASM_DECLARE_REGISTER_GLOBAL
-	      name = IDENTIFIER_POINTER (DECL_NAME (decl));
-	      ASM_DECLARE_REGISTER_GLOBAL (asm_out_file, decl, reg_number, name);
+	  name = IDENTIFIER_POINTER (DECL_NAME (decl));
+	  ASM_DECLARE_REGISTER_GLOBAL (asm_out_file, decl, reg_number, name);
 #endif
-	      nregs = hard_regno_nregs[reg_number][mode];
-	      while (nregs > 0)
-		globalize_reg (decl, reg_number + --nregs);
-	    }
-
-	  /* As a register variable, it has no section.  */
-	  return;
+	  int nregs = hard_regno_nregs[reg_number][mode];
+	  while (nregs > 0)
+	    globalize_reg (decl, reg_number + --nregs);
 	}
-      /* Avoid internal errors from invalid register
-	 specifications.  */
-      SET_DECL_ASSEMBLER_NAME (decl, NULL_TREE);
-      DECL_HARD_REGISTER (decl) = 0;
-      /* Also avoid SSA inconsistencies by pretending this is an external
-	 decl now.  */
-      DECL_EXTERNAL (decl) = 1;
+
+      /* As a register variable, it has no section.  */
       return;
-    }
+	}
   /* Now handle ordinary static variables and functions (in memory).
      Also handle vars declared register invalidly.  */
   else if (name[0] == '*')
@@ -1432,7 +1462,7 @@ make_decl_rtl (tree decl)
 #ifdef REGISTER_PREFIX
     if (strlen (REGISTER_PREFIX) != 0)
       {
-	reg_number = decode_reg_name (name);
+	int reg_number = decode_reg_name (name);
 	if (reg_number >= 0 || reg_number == -3)
 	  error ("register name given for non-register variable %q+D", decl);
       }
diff --git a/gcc/varasm.h b/gcc/varasm.h
index 51a5492..042ce1a 100644
--- a/gcc/varasm.h
+++ b/gcc/varasm.h
@@ -79,4 +79,7 @@ extern rtx assemble_static_space (unsigned HOST_WIDE_INT);
 
 extern rtx assemble_trampoline_template (void);
 
+extern bool register_asmspec_ok_p (tree decl, const char *spec,
+				   machine_mode mode, int *regnum = NULL);
+
 #endif  // GCC_VARASM_H
-- 
2.7.0

Re: [PATCH] pr 65702 - error out for invalid register asms earlier

[Bernd Schmidt]

On 01/25/2016 04:36 PM, tbsaunde+gcc@tbsaunde.org wrote:
> $subject.  To avoid regressions I kept the checks when generating rtl, but I
> believe its impossible for those to trigger now and we can remove the checks.
>
> bootstrapped + regtested on x86_64-linux-gnu, ok?

Is this still an issue? I committed a fix for a similar PR a few weeks 
ago, and I can't make the testcase from 65702 ICE.


Bernd

Re: [PATCH] pr 65702 - error out for invalid register asms earlier

[Trevor Saunders]

On Mon, Jan 25, 2016 at 04:42:58PM +0100, Bernd Schmidt wrote:
> On 01/25/2016 04:36 PM, tbsaunde+gcc@tbsaunde.org wrote:
> >$subject.  To avoid regressions I kept the checks when generating rtl, but I
> >believe its impossible for those to trigger now and we can remove the checks.
> >
> >bootstrapped + regtested on x86_64-linux-gnu, ok?
> 
> Is this still an issue? I committed a fix for a similar PR a few weeks ago,
> and I can't make the testcase from 65702 ICE.

 hrm, I guess my tree was more out of date than I thought, it doesn't
 ICE for me at r232662.

 Never mind then ;)

 Trev

> 
> 
> Bernd