Qing Zhao <qing.z...@oracle.com> writes: > @@ -4959,6 +4963,52 @@ handle_no_split_stack_attribute (tree *node, tree name, > return NULL_TREE; > } > > +/* Handle a "zero_call_used_regs" attribute; arguments as in > + struct attribute_spec.handler. */ > + > +static tree > +handle_zero_call_used_regs_attribute (tree *node, tree name, tree args, > + int ARG_UNUSED (flags), > + bool *no_add_attris)
s/attris/attrs/ > +{ > + tree decl = *node; > + tree id = TREE_VALUE (args); > + > + if (TREE_CODE (decl) != FUNCTION_DECL) > + { > + error_at (DECL_SOURCE_LOCATION (decl), > + "%qE attribute applies only to functions", name); > + *no_add_attris = true; > + return NULL_TREE; > + } > + > + if (TREE_CODE (id) != STRING_CST) > + { > + error ("attribute %qE arguments not a string", name); > + *no_add_attris = true; > + return NULL_TREE; > + } > + > + if ((strcmp (TREE_STRING_POINTER (id), "skip") != 0) > + && (strcmp (TREE_STRING_POINTER (id), "used-gpr-arg") != 0) > + && (strcmp (TREE_STRING_POINTER (id), "used-arg") != 0) > + && (strcmp (TREE_STRING_POINTER (id), "all-arg") != 0) > + && (strcmp (TREE_STRING_POINTER (id), "used-gpr") != 0) > + && (strcmp (TREE_STRING_POINTER (id), "all-gpr") != 0) > + && (strcmp (TREE_STRING_POINTER (id), "used") != 0) > + && (strcmp (TREE_STRING_POINTER (id), "all") != 0)) Any reason we don't support all-gpr-arg? Seems to be the only “missing” combination. Would be good to have a single piece of code that parses these arguments into a set of flags, rather than have one list here and one get_call_used_regs_seq. Maybe we could do something similar to sanitizer_opts, but that might not be necessary. > + { > + error ("attribute %qE argument must be one of %qs, %qs, %qs, %qs," > + "%qs, %qs, %qs, or %qs", > + name, "skip", "used-gpr-arg", "used-arg", "all-arg", > + "used-gpr", "all-gpr", "used", "all"); > + *no_add_attris = true; > + return NULL_TREE; > + } > + > + return NULL_TREE; > +} > + > /* Handle a "returns_nonnull" attribute; arguments as in > struct attribute_spec.handler. */ > > diff --git a/gcc/coretypes.h b/gcc/coretypes.h > index 6b6cfcd..0ce5eb4 100644 > --- a/gcc/coretypes.h > +++ b/gcc/coretypes.h > @@ -418,6 +418,19 @@ enum symbol_visibility > VISIBILITY_INTERNAL > }; > > +/* Zero call-used registers type. */ > +enum zero_call_used_regs { > + zero_call_used_regs_unset = 0, > + zero_call_used_regs_skip, > + zero_call_used_regs_used_gpr_arg, > + zero_call_used_regs_used_arg, > + zero_call_used_regs_all_arg, > + zero_call_used_regs_used_gpr, > + zero_call_used_regs_all_gpr, > + zero_call_used_regs_used, > + zero_call_used_regs_all > +}; I think a bitmask would be easier to use: SKIP ONLY_USED ONLY_GPR ONLY_ARG Should probably be a class enum given that we're C++11. > +/* Return true if REGNO is used by the epilogue. */ > +bool > +df_epilogue_uses_p (unsigned int regno) > +{ > + return (EPILOGUE_USES (regno) > + || TEST_HARD_REG_BIT (crtl->zeroed_reg_set, regno)); Nit: the { … } body should be indented by two spaces rather than four. > diff --git a/gcc/df.h b/gcc/df.h > index 8b6ca8c..0f098d7 100644 > --- a/gcc/df.h > +++ b/gcc/df.h > @@ -1085,6 +1085,7 @@ extern void df_update_entry_exit_and_calls (void); > extern bool df_hard_reg_used_p (unsigned int); > extern unsigned int df_hard_reg_used_count (unsigned int); > extern bool df_regs_ever_live_p (unsigned int); > +extern bool df_epilogue_uses_p (unsigned int); > extern void df_set_regs_ever_live (unsigned int, bool); > extern void df_compute_regs_ever_live (bool); > extern void df_scan_verify (void); > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > index c9f7299..f56f61a 100644 > --- a/gcc/doc/extend.texi > +++ b/gcc/doc/extend.texi > @@ -3992,6 +3992,30 @@ performing a link with relocatable output (i.e.@: > @code{ld -r}) on them. > A declaration to which @code{weakref} is attached and that is associated > with a named @code{target} must be @code{static}. > > +@item zero_call_used_regs ("@var{choice}") > +@cindex @code{zero_call_used_regs} function attribute > + > +The @code{zero_call_used_regs} attribute causes the compiler to zero > +call-used registers at function return according to @var{choice}. > +This is used to increase the program security by either mitigating > +Return-Oriented Programming (ROP) or preventing information leak > +through registers. > +@samp{skip} doesn't zero call-used registers. > + > +@samp{used-arg-gpr} zeros used call-used general purpose registers that used-gpr-arg > +pass parameters. @samp{used-arg} zeros used call-used registers that > +pass parameters. @samp{arg} zeros all call-used registers that pass > +parameters. These 3 choices are used for ROP mitigation. > + > +@samp{used-gpr} zeros call-used general purpose registers > +which are used in function. @samp{all-gpr} zeros all > +call-used registers. @samp{used} zeros call-used registers which > +are used in function. @samp{all} zeros all call-used registers. > +These 4 choices are used for preventing information leak through > +registers. The description for all-gpr doesn't look right. I think it would be easier to describe (and hopefully to follow) if we start with the three basic choices: “skip”, “used” and “all”. Then describe how “used” and “all” can be modified by adding “-gpr” to limit the clearing to general-purpose registers and “-arg” to limit the clearing to argument registers. We need to say what “call-used” and “used” mean in this context. In particular, “call-used” is also known as “call-clobbered”, “caller-saved“ and “volatile”, so it would be good to list those as alternatives. We need to say what “used” registers are. > @@ -12550,6 +12550,29 @@ int foo (void) > > Not all targets support this option. > > +@item -fzero-call-used-regs=@var{choice} > +@opindex fzero-call-used-regs > +Zero call-used registers at function return to increase the program > +security by either mitigating Return-Oriented Programming (ROP) or > +preventing information leak through registers. > + > +@samp{skip}, which is the default, doesn't zero call-used registers. > + > +@samp{used-gpr-arg} zeros used call-used general purpose registers that > +pass parameters. @samp{used-arg} zeros used call-used registers that > +pass parameters. @samp{all-arg} zeros all call-used registers that pass > +parameters. These 3 choices are used for ROP mitigation. > + > +@samp{used-gpr} zeros call-used general purpose registers > +which are used in function. @samp{all-gpr} zeros all > +call-used registers. @samp{used} zeros call-used registers which > +are used in function. @samp{all} zeros all call-used registers. > +These 4 choices are used for preventing information leak through > +registers. Same comment here. > @@ -310,6 +310,9 @@ struct GTY(()) rtl_data { > sets them. */ > HARD_REG_SET asm_clobbers; > > + /* All hard registers that are zeroed at the return of the routine. */ > + HARD_REG_SET zeroed_reg_set; How about “must_be_zero_on_return“? “zeroed_reg_set” isn't very specific about where the zeroing happens or is needed. E.g. we also zero uninitialised registers. > + > /* The highest address seen during shorten_branches. */ > int max_insn_address; > }; > diff --git a/gcc/function.c b/gcc/function.c > index c612959..c8181bd 100644 > --- a/gcc/function.c > +++ b/gcc/function.c > @@ -50,6 +50,7 @@ along with GCC; see the file COPYING3. If not see > #include "emit-rtl.h" > #include "recog.h" > #include "rtl-error.h" > +#include "hard-reg-set.h" > #include "alias.h" > #include "fold-const.h" > #include "stor-layout.h" > @@ -5815,6 +5816,182 @@ make_prologue_seq (void) > return seq; > } > > +/* Check whether the hard register REGNO is live before the return insn RET. > */ > +static bool > +is_live_reg_at_return (unsigned int regno, rtx_insn * ret) Nit: should be no space after “*”. > +{ > + basic_block bb = BLOCK_FOR_INSN (ret); > + auto_bitmap live_out; > + bitmap_copy (live_out, df_get_live_out (bb)); Sorry, forgot that here we should do: df_simulate_initialize_backwards (bb, live_out); But we should calculate this set once per return instruction rather than repeat the calculation for every register. > + df_simulate_one_insn_backwards (bb, ret, live_out); > + > + if (REGNO_REG_SET_P (live_out, regno)) > + return true; > + > + return false; > +} > + > +/* Emit a sequence of insns to zero the call-used-registers before RET. */ > + > +static void > +gen_call_used_regs_seq (rtx_insn *ret) > +{ > + bool gpr_only = true; > + bool used_only = true; > + bool arg_only = true; > + enum zero_call_used_regs zero_regs_type = zero_call_used_regs_unset; > + enum zero_call_used_regs attr_zero_regs_type > + = zero_call_used_regs_unset; > + tree attr_zero_regs > + = lookup_attribute ("zero_call_used_regs", > + DECL_ATTRIBUTES (cfun->decl)); > + > + /* Get the type of zero_call_used_regs from function attribute. */ > + if (attr_zero_regs) > + { > + /* The TREE_VALUE of an attribute is a TREE_LIST whose TREE_VALUE > + is the attribute argument's value. */ > + attr_zero_regs = TREE_VALUE (attr_zero_regs); > + gcc_assert (TREE_CODE (attr_zero_regs) == TREE_LIST); > + attr_zero_regs = TREE_VALUE (attr_zero_regs); > + gcc_assert (TREE_CODE (attr_zero_regs) == STRING_CST); > + > + if (strcmp (TREE_STRING_POINTER (attr_zero_regs), "skip") == 0) > + attr_zero_regs_type = zero_call_used_regs_skip; > + else if (strcmp (TREE_STRING_POINTER (attr_zero_regs), "used-gpr-arg") > + == 0) > + attr_zero_regs_type = zero_call_used_regs_used_gpr_arg; > + else if (strcmp (TREE_STRING_POINTER (attr_zero_regs), "used-arg") == > 0) > + attr_zero_regs_type = zero_call_used_regs_used_arg; > + else if (strcmp (TREE_STRING_POINTER (attr_zero_regs), "all-arg") == 0) > + attr_zero_regs_type = zero_call_used_regs_all_arg; > + else if (strcmp (TREE_STRING_POINTER (attr_zero_regs), "used-gpr") == > 0) > + attr_zero_regs_type = zero_call_used_regs_used_gpr; > + else if (strcmp (TREE_STRING_POINTER (attr_zero_regs), "all-gpr") == 0) > + attr_zero_regs_type = zero_call_used_regs_all_gpr; > + else if (strcmp (TREE_STRING_POINTER (attr_zero_regs), "used") == 0) > + attr_zero_regs_type = zero_call_used_regs_used; > + else if (strcmp (TREE_STRING_POINTER (attr_zero_regs), "all") == 0) > + attr_zero_regs_type = zero_call_used_regs_all; > + else > + gcc_assert (0); > + } > + > + if (flag_zero_call_used_regs) > + if (!attr_zero_regs) > + zero_regs_type = flag_zero_call_used_regs; > + else > + zero_regs_type = attr_zero_regs_type; > + else > + zero_regs_type = attr_zero_regs_type; > + > + /* No need to zero call-used-regs when no user request is present. */ > + if (zero_regs_type <= zero_call_used_regs_skip) > + return; > + > + /* No need to zero call-used-regs in main (). */ > + if (MAIN_NAME_P (DECL_NAME (current_function_decl))) > + return; > + > + /* No need to zero call-used-regs if __builtin_eh_return is called > + since it isn't a normal function return. */ > + if (crtl->calls_eh_return) > + return; > + > + /* If gpr_only is true, only zero call-used-registers that are > + general-purpose registers; if used_only is true, only zero > + call-used-registers that are used in the current function. */ > + > + switch (zero_regs_type) > + { > + case zero_call_used_regs_used_arg: > + gpr_only = false; > + break; > + case zero_call_used_regs_all_arg: > + gpr_only = false; > + used_only = false; > + break; > + case zero_call_used_regs_used_gpr: > + arg_only = false; > + break; > + case zero_call_used_regs_all_gpr: > + used_only = false; > + arg_only = false; > + break; > + case zero_call_used_regs_used: > + gpr_only = false; > + arg_only = false; > + break; > + case zero_call_used_regs_all: > + gpr_only = false; > + used_only = false; > + arg_only = false; > + break; > + default: > + break; > + } Using a bitmask would also simplify this. > + > + /* For each of the hard registers, check to see whether we should zero it > if: > + 1. it is a call-used-registers; > + and 2. it is not a fixed-registers; > + and 3. it is not live at the return of the routine; > + and 4. it is general registor if gpr_only is true; > + and 5. it is used in the routine if used_only is true; > + and 6. it is a register that passes parameter if arg_only is true; > + */ > + > + HARD_REG_SET need_zeroed_hardregs; > + CLEAR_HARD_REG_SET (need_zeroed_hardregs); > + for (unsigned int regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++) > + { > + if (!this_target_hard_regs->x_call_used_regs[regno]) > + continue; This should use crtl->abi instead. The set of call-used registers can vary from function to function. > + if (fixed_regs[regno]) > + continue; > + if (is_live_reg_at_return (regno, ret)) > + continue; > + if (gpr_only > + && !TEST_HARD_REG_BIT (reg_class_contents[GENERAL_REGS], regno)) > + continue; > + if (used_only && !df_regs_ever_live_p (regno)) > + continue; > + if (arg_only && !FUNCTION_ARG_REGNO_P (regno)) > + continue; > + > + /* Now this is a register that we might want to zero. */ > + SET_HARD_REG_BIT (need_zeroed_hardregs, regno); > + } > + > + if (hard_reg_set_empty_p (need_zeroed_hardregs)) > + return; > + > + /* Now we get a hard register set that need to be zeroed, pass it to > + target to generate zeroing sequence. */ > + HARD_REG_SET zeroed_hardregs; > + start_sequence (); > + zeroed_hardregs = targetm.calls.zero_call_used_regs (need_zeroed_hardregs); > + rtx_insn *seq = get_insns (); > + end_sequence (); > + if (seq) > + { > + /* emit the memory blockage and register clobber asm volatile before Nit: “Emit” > + the whole sequence. */ > + start_sequence (); > + expand_asm_reg_clobber_mem_blockage (zeroed_hardregs); > + rtx_insn *seq_barrier = get_insns (); > + end_sequence (); > + > + emit_insn_before (seq_barrier, ret); > + emit_insn_before (seq, ret); > + > + /* update the data flow information. */ “Update” > + crtl->zeroed_reg_set |= zeroed_hardregs; > + df_set_bb_dirty (EXIT_BLOCK_PTR_FOR_FN (cfun)); > + } > + return; GCC style is to avoid returns at the end of void functions. > +} > + > + > /* Return a sequence to be used as the epilogue for the current function, > or NULL. */ > > @@ -6486,7 +6663,75 @@ make_pass_thread_prologue_and_epilogue (gcc::context > *ctxt) > { > return new pass_thread_prologue_and_epilogue (ctxt); > } > - > > + > +static unsigned int > +rest_of_zero_call_used_regs (void) > +{ > + basic_block bb; > + rtx_insn *insn; > + > + /* This pass needs data flow information. */ > + df_analyze (); > + > + /* Search all the "return"s in the routine, and insert instruction > sequence to > + zero the call used registers. */ > + FOR_EACH_BB_REVERSE_FN (bb, cfun) > + if (bb == EXIT_BLOCK_PTR_FOR_FN (cfun) > + || (single_succ_p (bb) > + && single_succ (bb) == EXIT_BLOCK_PTR_FOR_FN (cfun))) > + FOR_BB_INSNS_REVERSE (bb, insn) > + if (JUMP_P (insn) && ANY_RETURN_P (JUMP_LABEL (insn))) > + { > + /* Now we can insert the instruction sequence to zero the call used > + registers before this insn. */ > + gen_call_used_regs_seq (insn); > + break; > + } The exit block never has instructions, so it's only necessary to process predecessors. A simpler way to do that is to iterate over the edges in: EXIT_BLOCK_PTR_FOR_FN (cfun)->preds You shouldn't need to use FOR_BB_INSNS_REVERSE: it should be enough to check only BB_END (bb), since returns always end blocks. > + > + return 0; > +} > + > +namespace { > + > +const pass_data pass_data_zero_call_used_regs = > +{ > + RTL_PASS, /* type */ > + "zero_call_used_regs", /* name */ > + OPTGROUP_NONE, /* optinfo_flags */ > + TV_NONE, /* tv_id */ > + 0, /* properties_required */ > + 0, /* properties_provided */ > + 0, /* properties_destroyed */ > + 0, /* todo_flags_start */ > + 0, /* todo_flags_finish */ > +}; > + > +class pass_zero_call_used_regs: public rtl_opt_pass > +{ > +public: > + pass_zero_call_used_regs (gcc::context *ctxt) > + : rtl_opt_pass (pass_data_zero_call_used_regs, ctxt) > + {} > + > + /* opt_pass methods: */ > + virtual bool gate (function *) > + { > + return flag_zero_call_used_regs > zero_call_used_regs_unset; I think this also needs to check the function attributes. > + } > + virtual unsigned int execute (function *) > + { > + return rest_of_zero_call_used_regs (); > + } > + > +}; // class pass_zero_call_used_regs > + > +} // anon namespace > + > +rtl_opt_pass * > +make_pass_zero_call_used_regs (gcc::context *ctxt) > +{ > + return new pass_zero_call_used_regs (ctxt); > +} > > /* If CONSTRAINT is a matching constraint, then return its number. > Otherwise, return -1. */ > diff --git a/gcc/optabs.c b/gcc/optabs.c > index 8ad7f4b..57e5c5d 100644 > --- a/gcc/optabs.c > +++ b/gcc/optabs.c > @@ -6484,6 +6484,49 @@ expand_memory_blockage (void) > expand_asm_memory_blockage (); > } > > +/* Generate asm volatile("" : : : "memory") as a memory blockage, at the > + same time clobbering the register set specified by ZEROED_REGS. */ > + > +void > +expand_asm_reg_clobber_mem_blockage (HARD_REG_SET zeroed_regs) Just “regs”: the interface is more general than registers that are being zeroed. > +{ > + rtx asm_op, clob_mem, clob_reg; > + > + unsigned int num_of_regs = 0; > + for (unsigned int i = 0; i < FIRST_PSEUDO_REGISTER; i++) > + if (TEST_HARD_REG_BIT (zeroed_regs, i)) > + num_of_regs++; > + > + if (num_of_regs == 0) > + return; For this interface, I think we should continue and just include a memory clobber. > + > + asm_op = gen_rtx_ASM_OPERANDS (VOIDmode, "", "", 0, > + rtvec_alloc (0), rtvec_alloc (0), > + rtvec_alloc (0), UNKNOWN_LOCATION); > + MEM_VOLATILE_P (asm_op) = 1; > + > + rtvec v = rtvec_alloc (num_of_regs + 2); > + > + clob_mem = gen_rtx_SCRATCH (VOIDmode); > + clob_mem = gen_rtx_MEM (BLKmode, clob_mem); > + clob_mem = gen_rtx_CLOBBER (VOIDmode, clob_mem); > + > + RTVEC_ELT (v,0) = asm_op; > + RTVEC_ELT (v,1) = clob_mem; > + > + unsigned int j = 2; > + for (unsigned int i = 0; i < FIRST_PSEUDO_REGISTER; i++) > + if (TEST_HARD_REG_BIT (zeroed_regs, i)) > + { > + clob_reg = gen_rtx_CLOBBER (VOIDmode, regno_reg_rtx[i]); Nit: should just be one space before “=”. However… > + RTVEC_ELT (v,j) = clob_reg; …IMO it would be more readable as just: RTVEC_ELT (v, j) = gen_rtx_CLOBBER (VOIDmode, regno_reg_rtx[i]); > diff --git a/gcc/recog.c b/gcc/recog.c > index ce83b7f..472c2dc 100644 > --- a/gcc/recog.c > +++ b/gcc/recog.c > @@ -923,6 +923,21 @@ validate_simplify_insn (rtx_insn *insn) > return ((num_changes_pending () > 0) && (apply_change_group () > 0)); > } > > > + > +bool > +valid_insn_p (rtx_insn *insn) This should have a function comment. E.g.: /* Check whether INSN matches a specific alternative of an .md pattern. */ > +{ > + recog_memoized (insn); > + if (INSN_CODE (insn) < 0) > + return false; > + extract_insn (insn); > + /* We don't know whether the insn will be in code that is optimized > + for size or speed, so consider all enabled alternatives. */ > + if (!constrain_operands (1, get_enabled_alternatives (insn))) > + return false; > + return true; > +} > + > /* Return 1 if OP is a valid general operand for machine mode MODE. > This is either a register reference, a memory reference, > or a constant. In the case of a memory reference, the address > diff --git a/gcc/target.def b/gcc/target.def > index ed2da15..7d6807d 100644 > --- a/gcc/target.def > +++ b/gcc/target.def > @@ -5080,6 +5080,19 @@ argument list due to stack realignment. Return > @code{NULL} if no DRAP\n\ > is needed.", > rtx, (void), NULL) > > +/* Generate instruction sequence to zero call used registers. */ > +DEFHOOK > +(zero_call_used_regs, > + "This target hook emits instructions to zero registers specified\n\ > +by @var{need_zeroed_hardregs} at function return, at the same time\n\ > +return the hard register set that are actually zeroed by the hook\n\ > +Define this hook if the target has more effecient instructions to\n\ > +zero call-used registers, or if the target only tries to zero a subset\n\ > +of @var{need_zeroed_hardregs}.\n\ > +If the hook is not defined, the default_zero_call_used_reg will be used.", > + HARD_REG_SET, (HARD_REG_SET need_zeroed_hardregs), I'd suggest: "Emit instructions to zero the subset of @var{selected_regs} that\n\ could conceivably contain values that are useful to an attacker.\n\ Return the set of registers that were actually cleared.\n\ \n\ The default implementation uses normal move instructions to zero\n\ all the registers in @var{selected_regs}. Define this hook if the\n\ target has more efficient ways of zeroing certain registers,\n\ or if you believe that certain registers would never contain\n\ values that are useful to an attacker." with the parameter called “selected_regs” instead of “need_zeroed_hardregs”. (“need” suggests that the target doesn't have the option of not zeroing.) > +default_zero_call_used_regs) > + > /* Return true if all function parameters should be spilled to the > stack. */ > DEFHOOK > diff --git a/gcc/targhooks.c b/gcc/targhooks.c > index 5d94fce..2318c324 100644 > --- a/gcc/targhooks.c > +++ b/gcc/targhooks.c > @@ -56,6 +56,9 @@ along with GCC; see the file COPYING3. If not see > #include "tree-ssa-alias.h" > #include "gimple-expr.h" > #include "memmodel.h" > +#include "backend.h" > +#include "emit-rtl.h" > +#include "df.h" > #include "tm_p.h" > #include "stringpool.h" > #include "tree-vrp.h" > @@ -987,6 +990,38 @@ default_function_value_regno_p (const unsigned int regno > ATTRIBUTE_UNUSED) > #endif > } > > +/* The default hook for TARGET_ZERO_CALL_USED_REGS. */ > + > +HARD_REG_SET > +default_zero_call_used_regs (HARD_REG_SET need_zeroed_hardregs) > +{ > + HARD_REG_SET zeroed_hardregs; > + gcc_assert (!hard_reg_set_empty_p (need_zeroed_hardregs)); > + > + CLEAR_HARD_REG_SET (zeroed_hardregs); > + for (unsigned int regno = 0; regno < FIRST_PSEUDO_REGISTER; regno++) > + if (TEST_HARD_REG_BIT (need_zeroed_hardregs, regno)) > + { > + rtx_insn *last_insn = get_last_insn (); > + machine_mode mode = GET_MODE (regno_reg_rtx[regno]); > + rtx zero = CONST0_RTX (mode); > + rtx_insn *insn = emit_move_insn (regno_reg_rtx[regno], zero); > + if (!valid_insn_p (insn)) > + { > + static bool issued_error; > + if (!issued_error) > + { > + issued_error = true; > + sorry ("-fzero-call-used-regs not supported on this target"); Should be "%qs not supported on this target", with the option name as a second argument. > + } > + delete_insns_since (last_insn); > + } > + else > + SET_HARD_REG_BIT (zeroed_hardregs, regno); > + } > + return zeroed_hardregs; I don't think it's worth building up a different return set. The only time it's different from need_zeroed_hardregs is if we emit the sorry, which will cause compilation to fail anyway. Sorry, I ran out of time to review the tests, but the code part otherwise looks good. Thanks, Richard