Szabolcs Nagy <szabolcs.n...@arm.com> writes: > The 05/13/2022 16:35, Richard Sandiford wrote: >> Szabolcs Nagy via Gcc-patches <gcc-patches@gcc.gnu.org> writes: >> > The RA_SIGN_STATE dwarf pseudo-register is normally only set using the >> > DW_CFA_AARCH64_negate_ra_state (== DW_CFA_window_save) operation which >> > toggles the return address signedness state (the default state is 0). >> > (It may be set by remember/restore_state CFI too, those save/restore >> > the state of all registers.) >> > >> > However RA_SIGN_STATE can be set directly via DW_CFA_val_expression too. >> > GCC does not generate such CFI but some other compilers reportedly do. >> > >> > Note: the toggle operation must not be mixed with other dwarf register >> > rule CFI within the same CIE and FDE. >> > >> > In libgcc we assume REG_UNSAVED means the RA_STATE is set using toggle >> > operations, otherwise we assume its value is set by other CFI. >> >> AFAIK, this is the first time I've looked at the RA_SIGN_STATE code, >> so this is probably a naive point/question, but: it seems a bit >> underhand for the existing code to be using REG_UNSAVED and >> loc.offset to hold the toggle state. Would it make sense to add >> a new enum value for known, pre-evaluated constants? _Unwind_GetGR >> would then DTRT for both cases. >> >> That's a comment about the pre-existing code though. I agree this >> patch looks like the right fix if we keep to the current approach. > > yes, this is a hack. i looked at introducing a generic REG_* > enum to deal with RA_SIGN_STATE now, but it's a bit awkward: > > normally frame state for a reg starts out REG_UNSAVED, which > should mean 0 value for the RA_SIGN_STATE pseudo register. > > when moving up frames the uw context gets copied and updated > according to the frame state (where REG_UNSAVED normally means > unmodified copy), this is not right for RA_SIGN_STATE which > should be reset in the absence of related dwarf ops. we can > fix this up in target hooks for update context, but we still > have to special case REG_UNSAVED. > > i think introducing a new REG_CONST does not simplify the > aarch64 target code (we might need further changes to get > a clean solution).
Ah, yeah, the zero reset would still need to be shoe-horned in somehow. OK for the original patch in that case. Thanks, Richard
