On Fri, Jan 31, 2014 at 3:41 AM, Tim Deegan <[email protected]> wrote: > Hi, > > At 00:35 -0800 on 31 Jan (1391124901), Arun Sharma wrote: >> On Mon, Jan 20, 2014 at 3:41 AM, Tim Deegan <[email protected]> >> 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 _______________________________________________ Libunwind-devel mailing list [email protected] https://lists.nongnu.org/mailman/listinfo/libunwind-devel
