Richard Earnshaw <richard.earns...@foss.arm.com> writes: > On 06/12/2022 15:46, Andrea Corallo wrote: >> Hi Richard, >> thanks for reviewing. >> Just one clarification before I complete the respin of this patch. >> Richard Earnshaw <richard.earns...@foss.arm.com> writes: >> [...] >> >>> Also, I think (out of an abundance of caution) we really need a >>> scheduling barrier placed before calls to gen_aut_nop() pattern is >>> emitted, to ensure that the scheduler never tries to move this >>> instruction away from the position we place it. Use gen_blockage() >>> for that (see TARGET_SCHED_PROLOG). Alternatively, we could make the >>> UNSPEC_PAC_NOP an unspec_volatile, which has the same effect (IIRC) >>> without needing an additional insn - if you use this approach, then >>> please make sure this is explained in a comment. >>> >>> +(define_insn "pacbti_nop" >>> + [(set (reg:SI IP_REGNUM) >>> + (unspec:SI [(reg:SI SP_REGNUM) (reg:SI LR_REGNUM)] >>> + UNSPEC_PACBTI_NOP))] >>> + "arm_arch8m_main" >>> + "pacbti\t%|ip, %|lr, %|sp" >>> + [(set_attr "conds" "unconditional")]) >>> >>> The additional side-effect of this being a BTI landing pad means that >>> we mustn't move any other instruction before it. So I think this >>> needs to be an unspec_volatile as well. >> IIUC from this we want to make all the three (UNSPEC_PAC_NOP, >> UNSPEC_PACBTI_NOP, UNSPEC_AUT_NOP) unspec volatile, correct? > > UNSPEC_PAC_NOP doesn't need to be volatile. The register constraints > will be enough to ensure it is run before any instruction that > consumes the result it produces. > > UNSPEC_PAC_BTI_NOP needs to be volatile, as it's essential that when > we have an instruction (for example ldr r3, [r3]) early in the program > that doesn't interact with the prologue then it cannot be migrated > before the BTI as the BTI is a landing pad and must be the first > instruction in the function. This is why UNSPEC_BTI_NOP is volatile. > > UNSPEC_AUT_NOP must be volatile because we want to ensure that no > instruction is moved after this one and before the return as that > might expose a ROP gadget to hackers. > > R.
Understood now, thanks. Andrea
