[gcc(refs/users/aoliva/heads/testme)] hardcfr: add checking at exceptions and noreturn calls: tweaks

Alexandre Oliva aoliva@gcc.gnu.org
Thu Aug 11 04:13:27 GMT 2022


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

commit c8d3669b617c5cebc5c7510b59e2d31a792cbd53
Author: Alexandre Oliva <oliva@gnu.org>
Date:   Wed Aug 10 22:56:28 2022 -0300

    hardcfr: add checking at exceptions and noreturn calls: tweaks

Diff:
---
 gcc/gimple-harden-control-flow.cc | 138 ++++++++++++++++++++++----------------
 libgcc/hardcfr.c                  | 104 +++++++++++++++++++++++++---
 2 files changed, 178 insertions(+), 64 deletions(-)

diff --git a/gcc/gimple-harden-control-flow.cc b/gcc/gimple-harden-control-flow.cc
index 5066a43fe6c..837f498c86d 100644
--- a/gcc/gimple-harden-control-flow.cc
+++ b/gcc/gimple-harden-control-flow.cc
@@ -669,7 +669,7 @@ public:
   /* Add to BB code to set its bit in VISITED, and add to RTCFG or
      CKSEQ the data or code needed to check BB's predecessors and
      successors.  Do NOT change the CFG.  */
-  void visit (basic_block bb)
+  void visit (basic_block bb, bool noreturn)
   {
     /* Set the bit in VISITED when entering the block.  */
     gimple_stmt_iterator gsi = gsi_after_labels (bb);
@@ -689,10 +689,13 @@ public:
 	rtcfg = tree_cons (NULL_TREE, build_int_cst (vword_type, 0), rtcfg);
 
 	/* Then, successors.  */
-	for (int i = EDGE_COUNT (bb->succs); i--; )
-	  if (push_rtcfg_pair (EDGE_SUCC (bb, i)->dest, bb,
-			       EXIT_BLOCK_PTR_FOR_FN (cfun)))
-	    break;
+	if (!noreturn
+	    || !push_rtcfg_pair (EXIT_BLOCK_PTR_FOR_FN (cfun),
+				 bb, EXIT_BLOCK_PTR_FOR_FN (cfun)))
+	  for (int i = EDGE_COUNT (bb->succs); i--; )
+	    if (push_rtcfg_pair (EDGE_SUCC (bb, i)->dest, bb,
+				 EXIT_BLOCK_PTR_FOR_FN (cfun)))
+	      break;
 	rtcfg = tree_cons (NULL_TREE, build_int_cst (vword_type, 0), rtcfg);
       }
     else
@@ -713,6 +716,8 @@ public:
 	gassign *blkruns = gimple_build_assign (ckpart, unshare_expr (bit));
 	gimple_seq_add_stmt (&ckseq, blkruns);
 
+	if (noreturn)
+	  build_block_check (EXIT_BLOCK_PTR_FOR_FN (cfun));
 	for (int i = 0, e = EDGE_COUNT (bb->succs); i < e; i++)
 	  build_block_check (EDGE_SUCC (bb, i)->dest);
 
@@ -729,10 +734,12 @@ public:
 unsigned int
 pass_harden_control_flow_redundancy::execute (function *fun)
 {
+  bool param_check_at_escaping_exceptions = true;
+  bool param_check_before_noreturn_calls = true;
   basic_block bb_eh_cleanup = NULL;
   basic_block bb;
 
-  if (flag_exceptions)
+  if (param_check_at_escaping_exceptions && flag_exceptions)
     {
       int lp_eh_cleanup = -1;
 
@@ -768,6 +775,11 @@ pass_harden_control_flow_redundancy::execute (function *fun)
 
 	      if (!stmt_ends_bb_p (stmt))
 		split_block (bb, stmt);
+	      /* A noreturn call needs not be associated with the
+		 cleanup handler, because we'll add checking before
+		 the call.  */
+	      else if (EDGE_COUNT (bb->succs) == 0)
+		continue;
 
 	      if (!bb_eh_cleanup)
 		{
@@ -791,14 +803,24 @@ pass_harden_control_flow_redundancy::execute (function *fun)
 		  gresx *resx = gimple_build_resx (new_r->index);
 		  gsi_insert_before (&ehgsi, resx, GSI_SAME_STMT);
 		}
-	      else
+	      else if (dom_info_available_p (CDI_DOMINATORS))
 		{
-		  // Update immedite dominator and loop?
+		  basic_block immdom;
+		  immdom = get_immediate_dominator (CDI_DOMINATORS, 
+						    bb_eh_cleanup);
+		  if (!dominated_by_p (CDI_DOMINATORS, immdom, bb))
+		    {
+		      immdom = nearest_common_dominator (CDI_DOMINATORS,
+							 immdom, bb);
+		      set_immediate_dominator (CDI_DOMINATORS,
+					       bb_eh_cleanup, immdom);
+		    }
 		}
 
 	      add_stmt_to_eh_lp (stmt, lp_eh_cleanup);
 	      /* Finally, wire the EH cleanup block into the CFG.  */
-	      make_eh_edges (stmt);		}
+	      make_eh_edges (stmt);
+	    }
 	}
     }
 
@@ -809,57 +831,61 @@ pass_harden_control_flow_redundancy::execute (function *fun)
   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++;
+  if (!param_check_before_noreturn_calls)
+    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;
-	}
 
-      /* 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;
-	}
+	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_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 (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++;
-    }
+	if (bitmap_set_bit (noreturn_blocks, bb->index))
+	  count_noreturn++;
+      }
+  else if (bb_eh_cleanup
+	   && bitmap_set_bit (noreturn_blocks, bb_eh_cleanup->index))
+    count_noreturn++;
 
   gcc_checking_assert (!bb_eh_cleanup
 		       || bitmap_bit_p (noreturn_blocks, bb_eh_cleanup->index));
@@ -874,7 +900,7 @@ pass_harden_control_flow_redundancy::execute (function *fun)
   rt_bb_visited vstd (count_noreturn);
 
   FOR_EACH_BB_FN (bb, fun)
-    vstd.visit (bb);
+    vstd.visit (bb, bitmap_bit_p (noreturn_blocks, bb->index));
 
   vstd.check (count_noreturn, noreturn_blocks);
 
diff --git a/libgcc/hardcfr.c b/libgcc/hardcfr.c
index 8ef29428111..3daf18bf0ae 100644
--- a/libgcc/hardcfr.c
+++ b/libgcc/hardcfr.c
@@ -60,14 +60,25 @@ extern void __hardcfr_check (size_t blocks,
 			     vword const *visited,
 			     vword const *cfg);
 
+/* Compute the MASK for the bit representing BLOCK in WORDIDX's vword in a
+   visited blocks bit array.  */
+static inline void
+block2mask (size_t const block, vword *const mask, size_t *const wordidx)
+{
+  size_t wbits = __CHAR_BIT__ * sizeof (vword);
+  *wordidx = block / wbits;
+  *mask = (vword)1 << (block % wbits);
+}
 
 /* Check whether the bit corresponding to BLOCK is set in VISITED.  */
 static inline bool
 visited_p (size_t const block, vword const *const visited)
 {
-  size_t wbits = __CHAR_BIT__ * sizeof (vword);
-  vword w = visited[block / wbits];
-  return (w & ((vword)1 << (block % wbits))) != 0;
+  vword mask;
+  size_t wordidx;
+  block2mask (block, &mask, &wordidx);
+  vword w = visited[wordidx];
+  return (w & mask) != 0;
 }
 
 /* Read and consume a mask from **CFG_IT.  (Consume meaning advancing the
@@ -118,19 +129,19 @@ consume_seq (vword const **const cfg_it)
    we reach the terminator without finding any.  Consume the entire sequence
    otherwise, so that *CFG_IT points just past the terminator, which may be the
    beginning of the next sequence.  */
-static inline void
+static inline bool
 check_seq (vword const *const visited, vword const **const cfg_it)
 {
   vword mask;
   size_t wordidx;
 
   /* If the block was visited, check that at least one of the
-     preds was also visited.  */
+     preds/succs was also visited.  */
   do
     /* If we get to the end of the sequence without finding any
        match, something is amiss.  */
     if (!next_pair (cfg_it, &mask, &wordidx))
-      __builtin_trap ();
+      return false;
   /* Keep searching until we find a match, at which point the
      condition is satisfied.  */
   while (!test_mask (visited, mask, wordidx));
@@ -139,6 +150,79 @@ check_seq (vword const *const visited, vword const **const cfg_it)
      skipped the block, so as to position the iterator at the beginning of the
      next .  */
   consume_seq (cfg_it);
+
+  return true;
+}
+
+static inline void
+__hardcfr_debug_cfg (size_t const blocks,
+		     void *caller,
+		     vword const *const cfg)
+{
+  __builtin_printf ("CFG at %p, for %p", cfg, caller);
+  vword const *cfg_it = cfg;
+  for (size_t i = 0; i < blocks; i++)
+    {
+      vword mask; size_t wordidx;
+      block2mask (i, &mask, &wordidx);
+      __builtin_printf ("\nblock %lu (%lu/0x%lx)\npreds: ",
+			(unsigned long)i,
+			(unsigned long)wordidx, (unsigned long)mask);
+      while (next_pair (&cfg_it, &mask, &wordidx))
+	__builtin_printf (" (%lux/0x%lx)",
+			  (unsigned long)wordidx, (unsigned long)mask);
+      __builtin_printf ("\nsuccs: ");
+      while (next_pair (&cfg_it, &mask, &wordidx))
+	__builtin_printf (" (%lux/0x%lx)",
+			  (unsigned long)wordidx, (unsigned long)mask);
+    }
+  __builtin_printf ("\n");
+}
+
+#ifndef ATTRIBUTE_UNUSED
+# define ATTRIBUTE_UNUSED __attribute__ ((__unused__))
+#endif
+
+static inline void
+__hardcfr_check_fail (size_t const blocks ATTRIBUTE_UNUSED,
+		      void *caller ATTRIBUTE_UNUSED,
+		      vword const *const cfg ATTRIBUTE_UNUSED,
+		      size_t block ATTRIBUTE_UNUSED,
+		      int which ATTRIBUTE_UNUSED)
+{
+#if 1
+  static const char *parts[] = { "preds", "succs" };
+
+  vword mask; size_t wordidx;
+  block2mask (block, &mask, &wordidx);
+  __builtin_printf ("-fharden-control-flow-redundancy fail at "
+		    " %p block %lu (%lu/0x%lx), expected %s:",
+		    caller, (unsigned long)block,
+		    (unsigned long)wordidx, (unsigned long)mask,
+		    parts[which]);
+
+  /* Skip data for previous blocks.  */
+  vword const *cfg_it = cfg;
+  for (size_t i = block; i--; )
+    {
+      consume_seq (&cfg_it);
+      consume_seq (&cfg_it);
+    }
+  for (size_t i = which; i--; )
+    consume_seq (&cfg_it);
+  
+  while (next_pair (&cfg_it, &mask, &wordidx))
+    __builtin_printf (" (%lux/0x%lx)",
+		      (unsigned long)wordidx, (unsigned long)mask);
+
+  __builtin_printf ("xn");
+
+  /* Reference __hardcfr_debug_cfg so that it's output out-of-line, so that it
+     can be called from a debugger.  */
+  if (!caller || caller == __hardcfr_debug_cfg)
+    return;
+#endif
+  __builtin_trap ();
 }
 
 /* Check that, for each of the BLOCKS basic blocks, if its bit is set in
@@ -168,9 +252,13 @@ __hardcfr_check (size_t const blocks,
       else
 	{
 	  /* Check predecessors.  */
-	  check_seq (visited, &cfg_it);
+	  if (!check_seq (visited, &cfg_it))
+	    __hardcfr_check_fail (blocks, __builtin_return_address (0),
+				  cfg, i, 0);
 	  /* Check successors.  */
-	  check_seq (visited, &cfg_it);
+	  if (!check_seq (visited, &cfg_it))
+	    __hardcfr_check_fail (blocks, __builtin_return_address (0),
+				  cfg, i, 1);
 	}
     }
 }


More information about the Gcc-cvs mailing list