The 01/03/2023 17:27, Wilco Dijkstra wrote: > > > Also, if I understood correctly, the reason we use REG_UNSAVED is to > > ensure that state from one frame isn't carried across to a parent frame, > > in cases where the parent frame lacks any signing. That is, each frame > > should start out with a zero bit even if a child frame is unwound while > > it has a set bit. > > This works fine since all registers are initialized to REG_UNSAVED every > frame. > > In v2 I've removed some clutter and encode the signing state in REG_UNSAVED/ > REG_UNDEFINED.
this looks good to me. > @@ -1206,8 +1205,10 @@ execute_cfa_program (const unsigned char *insn_ptr, > /* This CFA is multiplexed with Sparc. On AArch64 it's used to > toggle > return address signing status. */ > reg = DWARF_REGNUM_AARCH64_RA_STATE; > - gcc_assert (fs->regs.how[reg] == REG_UNSAVED); > - fs->regs.reg[reg].loc.offset ^= 1; > + if (fs->regs.how[reg] == REG_UNSAVED) > + fs->regs.how[reg] = REG_UNDEFINED; > + else > + fs->regs.how[reg] = REG_UNSAVED; > #else > /* ??? Hardcoded for SPARC register window configuration. */ > if (__LIBGCC_DWARF_FRAME_REGISTERS__ >= 32) i would keep the assert: how[reg] must be either UNSAVED or UNDEFINED here, other how[reg] means the toggle cfi instruction is mixed with incompatible instructions for the pseudo reg. and i would add a comment about this e.g. saying that UNSAVED/UNDEFINED how[reg] is used for tracking the return address signing status and other how[reg] is not allowed here. otherwise the patch looks good.
