[PATCH] readelf: DW_CFA_set_loc operand is an address

[PATCH] readelf: DW_CFA_set_loc operand is an address

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 10142 bytes --]

Previously this was incorrectly assumed to be an ULEB128. Hilarity
ensued.

This appears to be the case in both DWARF 3, DWARF 4, and LSB 5.

Also show set_loc argument as hexadecimal

Finally, add a test for this behavior. This was derived from the
following testsuite extract from output from the Glasgow Haskell
Compiler,

.section .text
_c2JJ:
        nop
_c2JP:
        nop
_c2K0:
        nop
_c2KN:
        nop
_c2KZ:
        nop
_c2L3:
        nop
.La2_r2HX_info_end:
        nop
a2_r2HX_info:
        nop
.LMain.ffiTest_info_end:
        nop
Main.ffiTest_info:
        nop
.Lsat_s2Ip_info_end:
        nop
sat_s2Ip_info:
        nop

.section .debug_frame,"",@progbits
.Lsection_frame:
_n2PT:
	.long .Ln2PT_end-.Ln2PT_start
.Ln2PT_start:
	.long -1
	.byte 3
	.asciz "S"
	.byte 1
	.byte 120
	.byte 16
	.byte 12
	.byte 6
	.byte 0
	.byte 144
	.byte 0
	.byte 8
	.byte 7
	.byte 20
	.byte 6
	.byte 0
	.align 8
.Ln2PT_end:
	.long .La2_r2HX_info_fde_end-.La2_r2HX_info_fde
.La2_r2HX_info_fde:
	.long _n2PT
	.quad a2_r2HX_info-1
	.quad .La2_r2HX_info_end-a2_r2HX_info+1
	.byte 1
	.quad _c2JJ
	.byte 14
	.byte 16
	.byte 1
	.quad _c2JP
	.byte 14
	.byte 0
	.byte 1
	.quad _c2K0-1
	.byte 14
	.byte 8
	.align 8
.La2_r2HX_info_fde_end:
	.long .LMain.ffiTest_info_fde_end-.LMain.ffiTest_info_fde
.LMain.ffiTest_info_fde:
	.long _n2PT
	.quad Main.ffiTest_info-1
	.quad .LMain.ffiTest_info_end-Main.ffiTest_info+1
	.align 8
.LMain.ffiTest_info_fde_end:
	.long .Lsat_s2Ip_info_fde_end-.Lsat_s2Ip_info_fde
.Lsat_s2Ip_info_fde:
	.long _n2PT
	.quad sat_s2Ip_info-1
	.quad .Lsat_s2Ip_info_end-sat_s2Ip_info+1
	.byte 1
	.quad _c2KN
	.byte 14
	.byte 8
	.byte 1
	.quad _c2KZ
	.byte 14
	.byte 0
	.byte 1
	.quad _c2L3
	.byte 14
	.byte 8
	.align 8
.Lsat_s2Ip_info_fde_end:

Update styule
---
 libdwfl/frame_unwind.c     |  40 +++++++++++++++++----
 src/readelf.c              |   4 +--
 tests/Makefile.am          |   3 +-
 tests/run-readelf-test5.sh |  88 +++++++++++++++++++++++++++++++++++++++++++++
 tests/testfile72           | Bin 0 -> 2640 bytes
 5 files changed, 126 insertions(+), 9 deletions(-)
 create mode 100755 tests/run-readelf-test5.sh
 create mode 100644 tests/testfile72

diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
index 39509b7..365da48 100644
--- a/libdwfl/frame_unwind.c
+++ b/libdwfl/frame_unwind.c
@@ -559,8 +559,24 @@ handle_cfi (Dwfl_Frame *state, Dwarf_Addr pc, Dwarf_CFI *cfi, Dwarf_Addr bias)
   bool ra_set = false;
   ebl_dwarf_to_regno (ebl, &ra);
 
+  fprintf(stderr, "\n");
+  fprintf(stderr, "Unwinding from %lx:\n", pc);
+  char *modname = "null";
+  if (cfi->dbg) modname = elf_getident(cfi->dbg->elf, NULL);
+  fprintf(stderr, "  mod        = %s\n", modname ? modname : "null");
+  fprintf(stderr, "  CIE offset = %lx\n", frame->fde->cie->offset);
+  if (frame->fde->cie->signal_frame)
+    fprintf(stderr, "  signal frame\n");
+
   for (unsigned regno = 0; regno < nregs; regno++)
     {
+      char regname[10];
+      {
+        const char *prefix, *setname;
+        int bits, type;
+        ebl_register_info(ebl, regno, regname, 10, &prefix, &setname, &bits, &type);
+      }
+
       Dwarf_Op reg_ops_mem[3], *reg_ops;
       size_t reg_nops;
       if (dwarf_frame_register (frame, regno, reg_ops_mem, &reg_ops,
@@ -568,22 +584,26 @@ handle_cfi (Dwfl_Frame *state, Dwarf_Addr pc, Dwarf_CFI *cfi, Dwarf_Addr bias)
 	{
 	  __libdwfl_seterrno (DWFL_E_LIBDW);
 	  continue;
-	}
+        }
       Dwarf_Addr regval;
       if (reg_nops == 0)
 	{
 	  if (reg_ops == reg_ops_mem)
 	    {
 	      /* REGNO is undefined.  */
-	      if (regno == ra)
-		unwound->pc_state = DWFL_FRAME_STATE_PC_UNDEFINED;
+              if (regno == ra) {
+                fprintf(stderr, "  reg %d = undefined\n", regno);
+                unwound->pc_state = DWFL_FRAME_STATE_PC_UNDEFINED;
+              }
 	      continue;
 	    }
 	  else if (reg_ops == NULL)
 	    {
 	      /* REGNO is same-value.  */
-	      if (! state_get_reg (state, regno, &regval))
-		continue;
+              if (! state_get_reg (state, regno, &regval)) {
+                fprintf(stderr, "  reg %d = same = %lx\n", regno, regval);
+                continue;
+              }
 	    }
 	  else
 	    {
@@ -597,7 +617,15 @@ handle_cfi (Dwfl_Frame *state, Dwarf_Addr pc, Dwarf_CFI *cfi, Dwarf_Addr bias)
 	     register will look as unset causing an error later, if used.
 	     But PPC32 does not use such registers.  */
 	  continue;
-	}
+        }
+
+      if (reg_nops > 0) {
+        fprintf(stderr, "  reg %s = ", regname);
+        for (unsigned i = 0; i < reg_nops; i++) {
+          fprintf(stderr, "i ");
+        }
+        fprintf(stderr, "    = %lx\n", regval);
+      }
 
       /* Some architectures encode some extra info in the return address.  */
       if (regno == frame->fde->cie->return_address_register)
diff --git a/src/readelf.c b/src/readelf.c
index 5f6e4ed..224ed88 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -4971,9 +4971,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 = 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)
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 30cf137..8d5df71 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -88,7 +88,8 @@ TESTS = run-arextract.sh run-arsymtest.sh newfile test-nlist \
 	run-find-prologues.sh run-allregs.sh run-addrcfi.sh \
 	run-nm-self.sh run-readelf-self.sh \
 	run-readelf-test1.sh run-readelf-test2.sh run-readelf-test3.sh \
-	run-readelf-test4.sh run-readelf-twofiles.sh \
+	run-readelf-test4.sh run-readelf-test5.sh \
+	run-readelf-twofiles.sh \
 	run-readelf-macro.sh run-readelf-loc.sh \
 	run-readelf-aranges.sh run-readelf-line.sh \
 	run-native-test.sh run-bug1-test.sh \
diff --git a/tests/run-readelf-test5.sh b/tests/run-readelf-test5.sh
new file mode 100755
index 0000000..b6ca830
--- /dev/null
+++ b/tests/run-readelf-test5.sh
@@ -0,0 +1,88 @@
+#! /bin/sh
+# Copyright (C) 2007 Red Hat, Inc.
+# This file is part of elfutils.
+#
+# This file is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or
+# (at your option) any later version.
+#
+# elfutils is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. $srcdir/test-subr.sh
+
+testfiles testfile72
+
+testrun_compare ${abs_top_builddir}/src/readelf --debug-dump=frames testfile72 <<\EOF
+
+DWARF section [ 4] '.debug_frame' at offset 0x50:
+
+ [     0] CIE length=20
+   CIE_id:                   18446744073709551615
+   version:                  3
+   augmentation:             "S"
+   code_alignment_factor:    1
+   data_alignment_factor:    -8
+   return_address_register:  16
+
+   Program:
+     def_cfa r6 (rbp) at offset 0
+     offset r16 (rip) at cfa+0
+     same_value r7 (rsp)
+     val_offset 6 at offset 0
+
+ [    18] FDE length=60 cie=[     0]
+   CIE_pointer:              0
+   initial_location:         .text+0x0000000000000006 <_c2L3+0x1>
+   address_range:            0
+
+   Program:
+     set_loc 0
+     def_cfa_offset 16
+     set_loc 0x1
+     def_cfa_offset 0
+     set_loc 0x1
+     def_cfa_offset 8
+     nop
+     nop
+     nop
+     nop
+     nop
+     nop
+     nop
+
+ [    58] FDE length=20 cie=[     0]
+   CIE_pointer:              0
+   initial_location:         .text+0x0000000000000008 <a2_r2HX_info+0x1>
+   address_range:            0
+
+   Program:
+
+ [    70] FDE length=60 cie=[     0]
+   CIE_pointer:              0
+   initial_location:         .text+0x000000000000000a <Main.ffiTest_info+0x1>
+   address_range:            0
+
+   Program:
+     set_loc 0x3
+     def_cfa_offset 8
+     set_loc 0x4
+     def_cfa_offset 0
+     set_loc 0x5
+     def_cfa_offset 8
+     nop
+     nop
+     nop
+     nop
+     nop
+     nop
+     nop
+EOF
+
+exit 0
diff --git a/tests/testfile72 b/tests/testfile72
new file mode 100644
index 0000000000000000000000000000000000000000..02c3c4da544c88dc8d79d348c7801ce3ffa1a4b6
GIT binary patch
literal 2640
zcmbW3&rcIk5XWb`T8cs~!a;+H)r9DQY+;QzB_;kq5U>$Uh%qT!*+P?2NOv*yr17K|
z!@;{(kNgQ9{WJVOJm}1OGy7vGn)pcHoA-G$^L>5ucFVg*tB(tg1JWE=fOIAlpixZi
zx-siefpIv_G{RH(`Hjup0B2AcE5b39My85z&%~VW60LG2HPaoGOB_OZhqZs_E&Vm`
zeN0+_Y3v>6S9u(Ja~K%oz+T6TtmbGzw5M*St>B>hvPnS8sgO9+Xp+<p4&z!lIIM-;
z_EFsFMzuJM;s}=3UsM-2s=nvn_2$8O?@U0YIN(a)A&+|qPurZjmzo5|R-*xSv>$yk(a)M
zQ=SaG=<R+Sw7`qvz8&|(io-z+-cAq);I*O%ynfgXyq&OhwA*a=gZ(f^x}9Dgbb9Rr
z3^(pUtJM$R<ottv(Ay390MFW;;ci;*oS)0zGzW3w;-7A|$VsV_<q>0Hp%P+^app;3
zd3}<(R=CRi9Q(;|ow=^m40Ee5-T*%H(G(f$A(a)hrh)6(a)m63h><co#xg*e>aS)$#X%>
z+~`8);m)kMFl0B)*Z#^1P~V{b>741;P5+Ym)jJUQ&A$I+t7&e?TF~iv?RIAqZ(a)eW@
z5H}-#`OwaqJ-(a)LD*I1W!Xlr9Zl{wXqKC;t!#d(^%XT)FXzNGwxzhbU+x0Sz~XncnM
zK>5o_e^OlT)fdI3l3x{<dv#lJo+jr(dKG~<%KL!$M23rxoR2JBVMDmY%Xw3tOX7^<
zS$Q%p|7DstF-D56B&o9@`w}j7l=%%bh{-t%Ull`*I{p@}Q?36up6;d2Ns<lfu$|Z;
z>E&Jtm_>u`LR|olANhh*Ie8g5A&HmzJwP^0U*IC=lR^6~;KDHdZ+sC&hxDmNT7MIn
z&WQ*X8LrVJ)3on1TpOnUo%Odlmh?$m>(jc<i3mP3)c;3lU;4e$jN(ri&e0^(<T()3
zLu1(ft9+p)p7u{~SG|9FR`uOO1V35-15Gkb(a)0I5IH;vr)82;b(a)dY0+8PV3X}(lGrw
N`cl}o?2z<Y{|`Qng6#kR

literal 0
HcmV?d00001

-- 
2.6.2

Re: [PATCH] readelf: DW_CFA_set_loc operand is an address

[Roland McGrath]

[-- Attachment #1: Type: text/plain, Size: 244 bytes --]

> Nice catch. Interestingly we seem to handle this correctly in cfi.c, but
> not eu-readelf.

Not surprising, as I wrote cfi.c from scratch based on the spec (and other
unwinder implementations) and not based on the readelf.c decoding code.

Re: [PATCH] readelf: DW_CFA_set_loc operand is an address

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 1422 bytes --]

Mark Wielaard <mjw@redhat.com> writes:

> On Thu, 2015-11-26 at 00:05 +0100, Ben Gamari wrote:
>> Previously this was incorrectly assumed to be an ULEB128. Hilarity
>> ensued.
>> 
>> This appears to be the case in both DWARF 3, DWARF 4, and LSB 5.
>> 
>> Also show set_loc argument as hexadecimal
>> 
>> Finally, add a test for this behavior. This was derived from the
>> following testsuite extract from output from the Glasgow Haskell
>> Compiler,
>> [...]
>> Update styule
>> [...]
>> diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
>> index 39509b7..365da48 100644
>> --- a/libdwfl/frame_unwind.c
>> +++ b/libdwfl/frame_unwind.c
>> @@ -559,8 +559,24 @@ handle_cfi (Dwfl_Frame *state, Dwarf_Addr pc, Dwarf_CFI *cfi, Dwarf_Addr bias)
>>    bool ra_set = false;
>>    ebl_dwarf_to_regno (ebl, &ra);
>>  
>> +  fprintf(stderr, "\n");
>> +  fprintf(stderr, "Unwinding from %lx:\n", pc);
>> +  char *modname = "null";
>> +  if (cfi->dbg) modname = elf_getident(cfi->dbg->elf, NULL);
>> +  fprintf(stderr, "  mod        = %s\n", modname ? modname : "null");
>> +  fprintf(stderr, "  CIE offset = %lx\n", frame->fde->cie->offset);
>> +  if (frame->fde->cie->signal_frame)
>> +    fprintf(stderr, "  signal frame\n");
>> +
>
> Looks like this patch wasn't fully finished/cleaned up yet.
>
Oh dear; yes, this must have snuck in while I was reworking things.

Cheers,

- Ben

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH] readelf: DW_CFA_set_loc operand is an address

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1272 bytes --]

On Thu, 2015-11-26 at 00:05 +0100, Ben Gamari wrote:
> Previously this was incorrectly assumed to be an ULEB128. Hilarity
> ensued.
> 
> This appears to be the case in both DWARF 3, DWARF 4, and LSB 5.
> 
> Also show set_loc argument as hexadecimal
> 
> Finally, add a test for this behavior. This was derived from the
> following testsuite extract from output from the Glasgow Haskell
> Compiler,
> [...]
> Update styule
> [...]
> diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
> index 39509b7..365da48 100644
> --- a/libdwfl/frame_unwind.c
> +++ b/libdwfl/frame_unwind.c
> @@ -559,8 +559,24 @@ handle_cfi (Dwfl_Frame *state, Dwarf_Addr pc, Dwarf_CFI *cfi, Dwarf_Addr bias)
>    bool ra_set = false;
>    ebl_dwarf_to_regno (ebl, &ra);
>  
> +  fprintf(stderr, "\n");
> +  fprintf(stderr, "Unwinding from %lx:\n", pc);
> +  char *modname = "null";
> +  if (cfi->dbg) modname = elf_getident(cfi->dbg->elf, NULL);
> +  fprintf(stderr, "  mod        = %s\n", modname ? modname : "null");
> +  fprintf(stderr, "  CIE offset = %lx\n", frame->fde->cie->offset);
> +  if (frame->fde->cie->signal_frame)
> +    fprintf(stderr, "  signal frame\n");
> +

Looks like this patch wasn't fully finished/cleaned up yet.

Cheers,

Mark

Re: [PATCH] readelf: DW_CFA_set_loc operand is an address

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 1465 bytes --]

Mark Wielaard <mjw@redhat.com> writes:

> On Wed, 2015-11-25 at 17:00 +0100, Ben Gamari wrote:
>> Previously this was incorrectly assumed to be an ULEB128. Hilarity
>> ensued.
>> 
>> This appears to be the case in both DWARF 3, DWARF 4, and LSB 5.
>
> Nice catch. Interestingly we seem to handle this correctly in cfi.c, but
> not eu-readelf. But note that DWARF and the LSB eh_frame extension are
> slightly different. For DWARF .debug_frame this is indeed a "plain"
> address and your read_addr_unaligned_inc call is correct. But for
> LSB .eh_frame addresses are "encoded" and you should call read_encoded
> using the fde_encoding. It looks like fde_encoding isn't passed to
> print_cfa_program, so we should probably pass it from
> print_debug_frame_section.
>
> It appears that there is no testfile which uses DW_CFA_set_loc. If you
> happen to have one that would be nice to add.
>
> Do you want to combine this with the other patch for printing set_loc?
> Could you add a ChangeLog entry and Signed-off-by line for your patch as
> described in the CONTRIBUTING document:
> https://git.fedorahosted.org/cgit/elfutils.git/plain/CONTRIBUTING
>
Oops, sorry about that. I can combine all three of these patches.

Also, I haven't forgotten about the local backtrace support. Your patch
nearly works but I've been under a bit of deadline pressure so I've not
had much time to commit to the elfutils side of things.

Cheers,

- Ben

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH] readelf: DW_CFA_set_loc operand is an address

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 327 bytes --]

Josh Stone <jistone@redhat.com> writes:

> On 11/25/2015 08:00 AM, Ben Gamari wrote:
>> Previously this was incorrectly assumed to be an ULEB128. Hilarity
>> ensued.
>
> Is the error visible in any testcase?  Or can you add a test?
>
I didn't see any effect on any existing tests. New test coming.

Cheers,

- Ben

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 472 bytes --]

Re: [PATCH] readelf: DW_CFA_set_loc operand is an address

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 1108 bytes --]

On Wed, 2015-11-25 at 17:00 +0100, Ben Gamari wrote:
> Previously this was incorrectly assumed to be an ULEB128. Hilarity
> ensued.
> 
> This appears to be the case in both DWARF 3, DWARF 4, and LSB 5.

Nice catch. Interestingly we seem to handle this correctly in cfi.c, but
not eu-readelf. But note that DWARF and the LSB eh_frame extension are
slightly different. For DWARF .debug_frame this is indeed a "plain"
address and your read_addr_unaligned_inc call is correct. But for
LSB .eh_frame addresses are "encoded" and you should call read_encoded
using the fde_encoding. It looks like fde_encoding isn't passed to
print_cfa_program, so we should probably pass it from
print_debug_frame_section.

It appears that there is no testfile which uses DW_CFA_set_loc. If you
happen to have one that would be nice to add.

Do you want to combine this with the other patch for printing set_loc?
Could you add a ChangeLog entry and Signed-off-by line for your patch as
described in the CONTRIBUTING document:
https://git.fedorahosted.org/cgit/elfutils.git/plain/CONTRIBUTING

Thanks,

Mark

Re: [PATCH] readelf: DW_CFA_set_loc operand is an address

[Josh Stone]

[-- Attachment #1: Type: text/plain, Size: 888 bytes --]

On 11/25/2015 08:00 AM, Ben Gamari wrote:
> Previously this was incorrectly assumed to be an ULEB128. Hilarity
> ensued.

Is the error visible in any testcase?  Or can you add a test?

> This appears to be the case in both DWARF 3, DWARF 4, and LSB 5.
> ---
>  src/readelf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/readelf.c b/src/readelf.c
> index 5f6e4ed..7c3237f 100644
> --- a/src/readelf.c
> +++ b/src/readelf.c
> @@ -4971,7 +4971,7 @@ 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 = read_addr_unaligned_inc (ptr_size, dbg, readp);
>  	    op1 += vma_base;
>  	    printf ("     set_loc %" PRIu64 "\n", op1 * code_align);
>  	    break;
> 

[PATCH] readelf: DW_CFA_set_loc operand is an address

[Ben Gamari]

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

Previously this was incorrectly assumed to be an ULEB128. Hilarity
ensued.

This appears to be the case in both DWARF 3, DWARF 4, and LSB 5.
---
 src/readelf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/readelf.c b/src/readelf.c
index 5f6e4ed..7c3237f 100644
--- a/src/readelf.c
+++ b/src/readelf.c
@@ -4971,7 +4971,7 @@ 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 = read_addr_unaligned_inc (ptr_size, dbg, readp);
 	    op1 += vma_base;
 	    printf ("     set_loc %" PRIu64 "\n", op1 * code_align);
 	    break;
-- 
2.6.2