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.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to trunk as r14-3376-g3b691e0190c6e7.
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.
---
gcc/analyzer/call-details.cc | 10 ++
gcc/analyzer/call-details.h | 30 +++++
gcc/analyzer/region-model.cc | 125 +++++++++++++-----
gcc/analyzer/region-model.h | 2 +
gcc/testsuite/gcc.dg/analyzer/attr-format-1.c | 31 +++++
gcc/testsuite/gcc.dg/analyzer/sprintf-1.c | 6 +-
6 files changed, 172 insertions(+), 32 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-format-1.c
diff --git a/gcc/analyzer/call-details.cc b/gcc/analyzer/call-details.cc
index e497fc58e028..8f5b28ce6c26 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 86f0e68072bd..58b5ccd2acde 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 0fce18896fbc..99817aee3a93 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 63a67b35350b..3979bf124783 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 000000000000..c7fa705585d7
--- /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 c79525d912f1..f8dc806d6192 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 } */
}
--
2.26.3
