libunwind: fix register numbering on OpenBSD/i386 (again)

classic Classic list List threaded Threaded
6 messages Options
Reply | Threaded
Open this post in threaded view
|

libunwind: fix register numbering on OpenBSD/i386 (again)

Patrick Wildt-3
Hi,

during the libunwind update to 6.0 we lost a particular patchset that
fixes register numbering for OpenBSD/i386, thus breaking exception
handling on that platform.  Looks like no one noticed until now.

ok?

Patrick

diff --git a/lib/libunwind/include/libunwind.h b/lib/libunwind/include/libunwind.h
index 29cf62e4335..4ff0c01b4e2 100644
--- a/lib/libunwind/include/libunwind.h
+++ b/lib/libunwind/include/libunwind.h
@@ -164,8 +164,13 @@ enum {
   UNW_X86_ECX = 1,
   UNW_X86_EDX = 2,
   UNW_X86_EBX = 3,
+#ifdef __OpenBSD__
+  UNW_X86_ESP = 4,
+  UNW_X86_EBP = 5,
+#else
   UNW_X86_EBP = 4,
   UNW_X86_ESP = 5,
+#endif
   UNW_X86_ESI = 6,
   UNW_X86_EDI = 7
 };

Reply | Threaded
Open this post in threaded view
|

Re: libunwind: fix register numbering on OpenBSD/i386 (again)

Robert Nagy
On 31/10/18 09:54 +0100, Patrick Wildt wrote:

> Hi,
>
> during the libunwind update to 6.0 we lost a particular patchset that
> fixes register numbering for OpenBSD/i386, thus breaking exception
> handling on that platform.  Looks like no one noticed until now.
>
> ok?
>
> Patrick
>
> diff --git a/lib/libunwind/include/libunwind.h b/lib/libunwind/include/libunwind.h
> index 29cf62e4335..4ff0c01b4e2 100644
> --- a/lib/libunwind/include/libunwind.h
> +++ b/lib/libunwind/include/libunwind.h
> @@ -164,8 +164,13 @@ enum {
>    UNW_X86_ECX = 1,
>    UNW_X86_EDX = 2,
>    UNW_X86_EBX = 3,
> +#ifdef __OpenBSD__
> +  UNW_X86_ESP = 4,
> +  UNW_X86_EBP = 5,
> +#else
>    UNW_X86_EBP = 4,
>    UNW_X86_ESP = 5,
> +#endif
>    UNW_X86_ESI = 6,
>    UNW_X86_EDI = 7
>  };
>

I think kettenis told me to remove this because they fixed it elsewhere.

Reply | Threaded
Open this post in threaded view
|

Re: libunwind: fix register numbering on OpenBSD/i386 (again)

Mark Kettenis
In reply to this post by Patrick Wildt-3
> Date: Wed, 31 Oct 2018 09:54:35 +0100
> From: Patrick Wildt <[hidden email]>
>
> Hi,
>
> during the libunwind update to 6.0 we lost a particular patchset that
> fixes register numbering for OpenBSD/i386, thus breaking exception
> handling on that platform.  Looks like no one noticed until now.
>
> ok?

I don't think so.  The code in src/Registers.hpp effectively swaps the
two registers on !__APPLE__ platforms.  Is that not working?


> diff --git a/lib/libunwind/include/libunwind.h b/lib/libunwind/include/libunwind.h
> index 29cf62e4335..4ff0c01b4e2 100644
> --- a/lib/libunwind/include/libunwind.h
> +++ b/lib/libunwind/include/libunwind.h
> @@ -164,8 +164,13 @@ enum {
>    UNW_X86_ECX = 1,
>    UNW_X86_EDX = 2,
>    UNW_X86_EBX = 3,
> +#ifdef __OpenBSD__
> +  UNW_X86_ESP = 4,
> +  UNW_X86_EBP = 5,
> +#else
>    UNW_X86_EBP = 4,
>    UNW_X86_ESP = 5,
> +#endif
>    UNW_X86_ESI = 6,
>    UNW_X86_EDI = 7
>  };
>
>

Reply | Threaded
Open this post in threaded view
|

Re: libunwind: fix register numbering on OpenBSD/i386 (again)

Patrick Wildt-3
On Wed, Oct 31, 2018 at 10:03:57AM +0100, Mark Kettenis wrote:

> > Date: Wed, 31 Oct 2018 09:54:35 +0100
> > From: Patrick Wildt <[hidden email]>
> >
> > Hi,
> >
> > during the libunwind update to 6.0 we lost a particular patchset that
> > fixes register numbering for OpenBSD/i386, thus breaking exception
> > handling on that platform.  Looks like no one noticed until now.
> >
> > ok?
>
> I don't think so.  The code in src/Registers.hpp effectively swaps the
> two registers on !__APPLE__ platforms.  Is that not working?

Good point, I missed that.  Well, there's something buggy.  Will dig
more and come back again.

Reply | Threaded
Open this post in threaded view
|

Re: libunwind: fix register numbering on OpenBSD/i386 (again)

Mark Kettenis
> Date: Wed, 31 Oct 2018 10:14:29 +0100
> From: Patrick Wildt <[hidden email]>
>
> On Wed, Oct 31, 2018 at 10:03:57AM +0100, Mark Kettenis wrote:
> > > Date: Wed, 31 Oct 2018 09:54:35 +0100
> > > From: Patrick Wildt <[hidden email]>
> > >
> > > Hi,
> > >
> > > during the libunwind update to 6.0 we lost a particular patchset that
> > > fixes register numbering for OpenBSD/i386, thus breaking exception
> > > handling on that platform.  Looks like no one noticed until now.
> > >
> > > ok?
> >
> > I don't think so.  The code in src/Registers.hpp effectively swaps the
> > two registers on !__APPLE__ platforms.  Is that not working?
>
> Good point, I missed that.  Well, there's something buggy.  Will dig
> more and come back again.

According to http://bluhm.genua.de/regress/results/regress.html the
misc/exception regress test still passes on i386.

Reply | Threaded
Open this post in threaded view
|

Re: libunwind: fix register numbering on OpenBSD/i386 (again)

Gerhard Roth-2
On Wed, 31 Oct 2018 11:06:54 +0100 (CET) Mark Kettenis <[hidden email]> wrote:

> > Date: Wed, 31 Oct 2018 10:14:29 +0100
> > From: Patrick Wildt <[hidden email]>
> >
> > On Wed, Oct 31, 2018 at 10:03:57AM +0100, Mark Kettenis wrote:  
> > > > Date: Wed, 31 Oct 2018 09:54:35 +0100
> > > > From: Patrick Wildt <[hidden email]>
> > > >
> > > > Hi,
> > > >
> > > > during the libunwind update to 6.0 we lost a particular patchset that
> > > > fixes register numbering for OpenBSD/i386, thus breaking exception
> > > > handling on that platform.  Looks like no one noticed until now.
> > > >
> > > > ok?  
> > >
> > > I don't think so.  The code in src/Registers.hpp effectively swaps the
> > > two registers on !__APPLE__ platforms.  Is that not working?  
> >
> > Good point, I missed that.  Well, there's something buggy.  Will dig
> > more and come back again.  
>
> According to http://bluhm.genua.de/regress/results/regress.html the
> misc/exception regress test still passes on i386.


After further digging I can provide a regression test for C++ exceptions
that fails with a segfault on i386 (see patch below).

After unwinding the stack the %esp register is wrong and when the catching
function returns it will segfault because 'ret' fetches an illegal return
address from the stack.

This bug shows up only in LLVM 6.x but not in 5.x and also not in 7.x
(verified on Linux). With 'llvm-dwarfdump -a' its obvious that the
DW_CFA_GNU_args_size items are missing and therefore libunwind can't
adjust the stack pointer correctly.

I suspect that https:// https://reviews.llvm.org/D42848 fixed the problem
in LLVM 7.x.


Gerhard


Index: regress/misc/exceptions/Makefile
===================================================================
RCS file: /cvs/src/regress/misc/exceptions/Makefile,v
retrieving revision 1.3
diff -u -p -u -p -r1.3 Makefile
--- regress/misc/exceptions/Makefile 28 Jan 2007 19:37:00 -0000 1.3
+++ regress/misc/exceptions/Makefile 5 Nov 2018 14:44:01 -0000
@@ -1,3 +1,3 @@
-SUBDIR+= simple libbar foo
+SUBDIR+= simple libbar foo throwup
 
 .include <bsd.subdir.mk>
Index: regress/misc/exceptions/throwup/Makefile
===================================================================
RCS file: regress/misc/exceptions/throwup/Makefile
diff -N regress/misc/exceptions/throwup/Makefile
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ regress/misc/exceptions/throwup/Makefile 5 Nov 2018 14:42:38 -0000
@@ -0,0 +1,6 @@
+.include <bsd.obj.mk>
+
+PROG= throwup
+SRCS= throwup.cc
+
+.include <bsd.regress.mk>
Index: regress/misc/exceptions/throwup/throwup.cc
===================================================================
RCS file: regress/misc/exceptions/throwup/throwup.cc
diff -N regress/misc/exceptions/throwup/throwup.cc
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ regress/misc/exceptions/throwup/throwup.cc 5 Nov 2018 14:42:38 -0000
@@ -0,0 +1,53 @@
+#include <stdio.h>
+#include <sys/types.h>
+
+class throwup {
+public:
+ throwup(void);
+ ~throwup(void);
+
+ void throwupnow(void);
+};
+
+int
+main(int argc, char **argv)
+{
+ throwup* t;
+
+ t = new throwup();
+ delete t;
+}
+
+#ifdef __i386__
+#define GETESP(s)       asm volatile("movl %%esp,%0" : "=r"(s))
+
+#define PRINT_ESP(msg) { \
+ ulong esp; \
+ asm volatile("movl %%esp,%0" : "=r"(esp)); \
+ printf("%s: %%esp == 0x%lx\n", (msg), esp); \
+}
+
+#else
+#define PRINT_ESP
+#endif
+
+throwup::throwup(void)
+{
+}
+
+throwup::~throwup(void)
+{
+ PRINT_ESP("at the beginning");
+ try {
+ throwupnow();
+ } catch (...) {
+ }
+ PRINT_ESP("after exception");
+}
+
+void
+throwup::throwupnow(void)
+{
+ throw false;
+}
+