Re: [Libunwind-devel] Breaking

[Dave Watson]

On 10/03/17 03:02 AM, Doug Moore wrote:
> It seems that this change:
> 
> index e8eaeac..9d405e7 100644
> --- a/src/dwarf/Gparser.c
> +++ b/src/dwarf/Gparser.c
> @@ -289,8 +289,10 @@ run_cfi_program (struct dwarf_cursor *c,
> dwarf_state_record_t *sr,
>                ret = -UNW_EINVAL;
>                break;
>              }
> -          memcpy (&sr->rs_current, &(*rs_stack)->state, sizeof
> (sr->rs_current));
> -         pop_rstate_stack(rs_stack);
> +          if (*ip < end_ip) {
> +            memcpy (&sr->rs_current, &(*rs_stack)->state, sizeof
> (sr->rs_current));
> +            pop_rstate_stack(rs_stack);
> +          }
>            Debug (15, "CFA_restore_state\n");
>            break;
> 
> 
> broke unw_apply_reg_state, because updating libunwind to include that change
> broke our software.

> So I'm highly motivated to understand what this fixed, so that I can fix
> unw_apply_reg_state without breaking something.

As far as I can tell, there are some paths through run_cfi_program
that need to *not* apply for the end ip, and some that do.  For
example the following dwarf

DW_CFA_def_cfa_offset: 112
DW_CFA_advance_loc: 5 to ...643
DW_CFA_restore state

should not apply restore_state if the return ip is 643, since the
return ip is one ahead of the calling ip.  Both the libgcc and llvm
unwinders actually have the while loop something more like

             vvvvv
while (*ip < /*=*/ end_ip && *addr < end_addr && ret >= 0)

but that also seems to break some tests.  There are repro steps in the
changelog.

If we need to I'm fine reverting this if we can't find a better fix,
currently the only complaint is multilib tests failing without it.