diff options
Diffstat (limited to 'gcc')
| -rw-r--r-- | gcc/analyzer/call-details.cc | 10 | ||||
| -rw-r--r-- | gcc/analyzer/call-details.h | 30 | ||||
| -rw-r--r-- | gcc/analyzer/region-model.cc | 125 | ||||
| -rw-r--r-- | gcc/analyzer/region-model.h | 2 | ||||
| -rw-r--r-- | gcc/testsuite/gcc.dg/analyzer/attr-format-1.c | 31 | ||||
| -rw-r--r-- | gcc/testsuite/gcc.dg/analyzer/sprintf-1.c | 6 |
6 files changed, 172 insertions, 32 deletions
diff --git a/gcc/analyzer/call-details.cc b/gcc/analyzer/call-details.cc index e497fc5..8f5b28c 100644 --- a/gcc/analyzer/call-details.cc +++ b/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 * diff --git a/gcc/analyzer/call-details.h b/gcc/analyzer/call-details.h index 86f0e68..58b5ccd 100644 --- a/gcc/analyzer/call-details.h +++ b/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 */ diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 0fce188..99817ae 100644 --- a/gcc/analyzer/region-model.cc +++ b/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. */ diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 63a67b3..3979bf1 100644 --- a/gcc/analyzer/region-model.h +++ b/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; diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-format-1.c b/gcc/testsuite/gcc.dg/analyzer/attr-format-1.c new file mode 100644 index 0000000..c7fa705 --- /dev/null +++ b/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 } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/sprintf-1.c b/gcc/testsuite/gcc.dg/analyzer/sprintf-1.c index c79525d..f8dc806 100644 --- a/gcc/testsuite/gcc.dg/analyzer/sprintf-1.c +++ b/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 } */ } |