[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Libunwind-devel] [PATCH] Implement DWARF DW_CFA_val_expression for
From: |
Arun Sharma |
Subject: |
Re: [Libunwind-devel] [PATCH] Implement DWARF DW_CFA_val_expression for x86_64 |
Date: |
Sat, 1 Feb 2014 20:06:06 -0800 |
On Fri, Jan 31, 2014 at 3:41 AM, Tim Deegan <address@hidden> wrote:
> Hi,
>
> At 00:35 -0800 on 31 Jan (1391124901), Arun Sharma wrote:
>> On Mon, Jan 20, 2014 at 3:41 AM, Tim Deegan <address@hidden> wrote:
>> > static inline int
>> > @@ -172,9 +183,12 @@ dwarf_put (struct dwarf_cursor *c, dwarf_loc_t loc,
>> > unw_word_t val)
>> > if (DWARF_IS_REG_LOC (loc))
>> > return (*c->as->acc.access_reg) (c->as, DWARF_GET_LOC (loc), &val,
>> > 1, c->as_arg);
>> > - else
>> > + if (DWARF_IS_MEM_LOC (loc))
>> > return (*c->as->acc.access_mem) (c->as, DWARF_GET_LOC (loc), &val,
>> > 1, c->as_arg);
>> > + assert(DWARF_IS_VAL_LOC (loc));
>> > + loc = DWARF_VAL_LOC(c, val);
>>
>> Is this code necessary?
>
> You mean, because nothing ever calls dwarf_put() with a VAL_LOC? I
> suppose not; would you be happy with an assert(!DWARF_IS_VAL_LOC (loc))
> at the head of the function instead?
Perfect.
>
>> > + case DWARF_WHERE_EXPR_VAL:
>>
>> DWARF_WHERE_VAL_EXPR reads better.
>
> Sure; I'll rename that for a v2.
>
>> > + addr = rs->reg[i].val;
>> > + if ((ret = eval_location_expr (c, as, a, addr, c->loc + i, arg,
>> > 1)) < 0)
>> > return ret;
>>
>> This is fine. Another way to do it would be to update the
>> DWARF_LOC_TYPE after the eval() instead of passing in an extra
>> parameter (is_val).
>
> I actually did it that way in my first fix but reworked it for
> upstreaming because I thought this was cleaner. :) Happy to switch
> back if you prefer.
I like your first implementation :) The only thing
eval_location_expr() can do is to tell if the result is in a register
or not. All the other logic to compute dwarf_loc_t belongs elsewhere.
-Arun
- Re: [Libunwind-devel] [PATCH] Implement DWARF DW_CFA_val_expression for x86_64,
Arun Sharma <=