Hi Torbjörn! On Thu, 6 Jun 2024 at 18:47, Torbjörn SVENSSON <torbjorn.svens...@foss.st.com> wrote: > > I would like to push this patch to the following branches: > > - releases/gcc-11 > - releases/gcc-12 > - releases/gcc-13 > - releases/gcc-14 > - trunk > > Ok? > > The problem was highlighted by https://linaro.atlassian.net/browse/GNU-1239 > > -- > > Properly handle zero and sign extension for Armv8-M.baseline as > Cortex-M23 can have the security extension active. > Currently, there is a internal compiler error on Cortex-M23 for the > epilog processing of sign extension. > > This patch addresses the following CVE-2024-0151 for Armv8-M.baseline. > > gcc/ChangeLog: > > * config/arm/arm.cc (cmse_nonsecure_call_inline_register_clear): > Sign extend for Thumb1. > (thumb1_expand_prologue): Add zero/sign extend.
Quick nitpicking: I think the ICE you are fixing was reported as https://linaro.atlassian.net/browse/GNU-1205 (GNU-1239 is about your test improvements failing too, in addition to the existing ones) and your patch is actually about fixing GCC bug report 115253. So your commit title should end with "[PR115253]" (or maybe "PR target/115253") and your ChangeLog should also contain "PR target/115253". You can use contrib/git_check_commit.py to check your patch is correctly formatted (otherwise it will be rejected by the commit hooks anyway). I haven't looked into the details of the patch yet :-) Thanks for looking at this, Christophe > > Signed-off-by: Torbjörn SVENSSON <torbjorn.svens...@foss.st.com> > Co-authored-by: Yvan ROUX <yvan.r...@foss.st.com> > --- > gcc/config/arm/arm.cc | 68 ++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 60 insertions(+), 8 deletions(-) > > diff --git a/gcc/config/arm/arm.cc b/gcc/config/arm/arm.cc > index ea0c963a4d6..077cb61f42a 100644 > --- a/gcc/config/arm/arm.cc > +++ b/gcc/config/arm/arm.cc > @@ -19220,17 +19220,23 @@ cmse_nonsecure_call_inline_register_clear (void) > || TREE_CODE (ret_type) == BOOLEAN_TYPE) > && known_lt (GET_MODE_SIZE (TYPE_MODE (ret_type)), 4)) > { > - machine_mode ret_mode = TYPE_MODE (ret_type); > + rtx ret_mode = gen_rtx_REG (TYPE_MODE (ret_type), R0_REGNUM); > + rtx si_mode = gen_rtx_REG (SImode, R0_REGNUM); > rtx extend; > if (TYPE_UNSIGNED (ret_type)) > - extend = gen_rtx_ZERO_EXTEND (SImode, > - gen_rtx_REG (ret_mode, > R0_REGNUM)); > + extend = gen_rtx_SET (si_mode, gen_rtx_ZERO_EXTEND (SImode, > + > ret_mode)); > + else if (TARGET_THUMB1) > + { > + if (known_lt (GET_MODE_SIZE (TYPE_MODE (ret_type)), 2)) > + extend = gen_thumb1_extendqisi2 (si_mode, ret_mode); > + else > + extend = gen_thumb1_extendhisi2 (si_mode, ret_mode); > + } > else > - extend = gen_rtx_SIGN_EXTEND (SImode, > - gen_rtx_REG (ret_mode, > R0_REGNUM)); > - emit_insn_after (gen_rtx_SET (gen_rtx_REG (SImode, R0_REGNUM), > - extend), insn); > - > + extend = gen_rtx_SET (si_mode, gen_rtx_SIGN_EXTEND (SImode, > + > ret_mode)); > + emit_insn_after (extend, insn); > } > > > @@ -27250,6 +27256,52 @@ thumb1_expand_prologue (void) > live_regs_mask = offsets->saved_regs_mask; > lr_needs_saving = live_regs_mask & (1 << LR_REGNUM); > > + /* The AAPCS requires the callee to widen integral types narrower > + than 32 bits to the full width of the register; but when handling > + calls to non-secure space, we cannot trust the callee to have > + correctly done so. So forcibly re-widen the result here. */ > + if (IS_CMSE_ENTRY (func_type)) > + { > + function_args_iterator args_iter; > + CUMULATIVE_ARGS args_so_far_v; > + cumulative_args_t args_so_far; > + bool first_param = true; > + tree arg_type; > + tree fndecl = current_function_decl; > + tree fntype = TREE_TYPE (fndecl); > + arm_init_cumulative_args (&args_so_far_v, fntype, NULL_RTX, fndecl); > + args_so_far = pack_cumulative_args (&args_so_far_v); > + FOREACH_FUNCTION_ARGS (fntype, arg_type, args_iter) > + { > + rtx arg_rtx; > + > + if (VOID_TYPE_P (arg_type)) > + break; > + > + function_arg_info arg (arg_type, /*named=*/true); > + if (!first_param) > + /* We should advance after processing the argument and pass > + the argument we're advancing past. */ > + arm_function_arg_advance (args_so_far, arg); > + first_param = false; > + arg_rtx = arm_function_arg (args_so_far, arg); > + gcc_assert (REG_P (arg_rtx)); > + if ((TREE_CODE (arg_type) == INTEGER_TYPE > + || TREE_CODE (arg_type) == ENUMERAL_TYPE > + || TREE_CODE (arg_type) == BOOLEAN_TYPE) > + && known_lt (GET_MODE_SIZE (GET_MODE (arg_rtx)), 4)) > + { > + rtx res_reg = gen_rtx_REG (SImode, REGNO(arg_rtx)); > + if (TYPE_UNSIGNED (arg_type)) > + emit_set_insn (res_reg, gen_rtx_ZERO_EXTEND (SImode, > arg_rtx)); > + else if (known_lt (GET_MODE_SIZE (GET_MODE (arg_rtx)), 2)) > + emit_insn (gen_thumb1_extendqisi2 (res_reg, arg_rtx)); > + else > + emit_insn (gen_thumb1_extendhisi2 (res_reg, arg_rtx)); > + } > + } > + } > + > /* Extract a mask of the ones we can give to the Thumb's push instruction. > */ > l_mask = live_regs_mask & 0x40ff; > /* Then count how many other high registers will need to be pushed. */ > -- > 2.25.1 >
