From: David Malcolm Date: Mon, 22 Feb 2021 23:46:05 +0000 (-0500) Subject: analyzer: handle error/error_at_line [PR99196] X-Git-Tag: upstream/12.2.0~9621 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5ee4ba031dd9fc60bf2494ca30f46c0acaa34805;p=platform%2Fupstream%2Fgcc.git analyzer: handle error/error_at_line [PR99196] 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. --- diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 89b3be2..0edeb49 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -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; diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc index d3b66ba..72404a5 100644 --- a/gcc/analyzer/region-model-impl-calls.cc +++ b/gcc/analyzer/region-model-impl-calls.cc @@ -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 . + 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, diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 159b0f18..2053f8f 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -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 diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index c73e39f..7768394 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -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 index 0000000..f82a4cd --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/error-1.c @@ -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 index 0000000..138ab9d --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/error-2.c @@ -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 index 0000000..b6ab6c8 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/error-3.c @@ -0,0 +1,11 @@ +/* Verify that we gracefully handle error functions that don't match + the signature of GNU's . */ + +extern void error (void); +extern void error_at_line (void); + +void test_1 (void) +{ + error (); + error_at_line (); +}