public inbox for elfutils@sourceware.org
 help / color / mirror / Atom feed
* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 10:50 Mark Wielaard
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Wielaard @ 2014-06-15 10:50 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, 2014-06-15 at 12:39 +0200, Kurt Roeckx wrote:
> On Sun, Jun 15, 2014 at 12:30:02PM +0200, Mark Wielaard wrote:
> > Another ARM oddity. A return value address in an unwind will contain an
> > extra bit to indicate whether to return to a regular ARM or THUMB function.
> > Add a new ebl function to return a mask to use to get the actual return
> > address during an unwind ebl_unwind_ret_mask.
> 
> Does this fix all the issues on armhf now, including the
> backtrace?

Yes, you'll need this patch, the "libebl: Add sym_func_value hook" and
the "tests: backtrace.c accept __libc_do_syscall as first frame symname"
patches. With libc6-dbg installed all tests should PASS with 3 SKIPs,
without libc6-dbg installed you should see 6 SKIPs (no FAILs).

The other recent patches should be optional, but help for arches which
don't have a fully working unwind backend and make the tests a bit more
robust and clear when something does go wrong.

Cheers,

Mark


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-17 14:50 Mark Wielaard
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Wielaard @ 2014-06-17 14:50 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, 2014-06-15 at 23:51 +0200, Kurt Roeckx wrote:
> On Sun, Jun 15, 2014 at 10:21:31PM +0200, Mark Wielaard wrote:
> > 
> > Indeed. You seem to be right that we are trying to read possibly
> > unaligned data which would cause a bus error on sparc. There was even a
> > helpful FIXME in the code about it. Why we didn't see that before is a
> > mystery. But the attached patch should solve it. Could you test it?
> 
> That changes things back to:
> # TOTAL: 123
> # PASS:  116
> # SKIP:  7
> # XFAIL: 0
> # FAIL:  0
> # XPASS: 0
> # ERROR: 0

Thanks for testing. Looks good. I pushed this to master now.


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-17  6:56 Mark Wielaard
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Wielaard @ 2014-06-17  6:56 UTC (permalink / raw)
  To: elfutils-devel

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

On Mon, 2014-06-16 at 15:23 -0700, Roland McGrath wrote:
> What does "actual return value" mean?  With the low bit intact, it is the
> actual value that must be used in a bx instruction.  So why is this a new
> hook rather than being the same as the "func_value" hook?

Good question. They are indeed used for the exact same thing. I just
didn't realize they were till now. Somehow I was working on the function
table issue and the unwind issue separately. It certainly makes sense to
have just one hook. Especially because ARM is the only backend using
them at the moment. I'll rework both patches/hooks into one.

Thanks,

Mark


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-16 22:23 Roland McGrath
  0 siblings, 0 replies; 16+ messages in thread
From: Roland McGrath @ 2014-06-16 22:23 UTC (permalink / raw)
  To: elfutils-devel

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

What does "actual return value" mean?  With the low bit intact, it is the
actual value that must be used in a bx instruction.  So why is this a new
hook rather than being the same as the "func_value" hook?

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 21:51 Kurt Roeckx
  0 siblings, 0 replies; 16+ messages in thread
From: Kurt Roeckx @ 2014-06-15 21:51 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, Jun 15, 2014 at 10:21:31PM +0200, Mark Wielaard wrote:
> 
> Indeed. You seem to be right that we are trying to read possibly
> unaligned data which would cause a bus error on sparc. There was even a
> helpful FIXME in the code about it. Why we didn't see that before is a
> mystery. But the attached patch should solve it. Could you test it?

That changes things back to:
# TOTAL: 123
# PASS:  116
# SKIP:  7
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 0

Thanks


Kurt


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 20:21 Mark Wielaard
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Wielaard @ 2014-06-15 20:21 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, 2014-06-15 at 21:59 +0200, Kurt Roeckx wrote:
> On Sun, Jun 15, 2014 at 09:27:32PM +0200, Kurt Roeckx wrote:
> > 
> > I'm not sure what that code is doing there.  But it seems to be
> > looking at a 64 bit core file (x86_64, arm64, s390x) and then
> > create a 64 bit pointer is trying to dereference that.
> 
> That would be a pointer to a 64 bit object of course.  Sparc
> probably has stricter alignment requirements than the other
> arches.  I just wonder why it worked before.

Indeed. You seem to be right that we are trying to read possibly
unaligned data which would cause a bus error on sparc. There was even a
helpful FIXME in the code about it. Why we didn't see that before is a
mystery. But the attached patch should solve it. Could you test it?

Thanks,

Mark

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-libdwfl-linux-core-attach.c-handle-possible-unaligne.patch --]
[-- Type: text/x-patch, Size: 4927 bytes --]

>From 7a6d8a0e3694af48a38e0461e15fcde0a06bad58 Mon Sep 17 00:00:00 2001
From: Mark Wielaard <mjw@redhat.com>
Date: Sun, 15 Jun 2014 22:14:04 +0200
Subject: [PATCH] libdwfl: linux-core-attach.c handle possible unaligned data access.

Use libdw/memory-access.h macros read_4ubyte_unaligned_noncvt and
read_8ubyte_unaligned_noncvt to access possibly unaligned data in
core files.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 libdwfl/ChangeLog           |    9 +++++++++
 libdwfl/linux-core-attach.c |   22 +++++++++++-----------
 2 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index ac92a21..f1bc1a7 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,12 @@
+2014-06-15  Mark Wielaard  <mjw@redhat.com>
+
+	* linux-core-attach.c (core_memory_read): Use libdw/memory-access.h
+	macros read_4ubyte_unaligned_noncvt and read_8ubyte_unaligned_noncvt
+	to read possibly unaligned data.
+	(core_next_thread): Likewise.
+	(core_set_initial_registers): Likewise.
+	(dwfl_core_file_attach): Likewise.
+
 2014-06-11  Mark Wielaard  <mjw@redhat.com>
 
 	* dwfl_frame.c (__libdwfl_process_free): Reset dwfl->attacherr.
diff --git a/libdwfl/linux-core-attach.c b/libdwfl/linux-core-attach.c
index 7ef3f25..5a7b3b3 100644
--- a/libdwfl/linux-core-attach.c
+++ b/libdwfl/linux-core-attach.c
@@ -30,6 +30,8 @@
 #include <fcntl.h>
 #include "system.h"
 
+#include "../libdw/memory-access.h"
+
 #ifndef MIN
 # define MIN(a, b) ((a) < (b) ? (a) : (b))
 #endif
@@ -83,12 +85,10 @@ core_memory_read (Dwfl *dwfl, Dwarf_Addr addr, Dwarf_Word *result,
 	  return false;
 	}
       assert (data->d_size == bytes);
-      /* FIXME: Currently any arch supported for unwinding supports
-	 unaligned access.  */
       if (bytes == 8)
-	*result = *(const uint64_t *) data->d_buf;
+	*result = read_8ubyte_unaligned_noncvt (data->d_buf);
       else
-	*result = *(const uint32_t *) data->d_buf;
+	*result = read_4ubyte_unaligned_noncvt (data->d_buf);
       return true;
     }
   __libdwfl_seterrno (DWFL_E_ADDR_OUTOFRANGE);
@@ -150,7 +150,7 @@ core_next_thread (Dwfl *dwfl __attribute__ ((unused)), void *dwfl_arg,
 	  break;
       if (item == items + nitems)
 	continue;
-      uint32_t val32 = *(const uint32_t *) (desc + item->offset);
+      uint32_t val32 = read_4ubyte_unaligned_noncvt (desc + item->offset);
       val32 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
 		? be32toh (val32) : le32toh (val32));
       pid_t tid = (int32_t) val32;
@@ -201,7 +201,7 @@ core_set_initial_registers (Dwfl_Thread *thread, void *thread_arg_voidp)
   assert (item < items + nitems);
   pid_t tid;
   {
-    uint32_t val32 = *(const uint32_t *) (desc + item->offset);
+    uint32_t val32 = read_4ubyte_unaligned_noncvt (desc + item->offset);
     val32 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
 	     ? be32toh (val32) : le32toh (val32));
     tid = (int32_t) val32;
@@ -218,14 +218,14 @@ core_set_initial_registers (Dwfl_Thread *thread, void *thread_arg_voidp)
       switch (gelf_getclass (core) == ELFCLASS32 ? 32 : 64)
       {
 	case 32:;
-	  uint32_t val32 = *(const uint32_t *) (desc + item->offset);
+	  uint32_t val32 = read_4ubyte_unaligned_noncvt (desc + item->offset);
 	  val32 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
 		   ? be32toh (val32) : le32toh (val32));
 	  /* Do a host width conversion.  */
 	  pc = val32;
 	  break;
 	case 64:;
-	  uint64_t val64 = *(const uint64_t *) (desc + item->offset);
+	  uint64_t val64 = read_8ubyte_unaligned_noncvt (desc + item->offset);
 	  val64 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
 		   ? be64toh (val64) : le64toh (val64));
 	  pc = val64;
@@ -259,7 +259,7 @@ core_set_initial_registers (Dwfl_Thread *thread, void *thread_arg_voidp)
 	  switch (regloc->bits)
 	  {
 	    case 32:;
-	      uint32_t val32 = *(const uint32_t *) reg_desc;
+	      uint32_t val32 = read_4ubyte_unaligned_noncvt (reg_desc);
 	      reg_desc += sizeof val32;
 	      val32 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
 		       ? be32toh (val32) : le32toh (val32));
@@ -267,7 +267,7 @@ core_set_initial_registers (Dwfl_Thread *thread, void *thread_arg_voidp)
 	      val = val32;
 	      break;
 	    case 64:;
-	      uint64_t val64 = *(const uint64_t *) reg_desc;
+	      uint64_t val64 = read_8ubyte_unaligned_noncvt (reg_desc);
 	      reg_desc += sizeof val64;
 	      val64 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
 		       ? be64toh (val64) : le64toh (val64));
@@ -392,7 +392,7 @@ dwfl_core_file_attach (Dwfl *dwfl, Elf *core)
 	  break;
       if (item == items + nitems)
 	continue;
-      uint32_t val32 = *(const uint32_t *) (desc + item->offset);
+      uint32_t val32 = read_4ubyte_unaligned_noncvt (desc + item->offset);
       val32 = (elf_getident (core, NULL)[EI_DATA] == ELFDATA2MSB
 		? be32toh (val32) : le32toh (val32));
       pid = (int32_t) val32;
-- 
1.7.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 19:59 Kurt Roeckx
  0 siblings, 0 replies; 16+ messages in thread
From: Kurt Roeckx @ 2014-06-15 19:59 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, Jun 15, 2014 at 09:27:32PM +0200, Kurt Roeckx wrote:
> 
> I'm not sure what that code is doing there.  But it seems to be
> looking at a 64 bit core file (x86_64, arm64, s390x) and then
> create a 64 bit pointer is trying to dereference that.

That would be a pointer to a 64 bit object of course.  Sparc
probably has stricter alignment requirements than the other
arches.  I just wonder why it worked before.


Kurt


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 19:27 Kurt Roeckx
  0 siblings, 0 replies; 16+ messages in thread
From: Kurt Roeckx @ 2014-06-15 19:27 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, Jun 15, 2014 at 08:56:17PM +0200, Mark Wielaard wrote:
> On Sun, 2014-06-15 at 20:47 +0200, Kurt Roeckx wrote:
> > On Sun, Jun 15, 2014 at 08:38:20PM +0200, Mark Wielaard wrote:
> > > 
> > > Since the sparc backend doesn't have unwinder support it sounds unlikely
> > > one of these patches triggered the bus error. Do you have more
> > > specifics? Which test triggers it, do you have a backtrace? Are there
> > > logs before/after, etc.
> > 
> > Log before:
> > https://buildd.debian.org/status/fetch.php?pkg=elfutils&arch=sparc&ver=0.159-1&stamp=1401337598
> > 
> > Log after:
> > https://buildd.debian.org/status/fetch.php?pkg=elfutils&arch=sparc&ver=0.159-2&stamp=1402854485
> 
> Strange indeed. But I am afraid you'll have to run one of the failing
> tests under GDB to help debug it. At least so we know where it is
> crashing and if it is crashing in elfutils code or somewhere else (using
> completely different gcc and glibc versions before/after is slightly
> suspicious, even though it might still be a bug in our code of course).

Program terminated with signal SIGBUS, Bus error.
b#0  core_set_initial_registers (thread=0xffef2018, thread_arg_voidp=<optimized out>) at linux-core-attach.c:299
299                   uint64_t val64 = *(const uint64_t *) reg_desc;
(gdb) bt
#0  core_set_initial_registers (thread=0xffef2018, thread_arg_voidp=<optimized out>) at linux-core-attach.c:299
#1  0xf79f43cc in dwfl_thread_getframes (thread=thread(a)entry=0xffef2018, callback=callback(a)entry=0x12520 <frame_callback>, arg=arg(a)entry=0xffef2088) at dwfl_frame.c:402
#2  0x00012ba8 in thread_callback (thread=0xffef2018, thread_arg=0xffef2088) at stack.c:458
#3  0xf79f41e0 in dwfl_getthreads (dwfl=0x28358, callback=callback(a)entry=0x12b80 <thread_callback>, arg=arg(a)entry=0xffef2088) at dwfl_frame.c:270
#4  0x0001150c in main (argc=8, argv=0xffef2354) at stack.c:731

Dropping the patches doesn't have any effect on the result.

Note that this is a 32 bit userland sparc, but the kernel is 64 bit.

I'm not sure what that code is doing there.  But it seems to be
looking at a 64 bit core file (x86_64, arm64, s390x) and then
create a 64 bit pointer is trying to dereference that.


Kurt


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 18:56 Mark Wielaard
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Wielaard @ 2014-06-15 18:56 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, 2014-06-15 at 20:47 +0200, Kurt Roeckx wrote:
> On Sun, Jun 15, 2014 at 08:38:20PM +0200, Mark Wielaard wrote:
> > 
> > Since the sparc backend doesn't have unwinder support it sounds unlikely
> > one of these patches triggered the bus error. Do you have more
> > specifics? Which test triggers it, do you have a backtrace? Are there
> > logs before/after, etc.
> 
> Log before:
> https://buildd.debian.org/status/fetch.php?pkg=elfutils&arch=sparc&ver=0.159-1&stamp=1401337598
> 
> Log after:
> https://buildd.debian.org/status/fetch.php?pkg=elfutils&arch=sparc&ver=0.159-2&stamp=1402854485

Strange indeed. But I am afraid you'll have to run one of the failing
tests under GDB to help debug it. At least so we know where it is
crashing and if it is crashing in elfutils code or somewhere else (using
completely different gcc and glibc versions before/after is slightly
suspicious, even though it might still be a bug in our code of course).

Thanks,

Mark


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 18:47 Kurt Roeckx
  0 siblings, 0 replies; 16+ messages in thread
From: Kurt Roeckx @ 2014-06-15 18:47 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, Jun 15, 2014 at 08:38:20PM +0200, Mark Wielaard wrote:
> 
> Since the sparc backend doesn't have unwinder support it sounds unlikely
> one of these patches triggered the bus error. Do you have more
> specifics? Which test triggers it, do you have a backtrace? Are there
> logs before/after, etc.

Log before:
https://buildd.debian.org/status/fetch.php?pkg=elfutils&arch=sparc&ver=0.159-1&stamp=1401337598

Log after:
https://buildd.debian.org/status/fetch.php?pkg=elfutils&arch=sparc&ver=0.159-2&stamp=1402854485


Kurt


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 18:42 Mark Wielaard
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Wielaard @ 2014-06-15 18:42 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, 2014-06-15 at 20:18 +0200, Kurt Roeckx wrote:
> On Sun, Jun 15, 2014 at 12:30:02PM +0200, Mark Wielaard wrote:
> > +  /* Mask to use to get the return address from an unwind in case the
> > +     architecture adds some extra non-address bits to it.  When not
> > +     initialized (0) then ebl_unwind_ret_mask will return ~0, otherwise
> > +     it should be the actual mask to use.  */
> > +  GElf_Addr unwind_ret_mask;
> 
> Shouldn't that be an Elf64_Addr?

No. None of the unwinder uses Elf32 or Elf64 specific types. GElf_Addr
is used here so the unwinder code doesn't need to care about 32-vs-64
issues.

Cheers,

Mark


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 18:38 Mark Wielaard
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Wielaard @ 2014-06-15 18:38 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, 2014-06-15 at 20:08 +0200, Kurt Roeckx wrote:
> > > Does this fix all the issues on armhf now, including the
> > > backtrace?
> > 
> > Yes, you'll need this patch, the "libebl: Add sym_func_value hook" and
> > the "tests: backtrace.c accept __libc_do_syscall as first frame symname"
> > patches. With libc6-dbg installed all tests should PASS with 3 SKIPs,
> > without libc6-dbg installed you should see 6 SKIPs (no FAILs).
> > 
> > The other recent patches should be optional, but help for arches which
> > don't have a fully working unwind backend and make the tests a bit more
> > robust and clear when something does go wrong.
> 
> Things seem to have worked on arm(hf) now, but I got a bus error
> on sparc instead.  Things that changed between the previous upload
> and this one are the 4 patches you made, and that we switched
> to gcc 4.9, eglibc 2.19, ...  But I suspect it's one of the
> patches.

Since the sparc backend doesn't have unwinder support it sounds unlikely
one of these patches triggered the bus error. Do you have more
specifics? Which test triggers it, do you have a backtrace? Are there
logs before/after, etc.

Thanks,

Mark


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 18:18 Kurt Roeckx
  0 siblings, 0 replies; 16+ messages in thread
From: Kurt Roeckx @ 2014-06-15 18:18 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, Jun 15, 2014 at 12:30:02PM +0200, Mark Wielaard wrote:
> +  /* Mask to use to get the return address from an unwind in case the
> +     architecture adds some extra non-address bits to it.  When not
> +     initialized (0) then ebl_unwind_ret_mask will return ~0, otherwise
> +     it should be the actual mask to use.  */
> +  GElf_Addr unwind_ret_mask;

Shouldn't that be an Elf64_Addr?


Kurt


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 18:08 Kurt Roeckx
  0 siblings, 0 replies; 16+ messages in thread
From: Kurt Roeckx @ 2014-06-15 18:08 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, Jun 15, 2014 at 12:50:14PM +0200, Mark Wielaard wrote:
> On Sun, 2014-06-15 at 12:39 +0200, Kurt Roeckx wrote:
> > On Sun, Jun 15, 2014 at 12:30:02PM +0200, Mark Wielaard wrote:
> > > Another ARM oddity. A return value address in an unwind will contain an
> > > extra bit to indicate whether to return to a regular ARM or THUMB function.
> > > Add a new ebl function to return a mask to use to get the actual return
> > > address during an unwind ebl_unwind_ret_mask.
> > 
> > Does this fix all the issues on armhf now, including the
> > backtrace?
> 
> Yes, you'll need this patch, the "libebl: Add sym_func_value hook" and
> the "tests: backtrace.c accept __libc_do_syscall as first frame symname"
> patches. With libc6-dbg installed all tests should PASS with 3 SKIPs,
> without libc6-dbg installed you should see 6 SKIPs (no FAILs).
> 
> The other recent patches should be optional, but help for arches which
> don't have a fully working unwind backend and make the tests a bit more
> robust and clear when something does go wrong.

Things seem to have worked on arm(hf) now, but I got a bus error
on sparc instead.  Things that changed between the previous upload
and this one are the 4 patches you made, and that we switched
to gcc 4.9, eglibc 2.19, ...  But I suspect it's one of the
patches.


Kurt


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 10:39 Kurt Roeckx
  0 siblings, 0 replies; 16+ messages in thread
From: Kurt Roeckx @ 2014-06-15 10:39 UTC (permalink / raw)
  To: elfutils-devel

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

On Sun, Jun 15, 2014 at 12:30:02PM +0200, Mark Wielaard wrote:
> Another ARM oddity. A return value address in an unwind will contain an
> extra bit to indicate whether to return to a regular ARM or THUMB function.
> Add a new ebl function to return a mask to use to get the actual return
> address during an unwind ebl_unwind_ret_mask.

Does this fix all the issues on armhf now, including the
backtrace?


Kurt


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH] libebl: Add ebl_unwind_ret_mask.
@ 2014-06-15 10:30 Mark Wielaard
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Wielaard @ 2014-06-15 10:30 UTC (permalink / raw)
  To: elfutils-devel

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

Another ARM oddity. A return value address in an unwind will contain an
extra bit to indicate whether to return to a regular ARM or THUMB function.
Add a new ebl function to return a mask to use to get the actual return
address during an unwind ebl_unwind_ret_mask.

Signed-off-by: Mark Wielaard <mjw@redhat.com>
---
 backends/ChangeLog     | 4 ++++
 backends/arm_init.c    | 3 +++
 libdwfl/ChangeLog      | 4 ++++
 libdwfl/frame_unwind.c | 6 +++++-
 libebl/ChangeLog       | 6 ++++++
 libebl/eblinitreg.c    | 9 ++++++++-
 libebl/libebl.h        | 4 ++++
 libebl/libeblP.h       | 8 +++++++-
 8 files changed, 41 insertions(+), 3 deletions(-)

diff --git a/backends/ChangeLog b/backends/ChangeLog
index 64b669e..11538b0 100644
--- a/backends/ChangeLog
+++ b/backends/ChangeLog
@@ -1,3 +1,7 @@
+2014-06-15  Mark Wielaard  <mjw@redhat.com>
+
+	* arm_init.c (arm_init): Set unwind_ret_mask.
+
 2014-06-14  Mark Wielaard  <mjw@redhat.com>
 
 	* arm_init.c (arm_init): Hook sym_func_value.
diff --git a/backends/arm_init.c b/backends/arm_init.c
index e2e20e4..f95949d 100644
--- a/backends/arm_init.c
+++ b/backends/arm_init.c
@@ -70,5 +70,8 @@ arm_init (elf, machine, eh, ehlen)
   eh->frame_nregs = 16;
   HOOK (eh, set_initial_registers_tid);
 
+  /* Bit zero encodes whether to return to a THUMB or ARM function. */
+  eh->unwind_ret_mask = ~(GElf_Addr)1;
+
   return MODVERSION;
 }
diff --git a/libdwfl/ChangeLog b/libdwfl/ChangeLog
index 888b400..195f5a0 100644
--- a/libdwfl/ChangeLog
+++ b/libdwfl/ChangeLog
@@ -1,3 +1,7 @@
+2014-06-15  Mark Wielaard  <mjw@redhat.com>
+
+	* frame_unwind.c (handle_cfi): Use ebl_unwind_ret_mask.
+
 2014-06-14  Mark Wielaard  <mjw@redhat.com>
 
 	* dwfl_module_getsym.c (__libdwfl_getsym): Call ebl_sym_func_value.
diff --git a/libdwfl/frame_unwind.c b/libdwfl/frame_unwind.c
index 18c808b..f93348f 100644
--- a/libdwfl/frame_unwind.c
+++ b/libdwfl/frame_unwind.c
@@ -1,5 +1,5 @@
 /* Get previous frame state for an existing frame state.
-   Copyright (C) 2013 Red Hat, Inc.
+   Copyright (C) 2013, 2014 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -582,6 +582,10 @@ handle_cfi (Dwfl_Frame *state, Dwarf_Addr pc, Dwarf_CFI *cfi, Dwarf_Addr bias)
 	  continue;
 	}
 
+      /* Some architectures encode some extra info in the return address.  */
+      if (regno == frame->fde->cie->return_address_register)
+	regval &= ebl_unwind_ret_mask (ebl);
+
       /* This is another strange PPC[64] case.  There are two
 	 registers numbers that can represent the same DWARF return
 	 register number.  We only want one to actually set the return
diff --git a/libebl/ChangeLog b/libebl/ChangeLog
index 1c7a2ba..d5fac1d 100644
--- a/libebl/ChangeLog
+++ b/libebl/ChangeLog
@@ -1,3 +1,9 @@
+2014-06-15  Mark Wielaard  <mjw@redhat.com>
+
+	* eblinitreg.c (ebl_unwind_ret_mask): New function.
+	* libebl.h (ebl_unwind_ret_mask): Define.
+	* libeblP.h (struct ebl): Add unwind_ret_mask.
+
 2014-06-14  Mark Wielaard  <mjw@redhat.com>
 
 	* Makefile.am (gen_SOURCES): Add eblsymfuncval.c.
diff --git a/libebl/eblinitreg.c b/libebl/eblinitreg.c
index 8909c50..a6bb00c 100644
--- a/libebl/eblinitreg.c
+++ b/libebl/eblinitreg.c
@@ -1,5 +1,5 @@
 /* Fetch live process Dwfl_Frame from PID.
-   Copyright (C) 2013 Red Hat, Inc.
+   Copyright (C) 2013, 2014 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -49,3 +49,10 @@ ebl_frame_nregs (Ebl *ebl)
 {
   return ebl == NULL ? 0 : ebl->frame_nregs;
 }
+
+GElf_Addr
+ebl_unwind_ret_mask (Ebl *ebl)
+{
+  return ((ebl == NULL || ebl->unwind_ret_mask == 0)
+	  ? ~(GElf_Addr)0 : ebl->unwind_ret_mask);
+}
diff --git a/libebl/libebl.h b/libebl/libebl.h
index bc1f491..768dab4 100644
--- a/libebl/libebl.h
+++ b/libebl/libebl.h
@@ -409,6 +409,10 @@ extern bool ebl_set_initial_registers_tid (Ebl *ebl,
 extern size_t ebl_frame_nregs (Ebl *ebl)
   __nonnull_attribute__ (1);
 
+/* Mask to use for unwind return address in case the architecture adds
+   some extra non-address bits to it.  */
+extern GElf_Addr ebl_unwind_ret_mask (Ebl *ebl);
+
 /* Convert *REGNO as is in DWARF to a lower range suitable for
    Dwarf_Frame->REGS indexing.  */
 extern bool ebl_dwarf_to_regno (Ebl *ebl, unsigned *regno)
diff --git a/libebl/libeblP.h b/libebl/libeblP.h
index f91c2a0..ac38258 100644
--- a/libebl/libeblP.h
+++ b/libebl/libeblP.h
@@ -1,5 +1,5 @@
 /* Internal definitions for interface for libebl.
-   Copyright (C) 2000-2009, 2013 Red Hat, Inc.
+   Copyright (C) 2000-2009, 2013, 2014 Red Hat, Inc.
    This file is part of elfutils.
 
    This file is free software; you can redistribute it and/or modify
@@ -64,6 +64,12 @@ struct ebl
      Ebl architecture can unwind iff FRAME_NREGS > 0.  */
   size_t frame_nregs;
 
+  /* Mask to use to get the return address from an unwind in case the
+     architecture adds some extra non-address bits to it.  When not
+     initialized (0) then ebl_unwind_ret_mask will return ~0, otherwise
+     it should be the actual mask to use.  */
+  GElf_Addr unwind_ret_mask;
+
   /* Function descriptor load address and table as used by
      ebl_resolve_sym_value if available for this arch.  */
   GElf_Addr fd_addr;
-- 
1.9.3


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2014-06-17 14:50 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-06-15 10:50 [PATCH] libebl: Add ebl_unwind_ret_mask Mark Wielaard
  -- strict thread matches above, loose matches on Subject: below --
2014-06-17 14:50 Mark Wielaard
2014-06-17  6:56 Mark Wielaard
2014-06-16 22:23 Roland McGrath
2014-06-15 21:51 Kurt Roeckx
2014-06-15 20:21 Mark Wielaard
2014-06-15 19:59 Kurt Roeckx
2014-06-15 19:27 Kurt Roeckx
2014-06-15 18:56 Mark Wielaard
2014-06-15 18:47 Kurt Roeckx
2014-06-15 18:42 Mark Wielaard
2014-06-15 18:38 Mark Wielaard
2014-06-15 18:18 Kurt Roeckx
2014-06-15 18:08 Kurt Roeckx
2014-06-15 10:39 Kurt Roeckx
2014-06-15 10:30 Mark Wielaard

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).