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 consistent). > > %#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 "\n", > 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 I believe the full change should be: diff --git a/src/readelf.c b/src/readelf.c index 0db192e..a25e4ac 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -5044,11 +5044,68 @@ register_info (Ebl *ebl, unsigned int regno, const Ebl_Register_Location *loc, return set; } +static const unsigned char * +read_encoded (unsigned int encoding, const unsigned char *readp, + const unsigned char *const endp, uint64_t *res, Dwarf *dbg) +{ + if ((encoding & 0xf) == DW_EH_PE_absptr) + encoding = gelf_getclass (dbg->elf) == ELFCLASS32 + ? DW_EH_PE_udata4 : DW_EH_PE_udata8; + + switch (encoding & 0xf) + { + case DW_EH_PE_uleb128: + get_uleb128 (*res, readp, endp); + break; + case DW_EH_PE_sleb128: + get_sleb128 (*res, readp, endp); + break; + case DW_EH_PE_udata2: + if (readp + 2 > endp) + goto invalid; + *res = read_2ubyte_unaligned_inc (dbg, readp); + break; + case DW_EH_PE_udata4: + if (readp + 4 > endp) + goto invalid; + *res = read_4ubyte_unaligned_inc (dbg, readp); + break; + case DW_EH_PE_udata8: + if (readp + 8 > endp) + goto invalid; + *res = read_8ubyte_unaligned_inc (dbg, readp); + break; + case DW_EH_PE_sdata2: + if (readp + 2 > endp) + goto invalid; + *res = read_2sbyte_unaligned_inc (dbg, readp); + break; + case DW_EH_PE_sdata4: + if (readp + 4 > endp) + goto invalid; + *res = read_4sbyte_unaligned_inc (dbg, readp); + break; + case DW_EH_PE_sdata8: + if (readp + 8 > endp) + goto invalid; + *res = read_8sbyte_unaligned_inc (dbg, readp); + break; + default: + invalid: + error (1, 0, + gettext ("invalid encoding")); + } + + return readp; +} + + static void print_cfa_program (const unsigned char *readp, const unsigned char *const endp, Dwarf_Word vma_base, unsigned int code_align, int data_align, unsigned int version, unsigned int ptr_size, + unsigned int encoding, Dwfl_Module *dwflmod, Ebl *ebl, Dwarf *dbg) { char regnamebuf[REGNAMESZ]; @@ -5079,9 +5136,9 @@ print_cfa_program (const unsigned char *readp, const unsigned char *const endp, case DW_CFA_set_loc: if ((uint64_t) (endp - readp) < 1) goto invalid; - get_uleb128 (op1, readp, endp); - op1 += vma_base; - printf (" set_loc %" PRIu64 "\n", op1 * code_align); + readp = read_encoded (encoding, readp, endp, &op1, dbg); + printf (" set_loc %#" PRIx64 " to %#" PRIx64 "\n", + op1, pc = vma_base + op1); break; case DW_CFA_advance_loc1: if ((uint64_t) (endp - readp) < 1) @@ -5421,62 +5478,6 @@ print_encoding_base (const char *pfx, unsigned int fde_encoding) } -static const unsigned char * -read_encoded (unsigned int encoding, const unsigned char *readp, - const unsigned char *const endp, uint64_t *res, Dwarf *dbg) -{ - if ((encoding & 0xf) == DW_EH_PE_absptr) - encoding = gelf_getclass (dbg->elf) == ELFCLASS32 - ? DW_EH_PE_udata4 : DW_EH_PE_udata8; - - switch (encoding & 0xf) - { - case DW_EH_PE_uleb128: - get_uleb128 (*res, readp, endp); - break; - case DW_EH_PE_sleb128: - get_sleb128 (*res, readp, endp); - break; - case DW_EH_PE_udata2: - if (readp + 2 > endp) - goto invalid; - *res = read_2ubyte_unaligned_inc (dbg, readp); - break; - case DW_EH_PE_udata4: - if (readp + 4 > endp) - goto invalid; - *res = read_4ubyte_unaligned_inc (dbg, readp); - break; - case DW_EH_PE_udata8: - if (readp + 8 > endp) - goto invalid; - *res = read_8ubyte_unaligned_inc (dbg, readp); - break; - case DW_EH_PE_sdata2: - if (readp + 2 > endp) - goto invalid; - *res = read_2sbyte_unaligned_inc (dbg, readp); - break; - case DW_EH_PE_sdata4: - if (readp + 4 > endp) - goto invalid; - *res = read_4sbyte_unaligned_inc (dbg, readp); - break; - case DW_EH_PE_sdata8: - if (readp + 8 > endp) - goto invalid; - *res = read_8sbyte_unaligned_inc (dbg, readp); - break; - default: - invalid: - error (1, 0, - gettext ("invalid encoding")); - } - - return readp; -} - - static void print_debug_frame_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr, Elf_Scn *scn, GElf_Shdr *shdr, Dwarf *dbg) @@ -5851,7 +5852,7 @@ print_debug_frame_section (Dwfl_Module *dwflmod, Ebl *ebl, GElf_Ehdr *ehdr, else print_cfa_program (readp, cieend, vma_base, code_alignment_factor, data_alignment_factor, version, ptr_size, - dwflmod, ebl, dbg); + fde_encoding, dwflmod, ebl, dbg); readp = cieend; } } Ben, do you happen to have a testcase around for this? Thanks, Mark