analyzer: check format strings for null termination [PR105899]

This patch extends -fanalyzer to check the format strings of calls
to functions marked with '__attribute__ ((format...))'.

The only checking done in this patch is to check that the format string
is a valid null-terminated string; this patch doesn't attempt to check
the content of the format string.

gcc/analyzer/ChangeLog:
	PR analyzer/105899
	* call-details.cc (call_details::call_details): New ctor.
	* call-details.h (call_details::call_details): New ctor decl.
	(struct call_arg_details): Move here from region-model.cc.
	* region-model.cc (region_model::check_call_format_attr): New.
	(region_model::check_call_args): Call it.
	(struct call_arg_details): Move it to call-details.h.
	* region-model.h (region_model::check_call_format_attr): New decl.

gcc/testsuite/ChangeLog:
	PR analyzer/105899
	* gcc.dg/analyzer/attr-format-1.c: New test.
	* gcc.dg/analyzer/sprintf-1.c: Update expected results for
	now-passing tests.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>

gcc/analyzer/call-details.cc

@@ -58,6 +58,16 @@ call_details::call_details (const gcall *call, region_model *model,
     }
 }
 
+/* call_details's ctor: copy CD, but override the context,
+   using CTXT instead.  */
+
+call_details::call_details (const call_details &cd,
+			    region_model_context *ctxt)
+{
+  *this = cd;
+  m_ctxt = ctxt;
+}
+
 /* Get the manager from m_model.  */
 
 region_model_manager *

gcc/analyzer/call-details.h

@@ -30,6 +30,7 @@ class call_details
 public:
   call_details (const gcall *call, region_model *model,
 		region_model_context *ctxt);
+  call_details (const call_details &cd, region_model_context *ctxt);
 
   region_model *get_model () const { return m_model; }
   region_model_manager *get_manager () const;
@@ -83,6 +84,35 @@ private:
   const region *m_lhs_region;
 };
 
+/* A bundle of information about a problematic argument at a callsite
+   for use by pending_diagnostic subclasses for reporting and
+   for deduplication.  */
+
+struct call_arg_details
+{
+public:
+  call_arg_details (const call_details &cd, unsigned arg_idx)
+  : m_call (cd.get_call_stmt ()),
+    m_called_fndecl (cd.get_fndecl_for_call ()),
+    m_arg_idx (arg_idx),
+    m_arg_expr (cd.get_arg_tree (arg_idx))
+  {
+  }
+
+  bool operator== (const call_arg_details &other) const
+  {
+    return (m_call == other.m_call
+	    && m_called_fndecl == other.m_called_fndecl
+	    && m_arg_idx == other.m_arg_idx
+	    && pending_diagnostic::same_tree_p (m_arg_expr, other.m_arg_expr));
+  }
+
+  const gcall *m_call;
+  tree m_called_fndecl;
+  unsigned m_arg_idx; // 0-based
+  tree m_arg_expr;
+};
+
 } // namespace ana
 
 #endif /* GCC_ANALYZER_CALL_DETAILS_H */

gcc/analyzer/region-model.cc

@@ -1271,14 +1271,108 @@ region_model::on_stmt_pre (const gimple *stmt,
     }
 }
 
+/* Given a call CD with function attribute FORMAT_ATTR, check that the
+   format arg to the call is a valid null-terminated string.  */
+
+void
+region_model::check_call_format_attr (const call_details &cd,
+				      tree format_attr) const
+{
+  /* We assume that FORMAT_ATTR has already been validated.  */
+
+  /* arg0 of the attribute should be kind of format strings
+     that this function expects (e.g. "printf").  */
+  const tree arg0_tree_list = TREE_VALUE (format_attr);
+  if (!arg0_tree_list)
+    return;
+
+  /* arg1 of the attribute should be the 1-based parameter index
+     to treat as the format string.  */
+  const tree arg1_tree_list = TREE_CHAIN (arg0_tree_list);
+  if (!arg1_tree_list)
+    return;
+  const tree arg1_value = TREE_VALUE (arg1_tree_list);
+  if (!arg1_value)
+    return;
+
+  unsigned format_arg_idx = TREE_INT_CST_LOW (arg1_value) - 1;
+  if (cd.num_args () <= format_arg_idx)
+    return;
+
+  /* Subclass of annotating_context that
+     adds a note about the format attr to any saved diagnostics.  */
+  class annotating_ctxt : public annotating_context
+  {
+  public:
+    annotating_ctxt (const call_details &cd,
+		     unsigned fmt_param_idx)
+    : annotating_context (cd.get_ctxt ()),
+      m_cd (cd),
+      m_fmt_param_idx (fmt_param_idx)
+    {
+    }
+    void add_annotations () final override
+    {
+      class reason_format_attr
+	: public pending_note_subclass<reason_format_attr>
+      {
+      public:
+	reason_format_attr (const call_arg_details &arg_details)
+	  : m_arg_details (arg_details)
+	{
+	}
+
+	const char *get_kind () const final override
+	{
+	  return "reason_format_attr";
+	}
+
+	void emit () const final override
+	{
+	  inform (DECL_SOURCE_LOCATION (m_arg_details.m_called_fndecl),
+		  "parameter %i of %qD marked as a format string"
+		  " via %qs attribute",
+		  m_arg_details.m_arg_idx + 1, m_arg_details.m_called_fndecl,
+		  "format");
+	}
+
+	bool operator== (const reason_format_attr &other) const
+	{
+	  return m_arg_details == other.m_arg_details;
+	}
+
+      private:
+	call_arg_details m_arg_details;
+      };
+
+      call_arg_details arg_details (m_cd, m_fmt_param_idx);
+      add_note (make_unique<reason_format_attr> (arg_details));
+    }
+  private:
+    const call_details &m_cd;
+    unsigned m_fmt_param_idx;
+  };
+
+  annotating_ctxt my_ctxt (cd, format_arg_idx);
+  call_details my_cd (cd, &my_ctxt);
+  my_cd.check_for_null_terminated_string_arg (format_arg_idx);
+}
+
 /* Ensure that all arguments at the call described by CD are checked
-   for poisoned values, by calling get_rvalue on each argument.  */
+   for poisoned values, by calling get_rvalue on each argument.
+
+   Check that calls to functions with "format" attribute have valid
+   null-terminated strings for their format argument.  */
 
 void
 region_model::check_call_args (const call_details &cd) const
 {
   for (unsigned arg_idx = 0; arg_idx < cd.num_args (); arg_idx++)
     cd.get_arg_svalue (arg_idx);
+
+  /* Handle attribute "format".  */
+  if (tree format_attr = cd.lookup_function_attribute ("format"))
+    check_call_format_attr (cd, format_attr);
 }
 
 /* Update this model for an outcome of a call that returns a specific
@@ -3175,35 +3269,6 @@ region_model::set_value (tree lhs, tree rhs, region_model_context *ctxt)
   set_value (lhs_reg, rhs_sval, ctxt);
 }
 
-/* A bundle of information about a problematic argument at a callsite
-   for use by pending_diagnostic subclasses for reporting and
-   for deduplication.  */
-
-struct call_arg_details
-{
-public:
-  call_arg_details (const call_details &cd, unsigned arg_idx)
-  : m_call (cd.get_call_stmt ()),
-    m_called_fndecl (cd.get_fndecl_for_call ()),
-    m_arg_idx (arg_idx),
-    m_arg_expr (cd.get_arg_tree (arg_idx))
-  {
-  }
-
-  bool operator== (const call_arg_details &other) const
-  {
-    return (m_call == other.m_call
-	    && m_called_fndecl == other.m_called_fndecl
-	    && m_arg_idx == other.m_arg_idx
-	    && pending_diagnostic::same_tree_p (m_arg_expr, other.m_arg_expr));
-  }
-
-  const gcall *m_call;
-  tree m_called_fndecl;
-  unsigned m_arg_idx; // 0-based
-  tree m_arg_expr;
-};
-
 /* Issue a note specifying that a particular function parameter is expected
    to be a valid null-terminated string.  */
 

gcc/analyzer/region-model.h

@@ -597,6 +597,8 @@ private:
 			    region_model_context *ctxt) const;
 
   void check_call_args (const call_details &cd) const;
+  void check_call_format_attr (const call_details &cd,
+			       tree format_attr) const;
   void check_external_function_for_access_attr (const gcall *call,
 						tree callee_fndecl,
 						region_model_context *ctxt) const;

gcc/testsuite/gcc.dg/analyzer/attr-format-1.c

@@ -0,0 +1,31 @@
+extern int
+my_printf (void *my_object, const char *my_format, ...)
+  __attribute__ ((format (printf, 2, 3)));
+/* { dg-message "parameter 2 of 'my_printf' marked as a format string via 'format' attribute" "attr note" { target *-*-* } .-2 } */
+/* { dg-message "argument 2 of 'my_printf' must be a pointer to a null-terminated string" "arg note" { target *-*-* } .-3 } */
+
+int test_empty (void *my_object, const char *msg)
+{
+  return my_printf (my_object, "");
+}
+
+int test_percent_s (void *my_object, const char *msg)
+{
+  return my_printf (my_object, "%s\n", msg);
+}
+
+int
+test_unterminated_format (void *my_object)
+{
+  char fmt[3] = "abc";
+  return my_printf (my_object, fmt); /* { dg-warning "stack-based buffer over-read" } */
+  /* { dg-message "while looking for null terminator for argument 2 \\('&fmt'\\) of 'my_printf'..." "event" { target *-*-* } .-1 } */
+}
+
+int
+test_uninitialized_format (void *my_object)
+{
+  char fmt[10];
+  return my_printf (my_object, fmt); /* { dg-warning "use of uninitialized value 'fmt\\\[0\\\]'" } */  
+  /* { dg-message "while looking for null terminator for argument 2 \\('&fmt'\\) of 'my_printf'..." "event" { target *-*-* } .-1 } */
+}

gcc/testsuite/gcc.dg/analyzer/sprintf-1.c

@@ -53,12 +53,14 @@ int
 test_uninit_fmt_buf (char *dst)
 {
   const char fmt[10];
-  return sprintf (dst, fmt); // TODO (PR analyzer/105899): complain about "fmt" not being initialized
+  return sprintf (dst, fmt); /* { dg-warning "use of uninitialized value 'fmt\\\[0\\\]'" } */
+  /* { dg-message "while looking for null terminator for argument 2 \\('&fmt'\\) of 'sprintf'..." "event" { target *-*-* } .-1 } */
 }
 
 int
 test_fmt_not_terminated (char *dst)
 {
   const char fmt[3] = "foo";
-  return sprintf (dst, fmt); // TODO (PR analyzer/105899): complain about "fmt" not being terminated
+  return sprintf (dst, fmt); /* { dg-warning "stack-based buffer over-read" } */
+  /* { dg-message "while looking for null terminator for argument 2 \\('&fmt'\\) of 'sprintf'..." "event" { target *-*-* } .-1 } */
 }