On Fri, 2012-12-21 at 14:13 -0800, Roland McGrath wrote:
> + /* Needed if we find pc relative addresses. */
"PC-relative"
Fixed.
> + GElf_Addr bias;
> + dwfl_module_getelf (dwflmod, &bias);
Check for errors?
Yes, it should, just in case.
> + /* pcrel for an FDE address really means data relative. */
Two spaces at the end of a sentence. "data-relative".
Or explain better: "relative to the runtime address of the start_address
field itself".
Added your better description.
> + Dwarf_Addr pc_start = initial_location;
> + if ((fde_encoding & 0x70) == DW_EH_PE_pcrel)
> + pc_start += ((uint64_t) shdr->sh_addr
> + + (base - (const unsigned char *) data->d_buf))
> + - bias;
Put it inside the parens so auto-indentation always works.
Yeah, brackets were wrong.
> + pc_start &= (ptr_size == 4
> + ? UINT64_C (0xffffffff)
> + : UINT64_C (0xffffffffffffffff));
Why is this necessary? It needs a comment.
That was a little obscure sorry. What I should have done and do now in
the updated patch is to sign extend the possible truncated pointer value
before the calculation. Hopefully that way, with an extra comment, it is
clearer what is going on.
Thanks,
Mark