[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libunwind-devel] Breaking
From: |
Doug Moore |
Subject: |
Re: [Libunwind-devel] Breaking |
Date: |
Wed, 4 Oct 2017 18:35:52 -0500 |
We’re going to ship our next release with a version of libunwind that undoes
this change.
I tried reproducing the bug that the change fixes, but the ‘configure’ command
in the checkin message fails on my x86_64 machine.
Where most who call run_cfi_program call it once, to get from wherever the
function starts to the key ip value, dwarf_reg_states_table_iterate calls it
repeatedly, jumping from one ip value to the next, so skipping that register
state change in the middle of the function screws us up for the rest of the
function, when we save broken register states that we’ll apply later.
If I understood the problem you’re trying to fix, I’d try to offer a solution
that worked for everyone. As it is, the best I can do is suggest another
parameter to run_cfi_program that indicates whether or not this
last-instruction hack should be applied or not.
Doug Moore
> On Oct 3, 2017, at 10:55 AM, Dave Watson <address@hidden> wrote:
>
> 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.