libgcc/CET: Skip signal frames when unwinding shadow stack

Message ID 20180411103715.GA33748@intel.com
State New
Headers show
Series
  • libgcc/CET: Skip signal frames when unwinding shadow stack
Related show

Commit Message

H.J. Lu April 11, 2018, 10:37 a.m.
When -fcf-protection -mcet is used, I got

FAIL: g++.dg/eh/sighandle.C

(gdb) bt
 #0  _Unwind_RaiseException (exc=exc@entry=0x416ed0)
    at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:140
 #1  0x00007ffff7d9936b in __cxxabiv1::__cxa_throw (obj=<optimized out>,
    tinfo=0x403dd0 <typeinfo for int@@CXXABI_1.3>, dest=0x0)
    at /export/gnu/import/git/sources/gcc/libstdc++-v3/libsupc++/eh_throw.cc:90
 #2  0x0000000000401255 in sighandler (signo=11, si=0x7fffffffd6f8,
    uc=0x7fffffffd5c0)
    at /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/eh/sighandle.C:9
 #3  <signal handler called> <<<< Signal frame which isn't on shadow stack
 #4  dosegv ()
    at /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/eh/sighandle.C:14
 #5  0x00000000004012e3 in main ()
    at /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/eh/sighandle.C:30
(gdb) p frames
$6 = 5
(gdb)

frame count should be 4, not 5.  This patch skips signal frames when
unwinding shadow stack.

Tested on i686 and x86-64.  OK for trunk?

H.J.
----
	PR libgcc/85334
	* unwind-generic.h (_Unwind_Frames_Increment): New.
	* config/i386/shadow-stack-unwind.h (_Unwind_Frames_Increment):
	Likewise.
	* unwind.inc (_Unwind_RaiseException_Phase2): Increment frame
	count with _Unwind_Frames_Increment.
	(_Unwind_ForcedUnwind_Phase2): Likewise.
---
 libgcc/config/i386/shadow-stack-unwind.h | 5 +++++
 libgcc/unwind-generic.h                  | 3 +++
 libgcc/unwind.inc                        | 6 ++++--
 3 files changed, 12 insertions(+), 2 deletions(-)

-- 
2.14.3

Comments

H.J. Lu April 12, 2018, 7:43 p.m. | #1
On Wed, Apr 11, 2018 at 3:37 AM, H.J. Lu <hongjiu.lu@intel.com> wrote:
> When -fcf-protection -mcet is used, I got

>

> FAIL: g++.dg/eh/sighandle.C

>

> (gdb) bt

>  #0  _Unwind_RaiseException (exc=exc@entry=0x416ed0)

>     at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:140

>  #1  0x00007ffff7d9936b in __cxxabiv1::__cxa_throw (obj=<optimized out>,

>     tinfo=0x403dd0 <typeinfo for int@@CXXABI_1.3>, dest=0x0)

>     at /export/gnu/import/git/sources/gcc/libstdc++-v3/libsupc++/eh_throw.cc:90

>  #2  0x0000000000401255 in sighandler (signo=11, si=0x7fffffffd6f8,

>     uc=0x7fffffffd5c0)

>     at /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/eh/sighandle.C:9

>  #3  <signal handler called> <<<< Signal frame which isn't on shadow stack

>  #4  dosegv ()

>     at /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/eh/sighandle.C:14

>  #5  0x00000000004012e3 in main ()

>     at /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/eh/sighandle.C:30

> (gdb) p frames

> $6 = 5

> (gdb)

>

> frame count should be 4, not 5.  This patch skips signal frames when

> unwinding shadow stack.

>

> Tested on i686 and x86-64.  OK for trunk?

>

> H.J.

> ----

>         PR libgcc/85334

>         * unwind-generic.h (_Unwind_Frames_Increment): New.

>         * config/i386/shadow-stack-unwind.h (_Unwind_Frames_Increment):

>         Likewise.

>         * unwind.inc (_Unwind_RaiseException_Phase2): Increment frame

>         count with _Unwind_Frames_Increment.

>         (_Unwind_ForcedUnwind_Phase2): Likewise.

> ---

>  libgcc/config/i386/shadow-stack-unwind.h | 5 +++++

>  libgcc/unwind-generic.h                  | 3 +++

>  libgcc/unwind.inc                        | 6 ++++--

>  3 files changed, 12 insertions(+), 2 deletions(-)

>

> diff --git a/libgcc/config/i386/shadow-stack-unwind.h b/libgcc/config/i386/shadow-stack-unwind.h

> index 40f48df2aec..a32f3e74b52 100644

> --- a/libgcc/config/i386/shadow-stack-unwind.h

> +++ b/libgcc/config/i386/shadow-stack-unwind.h

> @@ -49,3 +49,8 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see

>         }                                       \

>      }                                          \

>      while (0)

> +

> +/* Increment frame count.  Skip signal frames.  */

> +#undef _Unwind_Frames_Increment

> +#define _Unwind_Frames_Increment(context, frames) \

> +  if (!_Unwind_IsSignalFrame (context)) frames++

> diff --git a/libgcc/unwind-generic.h b/libgcc/unwind-generic.h

> index b5e3568e1bc..639c96f438e 100644

> --- a/libgcc/unwind-generic.h

> +++ b/libgcc/unwind-generic.h

> @@ -291,4 +291,7 @@ EXCEPTION_DISPOSITION _GCC_specific_handler (PEXCEPTION_RECORD, void *,

>  /* Additional actions to unwind number of stack frames.  */

>  #define _Unwind_Frames_Extra(frames)

>

> +/* Increment frame count.  */

> +#define _Unwind_Frames_Increment(context, frames) frames++

> +

>  #endif /* unwind.h */

> diff --git a/libgcc/unwind.inc b/libgcc/unwind.inc

> index 68c08964d30..b49f8797009 100644

> --- a/libgcc/unwind.inc

> +++ b/libgcc/unwind.inc

> @@ -72,8 +72,9 @@ _Unwind_RaiseException_Phase2(struct _Unwind_Exception *exc,

>        /* Don't let us unwind past the handler context.  */

>        gcc_assert (!match_handler);

>

> +      _Unwind_Frames_Increment (context, frames);

> +

>        uw_update_context (context, &fs);

> -      frames++;

>      }

>

>    *frames_p = frames;

> @@ -187,10 +188,11 @@ _Unwind_ForcedUnwind_Phase2 (struct _Unwind_Exception *exc,

>             return _URC_FATAL_PHASE2_ERROR;

>         }

>

> +      _Unwind_Frames_Increment (context, frames);

> +

>        /* Update cur_context to describe the same frame as fs, and discard

>          the previous context if necessary.  */

>        uw_advance_context (context, &fs);

> -      frames++;

>      }

>

>    *frames_p = frames;

> --

> 2.14.3

>


I need to increment frame count after uw_advance_context which will set
the signal frame bit.

OK for trunk?

-- 
H.J.
From 6ced07f8318d2c1faf616395b630c32c32e332f3 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 10 Apr 2018 20:46:04 -0700
Subject: [PATCH] libgcc/CET: Skip signal frames when unwinding shadow stack

When -fcf-protection -mcet is used, I got

FAIL: g++.dg/eh/sighandle.C

(gdb) bt
 #0  _Unwind_RaiseException (exc=exc@entry=0x416ed0)
    at /export/gnu/import/git/sources/gcc/libgcc/unwind.inc:140
 #1  0x00007ffff7d9936b in __cxxabiv1::__cxa_throw (obj=<optimized out>,
    tinfo=0x403dd0 <typeinfo for int@@CXXABI_1.3>, dest=0x0)
    at /export/gnu/import/git/sources/gcc/libstdc++-v3/libsupc++/eh_throw.cc:90
 #2  0x0000000000401255 in sighandler (signo=11, si=0x7fffffffd6f8,
    uc=0x7fffffffd5c0)
    at /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/eh/sighandle.C:9
 #3  <signal handler called> <<<< Signal frame which isn't on shadow stack
 #4  dosegv ()
    at /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/eh/sighandle.C:14
 #5  0x00000000004012e3 in main ()
    at /export/gnu/import/git/sources/gcc/gcc/testsuite/g++.dg/eh/sighandle.C:30
(gdb) p frames
$6 = 5
(gdb)

frame count should be 4, not 5.  This patch skips signal frames when
unwinding shadow stack.

gcc/testsuite/

	PR libgcc/85334
	* g++.dg/torture/pr85334.C: New test.

libgcc/

	PR libgcc/85334
	* unwind-generic.h (_Unwind_Frames_Increment): New.
	* config/i386/shadow-stack-unwind.h (_Unwind_Frames_Increment):
	Likewise.
	* unwind.inc (_Unwind_RaiseException_Phase2): Increment frame
	count with _Unwind_Frames_Increment.
	(_Unwind_ForcedUnwind_Phase2): Likewise.
---
 gcc/testsuite/g++.dg/torture/pr85334.C   | 40 ++++++++++++++++++++++++
 libgcc/config/i386/shadow-stack-unwind.h |  5 +++
 libgcc/unwind-generic.h                  |  3 ++
 libgcc/unwind.inc                        |  4 +--
 4 files changed, 50 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/torture/pr85334.C

diff --git a/gcc/testsuite/g++.dg/torture/pr85334.C b/gcc/testsuite/g++.dg/torture/pr85334.C
new file mode 100644
index 00000000000..7d0e2b7c465
--- /dev/null
+++ b/gcc/testsuite/g++.dg/torture/pr85334.C
@@ -0,0 +1,40 @@
+// { dg-do run { target { i?86-*-linux* i?86-*-gnu* x86_64-*-linux* } } }
+// { dg-require-effective-target cet }
+// { dg-options "-fexceptions -fnon-call-exceptions -O2 -fcf-protection -mcet" }
+
+#include <signal.h>
+#include <stdlib.h>
+
+void sighandler (int signo, siginfo_t * si, void * uc)
+{
+  throw (5);
+}
+
+char * dosegv ()
+{    
+  * ((volatile int *)0) = 12;
+  return 0;
+}
+
+int main ()
+{
+  struct sigaction sa;
+  int status;
+
+  sa.sa_sigaction = sighandler;
+  sa.sa_flags = SA_SIGINFO;
+    
+  status = sigaction (SIGSEGV, & sa, NULL);
+  status = sigaction (SIGBUS, & sa, NULL);
+
+  try {
+    dosegv ();
+  }
+  catch (int x) {
+    return (x != 5);
+  }
+
+  return 1;
+}
+
+
diff --git a/libgcc/config/i386/shadow-stack-unwind.h b/libgcc/config/i386/shadow-stack-unwind.h
index 40f48df2aec..a32f3e74b52 100644
--- a/libgcc/config/i386/shadow-stack-unwind.h
+++ b/libgcc/config/i386/shadow-stack-unwind.h
@@ -49,3 +49,8 @@ see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 	}					\
     }						\
     while (0)
+
+/* Increment frame count.  Skip signal frames.  */
+#undef _Unwind_Frames_Increment
+#define _Unwind_Frames_Increment(context, frames) \
+  if (!_Unwind_IsSignalFrame (context)) frames++
diff --git a/libgcc/unwind-generic.h b/libgcc/unwind-generic.h
index b5e3568e1bc..639c96f438e 100644
--- a/libgcc/unwind-generic.h
+++ b/libgcc/unwind-generic.h
@@ -291,4 +291,7 @@ EXCEPTION_DISPOSITION _GCC_specific_handler (PEXCEPTION_RECORD, void *,
 /* Additional actions to unwind number of stack frames.  */
 #define _Unwind_Frames_Extra(frames)
 
+/* Increment frame count.  */
+#define _Unwind_Frames_Increment(context, frames) frames++
+
 #endif /* unwind.h */
diff --git a/libgcc/unwind.inc b/libgcc/unwind.inc
index 68c08964d30..19a8e4f6c80 100644
--- a/libgcc/unwind.inc
+++ b/libgcc/unwind.inc
@@ -73,7 +73,7 @@ _Unwind_RaiseException_Phase2(struct _Unwind_Exception *exc,
       gcc_assert (!match_handler);
 
       uw_update_context (context, &fs);
-      frames++;
+      _Unwind_Frames_Increment (context, frames);
     }
 
   *frames_p = frames;
@@ -190,7 +190,7 @@ _Unwind_ForcedUnwind_Phase2 (struct _Unwind_Exception *exc,
       /* Update cur_context to describe the same frame as fs, and discard
 	 the previous context if necessary.  */
       uw_advance_context (context, &fs);
-      frames++;
+      _Unwind_Frames_Increment (context, frames);
     }
 
   *frames_p = frames;

Patch

diff --git a/libgcc/config/i386/shadow-stack-unwind.h b/libgcc/config/i386/shadow-stack-unwind.h
index 40f48df2aec..a32f3e74b52 100644
--- a/libgcc/config/i386/shadow-stack-unwind.h
+++ b/libgcc/config/i386/shadow-stack-unwind.h
@@ -49,3 +49,8 @@  see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
 	}					\
     }						\
     while (0)
+
+/* Increment frame count.  Skip signal frames.  */
+#undef _Unwind_Frames_Increment
+#define _Unwind_Frames_Increment(context, frames) \
+  if (!_Unwind_IsSignalFrame (context)) frames++
diff --git a/libgcc/unwind-generic.h b/libgcc/unwind-generic.h
index b5e3568e1bc..639c96f438e 100644
--- a/libgcc/unwind-generic.h
+++ b/libgcc/unwind-generic.h
@@ -291,4 +291,7 @@  EXCEPTION_DISPOSITION _GCC_specific_handler (PEXCEPTION_RECORD, void *,
 /* Additional actions to unwind number of stack frames.  */
 #define _Unwind_Frames_Extra(frames)
 
+/* Increment frame count.  */
+#define _Unwind_Frames_Increment(context, frames) frames++
+
 #endif /* unwind.h */
diff --git a/libgcc/unwind.inc b/libgcc/unwind.inc
index 68c08964d30..b49f8797009 100644
--- a/libgcc/unwind.inc
+++ b/libgcc/unwind.inc
@@ -72,8 +72,9 @@  _Unwind_RaiseException_Phase2(struct _Unwind_Exception *exc,
       /* Don't let us unwind past the handler context.  */
       gcc_assert (!match_handler);
 
+      _Unwind_Frames_Increment (context, frames);
+
       uw_update_context (context, &fs);
-      frames++;
     }
 
   *frames_p = frames;
@@ -187,10 +188,11 @@  _Unwind_ForcedUnwind_Phase2 (struct _Unwind_Exception *exc,
 	    return _URC_FATAL_PHASE2_ERROR;
 	}
 
+      _Unwind_Frames_Increment (context, frames);
+
       /* Update cur_context to describe the same frame as fs, and discard
 	 the previous context if necessary.  */
       uw_advance_context (context, &fs);
-      frames++;
     }
 
   *frames_p = frames;