This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Move static chain and non-local goto init after NOTE_INSN_FUNCTION_BEG (PR sanitize/81186).


Hi.

Following bug was for me very educative. I learned that we support non-local gotos that can be combined
with nested functions. Real fun :) Well, the problem is that both cfun->nonlocal_goto_save_area and
cfun->static_chain_decl (emitted in expand_function_start) are put before NOTE_INSN_FUNCTION_BEG.
And so result of expand_used_vars is put after these instrumentations. That causes problems as it uses
stack before we initialize it (use-after-return checking):

expanded cfun->static_chain_decl:

(note 1 0 5 NOTE_INSN_DELETED)
(note 5 1 2 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn 2 5 3 2 (set (reg/f:DI 88 [ CHAIN.1 ])
        (reg:DI 39 r10 [ CHAIN.1 ])) "pr81186.c":5 -1
     (nil))
(insn 3 2 4 2 (set (mem/c:DI (plus:DI (reg/f:DI 82 virtual-stack-vars)
                (const_int -8 [0xfffffffffffffff8])) [0  S8 A64])
        (reg:DI 39 r10 [ CHAIN.1 ])) "pr81186.c":5 -1
     (nil))
(note 4 3 7 2 NOTE_INSN_FUNCTION_BEG)
(insn 7 4 8 2 (set (reg/f:DI 87 [ _2 ])
        (reg/f:DI 88 [ CHAIN.1 ])) "pr81186.c":5 -1
     (nil))
(call_insn 8 7 9 2 (call (mem:QI (symbol_ref:DI ("__asan_handle_no_return") [flags 0x41]  <function_decl 0x2ba005ad5d00 __builtin___asan_handle_no_return>) [0 __builtin___asan_handle_no_return S1 A8])
        (const_int 0 [0])) "pr81186.c":5 -1
     (expr_list:REG_EH_REGION (const_int 0 [0])
        (nil))
    (nil))

expanded cfun->nonlocal_goto_save_area:

(note 1 0 34 NOTE_INSN_DELETED)
(note 34 1 31 2 [bb 2] NOTE_INSN_BASIC_BLOCK)
(insn 31 34 32 2 (set (mem/f/c:DI (plus:DI (reg:DI 95)
                (const_int -64 [0xffffffffffffffc0])) [4 FRAME.0.__nl_goto_buf+0 S8 A64])
        (reg/f:DI 82 virtual-stack-vars)) "pr81186.c":3 -1
     (nil))
(insn 32 31 2 2 (set (mem/f/c:DI (plus:DI (reg:DI 95)
                (const_int -56 [0xffffffffffffffc8])) [4 FRAME.0.__nl_goto_buf+8 S8 A64])
        (reg/f:DI 7 sp)) "pr81186.c":3 -1
     (nil))
(insn 2 32 3 2 (parallel [
            (set (reg:DI 96)
                (plus:DI (reg/f:DI 82 virtual-stack-vars)
                    (const_int -96 [0xffffffffffffffa0])))
            (clobber (reg:CC 17 flags))
        ]) "pr81186.c":3 -1
     (nil))
(insn 3 2 4 2 (set (reg:DI 97)
        (reg:DI 96)) "pr81186.c":3 -1
     (nil))
(insn 4 3 5 2 (set (reg:CCZ 17 flags)
        (compare:CCZ (mem/c:SI (symbol_ref:DI ("__asan_option_detect_stack_use_after_return") [flags 0x40]  <var_decl 0x2ba005b5b750 __asan_option_detect_stack_use_after_return>) [5 __asan_option_detect_stack_use_after_return+0 S4 A32])
            (const_int 0 [0]))) "pr81186.c":3 -1
     (nil))

And thus I suggest to move both these instrumentations after NOTE_INSN_FUNCTION_BEG.
Patch can bootstrap on ppc64le-redhat-linux and survives regression tests.

Ready to be installed?
Martin

gcc/ChangeLog:

2017-06-27  Martin Liska  <mliska@suse.cz>

        PR sanitize/81186
	* function.c (expand_function_start): Move static chain and non-local
	goto init after NOTE_INSN_FUNCTION_BEG.

gcc/testsuite/ChangeLog:

2017-06-27  Martin Liska  <mliska@suse.cz>

        PR sanitize/81186
	* gcc.dg/asan/pr81186.c: New test.
---
 gcc/function.c                      | 18 +++++++++---------
 gcc/testsuite/gcc.dg/asan/pr81186.c | 13 +++++++++++++
 2 files changed, 22 insertions(+), 9 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/asan/pr81186.c


diff --git a/gcc/function.c b/gcc/function.c
index f625489205b..5e8a56099a5 100644
--- a/gcc/function.c
+++ b/gcc/function.c
@@ -5220,6 +5220,14 @@ expand_function_start (tree subr)
      In some cases this requires emitting insns.  */
   assign_parms (subr);
 
+  /* The following was moved from init_function_start.
+     The move is supposed to make sdb output more accurate.  */
+  /* Indicate the beginning of the function body,
+     as opposed to parm setup.  */
+  rtx_note *b = emit_note (NOTE_INSN_FUNCTION_BEG);
+
+  gcc_assert (NOTE_P (get_last_insn ()));
+
   /* If function gets a static chain arg, store it.  */
   if (cfun->static_chain_decl)
     {
@@ -5284,15 +5292,7 @@ expand_function_start (tree subr)
       update_nonlocal_goto_save_area ();
     }
 
-  /* The following was moved from init_function_start.
-     The move is supposed to make sdb output more accurate.  */
-  /* Indicate the beginning of the function body,
-     as opposed to parm setup.  */
-  emit_note (NOTE_INSN_FUNCTION_BEG);
-
-  gcc_assert (NOTE_P (get_last_insn ()));
-
-  parm_birth_insn = get_last_insn ();
+  parm_birth_insn = b;
 
   if (crtl->profile)
     {
diff --git a/gcc/testsuite/gcc.dg/asan/pr81186.c b/gcc/testsuite/gcc.dg/asan/pr81186.c
new file mode 100644
index 00000000000..74d3837a482
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pr81186.c
@@ -0,0 +1,13 @@
+/* PR sanitizer/81186 */
+/* { dg-do run } */
+
+int
+main ()
+{
+  __label__ l;
+  void f () { goto l; }
+
+  f ();
+l:
+  return 0;
+}


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]