score gcc-8 warning fixes

Message ID 20180512070215.GC23663@bubble.grove.modra.org
State New
Headers show
Series
  • score gcc-8 warning fixes
Related show

Commit Message

Alan Modra May 12, 2018, 7:02 a.m.
Rather than just silencing the gcc-8 warnings, I decided to rewrite
the buffer handling in the two functions where gcc was warning.
The rest of the file could do with the same treatment.  We're not
supposed to have line length limits in the assembler.

	* config/tc-score.c (s3_do_macro_bcmp): Don't use fixed size
	buffers.
	(s3_do_macro_bcmpz): Likewise.


-- 
Alan Modra
Australia Development Lab, IBM

Comments

Carlos O'Donell May 14, 2018, 12:22 a.m. | #1
On 05/12/2018 03:02 AM, Alan Modra wrote:
> Rather than just silencing the gcc-8 warnings, I decided to rewrite

> the buffer handling in the two functions where gcc was warning.

> The rest of the file could do with the same treatment.  We're not

> supposed to have line length limits in the assembler.

> 

> 	* config/tc-score.c (s3_do_macro_bcmp): Don't use fixed size

> 	buffers.

> 	(s3_do_macro_bcmpz): Likewise.


+1

Where possible we fixup code in glibc to remove gcc-8 warnings where
it's a clear win.

These two cases you fix here are relatively easy to clean up, the rest
looks like quite a complicated refactoring, particularly when it comes
to copying error messages based on input from a line etc. There would
be quite a bit of work to clean it all up. It seems to me like you'd
want to delete s3_MAX_LITERAL_POOL_SIZE and fixup the fallout by
dynamically allocating whatever you needed? I've been looking for a
straight forward cleanup project for a new person on my team :-)

-- 
Cheers,
Carlos.
Alan Modra May 14, 2018, 3:30 a.m. | #2
On Sun, May 13, 2018 at 08:22:09PM -0400, Carlos O'Donell wrote:
> On 05/12/2018 03:02 AM, Alan Modra wrote:

> > Rather than just silencing the gcc-8 warnings, I decided to rewrite

> > the buffer handling in the two functions where gcc was warning.

> > The rest of the file could do with the same treatment.  We're not

> > supposed to have line length limits in the assembler.

> > 

> > 	* config/tc-score.c (s3_do_macro_bcmp): Don't use fixed size

> > 	buffers.

> > 	(s3_do_macro_bcmpz): Likewise.

> 

> +1

> 

> Where possible we fixup code in glibc to remove gcc-8 warnings where

> it's a clear win.

> 

> These two cases you fix here are relatively easy to clean up, the rest

> looks like quite a complicated refactoring, particularly when it comes

> to copying error messages based on input from a line etc. There would

> be quite a bit of work to clean it all up.


Yes, that's what I decided too.

> It seems to me like you'd

> want to delete s3_MAX_LITERAL_POOL_SIZE and fixup the fallout by

> dynamically allocating whatever you needed?


Yes, perhaps using an obstack for the memory.  I haven't analysed
anything to any detail but it's highly likely that a lot of the
string copying is not needed.

-- 
Alan Modra
Australia Development Lab, IBM

Patch

diff --git a/gas/config/tc-score.c b/gas/config/tc-score.c
index 8b587c8d64..c4e5ed90ef 100644
--- a/gas/config/tc-score.c
+++ b/gas/config/tc-score.c
@@ -4485,9 +4485,9 @@  static void
 s3_do_macro_bcmp (char *str)
 {
   int reg_a , reg_b;
-  char keep_data[s3_MAX_LITERAL_POOL_SIZE];
-  char* ptemp;
-  int i = 0;
+  char *keep_data;
+  size_t keep_data_size;
+  int i;
   struct s3_score_it inst_expand[2];
   struct s3_score_it inst_main;
 
@@ -4498,26 +4498,23 @@  s3_do_macro_bcmp (char *str)
       ||(reg_b = s3_reg_required_here (&str, 10, s3_REG_TYPE_SCORE)) == (int) s3_FAIL
       || s3_skip_past_comma (&str) == (int) s3_FAIL)
     return;
-  ptemp = str;
-  while (*ptemp != 0)
-    {
-      keep_data[i] = *ptemp;
-      i++;
-      ptemp++;
-    }
-  keep_data[i] = 0;
+
+  keep_data_size = strlen (str) + 1;
+  keep_data = xmalloc (keep_data_size * 2 + 14);
+  memcpy (keep_data, str, keep_data_size);
+
   if (s3_my_get_expression (&s3_inst.reloc.exp, &str) == (int) s3_FAIL
       ||reg_b == 0
       || s3_end_of_line (str) == (int) s3_FAIL)
-    return;
+    goto out;
   else if (s3_inst.reloc.exp.X_add_symbol == 0)
     {
       s3_inst.error = _("lacking label  ");
-      return;
+      goto out;
     }
   else
     {
-      char append_str[s3_MAX_LITERAL_POOL_SIZE];
+      char *append_str = keep_data + keep_data_size;
       s3_SET_INSN_ERROR (NULL);
 
       s3_inst.reloc.type = BFD_RELOC_SCORE_BCMP;
@@ -4536,15 +4533,15 @@  s3_do_macro_bcmp (char *str)
           /* support bcmp --> cmp!+beq (bne) */
           if (s3_score_pic == s3_NO_PIC)
             {
-              sprintf (&append_str[0], "cmp! r%d, r%d", reg_a, reg_b);
-              if (s3_append_insn (&append_str[0], TRUE) == (int) s3_FAIL)
-                return;
-              if ((inst_main.instruction & 0x3e00007e) == 0x0000004c)
-                sprintf (&append_str[1], "beq %s", keep_data);
-              else
-                sprintf (&append_str[1], "bne %s", keep_data);
-              if (s3_append_insn (&append_str[1], TRUE) == (int) s3_FAIL)
-                return;
+	      sprintf (append_str, "cmp! r%d, r%d", reg_a, reg_b);
+	      if (s3_append_insn (append_str, TRUE) == (int) s3_FAIL)
+		goto out;
+	      if ((inst_main.instruction & 0x3e00007e) == 0x0000004c)
+		sprintf (append_str, "beq %s", keep_data);
+	      else
+		sprintf (append_str, "bne %s", keep_data);
+	      if (s3_append_insn (append_str, TRUE) == (int) s3_FAIL)
+		goto out;
 	    }
 	  else
 	    {
@@ -4552,7 +4549,7 @@  s3_do_macro_bcmp (char *str)
 	    }
 	  /* Set bwarn as -1, so macro instruction itself will not be generated frag.  */
 	  s3_inst.bwarn = -1;
-	  return;
+	  goto out;
         }
       else
         {
@@ -4567,18 +4564,18 @@  s3_do_macro_bcmp (char *str)
 
       if (s3_score_pic == s3_NO_PIC)
         {
-          sprintf (&append_str[0], "cmp! r%d, r%d", reg_a, reg_b);
-          if (s3_append_insn (&append_str[0], FALSE) == (int) s3_FAIL)
-            return;
-          memcpy (&inst_expand[0], &s3_inst, sizeof (struct s3_score_it));
+	  sprintf (append_str, "cmp! r%d, r%d", reg_a, reg_b);
+	  if (s3_append_insn (append_str, FALSE) == (int) s3_FAIL)
+	    goto out;
+	  memcpy (&inst_expand[0], &s3_inst, sizeof (struct s3_score_it));
 
-          if ((inst_main.instruction & 0x3e00007e) == 0x0000004c)
-            sprintf (&append_str[1], "beq %s", keep_data);
-          else
-            sprintf (&append_str[1], "bne %s", keep_data);
-          if (s3_append_insn (&append_str[1], FALSE) == (int) s3_FAIL)
-            return;
-          memcpy (&inst_expand[1], &s3_inst, sizeof (struct s3_score_it));
+	  if ((inst_main.instruction & 0x3e00007e) == 0x0000004c)
+	    sprintf (append_str, "beq %s", keep_data);
+	  else
+	    sprintf (append_str, "bne %s", keep_data);
+	  if (s3_append_insn (append_str, FALSE) == (int) s3_FAIL)
+	    goto out;
+	  memcpy (&inst_expand[1], &s3_inst, sizeof (struct s3_score_it));
         }
       else
         {
@@ -4634,6 +4631,8 @@  s3_do_macro_bcmp (char *str)
       /* Set bwarn as -1, so macro instruction itself will not be generated frag.  */
       s3_inst.bwarn = -1;
     }
+ out:
+  free (keep_data);
 }
 
 /* Handle bcmpeqz / bcmpnez  */
@@ -4641,9 +4640,9 @@  static void
 s3_do_macro_bcmpz (char *str)
 {
   int reg_a;
-  char keep_data[s3_MAX_LITERAL_POOL_SIZE];
-  char* ptemp;
-  int i = 0;
+  char *keep_data;
+  size_t keep_data_size;
+  int i;
   struct s3_score_it inst_expand[2];
   struct s3_score_it inst_main;
 
@@ -4652,27 +4651,22 @@  s3_do_macro_bcmpz (char *str)
   if (( reg_a = s3_reg_required_here (&str, 15, s3_REG_TYPE_SCORE)) == (int) s3_FAIL
       || s3_skip_past_comma (&str) == (int) s3_FAIL)
     return;
-  ptemp = str;
-  while (*ptemp != 0)
-    {
-      keep_data[i] = *ptemp;
-      i++;
-      ptemp++;
-    }
 
-  keep_data[i] = 0;
+  keep_data_size = strlen (str) + 1;
+  keep_data = xmalloc (keep_data_size * 2 + 13);
+  memcpy (keep_data, str, keep_data_size);
 
   if (s3_my_get_expression (&s3_inst.reloc.exp, &str) == (int) s3_FAIL
       || s3_end_of_line (str) == (int) s3_FAIL)
-    return;
+    goto out;
   else if (s3_inst.reloc.exp.X_add_symbol == 0)
     {
       s3_inst.error = _("lacking label  ");
-      return;
+      goto out;
     }
   else
     {
-      char append_str[s3_MAX_LITERAL_POOL_SIZE];
+      char *append_str = keep_data + keep_data_size;
       s3_SET_INSN_ERROR (NULL);
       s3_inst.reloc.type = BFD_RELOC_SCORE_BCMP;
       s3_inst.reloc.pc_rel = 1;
@@ -4687,15 +4681,15 @@  s3_do_macro_bcmpz (char *str)
         {
           if (s3_score_pic == s3_NO_PIC)
             {
-              sprintf (&append_str[0], "cmpi! r%d,0", reg_a);
-              if (s3_append_insn (&append_str[0], TRUE) == (int) s3_FAIL)
-                return;
-              if ((inst_main.instruction & 0x3e00007e) == 0x0000004c)
-                sprintf (&append_str[1], "beq %s", keep_data);
-              else
-                sprintf (&append_str[1], "bne %s", keep_data);
-              if (s3_append_insn (&append_str[1], TRUE) == (int) s3_FAIL)
-                return;
+	      sprintf (append_str, "cmpi! r%d, 0", reg_a);
+	      if (s3_append_insn (append_str, TRUE) == (int) s3_FAIL)
+		goto out;
+	      if ((inst_main.instruction & 0x3e00007e) == 0x0000004c)
+		sprintf (append_str, "beq %s", keep_data);
+	      else
+		sprintf (append_str, "bne %s", keep_data);
+	      if (s3_append_insn (append_str, TRUE) == (int) s3_FAIL)
+		goto out;
             }
           else
             {
@@ -4703,7 +4697,7 @@  s3_do_macro_bcmpz (char *str)
             }
           /* Set bwarn as -1, so macro instruction itself will not be generated frag.  */
           s3_inst.bwarn = -1;
-          return;
+	  goto out;
         }
       else
         {
@@ -4718,17 +4712,17 @@  s3_do_macro_bcmpz (char *str)
 
       if (s3_score_pic == s3_NO_PIC)
         {
-          sprintf (&append_str[0], "cmpi! r%d, 0", reg_a);
-          if (s3_append_insn (&append_str[0], FALSE) == (int) s3_FAIL)
-            return;
-          memcpy (&inst_expand[0], &s3_inst, sizeof (struct s3_score_it));
-          if ((inst_main.instruction & 0x3e00007e) == 0x0000004c)
-            sprintf (&append_str[1], "beq %s", keep_data);
-          else
-            sprintf (&append_str[1], "bne %s", keep_data);
-          if (s3_append_insn (&append_str[1], FALSE) == (int) s3_FAIL)
-            return;
-          memcpy (&inst_expand[1], &s3_inst, sizeof (struct s3_score_it));
+	  sprintf (append_str, "cmpi! r%d, 0", reg_a);
+	  if (s3_append_insn (append_str, FALSE) == (int) s3_FAIL)
+	    goto out;
+	  memcpy (&inst_expand[0], &s3_inst, sizeof (struct s3_score_it));
+	  if ((inst_main.instruction & 0x3e00007e) == 0x0000004c)
+	    sprintf (append_str, "beq %s", keep_data);
+	  else
+	    sprintf (append_str, "bne %s", keep_data);
+	  if (s3_append_insn (append_str, FALSE) == (int) s3_FAIL)
+	    goto out;
+	  memcpy (&inst_expand[1], &s3_inst, sizeof (struct s3_score_it));
         }
       else
         {
@@ -4784,6 +4778,8 @@  s3_do_macro_bcmpz (char *str)
       /* Set bwarn as -1, so macro instruction itself will not be generated frag.  */
       s3_inst.bwarn = -1;
     }
+ out:
+  free (keep_data);
 }
 
 static int