From patchwork Mon Nov 9 11:36:28 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kyrylo Tkachov X-Patchwork-Id: 56218 Delivered-To: patch@linaro.org Received: by 10.112.155.196 with SMTP id vy4csp126680lbb; Mon, 9 Nov 2015 03:36:49 -0800 (PST) X-Received: by 10.68.91.98 with SMTP id cd2mr39891466pbb.9.1447069009183; Mon, 09 Nov 2015 03:36:49 -0800 (PST) Return-Path: Received: from sourceware.org (server1.sourceware.org. [209.132.180.131]) by mx.google.com with ESMTPS id iu1si21929330pbd.163.2015.11.09.03.36.48 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 09 Nov 2015 03:36:49 -0800 (PST) Received-SPF: pass (google.com: domain of gcc-patches-return-413221-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) client-ip=209.132.180.131; Authentication-Results: mx.google.com; spf=pass (google.com: domain of gcc-patches-return-413221-patch=linaro.org@gcc.gnu.org designates 209.132.180.131 as permitted sender) smtp.mailfrom=gcc-patches-return-413221-patch=linaro.org@gcc.gnu.org; dkim=pass header.i=@gcc.gnu.org DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; q=dns; s=default; b=EJdnniNSSsmD8KEDNiOh/rWDu5a77zC3v65N9Wpfvry iKBqRT3Pu3PA2uUcc2NHAGhtjgUWdX3O+4KrK1sQ4dTbSx/m0gaERIn8Yqp2Aylm FEWJpgO1x/F3dQYiLHP5ZN4qMpcg6+3lXnwtMy3IkpNmaRf1MpSNcWpxXQUABLa4 = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; s=default; bh=CX9YmmVOgiA8QiDH4FwHSHoR+A8=; b=BxqsQ1qLftyPrfs8y Wdn9e0QmZqPNs0P1y912Zur2AzNbP5j+zVb7m/Uw/B1IHPvTaEvJakZkoC8HvTLF XJx8NHl2JyjjaY+zuEw1m2uaK2QgIqEZxwR1EshRS3aXFvDTE3m/J6e50bpeZTv2 16VKmMJ4DVnEYZzLsuH9I6do8U= Received: (qmail 42459 invoked by alias); 9 Nov 2015 11:36:36 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Received: (qmail 42448 invoked by uid 89); 9 Nov 2015 11:36:35 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.7 required=5.0 tests=AWL, BAYES_00, SPF_PASS autolearn=ham version=3.3.2 X-HELO: eu-smtp-delivery-143.mimecast.com Received: from eu-smtp-delivery-143.mimecast.com (HELO eu-smtp-delivery-143.mimecast.com) (146.101.78.143) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 09 Nov 2015 11:36:33 +0000 Received: from cam-owa2.Emea.Arm.com (fw-tnat.cambridge.arm.com [217.140.96.140]) by eu-smtp-1.mimecast.com with ESMTP id uk-mta-5-zC06fycERQeubOtU1YMjBQ-1; Mon, 09 Nov 2015 11:36:28 +0000 Received: from [10.2.206.200] ([10.1.2.79]) by cam-owa2.Emea.Arm.com with Microsoft SMTPSVC(6.0.3790.3959); Mon, 9 Nov 2015 11:36:28 +0000 Message-ID: <5640853C.2030306@arm.com> Date: Mon, 09 Nov 2015 11:36:28 +0000 From: Kyrill Tkachov User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.2.0 MIME-Version: 1.0 To: GCC Patches CC: Ramana Radhakrishnan , Richard Earnshaw Subject: [PATCH][ARM] Use snprintf rather than sprintf where possible X-MC-Unique: zC06fycERQeubOtU1YMjBQ-1 X-IsSubscribed: yes Hi all, Judging by the thread at https://gcc.gnu.org/ml/gcc-patches/2015-10/msg01912.html I looked at replacing calls to sprintf with calls to snprintf in the arm backend. We use them a lot to print assembly mnemonics into static char buffers. This patch replaces the calls with snprintf and adds a size argument equal to the size of the buffer used. This way, if any of the format strings changes/increases past the size of the allocated buffer, snprintf will truncate it (and the assembler will catch it) rather than trying to write past the end of the buffer with unexpected results. I managed to replace all uses of sprintf in the arm backend except the one in aout.h: #define ASM_GENERATE_INTERNAL_LABEL(STRING, PREFIX, NUM) \ sprintf (STRING, "*%s%s%u", LOCAL_LABEL_PREFIX, PREFIX, (unsigned int)(NUM)) Here, ASM_GENERATE_INTERNAL_LABEL is used in various places in the midend to print labels to static buffers. I've seen those buffers have sizes ranging from 12 chars to 256 chars. The size of the buffer that ASM_GENERATE_INTERNAL_LABEL can depend on is not mandated in the documentation or passed down to the macro, so I think this is a bit dangerous. In practice, however, I don't think we print labels that long that that would cause an issue. Bootstrapped and tested on arm-none-linux-gnueabihf. Ok for trunk? Thanks, Kyrill 2015-11-09 Kyrylo Tkachov * 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, VDX): Likewise. (*neon_mov, VQXMOV): Likewise. (neon_vc_insn): Likewise. (neon_vc_insn_unspec): Likewise. commit bed313cb485dac52a5bb9984b55c9985af07ab35 Author: Kyrylo Tkachov Date: Wed Oct 21 14:40:07 2015 +0100 [ARM] Use snprintf rather than sprintf diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index f86ebd9..fa34a7b 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -2303,9 +2303,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); } @@ -2324,8 +2325,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); } @@ -3036,7 +3037,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; @@ -12631,9 +12633,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; } @@ -12653,9 +12657,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; } @@ -17672,9 +17678,7 @@ arm_output_multireg_pop (rtx *operands, bool return_pc, rtx cond, bool reverse, conditional = reverse ? "%?%D0" : "%?%d0"; if ((regno_base == SP_REGNUM) && update) - { - 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. @@ -17682,12 +17686,12 @@ arm_output_multireg_pop (rtx *operands, bool return_pc, rtx cond, bool reverse, if (regno_base == SP_REGNUM) /* update is never true here, hence there is no need to handle pop here. */ - sprintf (pattern, "ldmfd%s", conditional); + snprintf (pattern, sizeof (pattern), "ldmfd%s", conditional); 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) @@ -17743,7 +17747,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], "}"); @@ -18572,7 +18577,7 @@ output_move_vfp (rtx *operands) break; } - sprintf (buff, templ, + snprintf (buff, sizeof (buff), templ, load ? "ld" : "st", dp ? "64" : "32", dp ? "P" : "", @@ -18712,7 +18717,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); } } @@ -18720,7 +18726,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); } @@ -18731,7 +18738,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 ""; @@ -19474,7 +19481,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; @@ -19525,7 +19533,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 @@ -19545,7 +19554,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), @@ -19553,13 +19563,16 @@ 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), "ldmfdf%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)) @@ -19581,7 +19594,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, "^"); @@ -19611,24 +19624,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: - 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; } @@ -22679,9 +22695,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) @@ -27254,10 +27270,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 ""; @@ -27281,33 +27297,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 ""; @@ -27335,13 +27353,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 e5a2b0f..b674ec6 100644 --- a/gcc/config/arm/neon.md +++ b/gcc/config/arm/neon.md @@ -43,9 +43,10 @@ (define_insn "*neon_mov" gcc_assert (is_valid != 0); if (width == 0) - return "vmov.f32\t%P0, %1 @ "; + return "vmov.f32\t%P0, %1 @ "; else - sprintf (templ, "vmov.i%d\t%%P0, %%x1 @ ", width); + snprintf (templ, sizeof (templ), + "vmov.i%d\t%%P0, %%x1 @ ", width); return templ; } @@ -88,9 +89,10 @@ (define_insn "*neon_mov" gcc_assert (is_valid != 0); if (width == 0) - return "vmov.f32\t%q0, %1 @ "; + return "vmov.f32\t%q0, %1 @ "; else - sprintf (templ, "vmov.i%d\t%%q0, %%1 @ ", width); + snprintf (templ, sizeof (templ), + "vmov.i%d\t%%q0, %%1 @ ", width); return templ; } @@ -2187,12 +2189,13 @@ (define_insn "neon_vc_insn" && !flag_unsafe_math_optimizations)" { char pattern[100]; - sprintf (pattern, "vc.%s%%#\t%%0," - " %%1, %s", - GET_MODE_CLASS (mode) == MODE_VECTOR_FLOAT - ? "f" : "", - which_alternative == 0 - ? "%2" : "#0"); + snprintf (pattern, sizeof (pattern), + "vc.%s%%#\t%%0," + " %%1, %s", + GET_MODE_CLASS (mode) == MODE_VECTOR_FLOAT + ? "f" : "", + which_alternative == 0 + ? "%2" : "#0"); output_asm_insn (pattern, operands); return ""; } @@ -2211,10 +2214,11 @@ (define_insn "neon_vc_insn_unspec" "TARGET_NEON" { char pattern[100]; - sprintf (pattern, "vc.f%%#\t%%0," - " %%1, %s", - which_alternative == 0 - ? "%2" : "#0"); + snprintf (pattern, sizeof (pattern), + "vc.f%%#\t%%0," + " %%1, %s", + which_alternative == 0 + ? "%2" : "#0"); output_asm_insn (pattern, operands); return ""; }