From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: multipart/mixed; boundary="===============3306383565150454072==" MIME-Version: 1.0 From: Mark Wielaard To: elfutils-devel@lists.fedorahosted.org Subject: Re: [PATCH] readelf: Show set_loc argument as hexadecimal Date: Sat, 23 Jan 2016 21:39:57 +0100 Message-ID: <20160123203957.GD19390@blokker.redhat.com> In-Reply-To: 20160120231416.850642C3B71@topped-with-meat.com --===============3306383565150454072== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable 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 *=3D code_align; > printf (" set_loc %#" PRIx64 " to %#" PRIx64 "\n", > op1, pc =3D 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) =3D=3D DW_EH_PE_absptr) + encoding =3D gelf_getclass (dbg->elf) =3D=3D 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 =3D read_2ubyte_unaligned_inc (dbg, readp); + break; + case DW_EH_PE_udata4: + if (readp + 4 > endp) + goto invalid; + *res =3D read_4ubyte_unaligned_inc (dbg, readp); + break; + case DW_EH_PE_udata8: + if (readp + 8 > endp) + goto invalid; + *res =3D read_8ubyte_unaligned_inc (dbg, readp); + break; + case DW_EH_PE_sdata2: + if (readp + 2 > endp) + goto invalid; + *res =3D read_2sbyte_unaligned_inc (dbg, readp); + break; + case DW_EH_PE_sdata4: + if (readp + 4 > endp) + goto invalid; + *res =3D read_4sbyte_unaligned_inc (dbg, readp); + break; + case DW_EH_PE_sdata8: + if (readp + 8 > endp) + goto invalid; + *res =3D 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 +=3D vma_base; - printf (" set_loc %" PRIu64 "\n", op1 * code_align); + readp =3D read_encoded (encoding, readp, endp, &op1, dbg); + printf (" set_loc %#" PRIx64 " to %#" PRIx64 "\n", + op1, pc =3D 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 f= de_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) =3D=3D DW_EH_PE_absptr) - encoding =3D gelf_getclass (dbg->elf) =3D=3D 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 =3D read_2ubyte_unaligned_inc (dbg, readp); - break; - case DW_EH_PE_udata4: - if (readp + 4 > endp) - goto invalid; - *res =3D read_4ubyte_unaligned_inc (dbg, readp); - break; - case DW_EH_PE_udata8: - if (readp + 8 > endp) - goto invalid; - *res =3D read_8ubyte_unaligned_inc (dbg, readp); - break; - case DW_EH_PE_sdata2: - if (readp + 2 > endp) - goto invalid; - *res =3D read_2sbyte_unaligned_inc (dbg, readp); - break; - case DW_EH_PE_sdata4: - if (readp + 4 > endp) - goto invalid; - *res =3D read_4sbyte_unaligned_inc (dbg, readp); - break; - case DW_EH_PE_sdata8: - if (readp + 8 > endp) - goto invalid; - *res =3D 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 =3D cieend; } } Ben, do you happen to have a testcase around for this? Thanks, Mark --===============3306383565150454072==--