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(-)

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?

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;