analyzer: handle error/error_at_line [PR99196]
authorDavid Malcolm <dmalcolm@redhat.com>
Mon, 22 Feb 2021 23:46:05 +0000 (18:46 -0500)
committerDavid Malcolm <dmalcolm@redhat.com>
Mon, 22 Feb 2021 23:46:05 +0000 (18:46 -0500)
PR analyzer/99196 describes a false positive from -fanalyzer due to
the analyzer not "knowing" that calls to GNU libc's error(3) with a
nonzero status terminate the process and thus don't return.

This patch fixes the false positive by special-casing "error" and
"error_at_line".

gcc/analyzer/ChangeLog:
PR analyzer/99196
* engine.cc (exploded_node::on_stmt): Provide terminate_path
flag as a way for on_call_pre to terminate the current analysis
path.
* region-model-impl-calls.cc (call_details::num_args): New.
(region_model::impl_call_error): New.
* region-model.cc (region_model::on_call_pre): Add param
"out_terminate_path".  Handle "error" and "error_at_line".
* region-model.h (call_details::num_args): New decl.
(region_model::on_call_pre): Add param "out_terminate_path".
(region_model::impl_call_error): New decl.

gcc/testsuite/ChangeLog:
PR analyzer/99196
* gcc.dg/analyzer/error-1.c: New test.
* gcc.dg/analyzer/error-2.c: New test.
* gcc.dg/analyzer/error-3.c: New test.

gcc/analyzer/engine.cc
gcc/analyzer/region-model-impl-calls.cc
gcc/analyzer/region-model.cc
gcc/analyzer/region-model.h
gcc/testsuite/gcc.dg/analyzer/error-1.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/error-2.c [new file with mode: 0644]
gcc/testsuite/gcc.dg/analyzer/error-3.c [new file with mode: 0644]

index 89b3be2..0edeb49 100644 (file)
@@ -1132,6 +1132,7 @@ exploded_node::on_stmt (exploded_graph &eg,
                                  stmt);
 
   bool unknown_side_effects = false;
+  bool terminate_path = false;
 
   switch (gimple_code (stmt))
     {
@@ -1203,7 +1204,7 @@ exploded_node::on_stmt (exploded_graph &eg,
          }
        else
          unknown_side_effects
-           = state->m_region_model->on_call_pre (call, &ctxt);
+           = state->m_region_model->on_call_pre (call, &ctxt, &terminate_path);
       }
       break;
 
@@ -1215,6 +1216,9 @@ exploded_node::on_stmt (exploded_graph &eg,
       break;
     }
 
+  if (terminate_path)
+    return on_stmt_flags::terminate_path ();
+
   bool any_sm_changes = false;
   int sm_idx;
   sm_state_map *smap;
index d3b66ba..72404a5 100644 (file)
@@ -96,6 +96,14 @@ call_details::maybe_set_lhs (const svalue *result) const
     return false;
 }
 
+/* Return the number of arguments used by the call statement.  */
+
+unsigned
+call_details::num_args () const
+{
+  return gimple_call_num_args (m_call);
+}
+
 /* Get argument IDX at the callsite as a tree.  */
 
 tree
@@ -240,6 +248,36 @@ region_model::impl_call_calloc (const call_details &cd)
   return true;
 }
 
+/* Handle the on_call_pre part of "error" and "error_at_line" from
+   GNU's non-standard <error.h>.
+   MIN_ARGS identifies the minimum number of expected arguments
+   to be consistent with such a call (3 and 5 respectively).
+   Return true if handling it as one of these functions.
+   Write true to *OUT_TERMINATE_PATH if this execution path should be
+   terminated (e.g. the function call terminates the process).  */
+
+bool
+region_model::impl_call_error (const call_details &cd, unsigned min_args,
+                              bool *out_terminate_path)
+{
+  /* Bail if not enough args.  */
+  if (cd.num_args () < min_args)
+    return false;
+
+  /* Initial argument ought to be of type "int".  */
+  if (cd.get_arg_type (0) != integer_type_node)
+    return false;
+
+  /* The process exits if status != 0, so it only continues
+     for the case where status == 0.
+     Add that constraint, or terminate this analysis path.  */
+  tree status = cd.get_arg_tree (0);
+  if (!add_constraint (status, EQ_EXPR, integer_zero_node, cd.get_ctxt ()))
+    *out_terminate_path = true;
+
+  return true;
+}
+
 /* Handle the on_call_post part of "free", after sm-handling.
 
    If the ptr points to an underlying heap region, delete the region,
index 159b0f1..2053f8f 100644 (file)
@@ -741,10 +741,14 @@ region_model::on_assignment (const gassign *assign, region_model_context *ctxt)
 
    Return true if the function call has unknown side effects (it wasn't
    recognized and we don't have a body for it, or are unable to tell which
-   fndecl it is).  */
+   fndecl it is).
+
+   Write true to *OUT_TERMINATE_PATH if this execution path should be
+   terminated (e.g. the function call terminates the process).  */
 
 bool
-region_model::on_call_pre (const gcall *call, region_model_context *ctxt)
+region_model::on_call_pre (const gcall *call, region_model_context *ctxt,
+                          bool *out_terminate_path)
 {
   bool unknown_side_effects = false;
 
@@ -836,6 +840,20 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt)
        return impl_call_calloc (cd);
       else if (is_named_call_p (callee_fndecl, "alloca", call, 1))
        return impl_call_alloca (cd);
+      else if (is_named_call_p (callee_fndecl, "error"))
+       {
+         if (impl_call_error (cd, 3, out_terminate_path))
+           return false;
+         else
+           unknown_side_effects = true;
+       }
+      else if (is_named_call_p (callee_fndecl, "error_at_line"))
+       {
+         if (impl_call_error (cd, 5, out_terminate_path))
+           return false;
+         else
+           unknown_side_effects = true;
+       }
       else if (is_named_call_p (callee_fndecl, "getchar", call, 0))
        {
          /* No side-effects (tracking stream state is out-of-scope
index c73e39f..7768394 100644 (file)
@@ -383,6 +383,8 @@ public:
 
   bool maybe_set_lhs (const svalue *result) const;
 
+  unsigned num_args () const;
+
   tree get_arg_tree (unsigned idx) const;
   tree get_arg_type (unsigned idx) const;
   const svalue *get_arg_svalue (unsigned idx) const;
@@ -442,7 +444,8 @@ class region_model
   void on_assignment (const gassign *stmt, region_model_context *ctxt);
   const svalue *get_gassign_result (const gassign *assign,
                                    region_model_context *ctxt);
-  bool on_call_pre (const gcall *stmt, region_model_context *ctxt);
+  bool on_call_pre (const gcall *stmt, region_model_context *ctxt,
+                   bool *out_terminate_path);
   void on_call_post (const gcall *stmt,
                     bool unknown_side_effects,
                     region_model_context *ctxt);
@@ -455,6 +458,8 @@ class region_model
                                region_model_context *ctxt);
   bool impl_call_builtin_expect (const call_details &cd);
   bool impl_call_calloc (const call_details &cd);
+  bool impl_call_error (const call_details &cd, unsigned min_args,
+                       bool *out_terminate_path);
   void impl_call_free (const call_details &cd);
   bool impl_call_malloc (const call_details &cd);
   void impl_call_memcpy (const call_details &cd);
diff --git a/gcc/testsuite/gcc.dg/analyzer/error-1.c b/gcc/testsuite/gcc.dg/analyzer/error-1.c
new file mode 100644 (file)
index 0000000..f82a4cd
--- /dev/null
@@ -0,0 +1,66 @@
+#include "analyzer-decls.h"
+
+extern int errno;
+
+extern void error (int __status, int __errnum, const char *__format, ...)
+     __attribute__ ((__format__ (__printf__, 3, 4)));
+
+extern void error_at_line (int __status, int __errnum, const char *__fname,
+                          unsigned int __lineno, const char *__format, ...)
+     __attribute__ ((__format__ (__printf__, 5, 6)));
+
+/* When status is an unknown param.  */
+
+void test_1 (int st)
+{
+  error (st, errno, "test");
+  __analyzer_eval (st == 0); /* { dg-warning "TRUE" } */
+}
+
+/* When status is known zero.  */
+
+void test_2 (int st)
+{
+  error (0, errno, "test");
+  __analyzer_dump_path (); /* { dg-message "here" } */
+}
+
+/* When status is a non-zero known constant.  */
+
+void test_3 (int st)
+{
+  error (1, errno, "test");
+  __analyzer_dump_path (); /* { dg-bogus "here" } */
+}
+
+/* When status has been tested against zero.  */
+
+void test_4 (int st)
+{
+  if (st)
+    {
+      error (st, errno, "nonzero branch");
+      __analyzer_dump_path (); /* { dg-bogus "here" } */
+    }
+  else
+    {
+      error (st, errno, "zero branch");
+      __analyzer_dump_path (); /* { dg-message "here" } */
+    }
+}
+
+/* Similarly for error_at_line.  */
+
+void test_5 (int st)
+{
+  error_at_line (st, errno, __FILE__, __LINE__, "test");
+  __analyzer_eval (st == 0); /* { dg-warning "TRUE" } */
+}
+
+/* Non-trivial format string.  */
+
+void test_6 (int st, const char *str)
+{
+  error (st, errno, "test: %s", str);
+  __analyzer_eval (st == 0); /* { dg-warning "TRUE" } */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/error-2.c b/gcc/testsuite/gcc.dg/analyzer/error-2.c
new file mode 100644 (file)
index 0000000..138ab9d
--- /dev/null
@@ -0,0 +1,48 @@
+#define NULL ((void*)0)
+typedef __SIZE_TYPE__ size_t;
+
+extern int errno;
+
+extern void free (void *);
+char *strdup (const char *)
+  __attribute__((malloc (free)));
+
+extern size_t strlen (const char *__s)
+  __attribute__ ((__nothrow__ , __leaf__))
+  __attribute__ ((__pure__))
+  __attribute__ ((__nonnull__ (1)));
+
+extern void error (int __status, int __errnum, const char *__format, ...)
+     __attribute__ ((__format__ (__printf__, 3, 4)));
+
+extern void error_at_line (int __status, int __errnum, const char *__fname,
+                          unsigned int __lineno, const char *__format, ...)
+     __attribute__ ((__format__ (__printf__, 5, 6)));
+
+/* PR analyzer/99196; extract taken from
+     https://github.com/libguestfs/libguestfs/blob/f19fd566f6387ce7e4d82409528c9dde374d25e0/daemon/tar.c#L108
+   (which is GPLv2 or later).  */
+
+extern char *read_whole_file (const char *error_file, size_t *out);
+
+#define EXIT_FAILURE 1
+
+char *read_error_file (const char *error_file)
+{
+  size_t len;
+  char *str;
+
+  str = read_whole_file (error_file, &len);
+  if (str == NULL) {
+    str = strdup ("(no error)");
+    if (str == NULL)
+      error (EXIT_FAILURE, errno, "strdup");
+    len = strlen (str); /* { dg-bogus "NULL" } */
+  }
+
+  /* Remove trailing \n character if any. */
+  if (len > 0 && str[len-1] == '\n')
+    str[--len] = '\0';
+
+  return str;                   /* caller frees */
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/error-3.c b/gcc/testsuite/gcc.dg/analyzer/error-3.c
new file mode 100644 (file)
index 0000000..b6ab6c8
--- /dev/null
@@ -0,0 +1,11 @@
+/* Verify that we gracefully handle error functions that don't match
+   the signature of GNU's <error.h>.  */
+
+extern void error (void);
+extern void error_at_line (void);
+
+void test_1 (void)
+{
+  error ();
+  error_at_line ();
+}