On 20/09/16 15:13, Richard Earnshaw (lists) wrote:
On 08/09/16 12:00, Kyrill Tkachov wrote:
Hi all,

This is a rebase and slight respin of [1] that I sent out last November
to change all uses of
sprintf to snprintf in the arm backend.

Bootstrapped and tested on arm-none-linux-gnueabihf.
Ok for trunk?

Thanks,
Kyrill

[1] https://gcc.gnu.org/ml/gcc-patches/2015-11/msg00937.html

2016-09-08  Kyrylo Tkachov  <kyrylo.tkac...@arm.com>

     * config/arm/arm.c (arm_set_fixed_optab_libfunc): Use snprintf
     rather than sprintf.
     (arm_set_fixed_conv_libfunc): Likewise.
     (arm_option_override): Likewise.
     (neon_output_logic_immediate): Likewise.
     (neon_output_shift_immediate): Likewise.
     (arm_output_multireg_pop): Likewise.
     (vfp_output_vstmd): Likewise.
     (output_move_vfp): Likewise.
     (output_move_neon): Likewise.
     (output_return_instruction): Likewise.
     (arm_elf_asm_cdtor): Likewise.
     (arm_output_shift): Likewise.
     (arm_output_iwmmxt_shift_immediate): Likewise.
     (arm_output_iwmmxt_tinsr): Likewise.
     * config/arm/neon.md (*neon_mov<mode>, VDX): Likewise.
     (*neon_mov<mode>, VQXMOV): Likewise.
     (neon_vc<cmp_op><mode>_insn): Likewise.
     (neon_vc<cmp_op_unsp><mode>_insn_unspec): Likewise.
Most of these should assert that truncation did not occur (it's an
internal error if the buffers aren't large enough).  In a few cases we
should call output_operand_lossage on failure since it might be due to a
user writing inline assembly and overflowing a buffer.

I don't think we should silently accept a truncated write.

Ok, I'm proposing a new function defined in system.h
(though I'm open to suggestions for other location).
It would be something like:

static inline int ATTRIBUTE_PRINTF_3
gcc_snprintf (char *str, size_t size, const char *format, ...)
{
  va_list ap;
  va_start(ap, format);
  size_t res = vsnprintf (str, size, format, ap);
  va_end (ap);
  /* vsnprintf returns >= size if input was truncated.  */
  gcc_assert (res < size);
  return res;
}

Would that be acceptable?

Thanks,
Kyrill


R.

arm-snprintf.patch


diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 
946f308ca84e232af8af6eca4813464914cbd59c..8ff6c0e18f7c2a2eed72e56402ed582c9f692d2d
 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -2388,9 +2388,10 @@ arm_set_fixed_optab_libfunc (optab optable, machine_mode 
mode,
    char buffer[50];
if (num_suffix == 0)
-    sprintf (buffer, "__gnu_%s%s", funcname, modename);
+    snprintf (buffer, sizeof (buffer), "__gnu_%s%s", funcname, modename);
    else
-    sprintf (buffer, "__gnu_%s%s%d", funcname, modename, num_suffix);
+    snprintf (buffer, sizeof (buffer), "__gnu_%s%s%d", funcname,
+             modename, num_suffix);
set_optab_libfunc (optable, mode, buffer);
  }
@@ -2409,8 +2410,8 @@ arm_set_fixed_conv_libfunc (convert_optab optable, 
machine_mode to,
        && ALL_FRACT_MODE_P (from) == ALL_FRACT_MODE_P (to))
      maybe_suffix_2 = "2";
- sprintf (buffer, "__gnu_%s%s%s%s", funcname, fromname, toname,
-          maybe_suffix_2);
+  snprintf (buffer, sizeof (buffer), "__gnu_%s%s%s%s", funcname,
+           fromname, toname, maybe_suffix_2);
set_conv_libfunc (optable, to, from, buffer);
  }
@@ -3163,7 +3164,8 @@ arm_option_override (void)
    if (!arm_selected_tune)
      arm_selected_tune = &all_cores[arm_selected_cpu->core];
- sprintf (arm_arch_name, "__ARM_ARCH_%s__", arm_selected_cpu->arch);
+  snprintf (arm_arch_name, sizeof (arm_arch_name),
+           "__ARM_ARCH_%s__", arm_selected_cpu->arch);
    insn_flags = arm_selected_cpu->flags;
    arm_base_arch = arm_selected_cpu->base_arch;
@@ -12735,9 +12737,11 @@ neon_output_logic_immediate (const char *mnem, rtx *op2, machine_mode mode,
    gcc_assert (is_valid != 0);
if (quad)
-    sprintf (templ, "%s.i%d\t%%q0, %%2", mnem, width);
+    snprintf (templ, sizeof (templ), "%s.i%d\t%%q0, %%2",
+             mnem, width);
    else
-    sprintf (templ, "%s.i%d\t%%P0, %%2", mnem, width);
+    snprintf (templ, sizeof (templ), "%s.i%d\t%%P0, %%2",
+             mnem, width);
return templ;
  }
@@ -12757,9 +12761,11 @@ neon_output_shift_immediate (const char *mnem, char 
sign, rtx *op2,
    gcc_assert (is_valid != 0);
if (quad)
-    sprintf (templ, "%s.%c%d\t%%q0, %%q1, %%2", mnem, sign, width);
+    snprintf (templ, sizeof (templ),
+             "%s.%c%d\t%%q0, %%q1, %%2", mnem, sign, width);
    else
-    sprintf (templ, "%s.%c%d\t%%P0, %%P1, %%2", mnem, sign, width);
+    snprintf (templ, sizeof (templ),
+             "%s.%c%d\t%%P0, %%P1, %%2", mnem, sign, width);
return templ;
  }
@@ -17858,17 +17864,17 @@ arm_output_multireg_pop (rtx *operands, bool 
return_pc, rtx cond, bool reverse,
    conditional = reverse ? "%?%D0" : "%?%d0";
    /* Can't use POP if returning from an interrupt.  */
    if ((regno_base == SP_REGNUM) && update && !(interrupt_p && return_pc))
-    sprintf (pattern, "pop%s\t{", conditional);
+    snprintf (pattern, sizeof (pattern), "pop%s\t{", conditional);
    else
      {
        /* Output ldmfd when the base register is SP, otherwise output ldmia.
           It's just a convention, their semantics are identical.  */
        if (regno_base == SP_REGNUM)
-       sprintf (pattern, "ldmfd%s\t", conditional);
+       snprintf (pattern, sizeof (pattern), "ldmfd%s\t", conditional);
        else if (update)
-       sprintf (pattern, "ldmia%s\t", conditional);
+       snprintf (pattern, sizeof (pattern), "ldmia%s\t", conditional);
        else
-       sprintf (pattern, "ldm%s\t", conditional);
+       snprintf (pattern, sizeof (pattern), "ldm%s\t", conditional);
strcat (pattern, reg_names[regno_base]);
        if (update)
@@ -17924,7 +17930,8 @@ vfp_output_vstmd (rtx * operands)
    base = (REGNO (operands[1]) - FIRST_VFP_REGNUM) / 2;
    for (i = 1; i < XVECLEN (operands[2], 0); i++)
      {
-      p += sprintf (&pattern[p], ", d%d", base + i);
+      p += snprintf (&pattern[p], sizeof (pattern),
+                    ", d%d", base + i);
      }
    strcpy (&pattern[p], "}");
@@ -18719,7 +18726,7 @@ output_move_vfp (rtx *operands)
        break;
      }
- sprintf (buff, templ,
+  snprintf (buff, sizeof (buff), templ,
           load ? "ld" : "st",
           dp ? "64" : "32",
           dp ? "P" : "",
@@ -18859,7 +18866,8 @@ output_move_neon (rtx *operands)
              }
            else
              {
-               sprintf (buff, "v%sr%%?\t%%P0, %%1", load ? "ld" : "st");
+               snprintf (buff, sizeof (buff),
+                         "v%sr%%?\t%%P0, %%1", load ? "ld" : "st");
                output_asm_insn (buff, ops);
              }
          }
@@ -18867,7 +18875,8 @@ output_move_neon (rtx *operands)
          {
            ops[0] = gen_rtx_REG (DImode, REGNO (reg) + 2 * overlap);
            ops[1] = adjust_address (mem, SImode, 8 * overlap);
-           sprintf (buff, "v%sr%%?\t%%P0, %%1", load ? "ld" : "st");
+           snprintf (buff, sizeof (buff),
+                     "v%sr%%?\t%%P0, %%1", load ? "ld" : "st");
            output_asm_insn (buff, ops);
          }
@@ -18878,7 +18887,7 @@ output_move_neon (rtx *operands)
        gcc_unreachable ();
      }
- sprintf (buff, templ, load ? "ld" : "st");
+  snprintf (buff, sizeof (buff), templ, load ? "ld" : "st");
    output_asm_insn (buff, ops);
return "";
@@ -19606,7 +19615,8 @@ output_return_instruction (rtx operand, bool 
really_return, bool reverse,
gcc_assert (!cfun->calls_alloca || really_return); - sprintf (conditional, "%%?%%%c0", reverse ? 'D' : 'd');
+  snprintf (conditional, sizeof (conditional),
+          "%%?%%%c0", reverse ? 'D' : 'd');
cfun->machine->return_used_this_function = 1; @@ -19657,7 +19667,8 @@ output_return_instruction (rtx operand, bool really_return, bool reverse,
              || ! really_return
              || ! IS_INTERRUPT (func_type)))
        {
-         sprintf (instr, "ldr%s\t%%|%s, [%%|sp], #4", conditional,
+         snprintf (instr, sizeof (instr),
+                  "ldr%s\t%%|%s, [%%|sp], #4", conditional,
                   (reg == LR_REGNUM) ? return_reg : reg_names[reg]);
        }
        else
@@ -19677,7 +19688,8 @@ output_return_instruction (rtx operand, bool 
really_return, bool reverse,
              gcc_assert (stack_adjust == 0 || stack_adjust == 4);
if (stack_adjust && arm_arch5 && TARGET_ARM)
-                 sprintf (instr, "ldmib%s\t%%|sp, {", conditional);
+                 snprintf (instr, sizeof (instr), "ldmib%s\t%%|sp, {",
+                           conditional);
              else
                {
                  /* If we can't use ldmib (SA110 bug),
@@ -19685,17 +19697,20 @@ output_return_instruction (rtx operand, bool 
really_return, bool reverse,
                  if (stack_adjust)
                    live_regs_mask |= 1 << 3;
- sprintf (instr, "ldmfd%s\t%%|sp, {", conditional);
+                 snprintf (instr, sizeof (instr), "ldmfd%s\t%%|sp, {",
+                           conditional);
                }
            }
          /* For interrupt returns we have to use an LDM rather than
             a POP so that we can use the exception return variant.  */
          else if (IS_INTERRUPT (func_type))
-           sprintf (instr, "ldmfd%s\t%%|sp!, {", conditional);
+           snprintf (instr, sizeof (instr), "ldmfd%s\t%%|sp!, {", conditional);
          else
-           sprintf (instr, "pop%s\t{", conditional);
+           snprintf (instr, sizeof (instr), "pop%s\t{", conditional);
- p = instr + strlen (instr);
+         size_t len = strlen (instr);
+         p = instr + len;
+         size_t rem_size = sizeof (instr) - len;
for (reg = 0; reg <= SP_REGNUM; reg++)
            if (live_regs_mask & (1 << reg))
@@ -19717,7 +19732,7 @@ output_return_instruction (rtx operand, bool 
really_return, bool reverse,
if (live_regs_mask & (1 << LR_REGNUM))
            {
-             sprintf (p, "%s%%|%s}", first ? "" : ", ", return_reg);
+             snprintf (p, rem_size, "%s%%|%s}", first ? "" : ", ", return_reg);
              /* If returning from an interrupt, restore the CPSR.  */
              if (IS_INTERRUPT (func_type))
                strcat (p, "^");
@@ -19747,25 +19762,27 @@ output_return_instruction (rtx operand, bool 
really_return, bool reverse,
        case ARM_FT_ISR:
        case ARM_FT_FIQ:
          /* ??? This is wrong for unified assembly syntax.  */
-         sprintf (instr, "sub%ss\t%%|pc, %%|lr, #4", conditional);
+         snprintf (instr, sizeof (instr), "sub%ss\t%%|pc, %%|lr, #4",
+                   conditional);
          break;
case ARM_FT_INTERWORKED:
-         gcc_assert (arm_arch5 || arm_arch4t);
-         sprintf (instr, "bx%s\t%%|lr", conditional);
+         snprintf (instr, sizeof (instr), "bx%s\t%%|lr", conditional);
          break;
case ARM_FT_EXCEPTION:
          /* ??? This is wrong for unified assembly syntax.  */
-         sprintf (instr, "mov%ss\t%%|pc, %%|lr", conditional);
+         snprintf (instr, sizeof (instr), "mov%ss\t%%|pc, %%|lr",
+                   conditional);
          break;
default:
          /* Use bx if it's available.  */
          if (arm_arch5 || arm_arch4t)
-           sprintf (instr, "bx%s\t%%|lr", conditional);
+           snprintf (instr, sizeof (instr), "bx%s\t%%|lr", conditional);
          else
-           sprintf (instr, "mov%s\t%%|pc, %%|lr", conditional);
+           snprintf (instr, sizeof (instr), "mov%s\t%%|pc, %%|lr",
+                     conditional);
          break;
        }
@@ -22817,9 +22834,9 @@ arm_elf_asm_cdtor (rtx symbol, int priority, bool is_ctor)
    if (priority != DEFAULT_INIT_PRIORITY)
      {
        char buf[18];
-      sprintf (buf, "%s.%.5u",
-              is_ctor ? ".init_array" : ".fini_array",
-              priority);
+      snprintf (buf, sizeof (buf), "%s.%.5u",
+               is_ctor ? ".init_array" : ".fini_array",
+               priority);
        s = get_section (buf, SECTION_WRITE, NULL_TREE);
      }
    else if (is_ctor)
@@ -27429,10 +27446,10 @@ arm_output_shift(rtx * operands, int set_flags)
      {
        if (val != -1)
        operands[2] = GEN_INT(val);
-      sprintf (pattern, "%s%%%c\t%%0, %%1, %%2", shift, c);
+      snprintf (pattern, sizeof (pattern), "%s%%%c\t%%0, %%1, %%2", shift, c);
      }
    else
-    sprintf (pattern, "mov%%%c\t%%0, %%1", c);
+    snprintf (pattern, sizeof (pattern), "mov%%%c\t%%0, %%1", c);
output_asm_insn (pattern, operands);
    return "";
@@ -27456,33 +27473,35 @@ arm_output_iwmmxt_shift_immediate (const char 
*insn_name, rtx *operands, bool wr
    {
      if (wror_or_wsra)
        {
-        sprintf (templ, "%s\t%%0, %%1, #%d", insn_name, 32);
-        output_asm_insn (templ, operands);
-        if (opmode == DImode)
-          {
-           sprintf (templ, "%s\t%%0, %%0, #%d", insn_name, 32);
+       snprintf (templ, sizeof (templ), "%s\t%%0, %%1, #%d", insn_name, 32);
+       output_asm_insn (templ, operands);
+       if (opmode == DImode)
+         {
+           snprintf (templ, sizeof (templ), "%s\t%%0, %%0, #%d",
+                     insn_name, 32);
            output_asm_insn (templ, operands);
-          }
+         }
        }
      else
        {
-        /* The destination register will contain all zeros.  */
-        sprintf (templ, "wzero\t%%0");
-        output_asm_insn (templ, operands);
+       /* The destination register will contain all zeros.  */
+       snprintf (templ, sizeof (templ), "wzero\t%%0");
+       output_asm_insn (templ, operands);
        }
      return "";
    }
if ((opmode == DImode) && (shift > 32))
      {
-      sprintf (templ, "%s\t%%0, %%1, #%d", insn_name, 32);
+      snprintf (templ, sizeof (templ), "%s\t%%0, %%1, #%d", insn_name, 32);
        output_asm_insn (templ, operands);
-      sprintf (templ, "%s\t%%0, %%0, #%d", insn_name, shift - 32);
+      snprintf (templ, sizeof (templ), "%s\t%%0, %%0, #%d",
+               insn_name, shift - 32);
        output_asm_insn (templ, operands);
      }
    else
      {
-      sprintf (templ, "%s\t%%0, %%1, #%d", insn_name, shift);
+      snprintf (templ, sizeof (templ), "%s\t%%0, %%1, #%d", insn_name, shift);
        output_asm_insn (templ, operands);
      }
    return "";
@@ -27510,13 +27529,13 @@ arm_output_iwmmxt_tinsr (rtx *operands)
      switch (GET_MODE (operands[0]))
        {
        case V8QImode:
-       sprintf (templ, "tinsrb%%?\t%%0, %%2, #%d", i);
+       snprintf (templ, sizeof (templ), "tinsrb%%?\t%%0, %%2, #%d", i);
        break;
        case V4HImode:
-       sprintf (templ, "tinsrh%%?\t%%0, %%2, #%d", i);
+       snprintf (templ, sizeof (templ), "tinsrh%%?\t%%0, %%2, #%d", i);
        break;
        case V2SImode:
-       sprintf (templ, "tinsrw%%?\t%%0, %%2, #%d", i);
+       snprintf (templ, sizeof (templ), "tinsrw%%?\t%%0, %%2, #%d", i);
        break;
        default:
        gcc_unreachable ();
diff --git a/gcc/config/arm/neon.md b/gcc/config/arm/neon.md
index 
e2fdfbb04621ee6f8603849be089e8bce624214d..54e6edfc4c053f685c8ca0cd3d1227015654aca1
 100644
--- a/gcc/config/arm/neon.md
+++ b/gcc/config/arm/neon.md
@@ -43,9 +43,10 @@ (define_insn "*neon_mov<mode>"
        gcc_assert (is_valid != 0);
if (width == 0)
-        return "vmov.f32\t%P0, %1  @ <mode>";
+       return "vmov.f32\t%P0, %1  @ <mode>";
        else
-        sprintf (templ, "vmov.i%d\t%%P0, %%x1  @ <mode>", width);
+       snprintf (templ, sizeof (templ),
+                 "vmov.i%d\t%%P0, %%x1  @ <mode>", width);
return templ;
      }
@@ -88,9 +89,10 @@ (define_insn "*neon_mov<mode>"
        gcc_assert (is_valid != 0);
if (width == 0)
-        return "vmov.f32\t%q0, %1  @ <mode>";
+       return "vmov.f32\t%q0, %1  @ <mode>";
        else
-        sprintf (templ, "vmov.i%d\t%%q0, %%1  @ <mode>", width);
+       snprintf (templ, sizeof (templ),
+                 "vmov.i%d\t%%q0, %%1  @ <mode>", width);
return templ;
      }
@@ -2346,12 +2348,13 @@ (define_insn "neon_vc<cmp_op><mode>_insn"
                      && !flag_unsafe_math_optimizations)"
    {
      char pattern[100];
-    sprintf (pattern, "vc<cmp_op>.%s%%#<V_sz_elem>\t%%<V_reg>0,"
-                      " %%<V_reg>1, %s",
-                       GET_MODE_CLASS (<MODE>mode) == MODE_VECTOR_FLOAT
-                         ? "f" : "<cmp_type>",
-                       which_alternative == 0
-                         ? "%<V_reg>2" : "#0");
+    snprintf (pattern, sizeof (pattern),
+             "vc<cmp_op>.%s%%#<V_sz_elem>\t%%<V_reg>0,"
+             " %%<V_reg>1, %s",
+             GET_MODE_CLASS (<MODE>mode) == MODE_VECTOR_FLOAT
+               ? "f" : "<cmp_type>",
+             which_alternative == 0
+               ? "%<V_reg>2" : "#0");
      output_asm_insn (pattern, operands);
      return "";
    }
@@ -2370,10 +2373,11 @@ (define_insn "neon_vc<cmp_op_unsp><mode>_insn_unspec"
    "TARGET_NEON"
    {
      char pattern[100];
-    sprintf (pattern, "vc<cmp_op_unsp>.f%%#<V_sz_elem>\t%%<V_reg>0,"
-                       " %%<V_reg>1, %s",
-                       which_alternative == 0
-                         ? "%<V_reg>2" : "#0");
+    snprintf (pattern, sizeof (pattern),
+             "vc<cmp_op_unsp>.f%%#<V_sz_elem>\t%%<V_reg>0,"
+             " %%<V_reg>1, %s",
+             which_alternative == 0
+               ? "%<V_reg>2" : "#0");
      output_asm_insn (pattern, operands);
      return "";
  }


Reply via email to