On Sat, 2016-01-23 at 21:39 +0100, Mark Wielaard wrote:
On Wed, Jan 20, 2016 at 03:14:16PM -0800, Roland McGrath wrote:
> > Other addresses in this code are printed with ("%#" PRIx64).
> > (AFAIK '#' here will always give "0x", but lets keep it
> %#x prints "0" for zero (not "0x0"). Indeed it is the right
thing to use here.
> > It's also curious that nothing is done with op1 after printing it.
> > Shouldn't this set pc, at least?
> I think it should, but that is unrelated to this change.
> Arguably, it should be:
> get_uleb128 (op1, readp, endp);
> op1 *= code_align;
> printf (" set_loc %#" PRIx64 " to %#" PRIx64
> op1, pc = vma_base + op1);
> so that it displays the literal value (like the other cases do) as well as
> the final value (which in this case is just with VMA_BASE added in).
I agree with using %# for printing the address and setting the pc using
vma_base and op1. But the argument of a DW_CFA_set_loc is an address, not
a uleb128 and it should not be adjusted by code_align, which is only used
for CFA_advance operations as far as I understand it. To read an address
we need to know the FDE address encoding and use read_encoded (which
isn't defined till a bit later, so we need to move it up).
So, no comments about this at all? Sounds sane, sounds insane?
What if I attach a full patch and threaten to commit it?