@@ -851,6 +851,32 @@ kf_strcpy::impl_call_pre (const call_details &cd) const
model->set_value (sized_dest_reg, src_contents_sval, cd.get_ctxt ());
}
+/* Handler for "strdup" and "__builtin_strdup". */
+
+class kf_strdup : public known_function
+{
+public:
+ bool matches_call_types_p (const call_details &cd) const final override
+ {
+ return (cd.num_args () == 1 && cd.arg_is_pointer_p (0));
+ }
+ void impl_call_pre (const call_details &cd) const final override
+ {
+ region_model *model = cd.get_model ();
+ region_model_manager *mgr = cd.get_manager ();
+ /* Ideally we'd get the size here, and simulate copying the bytes. */
+ const region *new_reg
+ = model->get_or_create_region_for_heap_alloc (NULL, cd.get_ctxt ());
+ model->mark_region_as_unknown (new_reg, NULL);
+ if (cd.get_lhs_type ())
+ {
+ const svalue *ptr_sval
+ = mgr->get_ptr_svalue (cd.get_lhs_type (), new_reg);
+ cd.maybe_set_lhs (ptr_sval);
+ }
+ }
+};
+
/* Handle the on_call_pre part of "strlen". */
class kf_strlen : public known_function
@@ -892,6 +918,32 @@ kf_strlen::impl_call_pre (const call_details &cd) const
/* Otherwise a conjured value. */
}
+/* Handler for "strndup" and "__builtin_strndup". */
+
+class kf_strndup : public known_function
+{
+public:
+ bool matches_call_types_p (const call_details &cd) const final override
+ {
+ return (cd.num_args () == 2 && cd.arg_is_pointer_p (0));
+ }
+ void impl_call_pre (const call_details &cd) const final override
+ {
+ region_model *model = cd.get_model ();
+ region_model_manager *mgr = cd.get_manager ();
+ /* Ideally we'd get the size here, and simulate copying the bytes. */
+ const region *new_reg
+ = model->get_or_create_region_for_heap_alloc (NULL, cd.get_ctxt ());
+ model->mark_region_as_unknown (new_reg, NULL);
+ if (cd.get_lhs_type ())
+ {
+ const svalue *ptr_sval
+ = mgr->get_ptr_svalue (cd.get_lhs_type (), new_reg);
+ cd.maybe_set_lhs (ptr_sval);
+ }
+ }
+};
+
class kf_ubsan_bounds : public internal_known_function
{
/* Empty. */
@@ -943,6 +995,8 @@ register_known_functions (known_function_manager &kfm)
kfm.add (BUILT_IN_STRCHR, make_unique<kf_strchr> ());
kfm.add (BUILT_IN_STRCPY, make_unique<kf_strcpy> (2));
kfm.add (BUILT_IN_STRCPY_CHK, make_unique<kf_strcpy> (3));
+ kfm.add (BUILT_IN_STRDUP, make_unique<kf_strdup> ());
+ kfm.add (BUILT_IN_STRNDUP, make_unique<kf_strndup> ());
kfm.add (BUILT_IN_STRLEN, make_unique<kf_strlen> ());
register_varargs_builtins (kfm);
@@ -951,6 +1005,8 @@ register_known_functions (known_function_manager &kfm)
/* Known builtins and C standard library functions. */
{
kfm.add ("memset", make_unique<kf_memset> ());
+ kfm.add ("strdup", make_unique<kf_strdup> ());
+ kfm.add ("strndup", make_unique<kf_strndup> ());
}
/* Known POSIX functions, and some non-standard extensions. */
@@ -1441,6 +1441,8 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt)
if (ctxt)
check_call_args (cd);
+ tree callee_fndecl = get_fndecl_for_call (call, ctxt);
+
/* Some of the cases below update the lhs of the call based on the
return value, but not all. Provide a default value, which may
get overwritten below. */
@@ -1450,13 +1452,22 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt)
const svalue *sval = maybe_get_const_fn_result (cd);
if (!sval)
{
- /* For the common case of functions without __attribute__((const)),
- use a conjured value, and purge any prior state involving that
- value (in case this is in a loop). */
- sval = m_mgr->get_or_create_conjured_svalue (TREE_TYPE (lhs), call,
- lhs_region,
- conjured_purge (this,
- ctxt));
+ if (callee_fndecl
+ && lookup_attribute ("malloc", DECL_ATTRIBUTES (callee_fndecl)))
+ {
+ const region *new_reg
+ = get_or_create_region_for_heap_alloc (NULL, ctxt);
+ mark_region_as_unknown (new_reg, NULL);
+ sval = m_mgr->get_ptr_svalue (cd.get_lhs_type (), new_reg);
+ }
+ else
+ /* For the common case of functions without __attribute__((const)),
+ use a conjured value, and purge any prior state involving that
+ value (in case this is in a loop). */
+ sval = m_mgr->get_or_create_conjured_svalue (TREE_TYPE (lhs), call,
+ lhs_region,
+ conjured_purge (this,
+ ctxt));
}
set_value (lhs_region, sval, ctxt);
}
@@ -1469,7 +1480,7 @@ region_model::on_call_pre (const gcall *call, region_model_context *ctxt)
return false;
}
- if (tree callee_fndecl = get_fndecl_for_call (call, ctxt))
+ if (callee_fndecl)
{
int callee_fndecl_flags = flags_from_decl_or_type (callee_fndecl);
@@ -4909,8 +4920,9 @@ region_model::get_or_create_region_for_heap_alloc (const svalue *size_in_bytes,
get_referenced_base_regions (base_regs_in_use);
const region *reg
= m_mgr->get_or_create_region_for_heap_alloc (base_regs_in_use);
- if (compat_types_p (size_in_bytes->get_type (), size_type_node))
- set_dynamic_extents (reg, size_in_bytes, ctxt);
+ if (size_in_bytes)
+ if (compat_types_p (size_in_bytes->get_type (), size_type_node))
+ set_dynamic_extents (reg, size_in_bytes, ctxt);
return reg;
}
@@ -2578,6 +2578,8 @@ store::set_value (store_manager *mgr, const region *lhs_reg,
const region *ptr_base_reg = ptr_dst->get_base_region ();
mark_as_escaped (ptr_base_reg);
}
+ if (uncertainty)
+ uncertainty->on_maybe_bound_sval (rhs_sval);
}
else if (lhs_base_reg->tracked_p ())
{
new file mode 100644
@@ -0,0 +1,25 @@
+struct foo
+{
+ int m_int;
+};
+
+extern void foo_release (struct foo *);
+extern struct foo *foo_acquire (void)
+ __attribute__ ((malloc (foo_release)));
+
+struct {
+ /* [...snip...] */
+ struct foo *listen_default_ciphers;
+ struct foo *connect_default_ciphers;
+ /* [...snip...] */
+} g;
+
+int parse_global_ciphers(char **args)
+{
+ struct foo **target;
+ target = ((args[0][12] == 'b')
+ ? &g.listen_default_ciphers
+ : &g.connect_default_ciphers);
+ *target = foo_acquire ();
+ return 0; /* { dg-bogus "leak" } */
+}
new file mode 100644
@@ -0,0 +1,15 @@
+extern int open(const char *, int mode);
+#define O_RDONLY 0
+
+struct {
+ int fd_a;
+ int fd_b;
+} g;
+
+int test (const char *path, int flag)
+{
+ int *target;
+ target = flag ? &g.fd_a : &g.fd_b;
+ *target = open (path, O_RDONLY);
+ return 0; /* { dg-bogus "leak of file descriptor" } */
+}
@@ -913,8 +913,7 @@ static int yy_get_next_buffer (void)
if ( number_to_move == YY_MORE_ADJ )
{
ret_val = EOB_ACT_END_OF_FILE;
- yyrestart( yyin );/* { dg-bogus "leak" "" { xfail *-*-* } } */
- /* TODO: leak false positive: PR analyzer/103546. */
+ yyrestart( yyin );/* { dg-bogus "leak" } */
}
else
@@ -1101,8 +1100,7 @@ static int yy_get_next_buffer (void)
#ifdef __cplusplus
return yyinput();
#else
- return input(); /* { dg-bogus "infinite recursion" "" { xfail *-*-* } } */
- /* TODO: infinite recursion positive: PR analyzer/103546. */
+ return input(); /* { dg-bogus "infinite recursion" } */
#endif
}
@@ -1,3 +1,5 @@
+/* { dg-additional-options "-Wno-analyzer-too-complex" } */
+
typedef __SIZE_TYPE__ size_t;
extern void *calloc (size_t __nmemb, size_t __size)
@@ -1,3 +1,5 @@
+/* { dg-additional-options "-Wno-analyzer-too-complex" } */
+
extern char *strdup (const char *__s)
__attribute__ ((__nothrow__ , __leaf__, __malloc__, __nonnull__ (1)));
new file mode 100644
@@ -0,0 +1,19 @@
+extern char *strdup (const char *__s)
+ __attribute__ ((__nothrow__ , __leaf__, __malloc__, __nonnull__ (1)));
+
+struct {
+ /* [...snip...] */
+ char *listen_default_ciphers;
+ char *connect_default_ciphers;
+ /* [...snip...] */
+} g;
+
+int parse_global_ciphers(char **args)
+{
+ char **target;
+ target = ((args[0][12] == 'b')
+ ? &g.listen_default_ciphers
+ : &g.connect_default_ciphers);
+ *target = strdup(args[1]);
+ return 0; /* { dg-bogus "leak" } */
+}
new file mode 100644
@@ -0,0 +1,21 @@
+typedef __SIZE_TYPE__ size_t;
+
+extern char *strndup (const char *__s, size_t sz)
+ __attribute__ ((__nothrow__ , __leaf__, __malloc__, __nonnull__ (1)));
+
+struct {
+ /* [...snip...] */
+ char *listen_default_ciphers;
+ char *connect_default_ciphers;
+ /* [...snip...] */
+} g;
+
+int parse_global_ciphers(char **args)
+{
+ char **target;
+ target = ((args[0][12] == 'b')
+ ? &g.listen_default_ciphers
+ : &g.connect_default_ciphers);
+ *target = strndup(args[1], 42);
+ return 0; /* { dg-bogus "leak" } */
+}
PR analyzer/108252 reports a false positive from -Wanalyzer-malloc-leak on code like this: *ptr_ptr = strdup(EXPR); where ptr_ptr is an UNKNOWN_VALUE. When we handle: *UNKNOWN = PTR; store::set_value normally marks *PTR as having escaped, and this means we don't report PTR as leaking when the last usage of PTR is lost. However this only works for cases where PTR is a region_svalue. In the example in the bug, it's a conjured_svalue, rather than a region_svalue. A similar problem can arise for FDs, which aren't pointers. This patch fixes the bug by updating store::set_value to mark any values stored via *UNKNOWN = VAL as not leaking. Additionally, sm-malloc.cc's known_allocator_p hardcodes strdup and strndup as allocators (and thus transitioning their result to "unchecked"), but we don't implement known_functions for these, leading to the LHS being a CONJURED_SVALUE, rather than a region_svalue to a heap-allocated region. A similar issue happens with functions marked with __attribute__((malloc)). As part of a "belt and braces" fix, the patch also updates the handling of these functions, so that they use heap-allocated regions. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-5113-g688fc162b76dc6. gcc/analyzer/ChangeLog: PR analyzer/108252 * kf.cc (class kf_strdup): New. (class kf_strndup): New. (register_known_functions): Register them. * region-model.cc (region_model::on_call_pre): Use &HEAP_ALLOCATED_REGION for the default result of an external function with the "malloc" attribute, rather than CONJURED_SVALUE. (region_model::get_or_create_region_for_heap_alloc): Allow "size_in_bytes" to be NULL. * store.cc (store::set_value): When handling *UNKNOWN = VAL, mark VAL as "maybe bound". gcc/testsuite/ChangeLog: PR analyzer/108252 * gcc.dg/analyzer/attr-malloc-pr108252.c: New test. * gcc.dg/analyzer/fd-leak-pr108252.c: New test. * gcc.dg/analyzer/flex-with-call-summaries.c: Remove xfail from warning false +ve directives. * gcc.dg/analyzer/pr103217-2.c: Add -Wno-analyzer-too-complex. * gcc.dg/analyzer/pr103217-3.c: Likewise. * gcc.dg/analyzer/strdup-pr108252.c: New test. * gcc.dg/analyzer/strndup-pr108252.c: New test. Signed-off-by: David Malcolm <dmalcolm@redhat.com> --- gcc/analyzer/kf.cc | 56 +++++++++++++++++++ gcc/analyzer/region-model.cc | 32 +++++++---- gcc/analyzer/store.cc | 2 + .../gcc.dg/analyzer/attr-malloc-pr108252.c | 25 +++++++++ .../gcc.dg/analyzer/fd-leak-pr108252.c | 15 +++++ .../analyzer/flex-with-call-summaries.c | 6 +- gcc/testsuite/gcc.dg/analyzer/pr103217-2.c | 2 + gcc/testsuite/gcc.dg/analyzer/pr103217-3.c | 2 + .../gcc.dg/analyzer/strdup-pr108252.c | 19 +++++++ .../gcc.dg/analyzer/strndup-pr108252.c | 21 +++++++ 10 files changed, 166 insertions(+), 14 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-pr108252.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/fd-leak-pr108252.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/strdup-pr108252.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/strndup-pr108252.c