Re: [PATCH] gas: Improve documentation for cfi_remember/restore_state

[Alan Modra <amodra at gmail dot com>]

Thanks for doing this.

On Wed, Apr 13, 2016 at 06:49:20PM -0300, Martin Galvan wrote:
> diff --git a/gas/doc/as.texinfo b/gas/doc/as.texinfo
> index 7b36931..01fc17d 100644
> --- a/gas/doc/as.texinfo
> +++ b/gas/doc/as.texinfo
> @@ -4816,11 +4816,39 @@ From now on the previous value of @var{register} can't be restored anymore.
>  Current value of @var{register} is the same like in the previous frame,
>  i.e. no restoration needed.
>  
> -@subsection @code{.cfi_remember_state},
> -First save all current rules for all registers by @code{.cfi_remember_state},
> -then totally screw them up by subsequent @code{.cfi_*} directives and when
> -everything is hopelessly bad, use @code{.cfi_restore_state} to restore
> -the previous saved state.
> +@subsection @code{.cfi_remember_state} and @code{.cfi_restore_state}
> +@code{.cfi_remember_state} pushes the set of rules for every register onto an
> +implicit stack, while @code{.cfi_restore_state} pops them off the stack and
> +places them in the current row.  This is useful for situations where you have
> +@code{.cfi_*} directives that alter the following rows only under certain
> +conditions.  For example, we could have something like this:

Control flow in a program has no effect on interpretation of
debug/eh_frame info.  I think your wording here is giving the
impression that it does..

> +
> +@smallexample
> +        jne  label
> +        addq $42, %rsp
> +        .cfi_adjust_cfa_offset -42
> +        ret
> +label:
> +        /* Do something else */
> +@end smallexample
> +
> +Here, we want the @code{.cfi} directive to affect only the row corresponding to
> +the @code{ret} instruction, but not the code after @code{label}.  To do this we
> +can write:
> +
> +@smallexample
> +        .cfi_remember_state
> +        jne  label
> +        addq $42, %rsp
> +        .cfi_adjust_cfa_offset -42
> +        ret
> +label:
> +        .cfi_restore_state
> +        /* Do something else */
> +@end smallexample
> +
> +That way, the rules for the instructions after @code{label} will be the same
> +as before the @{.cfi_adjust_cfa_offset}.
>  
>  @subsection @code{.cfi_return_column @var{register}}
>  Change return column @var{register}, i.e. the return address is either

I have two suggestions for improving your example.  As I mentioned
before, control flow in a program is not relevant, so the
".cfi_remember_state" does not need to be placed before the "jne".
ie.

@smallexample
        jne  label
        addq $42, %rsp
        .cfi_remember_state
        .cfi_adjust_cfa_offset -42
        ret
label:
        .cfi_restore_state
        /* Do something else */
@end smallexample

works just as well, and encodes better (fewer advance_loc opcodes).

Secondly, this isn't a very good example of code that really needs
".cfi_remember_state".  You could solve the problem in your example
with

@smallexample
        jne  label
        addq $42, %rsp
        .cfi_adjust_cfa_offset -42
        ret
label:
        .cfi_adjust_cfa_offset 42
        /* Do something else */
@end smallexample

giving a DWARF "program" that is the same size but should execute a
little faster when unwinding.  Would you please update your example to
something that does benefit from using .cfi_remember_state?  The
simplest one I can think of is if your example restored a register or
two before adjusting sp, then you might want to describe that with
.cfi_restore.  That then would require a larger DWARF program to
recreate the register save locations if you wanted to do so "by hand"
and thus use of remember_state/restore_state is justified.

-- 
Alan Modra
Australia Development Lab, IBM