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.
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 "";
> }
>
