diff mbox series

[1/7,analyzer] Support paths for callbacks

Message ID 20191204162530.9285-2-dmalcolm@redhat.com
State New
Headers show
Series Add checking for unsafe calls within signal handlers | expand

Commit Message

David Malcolm Dec. 4, 2019, 4:25 p.m. UTC
This patch extends the path-printing support to cope with paths
for signal-handlers, popping the stack to a new entrypoint, with
events outside any function.

gcc/testsuite/ChangeLog:
	* gcc.dg/plugin/diagnostic-test-paths-4.c: New test.
	* gcc.dg/plugin/diagnostic_plugin_test_paths.c (example_3): New.
	(pass_test_show_path::execute): Call it.
	* gcc.dg/plugin/plugin.exp (plugin_test_list): Add
	diagnostic-test-paths-4.c.

gcc/ChangeLog:
	* tree-diagnostic-path.cc (path_summary::print): Support NULL_TREE
	for range->fndecl.  Handle disjoint paths in which the stack pops
	below the start point.
---
 .../gcc.dg/plugin/diagnostic-test-paths-4.c   | 83 +++++++++++++++++++
 .../plugin/diagnostic_plugin_test_paths.c     | 81 ++++++++++++++++++
 gcc/testsuite/gcc.dg/plugin/plugin.exp        |  1 +
 gcc/tree-diagnostic-path.cc                   | 75 +++++++++--------
 4 files changed, 207 insertions(+), 33 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
diff mbox series

Patch

diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
new file mode 100644
index 000000000000..41cbaaaaa976
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
@@ -0,0 +1,83 @@ 
+/* { dg-do compile } */
+/* { dg-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret -fdiagnostics-show-line-numbers -fdiagnostics-nn-line-numbers" } */
+
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+
+extern void body_of_program(void);
+
+void custom_logger(const char *msg)
+{
+  fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void int_handler(int signum)
+{
+  custom_logger("got signal");
+}
+
+static void register_handler ()
+{
+  signal(SIGINT, int_handler);
+}
+
+void test (void)
+{
+  register_handler ();
+  body_of_program();
+}
+
+/* { dg-begin-multiline-output "" }
+   NN |   fprintf(stderr, "LOG: %s", msg);
+      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+  'test': events 1-2
+    |
+    |   NN | {
+    |      | ^
+    |      | |
+    |      | (1) entering 'test'
+    |   NN |   register_handler ();
+    |      |   ~~~~~~~~~~~~~~~~~~~
+    |      |   |
+    |      |   (2) calling 'register_handler'
+    |
+    +--> 'register_handler': events 3-4
+           |
+           |   NN | {
+           |      | ^
+           |      | |
+           |      | (3) entering 'register_handler'
+           |   NN |   signal(SIGINT, int_handler);
+           |      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+           |      |   |
+           |      |   (4) registering 'int_handler' as signal handler
+           |
+  event 5
+    |
+    |cc1:
+    | (5): later on, when the signal is delivered to the process
+    |
+    +--> 'int_handler': events 6-7
+           |
+           |   NN | {
+           |      | ^
+           |      | |
+           |      | (6) entering 'int_handler'
+           |   NN |   custom_logger("got signal");
+           |      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+           |      |   |
+           |      |   (7) calling 'custom_logger'
+           |
+           +--> 'custom_logger': events 8-9
+                  |
+                  |   NN | {
+                  |      | ^
+                  |      | |
+                  |      | (8) entering 'custom_logger'
+                  |   NN |   fprintf(stderr, "LOG: %s", msg);
+                  |      |   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+                  |      |   |
+                  |      |   (9) calling 'fprintf'
+                  |
+   { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
index 823981550333..cf05ca3a5d32 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
@@ -341,11 +341,92 @@  example_2 ()
     }
 }
 
+/* Example 3: an interprocedural path with a callback.  */
+
+static void
+example_3 ()
+{
+  gimple_stmt_iterator gsi;
+  basic_block bb;
+
+  event_location_t entry_to_custom_logger;
+  event_location_t call_to_fprintf;
+
+  event_location_t entry_to_int_handler;
+  event_location_t call_to_custom_logger;
+
+  event_location_t entry_to_register_handler;
+  event_location_t call_to_signal;
+
+  event_location_t entry_to_test;
+  event_location_t call_to_register_handler;
+
+  cgraph_node *node;
+  FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
+    {
+      function *fun = node->get_fun ();
+      FOR_EACH_BB_FN (bb, fun)
+	{
+	  check_for_named_function (fun, "custom_logger",
+				    &entry_to_custom_logger);
+	  check_for_named_function (fun, "int_handler",
+				    &entry_to_int_handler);
+	  check_for_named_function (fun, "register_handler",
+				    &entry_to_register_handler);
+	  check_for_named_function (fun, "test",
+				    &entry_to_test);
+	  for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+	    {
+	      gimple *stmt = gsi_stmt (gsi);
+	      if (gcall *call = check_for_named_call (stmt, "fprintf", 3))
+		call_to_fprintf.set (call, fun);
+	      if (gcall *call = check_for_named_call (stmt, "custom_logger", 1))
+		call_to_custom_logger.set (call, fun);
+	      if (gcall *call = check_for_named_call (stmt, "register_handler",
+						      0))
+		call_to_register_handler.set (call, fun);
+	      if (gcall *call = check_for_named_call (stmt, "signal", 2))
+		call_to_signal.set (call, fun);
+	    }
+	}
+    }
+
+  if (call_to_fprintf.m_fun)
+    {
+      auto_diagnostic_group d;
+
+      gcc_rich_location richloc (call_to_fprintf.m_loc);
+      test_diagnostic_path path (global_dc->printer);
+      path.add_entry (entry_to_test, 1, "test");
+      path.add_call (call_to_register_handler, 1,
+		     entry_to_register_handler, "register_handler");
+      path.add_event (call_to_signal.m_loc, call_to_signal.m_fun->decl,
+		      2, "registering 'int_handler' as signal handler");
+      path.add_event (UNKNOWN_LOCATION, NULL_TREE, 0,
+		      "later on, when the signal is delivered to the process");
+      path.add_entry (entry_to_int_handler, 1, "int_handler");
+      path.add_call (call_to_custom_logger, 1,
+		     entry_to_custom_logger, "custom_logger");
+      path.add_leaf_call (call_to_fprintf, 2, "fprintf");
+
+      richloc.set_path (&path);
+
+      diagnostic_metadata m;
+      /* CWE-479: Signal Handler Use of a Non-reentrant Function.  */
+      m.add_cwe (479);
+
+      warning_at (&richloc, m, 0,
+		  "call to %qs from within signal handler",
+		  "fprintf");
+    }
+}
+
 unsigned int
 pass_test_show_path::execute (function *)
 {
   example_1 ();
   example_2 ();
+  example_3 ();
 
   return 0;
 }
diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index 4ae30481fb88..6fdb1c67b9e6 100644
--- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
+++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
@@ -98,6 +98,7 @@  set plugin_test_list [list \
 	  diagnostic-test-paths-1.c \
 	  diagnostic-test-paths-2.c \
 	  diagnostic-test-paths-3.c \
+	  diagnostic-test-paths-4.c \
 	  diagnostic-path-format-default.c \
 	  diagnostic-path-format-none.c \
 	  diagnostic-path-format-separate-events.c \
diff --git a/gcc/tree-diagnostic-path.cc b/gcc/tree-diagnostic-path.cc
index abb418dd2fa9..a6b51fd9228b 100644
--- a/gcc/tree-diagnostic-path.cc
+++ b/gcc/tree-diagnostic-path.cc
@@ -339,12 +339,16 @@  path_summary::print (diagnostic_context *dc, bool show_depths) const
 	      cur_indent += strlen (push_prefix);
 	    }
 	}
-      print_fndecl (pp, range->m_fndecl, true);
+      if (range->m_fndecl)
+	{
+	  print_fndecl (pp, range->m_fndecl, true);
+	  pp_string (pp, ": ");
+	}
       if (range->m_start_idx == range->m_end_idx)
-	pp_printf (pp, ": event %i",
+	pp_printf (pp, "event %i",
 		   range->m_start_idx + 1);
       else
-	pp_printf (pp, ": events %i-%i",
+	pp_printf (pp, "events %i-%i",
 		   range->m_start_idx + 1, range->m_end_idx + 1);
       if (show_depths)
 	pp_printf (pp, " (depth %i)", range->m_stack_depth);
@@ -387,36 +391,41 @@  path_summary::print (diagnostic_context *dc, bool show_depths) const
 
 	  if (range->m_stack_depth > next_range->m_stack_depth)
 	    {
-	      /* Show returning from stack frame(s), by printing
-		 something like:
-		 "                   |\n"
-		 "     <------------ +\n"
-		 "     |\n".  */
-
-	      gcc_assert (vbar_column_for_depth.get
-			  (next_range->m_stack_depth));
-
-	      int vbar_for_next_frame
-		= *vbar_column_for_depth.get (next_range->m_stack_depth);
-
-	      int indent_for_next_frame
-		= vbar_for_next_frame - per_frame_indent;
-	      write_indent (pp, vbar_for_next_frame);
-	      pp_string (pp, start_line_color);
-	      pp_character (pp, '<');
-	      for (int i = indent_for_next_frame + per_frame_indent;
-		   i < cur_indent + per_frame_indent - 1; i++)
-		pp_character (pp, '-');
-	      pp_character (pp, '+');
-	      pp_string (pp, end_line_color);
-	      pp_newline (pp);
-	      cur_indent = indent_for_next_frame;
-
-	      write_indent (pp, vbar_for_next_frame);
-	      pp_string (pp, start_line_color);
-	      pp_printf (pp, "|");
-	      pp_string (pp, end_line_color);
-	      pp_newline (pp);
+	      if (vbar_column_for_depth.get (next_range->m_stack_depth))
+		{
+		  /* Show returning from stack frame(s), by printing
+		     something like:
+		     "                   |\n"
+		     "     <------------ +\n"
+		     "     |\n".  */
+		  int vbar_for_next_frame
+		    = *vbar_column_for_depth.get (next_range->m_stack_depth);
+
+		  int indent_for_next_frame
+		    = vbar_for_next_frame - per_frame_indent;
+		  write_indent (pp, vbar_for_next_frame);
+		  pp_string (pp, start_line_color);
+		  pp_character (pp, '<');
+		  for (int i = indent_for_next_frame + per_frame_indent;
+		       i < cur_indent + per_frame_indent - 1; i++)
+		    pp_character (pp, '-');
+		  pp_character (pp, '+');
+		  pp_string (pp, end_line_color);
+		  pp_newline (pp);
+		  cur_indent = indent_for_next_frame;
+
+		  write_indent (pp, vbar_for_next_frame);
+		  pp_string (pp, start_line_color);
+		  pp_printf (pp, "|");
+		  pp_string (pp, end_line_color);
+		  pp_newline (pp);
+		}
+	      else
+		{
+		  /* Handle disjoint paths (e.g. a callback at some later
+		     time).  */
+		  cur_indent = base_indent;
+		}
 	    }
 	  else if (range->m_stack_depth < next_range->m_stack_depth)
 	    {