--- src/readelf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/readelf.c b/src/readelf.c index 7c3237f..446c591 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -4973,7 +4973,7 @@ print_cfa_program (const unsigned char *readp, const unsigned char *const endp, goto invalid; op1 = read_addr_unaligned_inc (ptr_size, dbg, readp); op1 += vma_base; - printf (" set_loc %" PRIu64 "\n", op1 * code_align); + printf (" set_loc %" PRIx64 "\n", op1 * code_align); break; case DW_CFA_advance_loc1: if ((uint64_t) (endp - readp) < 1)
Ben Gamari ben@smart-cactus.org writes:
src/readelf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/readelf.c b/src/readelf.c index 7c3237f..446c591 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -4973,7 +4973,7 @@ print_cfa_program (const unsigned char *readp, const unsigned char *const endp, goto invalid; op1 = read_addr_unaligned_inc (ptr_size, dbg, readp); op1 += vma_base;
printf (" set_loc %" PRIu64 "\n", op1 * code_align);
printf (" set_loc %" PRIx64 "\n", op1 * code_align); break;
Disregard this patch on account of the indentation change. New patch coming.
--- src/readelf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/readelf.c b/src/readelf.c index 7c3237f..e2f8fa9 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -4973,7 +4973,7 @@ print_cfa_program (const unsigned char *readp, const unsigned char *const endp, goto invalid; op1 = read_addr_unaligned_inc (ptr_size, dbg, readp); op1 += vma_base; - printf (" set_loc %" PRIu64 "\n", op1 * code_align); + printf (" set_loc %" PRIx64 "\n", op1 * code_align); break; case DW_CFA_advance_loc1: if ((uint64_t) (endp - readp) < 1)
Ben Gamari ben@smart-cactus.org writes:
src/readelf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/readelf.c b/src/readelf.c index 7c3237f..e2f8fa9 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -4973,7 +4973,7 @@ print_cfa_program (const unsigned char *readp, const unsigned char *const endp, goto invalid; op1 = read_addr_unaligned_inc (ptr_size, dbg, readp); op1 += vma_base;
printf (" set_loc %" PRIu64 "\n", op1 * code_align);
printf (" set_loc %" PRIx64 "\n", op1 * code_align); break;
On second thought, this should probably have a `0x` in front of it.
One last try coming. Sorry for the noise everyone.
Cheers,
- Ben
--- src/readelf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/readelf.c b/src/readelf.c index 7c3237f..3b6b4a0 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -4973,7 +4973,7 @@ print_cfa_program (const unsigned char *readp, const unsigned char *const endp, goto invalid; op1 = read_addr_unaligned_inc (ptr_size, dbg, readp); op1 += vma_base; - printf (" set_loc %" PRIu64 "\n", op1 * code_align); + printf (" set_loc 0x%" PRIx64 "\n", op1 * code_align); break; case DW_CFA_advance_loc1: if ((uint64_t) (endp - readp) < 1)
On 11/25/2015 08:31 AM, Ben Gamari wrote:
src/readelf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/readelf.c b/src/readelf.c index 7c3237f..3b6b4a0 100644 --- a/src/readelf.c +++ b/src/readelf.c @@ -4973,7 +4973,7 @@ print_cfa_program (const unsigned char *readp, const unsigned char *const endp, goto invalid; op1 = read_addr_unaligned_inc (ptr_size, dbg, readp); op1 += vma_base;
printf (" set_loc %" PRIu64 "\n", op1 * code_align);
case DW_CFA_advance_loc1: if ((uint64_t) (endp - readp) < 1)printf (" set_loc 0x%" PRIx64 "\n", op1 * code_align); break;
Other addresses in this code are printed with ("%#" PRIx64). (AFAIK '#' here will always give "0x", but lets keep it consistent).
It's also curious that nothing is done with op1 after printing it. Shouldn't this set pc, at least?
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).
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
Hi hackers,
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 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, no comments about this at all? Sounds sane, sounds insane? What if I attach a full patch and threaten to commit it? See attached.
Cheers,
Mark
On Tue, 2016-02-09 at 23:35 +0100, Mark Wielaard wrote:
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:
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?
OK, no comments... Then I am forced to execute my threat... Committed to master now:
+2016-02-09 Mark Wielaard mjw@redhat.com
- readelf.c (read_encoded): Move up.
- (print_cfa_program): Add encoding argument. Use it for read_encoded
- when reading DW_CFA_set_loc op.
- (print_debug_frame_section): Pass fde_encoding to print_cfa_program.
Cheers,
Mark
elfutils-devel@lists.fedorahosted.org