public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Alexandre Oliva <aoliva@gcc.gnu.org>
To: gcc-cvs@gcc.gnu.org
Subject: [gcc(refs/users/aoliva/heads/testme)] hardcfr: add checking at exceptions and noreturn calls
Date: Wed, 24 Aug 2022 22:59:31 +0000 (GMT)	[thread overview]
Message-ID: <20220824225931.9D29B385DC16@sourceware.org> (raw)

https://gcc.gnu.org/g:c7cc3fa861a35b4f67a6842143d817367c548c56

commit c7cc3fa861a35b4f67a6842143d817367c548c56
Author: Alexandre Oliva <oliva@adacore.com>
Date:   Wed Aug 24 19:57:59 2022 -0300

    hardcfr: add checking at exceptions and noreturn calls

Diff:
---
 gcc/doc/invoke.texi               |   7 +-
 gcc/gimple-harden-control-flow.cc | 270 +++++++++++++++++++++++++++++++-------
 2 files changed, 226 insertions(+), 51 deletions(-)

diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index df626a0ea39..9c8f2eecb6c 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -16556,9 +16556,10 @@ conditionals.
 @item -fharden-control-flow-redundancy
 @opindex fharden-control-flow-redundancy
 Emit extra code to set booleans when entering basic blocks, and to
-verify, at function exits, that they amount to an execution path that is
-consistent with the control flow graph, trapping otherwise.  Tuning
-options @option{--param hardcfr-max-blocks} and @option{--param
+verify, at function exits (returns, escaping exceptions, and before tail
+and noreturn calls), and trap when they indicate an execution path that
+is incompatible with the control flow graph.  Tuning options
+@option{--param hardcfr-max-blocks} and @option{--param
 hardcfr-max-inline-blocks} are available.
 
 @item -fstack-protector
diff --git a/gcc/gimple-harden-control-flow.cc b/gcc/gimple-harden-control-flow.cc
index 6b08846dbb1..00c84f6ada1 100644
--- a/gcc/gimple-harden-control-flow.cc
+++ b/gcc/gimple-harden-control-flow.cc
@@ -30,6 +30,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "ssa.h"
 #include "gimple-iterator.h"
 #include "tree-cfg.h"
+#include "tree-eh.h"
+#include "except.h"
+#include "sbitmap.h"
 #include "basic-block.h"
 #include "cfghooks.h"
 #include "cfgloop.h"
@@ -60,9 +63,7 @@ const pass_data pass_data_harden_control_flow_redundancy = {
   0,	    // properties_provided
   0,	    // properties_destroyed
   TODO_cleanup_cfg, // properties_start
-  TODO_update_ssa
-  | TODO_cleanup_cfg
-  | TODO_verify_il, // properties_finish
+  0,        // properties_finish
 };
 
 class pass_harden_control_flow_redundancy : public gimple_opt_pass
@@ -79,16 +80,6 @@ public:
     if (!flag_harden_control_flow_redundancy)
       return false;
 
-    /* We don't verify when an exception escapes, propagated or raised
-       by the function itself, so we're only concerned with edges to
-       the exit block.  If there aren't any, the function doesn't
-       return normally, so there won't be any checking point, so
-       there's no point in running the pass.  Should we add
-       verification at exception escapes, we should at least look at
-       !flag_exceptions here.  */
-    if (EDGE_COUNT (EXIT_BLOCK_PTR_FOR_FN (fun)->preds) == 0)
-      return false;
-
     /* Functions that return more than once, like setjmp and vfork
        (that also gets this flag set), will start recording a path
        after the first return, and then may take another path when
@@ -270,7 +261,7 @@ class rt_bb_visited
 
 public:
   /* Prepare to add control flow redundancy testing to CFUN.  */
-  rt_bb_visited ()
+  rt_bb_visited (int noreturn_blocks)
     : nblocks (n_basic_blocks_for_fn (cfun)),
       vword_type (NULL), ckseq (NULL), rtcfg (NULL)
   {
@@ -354,7 +345,9 @@ public:
     gimple_seq_add_stmt (&ckseq, detach);
 
     if (nblocks - 2 > blknum (param_hardcfr_max_inline_blocks)
-	|| !single_pred_p (EXIT_BLOCK_PTR_FOR_FN (cfun)))
+	|| !single_pred_p (EXIT_BLOCK_PTR_FOR_FN (cfun))
+	|| (EDGE_COUNT (EXIT_BLOCK_PTR_FOR_FN (cfun)->preds)
+	    + noreturn_blocks > 1))
       {
 	/* Make sure vword_bits is wide enough for the representation
 	   of nblocks in rtcfg.  Compare with vword_bits << vword_bits,
@@ -379,12 +372,10 @@ public:
     gimple_seq_add_stmt (&ckseq, ckfail_init);
   }
 
-  /* Insert SEQ on E, or close enough (e.g., before a noreturn or tail
-     call at the end of E->src).  */
-  void insert_exit_check (gimple_seq seq, edge e)
+  /* Insert SEQ before a resx, or noreturn or tail call at the end of
+     INSBB, and return TRUE, otherwise return FALSE.  */
+  bool insert_exit_check (gimple_seq seq, basic_block insbb)
   {
-    basic_block insbb = e->src;
-
     /* If the returning block ends with a noreturn call, insert
        checking before it.  This is particularly important for
        __builtin_return.  Other noreturn calls won't have an edge to
@@ -401,43 +392,44 @@ public:
        optimization is detected too late for us.  */
     gimple_stmt_iterator gsi = gsi_last_bb (insbb);
     gimple *ret = gsi_stmt (gsi);
+
+    if (ret && is_a <gresx *> (ret))
+      {
+	gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
+	return true;
+      }
+
     if (ret && is_a <greturn *> (ret))
       {
 	gsi_prev (&gsi);
 	if (!gsi_end_p (gsi))
 	  ret = gsi_stmt (gsi);
       }
-    if (ret && is_a <gcall *> (ret)
+    if (ret
+	&& is_a <gcall *> (ret)
 	&& (gimple_call_noreturn_p (ret)
 	    || gimple_call_must_tail_p (as_a <gcall *> (ret))
 	    || gimple_call_tail_p (as_a <gcall *> (ret))))
       gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
     else
+      return false;
+
+    return true;
+  }
+
+  /* Insert SEQ on E, or close enough (e.g., before a noreturn or tail
+     call at the end of E->src).  */
+  void insert_exit_check (gimple_seq seq, edge e)
+  {
+    if (!insert_exit_check (seq, e->src))
       gsi_insert_seq_on_edge_immediate (e, seq);
   }
 
   /* Add checking code on every exit edge, and initialization code on
      the entry edge.  Before this point, the CFG has been undisturbed,
      and all the needed data has been collected and safely stowed.  */
-  void check ()
+  void check (int count_noreturn, auto_sbitmap const &noreturn_blocks)
   {
-    /* Insert initializers for visited at the entry.  */
-    gimple_seq iseq = NULL;
-
-    gcall *vinit = gimple_build_call (builtin_decl_explicit
-				      (BUILT_IN_MEMSET), 3,
-				      build1 (ADDR_EXPR,
-					      build_pointer_type
-					      (TREE_TYPE (visited)),
-					      visited),
-				      integer_zero_node,
-				      TYPE_SIZE_UNIT (TREE_TYPE (visited)));
-    gimple_seq_add_stmt (&iseq, vinit);
-
-    gsi_insert_seq_on_edge_immediate (single_succ_edge
-				      (ENTRY_BLOCK_PTR_FOR_FN (cfun)),
-				      iseq);
-
     /* If we're using out-of-line checking, create and statically
        initialize the CFG checking representation, generate the
        checker call for the checking sequence, and insert it in all
@@ -506,12 +498,32 @@ public:
 	    gimple_seq seq = ckseq;
 	    /* Copy the sequence, unless we're dealing with the
 	       last edge (we're counting down to zero).  */
-	    if (i)
+	    if (i || count_noreturn)
+	      seq = gimple_seq_copy (seq);
+
+	    edge e = EDGE_PRED (EXIT_BLOCK_PTR_FOR_FN (cfun), i);
+
+	    insert_exit_check (seq, e);
+
+	    gcc_checking_assert (!bitmap_bit_p (noreturn_blocks, e->src->index));
+	  }
+
+	sbitmap_iterator it;
+	unsigned i;
+	EXECUTE_IF_SET_IN_BITMAP (noreturn_blocks, 0, i, it)
+	  {
+	    basic_block bb = BASIC_BLOCK_FOR_FN (cfun, i);
+
+	    gimple_seq seq = ckseq;
+	    gcc_checking_assert (count_noreturn > 0);
+	    if (--count_noreturn)
 	      seq = gimple_seq_copy (seq);
 
-	    insert_exit_check (seq,
-			       EDGE_PRED (EXIT_BLOCK_PTR_FOR_FN (cfun), i));
+	    if (!insert_exit_check (seq, bb))
+	      gcc_unreachable ();
 	  }
+
+	gcc_checking_assert (count_noreturn == 0);
       }
     else
       {
@@ -564,6 +576,24 @@ public:
 	if (dom_info_available_p (CDI_DOMINATORS))
 	  set_immediate_dominator (CDI_DOMINATORS, trp, gimple_bb (last));
       }
+
+    /* Insert initializers for visited at the entry.  Do this after
+       other insertions, to avoid messing with block numbers.  */
+    gimple_seq iseq = NULL;
+
+    gcall *vinit = gimple_build_call (builtin_decl_explicit
+				      (BUILT_IN_MEMSET), 3,
+				      build1 (ADDR_EXPR,
+					      build_pointer_type
+					      (TREE_TYPE (visited)),
+					      visited),
+				      integer_zero_node,
+				      TYPE_SIZE_UNIT (TREE_TYPE (visited)));
+    gimple_seq_add_stmt (&iseq, vinit);
+
+    gsi_insert_seq_on_edge_immediate (single_succ_edge
+				      (ENTRY_BLOCK_PTR_FOR_FN (cfun)),
+				      iseq);
   }
 
   /* Push onto RTCFG a (mask, index) pair to test for IBB when BB is
@@ -691,17 +721,161 @@ public:
    verify at exit that an expect path was taken.  */
 
 unsigned int
-pass_harden_control_flow_redundancy::execute (function *)
+pass_harden_control_flow_redundancy::execute (function *fun)
 {
-  rt_bb_visited vstd;
-
+  basic_block bb_eh_cleanup = NULL;
   basic_block bb;
-  FOR_EACH_BB_FN (bb, cfun)
+
+  if (flag_exceptions)
+    {
+      int lp_eh_cleanup = -1;
+
+      /* Record the preexisting blocks, to avoid visiting newly-created
+	 blocks.  */
+      auto_sbitmap to_visit (last_basic_block_for_fn (fun));
+      bitmap_clear (to_visit);
+
+      FOR_EACH_BB_FN (bb, fun)
+	bitmap_set_bit (to_visit, bb->index);
+
+      /* Scan the blocks for stmts with escaping exceptions, that
+	 wouldn't be denoted in the CFG, and associate them with an
+	 empty cleanup handler around the whole function.  Walk
+	 backwards, so that even when we split the block, */
+      sbitmap_iterator it;
+      unsigned i;
+      EXECUTE_IF_SET_IN_BITMAP (to_visit, 0, i, it)
+	{
+	  bb = BASIC_BLOCK_FOR_FN (fun, i);
+
+	  for (gimple_stmt_iterator gsi = gsi_last_bb (bb);
+	       !gsi_end_p (gsi); gsi_prev (&gsi))
+	    {
+	      gimple *stmt = gsi_stmt (gsi);
+	      if (!gimple_could_trap_p (stmt))
+		continue;
+
+	      /* If it must not throw, or if it already has a handler,
+		 we need not worry about it.  */
+	      if (lookup_stmt_eh_lp (stmt) != 0)
+		continue;
+
+	      if (!stmt_ends_bb_p (stmt))
+		split_block (bb, stmt);
+
+	      if (!bb_eh_cleanup)
+		{
+		  bb_eh_cleanup = create_empty_bb (bb);
+		  if (dom_info_available_p (CDI_DOMINATORS))
+		    set_immediate_dominator (CDI_DOMINATORS, bb_eh_cleanup, bb);
+		  if (current_loops)
+		    add_bb_to_loop (bb_eh_cleanup, current_loops->tree_root);
+
+		  /* Make the new block an EH cleanup for the call.  */
+		  eh_region new_r = gen_eh_region_cleanup (NULL);
+		  eh_landing_pad lp = gen_eh_landing_pad (new_r);
+		  tree label = gimple_block_label (bb_eh_cleanup);
+		  lp->post_landing_pad = label;
+		  EH_LANDING_PAD_NR (label) = lp_eh_cleanup = lp->index;
+
+		  /* Just propagate the exception.
+		     We will later insert the verifier call.  */
+		  gimple_stmt_iterator ehgsi;
+		  ehgsi = gsi_after_labels (bb_eh_cleanup);
+		  gresx *resx = gimple_build_resx (new_r->index);
+		  gsi_insert_before (&ehgsi, resx, GSI_SAME_STMT);
+		}
+	      else
+		{
+		  // Update immedite dominator and loop?
+		}
+
+	      add_stmt_to_eh_lp (stmt, lp_eh_cleanup);
+	      /* Finally, wire the EH cleanup block into the CFG.  */
+	      make_eh_edges (stmt);		}
+	}
+    }
+
+  /* We wish to add verification at blocks without successors, such as
+     noreturn calls (raising or not) and the reraise at the cleanup
+     block, but not other reraises: they will go through the cleanup
+     block.  */
+  int count_noreturn = 0;
+  auto_sbitmap noreturn_blocks (last_basic_block_for_fn (fun));
+  bitmap_clear (noreturn_blocks);
+  FOR_EACH_BB_FN (bb, fun)
+    {
+      if (EDGE_COUNT (bb->succs) == 0)
+	{
+	  if (bitmap_set_bit (noreturn_blocks, bb->index))
+	    count_noreturn++;
+	  continue;
+	}
+
+      /* If there are no exceptions, then any noreturn call must have
+	 zero successor edges.  Otherwise, check for blocks without
+	 non-EH successors, but skip those with resx stmts and edges
+	 (i.e., those other than that in bb_eh_cleanup), since those
+	 will go through bb_eh_cleanup, that will have been counted as
+	 noreturn above because it has no successors.  */
+      gcc_checking_assert (bb != bb_eh_cleanup);
+      if (!flag_exceptions)
+	continue;
+
+      bool found_non_eh_edge = false;
+      bool found_eh_edge = false;
+      edge e;
+      edge_iterator ei;
+      FOR_EACH_EDGE (e, ei, bb->succs)
+	{
+	  if ((e->flags & EDGE_EH))
+	    found_eh_edge = true;
+	  else
+	    found_non_eh_edge = true;
+	  if (found_non_eh_edge && found_eh_edge)
+	    break;
+	}
+
+      if (found_non_eh_edge)
+	continue;
+
+      if (found_eh_edge)
+	{
+	  /* We don't wish to check before (re?)raises, those will
+	     have checking performed at bb_eh_cleanup.  The one
+	     exception is bb_eh_cleanup itself.  */
+	  gimple_stmt_iterator gsi = gsi_last_bb (bb);
+	  gcc_checking_assert (!gsi_end_p (gsi));
+	  gimple *stmt = gsi_stmt (gsi);
+	  if (is_a <gresx *> (stmt))
+	    continue;
+	}
+
+      if (bitmap_set_bit (noreturn_blocks, bb->index))
+	count_noreturn++;
+    }
+
+  gcc_checking_assert (!bb_eh_cleanup
+		       || bitmap_bit_p (noreturn_blocks, bb_eh_cleanup->index));
+
+  /* If we don't have edges to exit nor noreturn calls (including the
+     cleanup reraise), then we may skip instrumentation: that would
+     amount to a function that ends with an infinite loop.  */
+  if (!count_noreturn
+      && EDGE_COUNT (EXIT_BLOCK_PTR_FOR_FN (fun)->preds) == 0)
+    return 0;
+
+  rt_bb_visited vstd (count_noreturn);
+
+  FOR_EACH_BB_FN (bb, fun)
     vstd.visit (bb);
 
-  vstd.check ();
+  vstd.check (count_noreturn, noreturn_blocks);
 
-  return 0;
+  return
+    TODO_update_ssa
+    | TODO_cleanup_cfg
+    | TODO_verify_il;
 }
 
 /* Instantiate a hardcfr pass.  */

             reply	other threads:[~2022-08-24 22:59 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-24 22:59 Alexandre Oliva [this message]
  -- strict thread matches above, loose matches on Subject: below --
2022-09-02 23:34 Alexandre Oliva
2022-08-27  2:55 Alexandre Oliva
2022-08-24 22:45 Alexandre Oliva
2022-08-24 19:39 Alexandre Oliva
2022-08-24 16:52 Alexandre Oliva
2022-08-10 23:51 Alexandre Oliva

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220824225931.9D29B385DC16@sourceware.org \
    --to=aoliva@gcc.gnu.org \
    --cc=gcc-cvs@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).