| Message ID | 20210118024946.4172507-1-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | analyzer: use "malloc" attribute | expand |
On Sun, 17 Jan 2021, David Malcolm wrote: > This is an updated version of this patch from October: > > 'RFC: add "deallocated_by" attribute for use by analyzer' > https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555544.html > > reworking it to build on top of Martin's work as noted below, reusing > the existing attribute rather than adding a new one. > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > Also tested by hand with valgrind. > > Apart from a trivial change to attrib.h and to builtins.c, this > is confined to the analyzer code and docs. The original patch was > posted in stage 1. Is this OK for master? (I'm hoping for > release manager permission to commit this code now; it's not > clear to me whether the end of stage 3 was on the 16th or is > today on the 17th). I guess it's still OK if you're quick. Richard. > Thanks > Dave > > In dce6c58db87ebf7f4477bd3126228e73e4eeee97 msebor extended the > "malloc" attribute to support user-defined allocator/deallocator > pairs. > > This patch extends the "malloc" checker within -fanalyzer to use > these attributes. It is based on an earlier patch: > 'RFC: add "deallocated_by" attribute for use by analyzer' > https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555544.html > which added a different attribute. I mistakenly thought that it would > be easy to merge our patches; the patch turned out to need a lot of > reworking, to support multiple deallocators per allocator. > > My hope was that this would provide a minimal level of markup that would > support library-checking without requiring lots of further markup. > I attempted to use this to detect a memory leak within a Linux > driver (CVE-2019-19078), by adding the attribute to mark these fns: > extern struct urb *usb_alloc_urb(int iso_packets, gfp_t mem_flags); > extern void usb_free_urb(struct urb *urb); > where there is a leak of a "urb" on an error-handling path. > Unfortunately I ran into the problem that there are various other fns > that take "struct urb *" and the analyzer conservatively assumes that a > urb passed to them might or might not be freed and thus stops tracking > state for them. > > Hence this will only detect issues for the simplest cases (without > adding another attribute). > > gcc/analyzer/ChangeLog: > * analyzer.h (is_std_named_call_p): New decl. > * diagnostic-manager.cc (path_builder::get_sm): New. > (state_change_event_creator::state_change_event_creator): Add "pb" > param. > (state_change_event_creator::on_global_state_change): Don't consider > state changes affecting other state_machines. > (state_change_event_creator::on_state_change): Likewise. > (state_change_event_creator::m_pb): New field. > (diagnostic_manager::add_events_for_eedge): Pass pb to visitor > ctor. > * region-model-impl-calls.cc > (region_model::impl_deallocation_call): New. > * region-model.cc: Include "attribs.h". > (region_model::on_call_post): Handle fndecls referenced by > __attribute__((deallocated_by(FOO))). > * region-model.h (region_model::impl_deallocation_call): New decl. > * sm-malloc.cc: Include "stringpool.h" and "attribs.h". Add > leading comment. > (class api): Delete. > (enum resource_state): Update comment for change from api to > deallocator and deallocator_set. > (allocation_state::allocation_state): Drop api param. Add > "deallocators" and "deallocator". > (allocation_state::m_api): Drop field in favor of... > (allocation_state::m_deallocators): New field. > (allocation_state::m_deallocator): New field. > (enum wording): Add WORDING_DEALLOCATED. > (struct deallocator): New. > (struct standard_deallocator): New. > (struct custom_deallocator): New. > (struct deallocator_set): New. > (struct custom_deallocator_set): New. > (struct standard_deallocator_set): New. > (struct deallocator_set_map_traits): New. > (malloc_state_machine::m_malloc): Drop field > (malloc_state_machine::m_scalar_new): Likewise. > (malloc_state_machine::m_vector_new): Likewise. > (malloc_state_machine::m_free): New field > (malloc_state_machine::m_scalar_delete): Likewise. > (malloc_state_machine::m_vector_delete): Likewise. > (malloc_state_machine::deallocator_map_t): New typedef. > (malloc_state_machine::m_deallocator_map): New field. > (malloc_state_machine::deallocator_set_cache_t): New typedef. > (malloc_state_machine::m_custom_deallocator_set_cache): New field. > (malloc_state_machine::custom_deallocator_set_map_t): New typedef. > (malloc_state_machine::m_custom_deallocator_set_map): New field. > (malloc_state_machine::m_dynamic_sets): New field. > (malloc_state_machine::m_dynamic_deallocators): New field. > (api::api): Delete. > (deallocator::deallocator): New ctor. > (deallocator::hash): New. > (deallocator::dump_to_pp): New. > (deallocator::cmp): New. > (deallocator::cmp_ptr_ptr): New. > (standard_deallocator::standard_deallocator): New ctor. > (deallocator_set::deallocator_set): New ctor. > (deallocator_set::dump): New. > (custom_deallocator_set::custom_deallocator_set): New ctor. > (custom_deallocator_set::contains_p): New. > (custom_deallocator_set::maybe_get_single): New. > (custom_deallocator_set::dump_to_pp): New. > (standard_deallocator_set::standard_deallocator_set): New ctor. > (standard_deallocator_set::contains_p): New. > (standard_deallocator_set::maybe_get_single): New. > (standard_deallocator_set::dump_to_pp): New. > (start_p): New. > (class mismatching_deallocation): Update for conversion from api > to deallocator_set and deallocator. > (double_free::emit): Use %qs. > (class use_after_free): Update for conversion from api to > deallocator_set and deallocator. > (malloc_leak::describe_state_change): Only emit "allocated here" on > a start->nonnull transition, rather than on other transitions to > nonnull. > (allocation_state::dump_to_pp): Update for conversion from api to > deallocator_set. > (allocation_state::get_nonnull): Likewise. > (malloc_state_machine::malloc_state_machine): Likewise. > (malloc_state_machine::~malloc_state_machine): New. > (malloc_state_machine::add_state): Update for conversion from api > to deallocator_set. > (malloc_state_machine::get_or_create_custom_deallocator_set): New. > (malloc_state_machine::maybe_create_custom_deallocator_set): New. > (malloc_state_machine::get_or_create_deallocator): New. > (malloc_state_machine::on_stmt): Update for conversion from api > to deallocator_set. Handle "__attribute__((malloc(FOO)))", and > the special attribute set on FOO. > (malloc_state_machine::on_allocator_call): Update for conversion > from api to deallocator_set. Add "returns_nonnull" param and use > it to affect which state to transition to. > (malloc_state_machine::on_deallocator_call): Update for conversion > from api to deallocator_set. > > gcc/ChangeLog: > * attribs.h (fndecl_dealloc_argno): New decl. > * builtins.c (call_dealloc_argno): Split out second half of > function into... > (fndecl_dealloc_argno): New. > * doc/extend.texi (Common Function Attributes): Document the > interaction between the analyzer and the malloc attribute. > * doc/invoke.texi (Static Analyzer Options): Likewise. > > gcc/testsuite/ChangeLog: > * gcc.dg/analyzer/attr-malloc-1.c: New test. > * gcc.dg/analyzer/attr-malloc-2.c: New test. > * gcc.dg/analyzer/attr-malloc-4.c: New test. > * gcc.dg/analyzer/attr-malloc-5.c: New test. > * gcc.dg/analyzer/attr-malloc-6.c: New test. > * gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c: New test. > * gcc.dg/analyzer/attr-malloc-misuses.c: New test. > --- > gcc/analyzer/analyzer.h | 1 + > gcc/analyzer/diagnostic-manager.cc | 15 +- > gcc/analyzer/region-model-impl-calls.cc | 9 + > gcc/analyzer/region-model.cc | 9 + > gcc/analyzer/region-model.h | 1 + > gcc/analyzer/sm-malloc.cc | 767 +++++++++++++++--- > gcc/attribs.h | 2 + > gcc/builtins.c | 10 + > gcc/doc/extend.texi | 52 ++ > gcc/doc/invoke.texi | 14 +- > gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c | 75 ++ > gcc/testsuite/gcc.dg/analyzer/attr-malloc-2.c | 24 + > gcc/testsuite/gcc.dg/analyzer/attr-malloc-4.c | 21 + > gcc/testsuite/gcc.dg/analyzer/attr-malloc-5.c | 12 + > gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c | 228 ++++++ > .../attr-malloc-CVE-2019-19078-usb-leak.c | 224 +++++ > .../gcc.dg/analyzer/attr-malloc-misuses.c | 18 + > 17 files changed, 1354 insertions(+), 128 deletions(-) > create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c > create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-2.c > create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-4.c > create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-5.c > create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c > create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c > create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-misuses.c > > diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h > index f603802c0d6..219fe9df5a6 100644 > --- a/gcc/analyzer/analyzer.h > +++ b/gcc/analyzer/analyzer.h > @@ -205,6 +205,7 @@ extern bool is_special_named_call_p (const gcall *call, const char *funcname, > extern bool is_named_call_p (tree fndecl, const char *funcname); > extern bool is_named_call_p (tree fndecl, const char *funcname, > const gcall *call, unsigned int num_args); > +extern bool is_std_named_call_p (tree fndecl, const char *funcname); > extern bool is_std_named_call_p (tree fndecl, const char *funcname, > const gcall *call, unsigned int num_args); > extern bool is_setjmp_call_p (const gcall *call); > diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc > index 3625e3fe746..22ca024f28e 100644 > --- a/gcc/analyzer/diagnostic-manager.cc > +++ b/gcc/analyzer/diagnostic-manager.cc > @@ -189,6 +189,8 @@ public: > return m_feasibility_problem; > } > > + const state_machine *get_sm () const { return m_sd.m_sm; } > + > private: > typedef reachability<eg_traits> enode_reachability; > > @@ -709,9 +711,11 @@ diagnostic_manager::build_emission_path (const path_builder &pb, > class state_change_event_creator : public state_change_visitor > { > public: > - state_change_event_creator (const exploded_edge &eedge, > + state_change_event_creator (const path_builder &pb, > + const exploded_edge &eedge, > checker_path *emission_path) > - : m_eedge (eedge), > + : m_pb (pb), > + m_eedge (eedge), > m_emission_path (emission_path) > {} > > @@ -720,6 +724,8 @@ public: > state_machine::state_t dst_sm_val) > FINAL OVERRIDE > { > + if (&sm != m_pb.get_sm ()) > + return false; > const exploded_node *src_node = m_eedge.m_src; > const program_point &src_point = src_node->get_point (); > const int src_stack_depth = src_point.get_stack_depth (); > @@ -748,6 +754,8 @@ public: > const svalue *sval, > const svalue *dst_origin_sval) FINAL OVERRIDE > { > + if (&sm != m_pb.get_sm ()) > + return false; > const exploded_node *src_node = m_eedge.m_src; > const program_point &src_point = src_node->get_point (); > const int src_stack_depth = src_point.get_stack_depth (); > @@ -783,6 +791,7 @@ public: > return false; > } > > + const path_builder &m_pb; > const exploded_edge &m_eedge; > checker_path *m_emission_path; > }; > @@ -1002,7 +1011,7 @@ diagnostic_manager::add_events_for_eedge (const path_builder &pb, > | ~~~~~~~~~~~~~^~~~~ > | | > | (3) ...to here (end_cfg_edge_event). */ > - state_change_event_creator visitor (eedge, emission_path); > + state_change_event_creator visitor (pb, eedge, emission_path); > for_each_state_change (src_state, dst_state, pb.get_ext_state (), > &visitor); > > diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc > index 1b81c742c72..d3b66ba7375 100644 > --- a/gcc/analyzer/region-model-impl-calls.cc > +++ b/gcc/analyzer/region-model-impl-calls.cc > @@ -436,6 +436,15 @@ region_model::impl_call_strlen (const call_details &cd) > return true; > } > > +/* Handle calls to functions referenced by > + __attribute__((malloc(FOO))). */ > + > +void > +region_model::impl_deallocation_call (const call_details &cd) > +{ > + impl_call_free (cd); > +} > + > } // namespace ana > > #endif /* #if ENABLE_ANALYZER */ > diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc > index 81e1a481b51..cfe8a391dd9 100644 > --- a/gcc/analyzer/region-model.cc > +++ b/gcc/analyzer/region-model.cc > @@ -65,6 +65,7 @@ along with GCC; see the file COPYING3. If not see > #include "analyzer/region-model-reachability.h" > #include "analyzer/analyzer-selftests.h" > #include "stor-layout.h" > +#include "attribs.h" > > #if ENABLE_ANALYZER > > @@ -917,6 +918,14 @@ region_model::on_call_post (const gcall *call, > impl_call_operator_delete (cd); > return; > } > + /* Was this fndecl referenced by > + __attribute__((malloc(FOO)))? */ > + if (lookup_attribute ("*dealloc", DECL_ATTRIBUTES (callee_fndecl))) > + { > + call_details cd (call, this, ctxt); > + impl_deallocation_call (cd); > + return; > + } > } > > if (unknown_side_effects) > diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h > index 0e303d0398a..efd1a09e362 100644 > --- a/gcc/analyzer/region-model.h > +++ b/gcc/analyzer/region-model.h > @@ -463,6 +463,7 @@ class region_model > bool impl_call_strlen (const call_details &cd); > bool impl_call_operator_new (const call_details &cd); > bool impl_call_operator_delete (const call_details &cd); > + void impl_deallocation_call (const call_details &cd); > > void handle_unrecognized_call (const gcall *call, > region_model_context *ctxt); > diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc > index e1fea02486f..d7c76e343ff 100644 > --- a/gcc/analyzer/sm-malloc.cc > +++ b/gcc/analyzer/sm-malloc.cc > @@ -42,6 +42,8 @@ along with GCC; see the file COPYING3. If not see > #include "analyzer/program-point.h" > #include "analyzer/store.h" > #include "analyzer/region-model.h" > +#include "stringpool.h" > +#include "attribs.h" > > #if ENABLE_ANALYZER > > @@ -49,14 +51,38 @@ namespace ana { > > namespace { > > -class api; > +/* This state machine and its various support classes track allocations > + and deallocations. > + > + It has a few standard allocation/deallocation pairs (e.g. new/delete), > + and also supports user-defined ones via > + __attribute__ ((malloc(DEALLOCATOR))). > + > + There can be more than one valid deallocator for a given allocator, > + for example: > + __attribute__ ((malloc (fclose))) > + __attribute__ ((malloc (freopen, 3))) > + FILE* fopen (const char*, const char*); > + A deallocator_set represents a particular set of valid deallocators. > + > + We track the expected deallocator_set for a value, but not the allocation > + function - there could be more than one allocator per deallocator_set. > + For example, there could be dozens of allocators for "free" beyond just > + malloc e.g. calloc, xstrdup, etc. We don't want to explode the number > + of states by tracking individual allocators in the exploded graph; > + we merely want to track "this value expects to have 'free' called on it". > + Perhaps we can reconstruct which allocator was used later, when emitting > + the path, if it's necessary for precision of wording of diagnostics. */ > + > +class deallocator; > +class deallocator_set; > class malloc_state_machine; > > /* An enum for discriminating between different kinds of allocation_state. */ > > enum resource_state > { > - /* States that are independent of api. */ > + /* States that are independent of allocator/deallocator. */ > > /* The start state. */ > RS_START, > @@ -71,28 +97,33 @@ enum resource_state > /* Stop state, for pointers we don't want to track any more. */ > RS_STOP, > > - /* States that relate to a specific api. */ > + /* States that relate to a specific deallocator_set. */ > > - /* State for a pointer returned from the api's allocator that hasn't > + /* State for a pointer returned from an allocator that hasn't > been checked for NULL. > It could be a pointer to heap-allocated memory, or could be NULL. */ > RS_UNCHECKED, > > - /* State for a pointer returned from the api's allocator, > + /* State for a pointer returned from an allocator, > known to be non-NULL. */ > RS_NONNULL, > > - /* State for a pointer passed to the api's deallocator. */ > + /* State for a pointer passed to a deallocator. */ > RS_FREED > }; > > -/* Custom state subclass, which can optionally refer to an an api. */ > +/* Custom state subclass, which can optionally refer to an a > + deallocator_set. */ > > struct allocation_state : public state_machine::state > { > allocation_state (const char *name, unsigned id, > - enum resource_state rs, const api *a) > - : state (name, id), m_rs (rs), m_api (a) > + enum resource_state rs, > + const deallocator_set *deallocators, > + const deallocator *deallocator) > + : state (name, id), m_rs (rs), > + m_deallocators (deallocators), > + m_deallocator (deallocator) > {} > > void dump_to_pp (pretty_printer *pp) const FINAL OVERRIDE; > @@ -100,7 +131,8 @@ struct allocation_state : public state_machine::state > const allocation_state *get_nonnull () const; > > enum resource_state m_rs; > - const api *m_api; > + const deallocator_set *m_deallocators; > + const deallocator *m_deallocator; > }; > > /* An enum for choosing which wording to use in various diagnostics > @@ -109,52 +141,186 @@ struct allocation_state : public state_machine::state > enum wording > { > WORDING_FREED, > - WORDING_DELETED > + WORDING_DELETED, > + WORDING_DEALLOCATED > }; > > -/* Represents a particular family of API calls for allocating/deallocating > - heap memory that should be matched e.g. > - - malloc/free > - - scalar new/delete > - - vector new[]/delete[] > - etc. > +/* Base class representing a deallocation function, > + either a built-in one we know about, or one exposed via > + __attribute__((malloc(DEALLOCATOR))). */ > > - We track the expected deallocation function, but not the allocation > - function - there could be more than one allocator per deallocator. For > - example, there could be dozens of allocators for "free" beyond just > - malloc e.g. calloc, xstrdup, etc. We don't want to explode the number > - of states by tracking individual allocators in the exploded graph; > - we merely want to track "this value expects to have 'free' called on it". > - Perhaps we can reconstruct which allocator was used later, when emitting > - the path, if it's necessary for precision of wording of diagnostics. */ > - > -struct api > +struct deallocator > { > - api (malloc_state_machine *sm, > - const char *name, > - const char *dealloc_funcname, > - enum wording wording); > + hashval_t hash () const; > + void dump_to_pp (pretty_printer *pp) const; > + static int cmp (const deallocator *a, const deallocator *b); > + static int cmp_ptr_ptr (const void *, const void *); > > - /* An internal name for identifying this API in dumps. */ > + /* Name to use in diagnostics. */ > const char *m_name; > > - /* The name of the deallocation function, for use in diagnostics. */ > - const char *m_dealloc_funcname; > + /* Which wording to use in diagnostics. */ > + enum wording m_wording; > + > + /* State for a value passed to one of the deallocators. */ > + state_machine::state_t m_freed; > + > +protected: > + deallocator (malloc_state_machine *sm, > + const char *name, > + enum wording wording); > +}; > + > +/* Subclass representing a predefined deallocator. > + e.g. "delete []", without needing a specific FUNCTION_DECL > + ahead of time. */ > + > +struct standard_deallocator : public deallocator > +{ > + standard_deallocator (malloc_state_machine *sm, > + const char *name, > + enum wording wording); > +}; > + > +/* Subclass representing a user-defined deallocator > + via __attribute__((malloc(DEALLOCATOR))) given > + a specific FUNCTION_DECL. */ > + > +struct custom_deallocator : public deallocator > +{ > + custom_deallocator (malloc_state_machine *sm, > + tree deallocator_fndecl, > + enum wording wording) > + : deallocator (sm, IDENTIFIER_POINTER (DECL_NAME (deallocator_fndecl)), > + wording) > + { > + } > +}; > + > +/* Base class representing a set of possible deallocators. > + Often this will be just a single deallocator, but some > + allocators have multiple valid deallocators (e.g. the result of > + "fopen" can be closed by either "fclose" or "freopen"). */ > + > +struct deallocator_set > +{ > + deallocator_set (malloc_state_machine *sm, > + enum wording wording); > + virtual ~deallocator_set () {} > + > + virtual bool contains_p (const deallocator *d) const = 0; > + virtual const deallocator *maybe_get_single () const = 0; > + virtual void dump_to_pp (pretty_printer *pp) const = 0; > + void dump () const; > > /* Which wording to use in diagnostics. */ > enum wording m_wording; > > - /* Pointers to api-specific states. > + /* Pointers to states. > These states are owned by the state_machine base class. */ > > - /* State for an unchecked result from this api's allocator. */ > + /* State for an unchecked result from an allocator using this set. */ > state_machine::state_t m_unchecked; > > - /* State for a known non-NULL result from this apis's allocator. */ > + /* State for a known non-NULL result from such an allocator. */ > state_machine::state_t m_nonnull; > +}; > > - /* State for a value passed to this api's deallocator. */ > - state_machine::state_t m_freed; > +/* Subclass of deallocator_set representing a set of deallocators > + defined by one or more __attribute__((malloc(DEALLOCATOR))). */ > + > +struct custom_deallocator_set : public deallocator_set > +{ > + typedef const auto_vec <const deallocator *> *key_t; > + > + custom_deallocator_set (malloc_state_machine *sm, > + const auto_vec <const deallocator *> *vec, > + //const char *name, > + //const char *dealloc_funcname, > + //unsigned arg_idx, > + enum wording wording); > + > + bool contains_p (const deallocator *d) const FINAL OVERRIDE; > + const deallocator *maybe_get_single () const FINAL OVERRIDE; > + void dump_to_pp (pretty_printer *pp) const FINAL OVERRIDE; > + > + auto_vec <const deallocator *> m_deallocator_vec; > +}; > + > +/* Subclass of deallocator_set representing a set of deallocators > + with a single standard_deallocator, e.g. "delete []". */ > + > +struct standard_deallocator_set : public deallocator_set > +{ > + standard_deallocator_set (malloc_state_machine *sm, > + const char *name, > + enum wording wording); > + > + bool contains_p (const deallocator *d) const FINAL OVERRIDE; > + const deallocator *maybe_get_single () const FINAL OVERRIDE; > + void dump_to_pp (pretty_printer *pp) const FINAL OVERRIDE; > + > + standard_deallocator m_deallocator; > +}; > + > +/* Traits class for ensuring uniqueness of deallocator_sets within > + malloc_state_machine. */ > + > +struct deallocator_set_map_traits > +{ > + typedef custom_deallocator_set::key_t key_type; > + typedef custom_deallocator_set *value_type; > + typedef custom_deallocator_set *compare_type; > + > + static inline hashval_t hash (const key_type &k) > + { > + gcc_assert (k != NULL); > + gcc_assert (k != reinterpret_cast<key_type> (1)); > + > + hashval_t result = 0; > + unsigned i; > + const deallocator *d; > + FOR_EACH_VEC_ELT (*k, i, d) > + result ^= d->hash (); > + return result; > + } > + static inline bool equal_keys (const key_type &k1, const key_type &k2) > + { > + if (k1->length () != k2->length ()) > + return false; > + > + for (unsigned i = 0; i < k1->length (); i++) > + if ((*k1)[i] != (*k2)[i]) > + return false; > + > + return true; > + } > + template <typename T> > + static inline void remove (T &) > + { > + /* empty; the nodes are handled elsewhere. */ > + } > + template <typename T> > + static inline void mark_deleted (T &entry) > + { > + entry.m_key = reinterpret_cast<key_type> (1); > + } > + template <typename T> > + static inline void mark_empty (T &entry) > + { > + entry.m_key = NULL; > + } > + template <typename T> > + static inline bool is_deleted (const T &entry) > + { > + return entry.m_key == reinterpret_cast<key_type> (1); > + } > + template <typename T> > + static inline bool is_empty (const T &entry) > + { > + return entry.m_key == NULL; > + } > + static const bool empty_zero_p = false; > }; > > /* A state machine for detecting misuses of the malloc/free API. > @@ -167,9 +333,12 @@ public: > typedef allocation_state custom_data_t; > > malloc_state_machine (logger *logger); > + ~malloc_state_machine (); > > state_t > - add_state (const char *name, enum resource_state rs, const api *a); > + add_state (const char *name, enum resource_state rs, > + const deallocator_set *deallocators, > + const deallocator *deallocator); > > bool inherited_state_p () const FINAL OVERRIDE { return false; } > > @@ -214,9 +383,9 @@ public: > bool reset_when_passed_to_unknown_fn_p (state_t s, > bool is_mutable) const FINAL OVERRIDE; > > - api m_malloc; > - api m_scalar_new; > - api m_vector_new; > + standard_deallocator_set m_free; > + standard_deallocator_set m_scalar_delete; > + standard_deallocator_set m_vector_delete; > > /* States that are independent of api. */ > > @@ -232,33 +401,194 @@ public: > state_t m_stop; > > private: > + const custom_deallocator_set * > + get_or_create_custom_deallocator_set (tree allocator_fndecl); > + custom_deallocator_set * > + maybe_create_custom_deallocator_set (tree allocator_fndecl); > + const deallocator * > + get_or_create_deallocator (tree deallocator_fndecl); > + > void on_allocator_call (sm_context *sm_ctxt, > const gcall *call, > - const api &ap) const; > + const deallocator_set *deallocators, > + bool returns_nonnull = false) const; > void on_deallocator_call (sm_context *sm_ctxt, > const supernode *node, > const gcall *call, > - const api &ap) const; > + const deallocator *d, > + unsigned argno) const; > void on_zero_assignment (sm_context *sm_ctxt, > const gimple *stmt, > tree lhs) const; > + > + /* A map for consolidating deallocators so that they are > + unique per deallocator FUNCTION_DECL. */ > + typedef hash_map<tree, deallocator *> deallocator_map_t; > + deallocator_map_t m_deallocator_map; > + > + /* Memoized lookups from FUNCTION_DECL to custom_deallocator_set *. */ > + typedef hash_map<tree, custom_deallocator_set *> deallocator_set_cache_t; > + deallocator_set_cache_t m_custom_deallocator_set_cache; > + > + /* A map for consolidating custom_deallocator_set instances. */ > + typedef hash_map<custom_deallocator_set::key_t, > + custom_deallocator_set *, > + deallocator_set_map_traits> custom_deallocator_set_map_t; > + custom_deallocator_set_map_t m_custom_deallocator_set_map; > + > + /* Record of dynamically-allocated objects, for cleanup. */ > + auto_vec <custom_deallocator_set *> m_dynamic_sets; > + auto_vec <custom_deallocator *> m_dynamic_deallocators; > }; > > -/* struct api. */ > +/* struct deallocator. */ > > -api::api (malloc_state_machine *sm, > - const char *name, > - const char *dealloc_funcname, > - enum wording wording) > +deallocator::deallocator (malloc_state_machine *sm, > + const char *name, > + enum wording wording) > : m_name (name), > - m_dealloc_funcname (dealloc_funcname), > m_wording (wording), > - m_unchecked (sm->add_state ("unchecked", RS_UNCHECKED, this)), > - m_nonnull (sm->add_state ("nonnull", RS_NONNULL, this)), > - m_freed (sm->add_state ("freed", RS_FREED, this)) > + m_freed (sm->add_state ("freed", RS_FREED, NULL, this)) > +{ > +} > + > +hashval_t > +deallocator::hash () const > +{ > + return (hashval_t)m_freed->get_id (); > +} > + > +void > +deallocator::dump_to_pp (pretty_printer *pp) const > +{ > + pp_printf (pp, "%qs", m_name); > +} > + > +int > +deallocator::cmp (const deallocator *a, const deallocator *b) > +{ > + return (int)a->m_freed->get_id () - (int)b->m_freed->get_id (); > +} > + > +int > +deallocator::cmp_ptr_ptr (const void *a, const void *b) > +{ > + return cmp (*(const deallocator * const *)a, > + *(const deallocator * const *)b); > +} > + > + > +/* struct standard_deallocator : public deallocator. */ > + > +standard_deallocator::standard_deallocator (malloc_state_machine *sm, > + const char *name, > + enum wording wording) > +: deallocator (sm, name, wording) > +{ > +} > + > +/* struct deallocator_set. */ > + > +deallocator_set::deallocator_set (malloc_state_machine *sm, > + enum wording wording) > +: m_wording (wording), > + m_unchecked (sm->add_state ("unchecked", RS_UNCHECKED, this, NULL)), > + m_nonnull (sm->add_state ("nonnull", RS_NONNULL, this, NULL)) > +{ > +} > + > +/* Dump a description of this deallocator_set to stderr. */ > + > +DEBUG_FUNCTION void > +deallocator_set::dump () const > +{ > + pretty_printer pp; > + pp_show_color (&pp) = pp_show_color (global_dc->printer); > + pp.buffer->stream = stderr; > + dump_to_pp (&pp); > + pp_newline (&pp); > + pp_flush (&pp); > +} > + > +/* struct custom_deallocator_set : public deallocator_set. */ > + > +custom_deallocator_set:: > +custom_deallocator_set (malloc_state_machine *sm, > + const auto_vec <const deallocator *> *vec, > + enum wording wording) > +: deallocator_set (sm, wording), > + m_deallocator_vec (vec->length ()) > +{ > + unsigned i; > + const deallocator *d; > + FOR_EACH_VEC_ELT (*vec, i, d) > + m_deallocator_vec.safe_push (d); > +} > + > +bool > +custom_deallocator_set::contains_p (const deallocator *d) const > +{ > + unsigned i; > + const deallocator *cd; > + FOR_EACH_VEC_ELT (m_deallocator_vec, i, cd) > + if (cd == d) > + return true; > + return false; > +} > + > +const deallocator * > +custom_deallocator_set::maybe_get_single () const > +{ > + if (m_deallocator_vec.length () == 1) > + return m_deallocator_vec[0]; > + return NULL; > +} > + > +void > +custom_deallocator_set::dump_to_pp (pretty_printer *pp) const > +{ > + pp_character (pp, '{'); > + unsigned i; > + const deallocator *d; > + FOR_EACH_VEC_ELT (m_deallocator_vec, i, d) > + { > + if (i > 0) > + pp_string (pp, ", "); > + d->dump_to_pp (pp); > + } > + pp_character (pp, '}'); > +} > + > +/* struct standard_deallocator_set : public deallocator_set. */ > + > +standard_deallocator_set::standard_deallocator_set (malloc_state_machine *sm, > + const char *name, > + enum wording wording) > +: deallocator_set (sm, wording), > + m_deallocator (sm, name, wording) > { > } > > +bool > +standard_deallocator_set::contains_p (const deallocator *d) const > +{ > + return d == &m_deallocator; > +} > + > +const deallocator * > +standard_deallocator_set::maybe_get_single () const > +{ > + return &m_deallocator; > +} > + > +void > +standard_deallocator_set::dump_to_pp (pretty_printer *pp) const > +{ > + pp_character (pp, '{'); > + pp_string (pp, m_deallocator.m_name); > + pp_character (pp, '}'); > +} > + > /* Return STATE cast to the custom state subclass, or NULL for the start state. > Everything should be an allocation_state apart from the start state. */ > > @@ -291,6 +621,14 @@ get_rs (state_machine::state_t state) > return RS_START; > } > > +/* Return true if STATE is the start state. */ > + > +static bool > +start_p (state_machine::state_t state) > +{ > + return get_rs (state) == RS_START; > +} > + > /* Return true if STATE is an unchecked result from an allocator. */ > > static bool > @@ -383,10 +721,10 @@ class mismatching_deallocation : public malloc_diagnostic > { > public: > mismatching_deallocation (const malloc_state_machine &sm, tree arg, > - const api *expected_dealloc, > - const api *actual_dealloc) > + const deallocator_set *expected_deallocators, > + const deallocator *actual_dealloc) > : malloc_diagnostic (sm, arg), > - m_expected_dealloc (expected_dealloc), > + m_expected_deallocators (expected_deallocators), > m_actual_dealloc (actual_dealloc) > {} > > @@ -400,11 +738,18 @@ public: > auto_diagnostic_group d; > diagnostic_metadata m; > m.add_cwe (762); /* CWE-762: Mismatched Memory Management Routines. */ > - return warning_meta (rich_loc, m, OPT_Wanalyzer_mismatching_deallocation, > - "%qE should have been deallocated with %qs" > - " but was deallocated with %qs", > - m_arg, m_expected_dealloc->m_dealloc_funcname, > - m_actual_dealloc->m_dealloc_funcname); > + if (const deallocator *expected_dealloc > + = m_expected_deallocators->maybe_get_single ()) > + return warning_meta (rich_loc, m, OPT_Wanalyzer_mismatching_deallocation, > + "%qE should have been deallocated with %qs" > + " but was deallocated with %qs", > + m_arg, expected_dealloc->m_name, > + m_actual_dealloc->m_name); > + else > + return warning_meta (rich_loc, m, OPT_Wanalyzer_mismatching_deallocation, > + "%qs called on %qE returned from a mismatched" > + " allocation function", > + m_actual_dealloc->m_name, m_arg); > } > > label_text describe_state_change (const evdesc::state_change &change) > @@ -413,9 +758,13 @@ public: > if (unchecked_p (change.m_new_state)) > { > m_alloc_event = change.m_event_id; > - return change.formatted_print ("allocated here" > - " (expects deallocation with %qs)", > - m_expected_dealloc->m_dealloc_funcname); > + if (const deallocator *expected_dealloc > + = m_expected_deallocators->maybe_get_single ()) > + return change.formatted_print ("allocated here" > + " (expects deallocation with %qs)", > + expected_dealloc->m_name); > + else > + return change.formatted_print ("allocated here"); > } > return malloc_diagnostic::describe_state_change (change); > } > @@ -423,19 +772,28 @@ public: > label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE > { > if (m_alloc_event.known_p ()) > - return ev.formatted_print > - ("deallocated with %qs here;" > - " allocation at %@ expects deallocation with %qs", > - m_actual_dealloc->m_dealloc_funcname, &m_alloc_event, > - m_expected_dealloc->m_dealloc_funcname); > + { > + if (const deallocator *expected_dealloc > + = m_expected_deallocators->maybe_get_single ()) > + return ev.formatted_print > + ("deallocated with %qs here;" > + " allocation at %@ expects deallocation with %qs", > + m_actual_dealloc->m_name, &m_alloc_event, > + expected_dealloc->m_name); > + else > + return ev.formatted_print > + ("deallocated with %qs here;" > + " allocated at %@", > + m_actual_dealloc->m_name, &m_alloc_event); > + } > return ev.formatted_print ("deallocated with %qs here", > - m_actual_dealloc->m_dealloc_funcname); > + m_actual_dealloc->m_name); > } > > private: > diagnostic_event_id_t m_alloc_event; > - const api *m_expected_dealloc; > - const api *m_actual_dealloc; > + const deallocator_set *m_expected_deallocators; > + const deallocator *m_actual_dealloc; > }; > > /* Concrete subclass for reporting double-free diagnostics. */ > @@ -455,7 +813,7 @@ public: > diagnostic_metadata m; > m.add_cwe (415); /* CWE-415: Double Free. */ > return warning_meta (rich_loc, m, OPT_Wanalyzer_double_free, > - "double-%<%s%> of %qE", m_funcname, m_arg); > + "double-%qs of %qE", m_funcname, m_arg); > } > > label_text describe_state_change (const evdesc::state_change &change) > @@ -765,9 +1123,12 @@ class use_after_free : public malloc_diagnostic > { > public: > use_after_free (const malloc_state_machine &sm, tree arg, > - const api *a) > - : malloc_diagnostic (sm, arg), m_api (a) > - {} > + const deallocator *deallocator) > + : malloc_diagnostic (sm, arg), > + m_deallocator (deallocator) > + { > + gcc_assert (deallocator); > + } > > const char *get_kind () const FINAL OVERRIDE { return "use_after_free"; } > > @@ -778,7 +1139,7 @@ public: > m.add_cwe (416); > return warning_meta (rich_loc, m, OPT_Wanalyzer_use_after_free, > "use after %<%s%> of %qE", > - m_api->m_dealloc_funcname, m_arg); > + m_deallocator->m_name, m_arg); > } > > label_text describe_state_change (const evdesc::state_change &change) > @@ -787,7 +1148,7 @@ public: > if (freed_p (change.m_new_state)) > { > m_free_event = change.m_event_id; > - switch (m_api->m_wording) > + switch (m_deallocator->m_wording) > { > default: > gcc_unreachable (); > @@ -795,6 +1156,8 @@ public: > return label_text::borrow ("freed here"); > case WORDING_DELETED: > return label_text::borrow ("deleted here"); > + case WORDING_DEALLOCATED: > + return label_text::borrow ("deallocated here"); > } > } > return malloc_diagnostic::describe_state_change (change); > @@ -802,9 +1165,9 @@ public: > > label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE > { > - const char *funcname = m_api->m_dealloc_funcname; > + const char *funcname = m_deallocator->m_name; > if (m_free_event.known_p ()) > - switch (m_api->m_wording) > + switch (m_deallocator->m_wording) > { > default: > gcc_unreachable (); > @@ -814,6 +1177,10 @@ public: > case WORDING_DELETED: > return ev.formatted_print ("use after %<%s%> of %qE; deleted at %@", > funcname, ev.m_expr, &m_free_event); > + case WORDING_DEALLOCATED: > + return ev.formatted_print ("use after %<%s%> of %qE;" > + " deallocated at %@", > + funcname, ev.m_expr, &m_free_event); > } > else > return ev.formatted_print ("use after %<%s%> of %qE", > @@ -822,7 +1189,7 @@ public: > > private: > diagnostic_event_id_t m_free_event; > - const api *m_api; > + const deallocator *m_deallocator; > }; > > class malloc_leak : public malloc_diagnostic > @@ -848,7 +1215,8 @@ public: > label_text describe_state_change (const evdesc::state_change &change) > FINAL OVERRIDE > { > - if (unchecked_p (change.m_new_state)) > + if (unchecked_p (change.m_new_state) > + || (start_p (change.m_old_state) && nonnull_p (change.m_new_state))) > { > m_alloc_event = change.m_event_id; > return label_text::borrow ("allocated here"); > @@ -969,40 +1337,161 @@ void > allocation_state::dump_to_pp (pretty_printer *pp) const > { > state_machine::state::dump_to_pp (pp); > - if (m_api) > - pp_printf (pp, " (%s)", m_api->m_name); > + if (m_deallocators) > + { > + pp_string (pp, " ("); > + m_deallocators->dump_to_pp (pp); > + pp_character (pp, ')'); > + } > } > > -/* Given a allocation_state for an api, get the "nonnull" state > - for the corresponding allocator. */ > +/* Given a allocation_state for a deallocator_set, get the "nonnull" state > + for the corresponding allocator(s). */ > > const allocation_state * > allocation_state::get_nonnull () const > { > - gcc_assert (m_api); > - return as_a_allocation_state (m_api->m_nonnull); > + gcc_assert (m_deallocators); > + return as_a_allocation_state (m_deallocators->m_nonnull); > } > > /* malloc_state_machine's ctor. */ > > malloc_state_machine::malloc_state_machine (logger *logger) > : state_machine ("malloc", logger), > - m_malloc (this, "malloc", "free", WORDING_FREED), > - m_scalar_new (this, "new", "delete", WORDING_DELETED), > - m_vector_new (this, "new[]", "delete[]", WORDING_DELETED) > + m_free (this, "free", WORDING_FREED), > + m_scalar_delete (this, "delete", WORDING_DELETED), > + m_vector_delete (this, "delete[]", WORDING_DELETED) > { > gcc_assert (m_start->get_id () == 0); > - m_null = add_state ("null", RS_FREED, NULL); > - m_non_heap = add_state ("non-heap", RS_NON_HEAP, NULL); > - m_stop = add_state ("stop", RS_STOP, NULL); > + m_null = add_state ("null", RS_FREED, NULL, NULL); > + m_non_heap = add_state ("non-heap", RS_NON_HEAP, NULL, NULL); > + m_stop = add_state ("stop", RS_STOP, NULL, NULL); > +} > + > +malloc_state_machine::~malloc_state_machine () > +{ > + unsigned i; > + custom_deallocator_set *set; > + FOR_EACH_VEC_ELT (m_dynamic_sets, i, set) > + delete set; > + custom_deallocator *d; > + FOR_EACH_VEC_ELT (m_dynamic_deallocators, i, d) > + delete d; > } > > state_machine::state_t > malloc_state_machine::add_state (const char *name, enum resource_state rs, > - const api *a) > + const deallocator_set *deallocators, > + const deallocator *deallocator) > { > return add_custom_state (new allocation_state (name, alloc_state_id (), > - rs, a)); > + rs, deallocators, > + deallocator)); > +} > + > +/* If ALLOCATOR_FNDECL has any "__attribute__((malloc(FOO)))", > + return a custom_deallocator_set for them, consolidating them > + to ensure uniqueness of the sets. > + > + Return NULL if it has no such attributes. */ > + > +const custom_deallocator_set * > +malloc_state_machine:: > +get_or_create_custom_deallocator_set (tree allocator_fndecl) > +{ > + /* Early rejection of decls without attributes. */ > + tree attrs = DECL_ATTRIBUTES (allocator_fndecl); > + if (!attrs) > + return NULL; > + > + /* Otherwise, call maybe_create_custom_deallocator_set, > + memoizing the result. */ > + if (custom_deallocator_set **slot > + = m_custom_deallocator_set_cache.get (allocator_fndecl)) > + return *slot; > + custom_deallocator_set *set > + = maybe_create_custom_deallocator_set (allocator_fndecl); > + m_custom_deallocator_set_cache.put (allocator_fndecl, set); > + return set; > +} > + > +/* Given ALLOCATOR_FNDECL, a FUNCTION_DECL with attributes, > + look for any "__attribute__((malloc(FOO)))" and return a > + custom_deallocator_set for them, consolidating them > + to ensure uniqueness of the sets. > + > + Return NULL if it has no such attributes. > + > + Subroutine of get_or_create_custom_deallocator_set which > + memoizes the result. */ > + > +custom_deallocator_set * > +malloc_state_machine:: > +maybe_create_custom_deallocator_set (tree allocator_fndecl) > +{ > + tree attrs = DECL_ATTRIBUTES (allocator_fndecl); > + gcc_assert (attrs); > + > + /* Look for instances of __attribute__((malloc(FOO))). */ > + auto_vec<const deallocator *> deallocator_vec; > + for (tree allocs = attrs; > + (allocs = lookup_attribute ("malloc", allocs)); > + allocs = TREE_CHAIN (allocs)) > + { > + tree args = TREE_VALUE (allocs); > + if (!args) > + continue; > + if (TREE_VALUE (args)) > + { > + const deallocator *d > + = get_or_create_deallocator (TREE_VALUE (args)); > + deallocator_vec.safe_push (d); > + } > + } > + > + /* If there weren't any deallocators, bail. */ > + if (deallocator_vec.length () == 0) > + return NULL; > + > + /* Consolidate, so that we reuse existing deallocator_set > + instances. */ > + deallocator_vec.qsort (deallocator::cmp_ptr_ptr); > + custom_deallocator_set **slot > + = m_custom_deallocator_set_map.get (&deallocator_vec); > + if (slot) > + return *slot; > + custom_deallocator_set *set > + = new custom_deallocator_set (this, &deallocator_vec, WORDING_DEALLOCATED); > + m_custom_deallocator_set_map.put (&set->m_deallocator_vec, set); > + m_dynamic_sets.safe_push (set); > + return set; > +} > + > +/* Get the deallocator for DEALLOCATOR_FNDECL, creating it if necessary. */ > + > +const deallocator * > +malloc_state_machine::get_or_create_deallocator (tree deallocator_fndecl) > +{ > + deallocator **slot = m_deallocator_map.get (deallocator_fndecl); > + if (slot) > + return *slot; > + > + /* Reuse "free". */ > + deallocator *d; > + if (is_named_call_p (deallocator_fndecl, "free") > + || is_std_named_call_p (deallocator_fndecl, "free")) > + d = &m_free.m_deallocator; > + else > + { > + custom_deallocator *cd > + = new custom_deallocator (this, deallocator_fndecl, > + WORDING_DEALLOCATED); > + m_dynamic_deallocators.safe_push (cd); > + d = cd; > + } > + m_deallocator_map.put (deallocator_fndecl, d); > + return d; > } > > /* Implementation of state_machine::on_stmt vfunc for malloc_state_machine. */ > @@ -1024,23 +1513,25 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, > || is_named_call_p (callee_fndecl, "strdup", call, 1) > || is_named_call_p (callee_fndecl, "strndup", call, 2)) > { > - on_allocator_call (sm_ctxt, call, m_malloc); > + on_allocator_call (sm_ctxt, call, &m_free); > return true; > } > > if (is_named_call_p (callee_fndecl, "operator new", call, 1)) > - on_allocator_call (sm_ctxt, call, m_scalar_new); > + on_allocator_call (sm_ctxt, call, &m_scalar_delete); > else if (is_named_call_p (callee_fndecl, "operator new []", call, 1)) > - on_allocator_call (sm_ctxt, call, m_vector_new); > + on_allocator_call (sm_ctxt, call, &m_vector_delete); > else if (is_named_call_p (callee_fndecl, "operator delete", call, 1) > || is_named_call_p (callee_fndecl, "operator delete", call, 2)) > { > - on_deallocator_call (sm_ctxt, node, call, m_scalar_new); > + on_deallocator_call (sm_ctxt, node, call, > + &m_scalar_delete.m_deallocator, 0); > return true; > } > else if (is_named_call_p (callee_fndecl, "operator delete []", call, 1)) > { > - on_deallocator_call (sm_ctxt, node, call, m_vector_new); > + on_deallocator_call (sm_ctxt, node, call, > + &m_vector_delete.m_deallocator, 0); > return true; > } > > @@ -1057,10 +1548,26 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, > || is_std_named_call_p (callee_fndecl, "free", call, 1) > || is_named_call_p (callee_fndecl, "__builtin_free", call, 1)) > { > - on_deallocator_call (sm_ctxt, node, call, m_malloc); > + on_deallocator_call (sm_ctxt, node, call, > + &m_free.m_deallocator, 0); > return true; > } > > + /* Cast away const-ness for cache-like operations. */ > + malloc_state_machine *mutable_this > + = const_cast <malloc_state_machine *> (this); > + > + /* Handle "__attribute__((malloc(FOO)))". */ > + if (const deallocator_set *deallocators > + = mutable_this->get_or_create_custom_deallocator_set > + (callee_fndecl)) > + { > + tree attrs = TYPE_ATTRIBUTES (TREE_TYPE (callee_fndecl)); > + bool returns_nonnull > + = lookup_attribute ("returns_nonnull", attrs); > + on_allocator_call (sm_ctxt, call, deallocators, returns_nonnull); > + } > + > /* Handle "__attribute__((nonnull))". */ > { > tree fntype = TREE_TYPE (callee_fndecl); > @@ -1103,6 +1610,16 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, > BITMAP_FREE (nonnull_args); > } > } > + > + /* Check for this after nonnull, so that if we have both > + then we transition to "freed", rather than "checked". */ > + unsigned dealloc_argno = fndecl_dealloc_argno (callee_fndecl); > + if (dealloc_argno != UINT_MAX) > + { > + const deallocator *d > + = mutable_this->get_or_create_deallocator (callee_fndecl); > + on_deallocator_call (sm_ctxt, node, call, d, dealloc_argno); > + } > } > > if (tree lhs = sm_ctxt->is_zero_assignment (stmt)) > @@ -1162,7 +1679,7 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, > const allocation_state *astate = as_a_allocation_state (state); > sm_ctxt->warn (node, stmt, arg, > new use_after_free (*this, diag_arg, > - astate->m_api)); > + astate->m_deallocator)); > sm_ctxt->set_next_state (stmt, arg, m_stop); > } > } > @@ -1170,18 +1687,24 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, > return false; > } > > -/* Handle a call to an allocator. */ > +/* Handle a call to an allocator. > + RETURNS_NONNULL is true if CALL is to a fndecl known to have > + __attribute__((returns_nonnull)). */ > > void > malloc_state_machine::on_allocator_call (sm_context *sm_ctxt, > const gcall *call, > - const api &ap) const > + const deallocator_set *deallocators, > + bool returns_nonnull) const > { > tree lhs = gimple_call_lhs (call); > if (lhs) > { > if (sm_ctxt->get_state (call, lhs) == m_start) > - sm_ctxt->set_next_state (call, lhs, ap.m_unchecked); > + sm_ctxt->set_next_state (call, lhs, > + (returns_nonnull > + ? deallocators->m_nonnull > + : deallocators->m_unchecked)); > } > else > { > @@ -1193,40 +1716,42 @@ void > malloc_state_machine::on_deallocator_call (sm_context *sm_ctxt, > const supernode *node, > const gcall *call, > - const api &ap) const > + const deallocator *d, > + unsigned argno) const > { > - tree arg = gimple_call_arg (call, 0); > + if (argno >= gimple_call_num_args (call)) > + return; > + tree arg = gimple_call_arg (call, argno); > tree diag_arg = sm_ctxt->get_diagnostic_tree (arg); > > state_t state = sm_ctxt->get_state (call, arg); > > /* start/unchecked/nonnull -> freed. */ > if (state == m_start) > - sm_ctxt->set_next_state (call, arg, ap.m_freed); > + sm_ctxt->set_next_state (call, arg, d->m_freed); > else if (unchecked_p (state) || nonnull_p (state)) > { > const allocation_state *astate = as_a_allocation_state (state); > - > - if (astate->m_api != &ap) > + gcc_assert (astate->m_deallocators); > + if (!astate->m_deallocators->contains_p (d)) > { > /* Wrong allocator. */ > - pending_diagnostic *d > + pending_diagnostic *pd > = new mismatching_deallocation (*this, diag_arg, > - astate->m_api, &ap); > - sm_ctxt->warn (node, call, arg, d); > + astate->m_deallocators, > + d); > + sm_ctxt->warn (node, call, arg, pd); > } > - sm_ctxt->set_next_state (call, arg, ap.m_freed); > + sm_ctxt->set_next_state (call, arg, d->m_freed); > } > > /* Keep state "null" as-is, rather than transitioning to "freed"; > we don't want to complain about double-free of NULL. */ > - > - else if (state == ap.m_freed) > + else if (state == d->m_freed) > { > /* freed -> stop, with warning. */ > sm_ctxt->warn (node, call, arg, > - new double_free (*this, diag_arg, > - ap.m_dealloc_funcname)); > + new double_free (*this, diag_arg, d->m_name)); > sm_ctxt->set_next_state (call, arg, m_stop); > } > else if (state == m_non_heap) > @@ -1234,7 +1759,7 @@ malloc_state_machine::on_deallocator_call (sm_context *sm_ctxt, > /* non-heap -> stop, with warning. */ > sm_ctxt->warn (node, call, arg, > new free_of_non_heap (*this, diag_arg, > - ap.m_dealloc_funcname)); > + d->m_name)); > sm_ctxt->set_next_state (call, arg, m_stop); > } > } > diff --git a/gcc/attribs.h b/gcc/attribs.h > index 9e3e56a73fb..21d28a47f39 100644 > --- a/gcc/attribs.h > +++ b/gcc/attribs.h > @@ -310,4 +310,6 @@ extern void init_attr_rdwr_indices (rdwr_map *, tree); > extern attr_access *get_parm_access (rdwr_map &, tree, > tree = current_function_decl); > > +extern unsigned fndecl_dealloc_argno (tree fndecl); > + > #endif // GCC_ATTRIBS_H > diff --git a/gcc/builtins.c b/gcc/builtins.c > index 02e7815aee5..c1115a32d91 100644 > --- a/gcc/builtins.c > +++ b/gcc/builtins.c > @@ -13014,6 +13014,16 @@ call_dealloc_argno (tree exp) > if (!fndecl) > return UINT_MAX; > > + return fndecl_dealloc_argno (fndecl); > +} > + > +/* Return the zero-based number corresponding to the argument being > + deallocated if FNDECL is a deallocation function or UINT_MAX > + if it isn't. */ > + > +unsigned > +fndecl_dealloc_argno (tree fndecl) > +{ > /* A call to operator delete isn't recognized as one to a built-in. */ > if (DECL_IS_OPERATOR_DELETE_P (fndecl)) > return 0; > diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi > index 2748e98462e..6f8edc7b576 100644 > --- a/gcc/doc/extend.texi > +++ b/gcc/doc/extend.texi > @@ -3291,6 +3291,58 @@ __attribute__ ((malloc, malloc (fclose (1)))) > FILE* tmpfile (void); > @end smallexample > > +The warnings guarded by @option{-fanalyzer} respect allocation and > +deallocation pairs marked with the @code{malloc}. In particular: > + > +@itemize @bullet > + > +@item > +The analyzer will emit a @option{-Wanalyzer-mismatching-deallocation} > +diagnostic if there is an execution path in which the result of an > +allocation call is passed to a different deallocator. > + > +@item > +The analyzer will emit a @option{-Wanalyzer-double-free} > +diagnostic if there is an execution path in which a value is passed > +more than once to a deallocation call. > + > +@item > +The analyzer will consider the possibility that an allocation function > +could fail and return NULL. It will emit > +@option{-Wanalyzer-possible-null-dereference} and > +@option{-Wanalyzer-possible-null-argument} diagnostics if there are > +execution paths in which an unchecked result of an allocation call is > +dereferenced or passed to a function requiring a non-null argument. > +If the allocator always returns non-null, use > +@code{__attribute__ ((returns_nonnull))} to suppress these warnings. > +For example: > +@smallexample > +char *xstrdup (const char *) > + __attribute__((malloc (free), returns_nonnull)); > +@end smallexample > + > +@item > +The analyzer will emit a @option{-Wanalyzer-use-after-free} > +diagnostic if there is an execution path in which the memory passed > +by pointer to a deallocation call is used after the deallocation. > + > +@item > +The analyzer will emit a @option{-Wanalyzer-malloc-leak} diagnostic if > +there is an execution path in which the result of an allocation call > +is leaked (without being passed to the deallocation function). > + > +@item > +The analyzer will emit a @option{-Wanalyzer-free-of-non-heap} diagnostic > +if a deallocation function is used on a global or on-stack variable. > + > +@end itemize > + > +The analyzer assumes that deallocators can gracefully handle the @code{NULL} > +pointer. If this is not the case, the deallocator can be marked with > +@code{__attribute__((nonnull))} so that @option{-fanalyzer} can emit > +a @option{-Wanalyzer-possible-null-argument} diagnostic for code paths > +in which the deallocator is called with NULL. > + > @item no_icf > @cindex @code{no_icf} function attribute > This function attribute prevents a functions from being merged with another > diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi > index 298f1f873e3..e053d569aec 100644 > --- a/gcc/doc/invoke.texi > +++ b/gcc/doc/invoke.texi > @@ -9155,7 +9155,8 @@ This warning requires @option{-fanalyzer}, which enables it; use > @option{-Wno-analyzer-double-free} to disable it. > > This diagnostic warns for paths through the code in which a pointer > -can have @code{free} called on it more than once. > +can have a deallocator called on it more than once, either @code{free}, > +or a deallocator referenced by attribute @code{malloc}. > > @item -Wno-analyzer-exposure-through-output-file > @opindex Wanalyzer-exposure-through-output-file > @@ -9196,7 +9197,8 @@ This warning requires @option{-fanalyzer}, which enables it; use > to disable it. > > This diagnostic warns for paths through the code in which a > -pointer allocated via @code{malloc} is leaked. > +pointer allocated via an allocator is leaked: either @code{malloc}, > +or a function marked with attribute @code{malloc}. > > @item -Wno-analyzer-mismatching-deallocation > @opindex Wanalyzer-mismatching-deallocation > @@ -9207,7 +9209,10 @@ to disable it. > > This diagnostic warns for paths through the code in which the > wrong deallocation function is called on a pointer value, based on > -which function was used to allocate the pointer value. > +which function was used to allocate the pointer value. The diagnostic > +will warn about mismatches between @code{free}, scalar @code{delete} > +and vector @code{delete[]}, and those marked as allocator/deallocator > +pairs using attribute @code{malloc}. > > @item -Wno-analyzer-possible-null-argument > @opindex Wanalyzer-possible-null-argument > @@ -9322,7 +9327,8 @@ This warning requires @option{-fanalyzer}, which enables it; use > @option{-Wno-analyzer-use-after-free} to disable it. > > This diagnostic warns for paths through the code in which a > -pointer is used after @code{free} is called on it. > +pointer is used after a deallocator is called on it: either @code{free}, > +or a deallocator referenced by attribute @code{malloc}. > > @item -Wno-analyzer-use-of-pointer-in-stale-stack-frame > @opindex Wanalyzer-use-of-pointer-in-stale-stack-frame > diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c > new file mode 100644 > index 00000000000..3de32b1b14b > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c > @@ -0,0 +1,75 @@ > +extern void free (void *); > + > +struct foo > +{ > + int m_int; > +}; > + > +extern void foo_release (struct foo *); > +extern struct foo *foo_acquire (void) > + __attribute__ ((malloc (foo_release))); > +extern void use_foo (const struct foo *) > + __attribute__((nonnull)); > + > +void test_1 (void) > +{ > + struct foo *p = foo_acquire (); > + foo_release (p); > +} > + > +void test_2 (void) > +{ > + struct foo *p = foo_acquire (); /* { dg-message "this call could return NULL" } */ > + p->m_int = 42; /* { dg-warning "dereference of possibly-NULL 'p'" } */ > + foo_release (p); > +} > + > +void test_2a (void) > +{ > + struct foo *p = foo_acquire (); /* { dg-message "this call could return NULL" } */ > + use_foo (p); /* { dg-warning "use of possibly-NULL 'p' where non-null expected" } */ > + foo_release (p); > +} > + > +void test_3 (void) > +{ > + struct foo *p = foo_acquire (); /* { dg-message "allocated here" } */ > +} /* { dg-warning "leak of 'p'" } */ > + > +void test_4 (struct foo *p) > +{ > + foo_release (p); > + foo_release (p); /* { dg-warning "double-'foo_release' of 'p'" } */ > +} > + > +void test_4a (void) > +{ > + struct foo *p = foo_acquire (); > + foo_release (p); > + foo_release (p); /* { dg-warning "double-'foo_release' of 'p'" } */ > +} > + > +void test_5 (void) > +{ > + struct foo *p = foo_acquire (); /* { dg-message "allocated here \\(expects deallocation with 'foo_release'\\)" } */ > + free (p); /* { dg-warning "'p' should have been deallocated with 'foo_release' but was deallocated with 'free'" } */ > +} > + > +void test_6 (struct foo *p) > +{ > + foo_release (p); > + free (p); // TODO: double-release warning! > +} > + > +void test_7 () > +{ > + struct foo f; > + foo_release (&f); /* { dg-warning "not on the heap" "analyzer" } */ > + /* { dg-warning "'foo_release' called on unallocated object 'f'" "non-analyzer" { target *-*-* } .-1 } */ > +} > + > +int test_8 (struct foo *p) > +{ > + foo_release (p); > + return p->m_int; /* { dg-warning "use after 'foo_release' of 'p'" } */ > +} > diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-2.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-2.c > new file mode 100644 > index 00000000000..09d941fe082 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-2.c > @@ -0,0 +1,24 @@ > +extern void free (void *); > +char *xstrdup (const char *) > + __attribute__((malloc (free), returns_nonnull)); > + > +void test_1 (const char *s) > +{ > + char *p = xstrdup (s); > + free (p); > +} > + > +/* Verify that we don't issue -Wanalyzer-possible-null-dereference > + when the allocator has __attribute__((returns_nonnull)). */ > + > +char *test_2 (const char *s) > +{ > + char *p = xstrdup (s); > + p[0] = 'a'; /* { dg-bogus "possibly-NULL" } */ > + return p; > +} > + > +void test_3 (const char *s) > +{ > + char *p = xstrdup (s); /* { dg-message "allocated here" } */ > +} /* { dg-warning "leak of 'p'" } */ > diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-4.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-4.c > new file mode 100644 > index 00000000000..1517667dfb2 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-4.c > @@ -0,0 +1,21 @@ > +/* An example where the deallocator requires non-NULL. */ > + > +struct foo; > +extern void foo_release (struct foo *) > + __attribute__((nonnull)); > +extern struct foo *foo_acquire (void) > + __attribute__ ((malloc (foo_release))); > + > +void test_1 (void) > +{ > + struct foo *p = foo_acquire (); /* { dg-message "this call could return NULL" } */ > + foo_release (p); /* { dg-warning "use of possibly-NULL 'p' where non-null" } */ > +} > + > +void test_2 (void) > +{ > + struct foo *p = foo_acquire (); > + if (!p) > + return; > + foo_release (p); > +} > diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-5.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-5.c > new file mode 100644 > index 00000000000..7ff4e57fcfb > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-5.c > @@ -0,0 +1,12 @@ > +/* Example of extra argument to "malloc" attribute. */ > + > +struct foo; > +extern void foo_release (int, struct foo *); > +extern struct foo *foo_acquire (void) > + __attribute__ ((malloc (foo_release, 2))); > + > +void test_1 (void) > +{ > + struct foo *p = foo_acquire (); > + foo_release (0, p); > +} > diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c > new file mode 100644 > index 00000000000..bd28107d0d7 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c > @@ -0,0 +1,228 @@ > +/* Adapted from gcc.dg/Wmismatched-dealloc.c. */ > + > +#define A(...) __attribute__ ((malloc (__VA_ARGS__))) > + > +typedef struct FILE FILE; > +typedef __SIZE_TYPE__ size_t; > + > +void free (void*); > +void* malloc (size_t); > +void* realloc (void*, size_t); > + > +int fclose (FILE*); > +FILE* freopen (const char*, const char*, FILE*); > +int pclose (FILE*); > + > +A (fclose) A (freopen, 3) > + FILE* fdopen (int); > +A (fclose) A (freopen, 3) > + FILE* fopen (const char*, const char*); > +A (fclose) A (freopen, 3) > + FILE* fmemopen(void *, size_t, const char *); > +A (fclose) A (freopen, 3) > + FILE* freopen (const char*, const char*, FILE*); > +A (pclose) A (freopen, 3) > + FILE* popen (const char*, const char*); > +A (fclose) A (freopen, 3) > + FILE* tmpfile (void); > + > +void sink (FILE*); > + > + > + void release (void*); > +A (release) FILE* acquire (void); > + > +void nowarn_fdopen (void) > +{ > + { > + FILE *q = fdopen (0); > + if (!q) > + return; > + > + fclose (q); > + } > + > + { > + FILE *q = fdopen (0); > + if (!q) > + return; > + > + q = freopen ("1", "r", q); > + fclose (q); > + } > + > + { > + FILE *q = fdopen (0); > + if (!q) > + return; > + > + sink (q); > + } > +} > + > + > +void warn_fdopen (void) > +{ > + { > + FILE *q = fdopen (0); // { dg-message "allocated here" } > + release (q); // { dg-warning "'release' called on 'q' returned from a mismatched allocation function" } > + } > + { > + FILE *q = fdopen (0); // { dg-message "allocated here" } > + free (q); // { dg-warning "'free' called on 'q' returned from a mismatched allocation function" } > + } > + > + { > + FILE *q = fdopen (0); // { dg-message "allocated here" } > + q = realloc (q, 7); // { dg-warning "'realloc' called on 'q' returned from a mismatched allocation function" } > + sink (q); > + } > +} > + > + > +void nowarn_fopen (void) > +{ > + { > + FILE *q = fopen ("1", "r"); > + sink (q); > + fclose (q); > + } > + > + { > + FILE *q = fopen ("2", "r"); > + sink (q); > + q = freopen ("3", "r", q); > + sink (q); > + fclose (q); > + } > + > + { > + FILE *q = fopen ("4", "r"); > + sink (q); > + } > +} > + > + > +void warn_fopen (void) > +{ > + { > + FILE *q = fopen ("1", "r"); > + release (q); // { dg-warning "'release' called on 'q' returned from a mismatched allocation function" } > + fclose (q); > + } > + { > + FILE *q = fdopen (0); > + free (q); // { dg-warning "'free' called on 'q' returned from a mismatched allocation function" } > + } > + > + { > + FILE *q = fdopen (0); > + q = realloc (q, 7); // { dg-warning "'realloc' called on 'q' returned from a mismatched allocation function" } > + sink (q); > + } > +} > + > + > +void test_popen (void) > +{ > + { > + FILE *p = popen ("1", "r"); > + sink (p); > + pclose (p); > + } > + > + { > + FILE *p; > + p = popen ("2", "r"); // { dg-message "allocated here" } > + fclose (p); // { dg-warning "'fclose' called on 'p' returned from a mismatched allocation function" } > + } > + > + { > + /* freopen() can close a stream open by popen() but pclose() can't > + close the stream returned from freopen(). */ > + FILE *p = popen ("2", "r"); > + p = freopen ("3", "r", p); // { dg-message "allocated here" } > + pclose (p); // { dg-warning "'pclose' called on 'p' returned from a mismatched allocation function" } > + } > +} > + > + > +void test_tmpfile (void) > +{ > + { > + FILE *p = tmpfile (); > + fclose (p); > + } > + > + { > + FILE *p = tmpfile (); > + p = freopen ("1", "r", p); > + fclose (p); > + } > + > + { > + FILE *p = tmpfile (); // { dg-message "allocated here" } > + pclose (p); // { dg-warning "'pclose' called on 'p' returned from a mismatched allocation function" } > + } > +} > + > + > +void warn_malloc (void) > +{ > + { > + FILE *p = malloc (100); // { dg-message "allocated here" } > + fclose (p); // { dg-warning "'p' should have been deallocated with 'free' but was deallocated with 'fclose'" } > + } > + > + { > + FILE *p = malloc (100); // { dg-message "allocated here" } > + p = freopen ("1", "r", p);// { dg-warning "'p' should have been deallocated with 'free' but was deallocated with 'freopen'" } > + fclose (p); > + } > + > + { > + FILE *p = malloc (100); // { dg-message "allocated here" } > + pclose (p); // { dg-warning "'p' should have been deallocated with 'free' but was deallocated with 'pclose'" } > + } > +} > + > + > +void test_acquire (void) > +{ > + { > + FILE *p = acquire (); > + release (p); > + } > + > + { > + FILE *p = acquire (); > + release (p); > + } > + > + { > + FILE *p = acquire (); // { dg-message "allocated here \\(expects deallocation with 'release'\\)" } > + fclose (p); // { dg-warning "'p' should have been deallocated with 'release' but was deallocated with 'fclose'" } > + } > + > + { > + FILE *p = acquire (); // { dg-message "allocated here \\(expects deallocation with 'release'\\)" } > + pclose (p); // { dg-warning "'p' should have been deallocated with 'release' but was deallocated with 'pclose'" } > + } > + > + { > + FILE *p = acquire (); // { dg-message "allocated here \\(expects deallocation with 'release'\\)" } > + p = freopen ("1", "r", p); // { dg-warning "'p' should have been deallocated with 'release' but was deallocated with 'freopen'" } > + sink (p); > + } > + > + { > + FILE *p = acquire (); // { dg-message "allocated here \\(expects deallocation with 'release'\\)" } > + free (p); // { dg-warning "'p' should have been deallocated with 'release' but was deallocated with 'free'" } > + } > + > + { > + FILE *p = acquire (); // { dg-message "allocated here \\(expects deallocation with 'release'\\)" } > + p = realloc (p, 123); // { dg-warning "'p' should have been deallocated with 'release' but was deallocated with 'realloc'" } > + sink (p); > + } > +} > diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c > new file mode 100644 > index 00000000000..905d50ec3f9 > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c > @@ -0,0 +1,224 @@ > +/* Adapted from linux 5.3.11: drivers/net/wireless/ath/ath10k/usb.c > + Reduced reproducer for CVE-2019-19078 (leak of struct urb). */ > + > +typedef unsigned char u8; > +typedef unsigned short u16; > +typedef _Bool bool; > + > +#define ENOMEM 12 > +#define EINVAL 22 > + > +/* The original file has this licence header. */ > + > +// SPDX-License-Identifier: ISC > +/* > + * Copyright (c) 2007-2011 Atheros Communications Inc. > + * Copyright (c) 2011-2012,2017 Qualcomm Atheros, Inc. > + * Copyright (c) 2016-2017 Erik Stromdahl <erik.stromdahl@gmail.com> > + */ > + > +/* Adapted from include/linux/compiler_attributes.h. */ > +#define __aligned(x) __attribute__((__aligned__(x))) > +#define __printf(a, b) __attribute__((__format__(printf, a, b))) > + > +/* Possible macro for the new attribute. */ > +#define __malloc(f) __attribute__((malloc(f))); > + > +/* From include/linux/types.h. */ > + > +typedef unsigned int gfp_t; > + > +/* Not the real value, which is in include/linux/gfp.h. */ > +#define GFP_ATOMIC 32 > + > +/* From include/linux/usb.h. */ > + > +struct urb; > +extern void usb_free_urb(struct urb *urb); > +extern struct urb *usb_alloc_urb(int iso_packets, gfp_t mem_flags) > + __malloc(usb_free_urb); > +/* attribute added as part of testcase */ > + > +extern int usb_submit_urb(/*struct urb *urb, */gfp_t mem_flags); > +extern void usb_unanchor_urb(struct urb *urb); > + > +/* From drivers/net/wireless/ath/ath10k/core.h. */ > + > +struct ath10k; > + > +struct ath10k { > + /* [...many other fields removed...] */ > + > + /* must be last */ > + u8 drv_priv[0] __aligned(sizeof(void *)); > +}; > + > +/* From drivers/net/wireless/ath/ath10k/debug.h. */ > + > +enum ath10k_debug_mask { > + /* [...other values removed...] */ > + ATH10K_DBG_USB_BULK = 0x00080000, > +}; > + > +extern unsigned int ath10k_debug_mask; > + > +__printf(3, 4) void __ath10k_dbg(struct ath10k *ar, > + enum ath10k_debug_mask mask, > + const char *fmt, ...); > + > +/* Simplified for now, to avoid pulling in tracepoint code. */ > +static inline > +bool trace_ath10k_log_dbg_enabled(void) { return 0; } > + > +#define ath10k_dbg(ar, dbg_mask, fmt, ...) \ > +do { \ > + if ((ath10k_debug_mask & dbg_mask) || \ > + trace_ath10k_log_dbg_enabled()) \ > + __ath10k_dbg(ar, dbg_mask, fmt, ##__VA_ARGS__); \ > +} while (0) > + > +/* From drivers/net/wireless/ath/ath10k/hif.h. */ > + > +struct ath10k_hif_sg_item { > + /* [...other fields removed...] */ > + void *transfer_context; /* NULL = tx completion callback not called */ > +}; > + > +struct ath10k_hif_ops { > + /* send a scatter-gather list to the target */ > + int (*tx_sg)(struct ath10k *ar, u8 pipe_id, > + struct ath10k_hif_sg_item *items, int n_items); > + /* [...other fields removed...] */ > +}; > + > +/* From drivers/net/wireless/ath/ath10k/usb.h. */ > + > +/* tx/rx pipes for usb */ > +enum ath10k_usb_pipe_id { > + /* [...other values removed...] */ > + ATH10K_USB_PIPE_MAX = 8 > +}; > + > +struct ath10k_usb_pipe { > + /* [...all fields removed...] */ > +}; > + > +/* usb device object */ > +struct ath10k_usb { > + /* [...other fields removed...] */ > + struct ath10k_usb_pipe pipes[ATH10K_USB_PIPE_MAX]; > +}; > + > +/* usb urb object */ > +struct ath10k_urb_context { > + /* [...other fields removed...] */ > + struct ath10k_usb_pipe *pipe; > + struct sk_buff *skb; > +}; > + > +static inline struct ath10k_usb *ath10k_usb_priv(struct ath10k *ar) > +{ > + return (struct ath10k_usb *)ar->drv_priv; > +} > + > +/* The source file. */ > + > +static void ath10k_usb_post_recv_transfers(struct ath10k *ar, > + struct ath10k_usb_pipe *recv_pipe); > + > +struct ath10k_urb_context * > +ath10k_usb_alloc_urb_from_pipe(struct ath10k_usb_pipe *pipe); > + > +void ath10k_usb_free_urb_to_pipe(struct ath10k_usb_pipe *pipe, > + struct ath10k_urb_context *urb_context); > + > +static int ath10k_usb_hif_tx_sg(struct ath10k *ar, u8 pipe_id, > + struct ath10k_hif_sg_item *items, int n_items) > +{ > + struct ath10k_usb *ar_usb = ath10k_usb_priv(ar); > + struct ath10k_usb_pipe *pipe = &ar_usb->pipes[pipe_id]; > + struct ath10k_urb_context *urb_context; > + struct sk_buff *skb; > + struct urb *urb; > + int ret, i; > + > + for (i = 0; i < n_items; i++) { > + urb_context = ath10k_usb_alloc_urb_from_pipe(pipe); > + if (!urb_context) { > + ret = -ENOMEM; > + goto err; > + } > + > + skb = items[i].transfer_context; > + urb_context->skb = skb; > + > + urb = usb_alloc_urb(0, GFP_ATOMIC); /* { dg-message "allocated here" } */ > + if (!urb) { > + ret = -ENOMEM; > + goto err_free_urb_to_pipe; > + } > + > + /* TODO: these are disabled, otherwise we conservatively > + assume that they could free urb. */ > +#if 0 > + usb_fill_bulk_urb(urb, > + ar_usb->udev, > + pipe->usb_pipe_handle, > + skb->data, > + skb->len, > + ath10k_usb_transmit_complete, urb_context); > + if (!(skb->len % pipe->max_packet_size)) { > + /* hit a max packet boundary on this pipe */ > + urb->transfer_flags |= URB_ZERO_PACKET; > + } > + > + usb_anchor_urb(urb, &pipe->urb_submitted); > +#endif > + /* TODO: initial argument disabled, otherwise we conservatively > + assume that it could free urb. */ > + ret = usb_submit_urb(/*urb, */GFP_ATOMIC); > + if (ret) { /* TODO: why doesn't it show this condition at default verbosity? */ > + ath10k_dbg(ar, ATH10K_DBG_USB_BULK, > + "usb bulk transmit failed: %d\n", ret); > + > + /* TODO: this is disabled, otherwise we conservatively > + assume that it could free urb. */ > +#if 0 > + usb_unanchor_urb(urb); > +#endif > + > + ret = -EINVAL; > + /* Leak of urb happens here. */ > + goto err_free_urb_to_pipe; > + } > + > + /* TODO: the loop confuses the double-free checker (another > + instance of PR analyzer/93695). */ > + usb_free_urb(urb); /* { dg-bogus "double-'usb_free_urb' of 'urb'" "" { xfail *-*-* } } */ > + } > + > + return 0; > + > +err_free_urb_to_pipe: > + ath10k_usb_free_urb_to_pipe(urb_context->pipe, urb_context); > +err: > + return ret; /* { dg-warning "leak of 'urb'" } */ > +} > + > +static const struct ath10k_hif_ops ath10k_usb_hif_ops = { > + .tx_sg = ath10k_usb_hif_tx_sg, > +}; > + > +/* Simulate code to register the callback. */ > +extern void callback_registration (const void *); > +int ath10k_usb_probe(void) > +{ > + callback_registration(&ath10k_usb_hif_ops); > +} > + > + > +/* The original source file ends with: > +MODULE_AUTHOR("Atheros Communications, Inc."); > +MODULE_DESCRIPTION("Driver support for Qualcomm Atheros 802.11ac WLAN USB devices"); > +MODULE_LICENSE("Dual BSD/GPL"); > +*/ > diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-misuses.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-misuses.c > new file mode 100644 > index 00000000000..3c6c17bddde > --- /dev/null > +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-misuses.c > @@ -0,0 +1,18 @@ > +extern void free (void *); > + > +int not_a_fn __attribute__ ((malloc (free))); /* { dg-warning "'malloc' attribute ignored; valid only for functions" } */ > + > +void void_return (void) __attribute__ ((malloc(free))); /* { dg-warning "'malloc' attribute ignored on functions returning 'void'" } */ > + > +void test_void_return (void) > +{ > + void_return (); > +} > + > +extern void void_args (void); /* { dg-message "declared here" } */ > +void *has_malloc_with_void_args (void) > + __attribute__ ((malloc(void_args))); /* { dg-error "'malloc' attribute argument 1 must take a pointer type as its first argument; have 'void'" } */ > + > +extern void no_args (); /* { dg-message "declared here" } */ > +void *has_malloc_with_no_args (void) > + __attribute__ ((malloc(no_args))); /* { dg-error "'malloc' attribute argument 1 must take a pointer type as its first argument" } */ >
On Mon, 2021-01-18 at 14:26 +0100, Richard Biener wrote: > On Sun, 17 Jan 2021, David Malcolm wrote: > > > This is an updated version of this patch from October: > > > > 'RFC: add "deallocated_by" attribute for use by analyzer' > > > > https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555544.html > > > > reworking it to build on top of Martin's work as noted below, > > reusing > > the existing attribute rather than adding a new one. > > > > Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. > > Also tested by hand with valgrind. > > > > Apart from a trivial change to attrib.h and to builtins.c, this > > is confined to the analyzer code and docs. The original patch was > > posted in stage 1. Is this OK for master? (I'm hoping for > > release manager permission to commit this code now; it's not > > clear to me whether the end of stage 3 was on the 16th or is > > today on the 17th). > > I guess it's still OK if you're quick. > > Richard. Thanks; pushed as c7e276b869bdeb4a95735c1f037ee1a5f629de3d.
diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h index f603802c0d6..219fe9df5a6 100644 --- a/gcc/analyzer/analyzer.h +++ b/gcc/analyzer/analyzer.h @@ -205,6 +205,7 @@ extern bool is_special_named_call_p (const gcall *call, const char *funcname, extern bool is_named_call_p (tree fndecl, const char *funcname); extern bool is_named_call_p (tree fndecl, const char *funcname, const gcall *call, unsigned int num_args); +extern bool is_std_named_call_p (tree fndecl, const char *funcname); extern bool is_std_named_call_p (tree fndecl, const char *funcname, const gcall *call, unsigned int num_args); extern bool is_setjmp_call_p (const gcall *call); diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index 3625e3fe746..22ca024f28e 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -189,6 +189,8 @@ public: return m_feasibility_problem; } + const state_machine *get_sm () const { return m_sd.m_sm; } + private: typedef reachability<eg_traits> enode_reachability; @@ -709,9 +711,11 @@ diagnostic_manager::build_emission_path (const path_builder &pb, class state_change_event_creator : public state_change_visitor { public: - state_change_event_creator (const exploded_edge &eedge, + state_change_event_creator (const path_builder &pb, + const exploded_edge &eedge, checker_path *emission_path) - : m_eedge (eedge), + : m_pb (pb), + m_eedge (eedge), m_emission_path (emission_path) {} @@ -720,6 +724,8 @@ public: state_machine::state_t dst_sm_val) FINAL OVERRIDE { + if (&sm != m_pb.get_sm ()) + return false; const exploded_node *src_node = m_eedge.m_src; const program_point &src_point = src_node->get_point (); const int src_stack_depth = src_point.get_stack_depth (); @@ -748,6 +754,8 @@ public: const svalue *sval, const svalue *dst_origin_sval) FINAL OVERRIDE { + if (&sm != m_pb.get_sm ()) + return false; const exploded_node *src_node = m_eedge.m_src; const program_point &src_point = src_node->get_point (); const int src_stack_depth = src_point.get_stack_depth (); @@ -783,6 +791,7 @@ public: return false; } + const path_builder &m_pb; const exploded_edge &m_eedge; checker_path *m_emission_path; }; @@ -1002,7 +1011,7 @@ diagnostic_manager::add_events_for_eedge (const path_builder &pb, | ~~~~~~~~~~~~~^~~~~ | | | (3) ...to here (end_cfg_edge_event). */ - state_change_event_creator visitor (eedge, emission_path); + state_change_event_creator visitor (pb, eedge, emission_path); for_each_state_change (src_state, dst_state, pb.get_ext_state (), &visitor); diff --git a/gcc/analyzer/region-model-impl-calls.cc b/gcc/analyzer/region-model-impl-calls.cc index 1b81c742c72..d3b66ba7375 100644 --- a/gcc/analyzer/region-model-impl-calls.cc +++ b/gcc/analyzer/region-model-impl-calls.cc @@ -436,6 +436,15 @@ region_model::impl_call_strlen (const call_details &cd) return true; } +/* Handle calls to functions referenced by + __attribute__((malloc(FOO))). */ + +void +region_model::impl_deallocation_call (const call_details &cd) +{ + impl_call_free (cd); +} + } // namespace ana #endif /* #if ENABLE_ANALYZER */ diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 81e1a481b51..cfe8a391dd9 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -65,6 +65,7 @@ along with GCC; see the file COPYING3. If not see #include "analyzer/region-model-reachability.h" #include "analyzer/analyzer-selftests.h" #include "stor-layout.h" +#include "attribs.h" #if ENABLE_ANALYZER @@ -917,6 +918,14 @@ region_model::on_call_post (const gcall *call, impl_call_operator_delete (cd); return; } + /* Was this fndecl referenced by + __attribute__((malloc(FOO)))? */ + if (lookup_attribute ("*dealloc", DECL_ATTRIBUTES (callee_fndecl))) + { + call_details cd (call, this, ctxt); + impl_deallocation_call (cd); + return; + } } if (unknown_side_effects) diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 0e303d0398a..efd1a09e362 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -463,6 +463,7 @@ class region_model bool impl_call_strlen (const call_details &cd); bool impl_call_operator_new (const call_details &cd); bool impl_call_operator_delete (const call_details &cd); + void impl_deallocation_call (const call_details &cd); void handle_unrecognized_call (const gcall *call, region_model_context *ctxt); diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc index e1fea02486f..d7c76e343ff 100644 --- a/gcc/analyzer/sm-malloc.cc +++ b/gcc/analyzer/sm-malloc.cc @@ -42,6 +42,8 @@ along with GCC; see the file COPYING3. If not see #include "analyzer/program-point.h" #include "analyzer/store.h" #include "analyzer/region-model.h" +#include "stringpool.h" +#include "attribs.h" #if ENABLE_ANALYZER @@ -49,14 +51,38 @@ namespace ana { namespace { -class api; +/* This state machine and its various support classes track allocations + and deallocations. + + It has a few standard allocation/deallocation pairs (e.g. new/delete), + and also supports user-defined ones via + __attribute__ ((malloc(DEALLOCATOR))). + + There can be more than one valid deallocator for a given allocator, + for example: + __attribute__ ((malloc (fclose))) + __attribute__ ((malloc (freopen, 3))) + FILE* fopen (const char*, const char*); + A deallocator_set represents a particular set of valid deallocators. + + We track the expected deallocator_set for a value, but not the allocation + function - there could be more than one allocator per deallocator_set. + For example, there could be dozens of allocators for "free" beyond just + malloc e.g. calloc, xstrdup, etc. We don't want to explode the number + of states by tracking individual allocators in the exploded graph; + we merely want to track "this value expects to have 'free' called on it". + Perhaps we can reconstruct which allocator was used later, when emitting + the path, if it's necessary for precision of wording of diagnostics. */ + +class deallocator; +class deallocator_set; class malloc_state_machine; /* An enum for discriminating between different kinds of allocation_state. */ enum resource_state { - /* States that are independent of api. */ + /* States that are independent of allocator/deallocator. */ /* The start state. */ RS_START, @@ -71,28 +97,33 @@ enum resource_state /* Stop state, for pointers we don't want to track any more. */ RS_STOP, - /* States that relate to a specific api. */ + /* States that relate to a specific deallocator_set. */ - /* State for a pointer returned from the api's allocator that hasn't + /* State for a pointer returned from an allocator that hasn't been checked for NULL. It could be a pointer to heap-allocated memory, or could be NULL. */ RS_UNCHECKED, - /* State for a pointer returned from the api's allocator, + /* State for a pointer returned from an allocator, known to be non-NULL. */ RS_NONNULL, - /* State for a pointer passed to the api's deallocator. */ + /* State for a pointer passed to a deallocator. */ RS_FREED }; -/* Custom state subclass, which can optionally refer to an an api. */ +/* Custom state subclass, which can optionally refer to an a + deallocator_set. */ struct allocation_state : public state_machine::state { allocation_state (const char *name, unsigned id, - enum resource_state rs, const api *a) - : state (name, id), m_rs (rs), m_api (a) + enum resource_state rs, + const deallocator_set *deallocators, + const deallocator *deallocator) + : state (name, id), m_rs (rs), + m_deallocators (deallocators), + m_deallocator (deallocator) {} void dump_to_pp (pretty_printer *pp) const FINAL OVERRIDE; @@ -100,7 +131,8 @@ struct allocation_state : public state_machine::state const allocation_state *get_nonnull () const; enum resource_state m_rs; - const api *m_api; + const deallocator_set *m_deallocators; + const deallocator *m_deallocator; }; /* An enum for choosing which wording to use in various diagnostics @@ -109,52 +141,186 @@ struct allocation_state : public state_machine::state enum wording { WORDING_FREED, - WORDING_DELETED + WORDING_DELETED, + WORDING_DEALLOCATED }; -/* Represents a particular family of API calls for allocating/deallocating - heap memory that should be matched e.g. - - malloc/free - - scalar new/delete - - vector new[]/delete[] - etc. +/* Base class representing a deallocation function, + either a built-in one we know about, or one exposed via + __attribute__((malloc(DEALLOCATOR))). */ - We track the expected deallocation function, but not the allocation - function - there could be more than one allocator per deallocator. For - example, there could be dozens of allocators for "free" beyond just - malloc e.g. calloc, xstrdup, etc. We don't want to explode the number - of states by tracking individual allocators in the exploded graph; - we merely want to track "this value expects to have 'free' called on it". - Perhaps we can reconstruct which allocator was used later, when emitting - the path, if it's necessary for precision of wording of diagnostics. */ - -struct api +struct deallocator { - api (malloc_state_machine *sm, - const char *name, - const char *dealloc_funcname, - enum wording wording); + hashval_t hash () const; + void dump_to_pp (pretty_printer *pp) const; + static int cmp (const deallocator *a, const deallocator *b); + static int cmp_ptr_ptr (const void *, const void *); - /* An internal name for identifying this API in dumps. */ + /* Name to use in diagnostics. */ const char *m_name; - /* The name of the deallocation function, for use in diagnostics. */ - const char *m_dealloc_funcname; + /* Which wording to use in diagnostics. */ + enum wording m_wording; + + /* State for a value passed to one of the deallocators. */ + state_machine::state_t m_freed; + +protected: + deallocator (malloc_state_machine *sm, + const char *name, + enum wording wording); +}; + +/* Subclass representing a predefined deallocator. + e.g. "delete []", without needing a specific FUNCTION_DECL + ahead of time. */ + +struct standard_deallocator : public deallocator +{ + standard_deallocator (malloc_state_machine *sm, + const char *name, + enum wording wording); +}; + +/* Subclass representing a user-defined deallocator + via __attribute__((malloc(DEALLOCATOR))) given + a specific FUNCTION_DECL. */ + +struct custom_deallocator : public deallocator +{ + custom_deallocator (malloc_state_machine *sm, + tree deallocator_fndecl, + enum wording wording) + : deallocator (sm, IDENTIFIER_POINTER (DECL_NAME (deallocator_fndecl)), + wording) + { + } +}; + +/* Base class representing a set of possible deallocators. + Often this will be just a single deallocator, but some + allocators have multiple valid deallocators (e.g. the result of + "fopen" can be closed by either "fclose" or "freopen"). */ + +struct deallocator_set +{ + deallocator_set (malloc_state_machine *sm, + enum wording wording); + virtual ~deallocator_set () {} + + virtual bool contains_p (const deallocator *d) const = 0; + virtual const deallocator *maybe_get_single () const = 0; + virtual void dump_to_pp (pretty_printer *pp) const = 0; + void dump () const; /* Which wording to use in diagnostics. */ enum wording m_wording; - /* Pointers to api-specific states. + /* Pointers to states. These states are owned by the state_machine base class. */ - /* State for an unchecked result from this api's allocator. */ + /* State for an unchecked result from an allocator using this set. */ state_machine::state_t m_unchecked; - /* State for a known non-NULL result from this apis's allocator. */ + /* State for a known non-NULL result from such an allocator. */ state_machine::state_t m_nonnull; +}; - /* State for a value passed to this api's deallocator. */ - state_machine::state_t m_freed; +/* Subclass of deallocator_set representing a set of deallocators + defined by one or more __attribute__((malloc(DEALLOCATOR))). */ + +struct custom_deallocator_set : public deallocator_set +{ + typedef const auto_vec <const deallocator *> *key_t; + + custom_deallocator_set (malloc_state_machine *sm, + const auto_vec <const deallocator *> *vec, + //const char *name, + //const char *dealloc_funcname, + //unsigned arg_idx, + enum wording wording); + + bool contains_p (const deallocator *d) const FINAL OVERRIDE; + const deallocator *maybe_get_single () const FINAL OVERRIDE; + void dump_to_pp (pretty_printer *pp) const FINAL OVERRIDE; + + auto_vec <const deallocator *> m_deallocator_vec; +}; + +/* Subclass of deallocator_set representing a set of deallocators + with a single standard_deallocator, e.g. "delete []". */ + +struct standard_deallocator_set : public deallocator_set +{ + standard_deallocator_set (malloc_state_machine *sm, + const char *name, + enum wording wording); + + bool contains_p (const deallocator *d) const FINAL OVERRIDE; + const deallocator *maybe_get_single () const FINAL OVERRIDE; + void dump_to_pp (pretty_printer *pp) const FINAL OVERRIDE; + + standard_deallocator m_deallocator; +}; + +/* Traits class for ensuring uniqueness of deallocator_sets within + malloc_state_machine. */ + +struct deallocator_set_map_traits +{ + typedef custom_deallocator_set::key_t key_type; + typedef custom_deallocator_set *value_type; + typedef custom_deallocator_set *compare_type; + + static inline hashval_t hash (const key_type &k) + { + gcc_assert (k != NULL); + gcc_assert (k != reinterpret_cast<key_type> (1)); + + hashval_t result = 0; + unsigned i; + const deallocator *d; + FOR_EACH_VEC_ELT (*k, i, d) + result ^= d->hash (); + return result; + } + static inline bool equal_keys (const key_type &k1, const key_type &k2) + { + if (k1->length () != k2->length ()) + return false; + + for (unsigned i = 0; i < k1->length (); i++) + if ((*k1)[i] != (*k2)[i]) + return false; + + return true; + } + template <typename T> + static inline void remove (T &) + { + /* empty; the nodes are handled elsewhere. */ + } + template <typename T> + static inline void mark_deleted (T &entry) + { + entry.m_key = reinterpret_cast<key_type> (1); + } + template <typename T> + static inline void mark_empty (T &entry) + { + entry.m_key = NULL; + } + template <typename T> + static inline bool is_deleted (const T &entry) + { + return entry.m_key == reinterpret_cast<key_type> (1); + } + template <typename T> + static inline bool is_empty (const T &entry) + { + return entry.m_key == NULL; + } + static const bool empty_zero_p = false; }; /* A state machine for detecting misuses of the malloc/free API. @@ -167,9 +333,12 @@ public: typedef allocation_state custom_data_t; malloc_state_machine (logger *logger); + ~malloc_state_machine (); state_t - add_state (const char *name, enum resource_state rs, const api *a); + add_state (const char *name, enum resource_state rs, + const deallocator_set *deallocators, + const deallocator *deallocator); bool inherited_state_p () const FINAL OVERRIDE { return false; } @@ -214,9 +383,9 @@ public: bool reset_when_passed_to_unknown_fn_p (state_t s, bool is_mutable) const FINAL OVERRIDE; - api m_malloc; - api m_scalar_new; - api m_vector_new; + standard_deallocator_set m_free; + standard_deallocator_set m_scalar_delete; + standard_deallocator_set m_vector_delete; /* States that are independent of api. */ @@ -232,33 +401,194 @@ public: state_t m_stop; private: + const custom_deallocator_set * + get_or_create_custom_deallocator_set (tree allocator_fndecl); + custom_deallocator_set * + maybe_create_custom_deallocator_set (tree allocator_fndecl); + const deallocator * + get_or_create_deallocator (tree deallocator_fndecl); + void on_allocator_call (sm_context *sm_ctxt, const gcall *call, - const api &ap) const; + const deallocator_set *deallocators, + bool returns_nonnull = false) const; void on_deallocator_call (sm_context *sm_ctxt, const supernode *node, const gcall *call, - const api &ap) const; + const deallocator *d, + unsigned argno) const; void on_zero_assignment (sm_context *sm_ctxt, const gimple *stmt, tree lhs) const; + + /* A map for consolidating deallocators so that they are + unique per deallocator FUNCTION_DECL. */ + typedef hash_map<tree, deallocator *> deallocator_map_t; + deallocator_map_t m_deallocator_map; + + /* Memoized lookups from FUNCTION_DECL to custom_deallocator_set *. */ + typedef hash_map<tree, custom_deallocator_set *> deallocator_set_cache_t; + deallocator_set_cache_t m_custom_deallocator_set_cache; + + /* A map for consolidating custom_deallocator_set instances. */ + typedef hash_map<custom_deallocator_set::key_t, + custom_deallocator_set *, + deallocator_set_map_traits> custom_deallocator_set_map_t; + custom_deallocator_set_map_t m_custom_deallocator_set_map; + + /* Record of dynamically-allocated objects, for cleanup. */ + auto_vec <custom_deallocator_set *> m_dynamic_sets; + auto_vec <custom_deallocator *> m_dynamic_deallocators; }; -/* struct api. */ +/* struct deallocator. */ -api::api (malloc_state_machine *sm, - const char *name, - const char *dealloc_funcname, - enum wording wording) +deallocator::deallocator (malloc_state_machine *sm, + const char *name, + enum wording wording) : m_name (name), - m_dealloc_funcname (dealloc_funcname), m_wording (wording), - m_unchecked (sm->add_state ("unchecked", RS_UNCHECKED, this)), - m_nonnull (sm->add_state ("nonnull", RS_NONNULL, this)), - m_freed (sm->add_state ("freed", RS_FREED, this)) + m_freed (sm->add_state ("freed", RS_FREED, NULL, this)) +{ +} + +hashval_t +deallocator::hash () const +{ + return (hashval_t)m_freed->get_id (); +} + +void +deallocator::dump_to_pp (pretty_printer *pp) const +{ + pp_printf (pp, "%qs", m_name); +} + +int +deallocator::cmp (const deallocator *a, const deallocator *b) +{ + return (int)a->m_freed->get_id () - (int)b->m_freed->get_id (); +} + +int +deallocator::cmp_ptr_ptr (const void *a, const void *b) +{ + return cmp (*(const deallocator * const *)a, + *(const deallocator * const *)b); +} + + +/* struct standard_deallocator : public deallocator. */ + +standard_deallocator::standard_deallocator (malloc_state_machine *sm, + const char *name, + enum wording wording) +: deallocator (sm, name, wording) +{ +} + +/* struct deallocator_set. */ + +deallocator_set::deallocator_set (malloc_state_machine *sm, + enum wording wording) +: m_wording (wording), + m_unchecked (sm->add_state ("unchecked", RS_UNCHECKED, this, NULL)), + m_nonnull (sm->add_state ("nonnull", RS_NONNULL, this, NULL)) +{ +} + +/* Dump a description of this deallocator_set to stderr. */ + +DEBUG_FUNCTION void +deallocator_set::dump () const +{ + pretty_printer pp; + pp_show_color (&pp) = pp_show_color (global_dc->printer); + pp.buffer->stream = stderr; + dump_to_pp (&pp); + pp_newline (&pp); + pp_flush (&pp); +} + +/* struct custom_deallocator_set : public deallocator_set. */ + +custom_deallocator_set:: +custom_deallocator_set (malloc_state_machine *sm, + const auto_vec <const deallocator *> *vec, + enum wording wording) +: deallocator_set (sm, wording), + m_deallocator_vec (vec->length ()) +{ + unsigned i; + const deallocator *d; + FOR_EACH_VEC_ELT (*vec, i, d) + m_deallocator_vec.safe_push (d); +} + +bool +custom_deallocator_set::contains_p (const deallocator *d) const +{ + unsigned i; + const deallocator *cd; + FOR_EACH_VEC_ELT (m_deallocator_vec, i, cd) + if (cd == d) + return true; + return false; +} + +const deallocator * +custom_deallocator_set::maybe_get_single () const +{ + if (m_deallocator_vec.length () == 1) + return m_deallocator_vec[0]; + return NULL; +} + +void +custom_deallocator_set::dump_to_pp (pretty_printer *pp) const +{ + pp_character (pp, '{'); + unsigned i; + const deallocator *d; + FOR_EACH_VEC_ELT (m_deallocator_vec, i, d) + { + if (i > 0) + pp_string (pp, ", "); + d->dump_to_pp (pp); + } + pp_character (pp, '}'); +} + +/* struct standard_deallocator_set : public deallocator_set. */ + +standard_deallocator_set::standard_deallocator_set (malloc_state_machine *sm, + const char *name, + enum wording wording) +: deallocator_set (sm, wording), + m_deallocator (sm, name, wording) { } +bool +standard_deallocator_set::contains_p (const deallocator *d) const +{ + return d == &m_deallocator; +} + +const deallocator * +standard_deallocator_set::maybe_get_single () const +{ + return &m_deallocator; +} + +void +standard_deallocator_set::dump_to_pp (pretty_printer *pp) const +{ + pp_character (pp, '{'); + pp_string (pp, m_deallocator.m_name); + pp_character (pp, '}'); +} + /* Return STATE cast to the custom state subclass, or NULL for the start state. Everything should be an allocation_state apart from the start state. */ @@ -291,6 +621,14 @@ get_rs (state_machine::state_t state) return RS_START; } +/* Return true if STATE is the start state. */ + +static bool +start_p (state_machine::state_t state) +{ + return get_rs (state) == RS_START; +} + /* Return true if STATE is an unchecked result from an allocator. */ static bool @@ -383,10 +721,10 @@ class mismatching_deallocation : public malloc_diagnostic { public: mismatching_deallocation (const malloc_state_machine &sm, tree arg, - const api *expected_dealloc, - const api *actual_dealloc) + const deallocator_set *expected_deallocators, + const deallocator *actual_dealloc) : malloc_diagnostic (sm, arg), - m_expected_dealloc (expected_dealloc), + m_expected_deallocators (expected_deallocators), m_actual_dealloc (actual_dealloc) {} @@ -400,11 +738,18 @@ public: auto_diagnostic_group d; diagnostic_metadata m; m.add_cwe (762); /* CWE-762: Mismatched Memory Management Routines. */ - return warning_meta (rich_loc, m, OPT_Wanalyzer_mismatching_deallocation, - "%qE should have been deallocated with %qs" - " but was deallocated with %qs", - m_arg, m_expected_dealloc->m_dealloc_funcname, - m_actual_dealloc->m_dealloc_funcname); + if (const deallocator *expected_dealloc + = m_expected_deallocators->maybe_get_single ()) + return warning_meta (rich_loc, m, OPT_Wanalyzer_mismatching_deallocation, + "%qE should have been deallocated with %qs" + " but was deallocated with %qs", + m_arg, expected_dealloc->m_name, + m_actual_dealloc->m_name); + else + return warning_meta (rich_loc, m, OPT_Wanalyzer_mismatching_deallocation, + "%qs called on %qE returned from a mismatched" + " allocation function", + m_actual_dealloc->m_name, m_arg); } label_text describe_state_change (const evdesc::state_change &change) @@ -413,9 +758,13 @@ public: if (unchecked_p (change.m_new_state)) { m_alloc_event = change.m_event_id; - return change.formatted_print ("allocated here" - " (expects deallocation with %qs)", - m_expected_dealloc->m_dealloc_funcname); + if (const deallocator *expected_dealloc + = m_expected_deallocators->maybe_get_single ()) + return change.formatted_print ("allocated here" + " (expects deallocation with %qs)", + expected_dealloc->m_name); + else + return change.formatted_print ("allocated here"); } return malloc_diagnostic::describe_state_change (change); } @@ -423,19 +772,28 @@ public: label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE { if (m_alloc_event.known_p ()) - return ev.formatted_print - ("deallocated with %qs here;" - " allocation at %@ expects deallocation with %qs", - m_actual_dealloc->m_dealloc_funcname, &m_alloc_event, - m_expected_dealloc->m_dealloc_funcname); + { + if (const deallocator *expected_dealloc + = m_expected_deallocators->maybe_get_single ()) + return ev.formatted_print + ("deallocated with %qs here;" + " allocation at %@ expects deallocation with %qs", + m_actual_dealloc->m_name, &m_alloc_event, + expected_dealloc->m_name); + else + return ev.formatted_print + ("deallocated with %qs here;" + " allocated at %@", + m_actual_dealloc->m_name, &m_alloc_event); + } return ev.formatted_print ("deallocated with %qs here", - m_actual_dealloc->m_dealloc_funcname); + m_actual_dealloc->m_name); } private: diagnostic_event_id_t m_alloc_event; - const api *m_expected_dealloc; - const api *m_actual_dealloc; + const deallocator_set *m_expected_deallocators; + const deallocator *m_actual_dealloc; }; /* Concrete subclass for reporting double-free diagnostics. */ @@ -455,7 +813,7 @@ public: diagnostic_metadata m; m.add_cwe (415); /* CWE-415: Double Free. */ return warning_meta (rich_loc, m, OPT_Wanalyzer_double_free, - "double-%<%s%> of %qE", m_funcname, m_arg); + "double-%qs of %qE", m_funcname, m_arg); } label_text describe_state_change (const evdesc::state_change &change) @@ -765,9 +1123,12 @@ class use_after_free : public malloc_diagnostic { public: use_after_free (const malloc_state_machine &sm, tree arg, - const api *a) - : malloc_diagnostic (sm, arg), m_api (a) - {} + const deallocator *deallocator) + : malloc_diagnostic (sm, arg), + m_deallocator (deallocator) + { + gcc_assert (deallocator); + } const char *get_kind () const FINAL OVERRIDE { return "use_after_free"; } @@ -778,7 +1139,7 @@ public: m.add_cwe (416); return warning_meta (rich_loc, m, OPT_Wanalyzer_use_after_free, "use after %<%s%> of %qE", - m_api->m_dealloc_funcname, m_arg); + m_deallocator->m_name, m_arg); } label_text describe_state_change (const evdesc::state_change &change) @@ -787,7 +1148,7 @@ public: if (freed_p (change.m_new_state)) { m_free_event = change.m_event_id; - switch (m_api->m_wording) + switch (m_deallocator->m_wording) { default: gcc_unreachable (); @@ -795,6 +1156,8 @@ public: return label_text::borrow ("freed here"); case WORDING_DELETED: return label_text::borrow ("deleted here"); + case WORDING_DEALLOCATED: + return label_text::borrow ("deallocated here"); } } return malloc_diagnostic::describe_state_change (change); @@ -802,9 +1165,9 @@ public: label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE { - const char *funcname = m_api->m_dealloc_funcname; + const char *funcname = m_deallocator->m_name; if (m_free_event.known_p ()) - switch (m_api->m_wording) + switch (m_deallocator->m_wording) { default: gcc_unreachable (); @@ -814,6 +1177,10 @@ public: case WORDING_DELETED: return ev.formatted_print ("use after %<%s%> of %qE; deleted at %@", funcname, ev.m_expr, &m_free_event); + case WORDING_DEALLOCATED: + return ev.formatted_print ("use after %<%s%> of %qE;" + " deallocated at %@", + funcname, ev.m_expr, &m_free_event); } else return ev.formatted_print ("use after %<%s%> of %qE", @@ -822,7 +1189,7 @@ public: private: diagnostic_event_id_t m_free_event; - const api *m_api; + const deallocator *m_deallocator; }; class malloc_leak : public malloc_diagnostic @@ -848,7 +1215,8 @@ public: label_text describe_state_change (const evdesc::state_change &change) FINAL OVERRIDE { - if (unchecked_p (change.m_new_state)) + if (unchecked_p (change.m_new_state) + || (start_p (change.m_old_state) && nonnull_p (change.m_new_state))) { m_alloc_event = change.m_event_id; return label_text::borrow ("allocated here"); @@ -969,40 +1337,161 @@ void allocation_state::dump_to_pp (pretty_printer *pp) const { state_machine::state::dump_to_pp (pp); - if (m_api) - pp_printf (pp, " (%s)", m_api->m_name); + if (m_deallocators) + { + pp_string (pp, " ("); + m_deallocators->dump_to_pp (pp); + pp_character (pp, ')'); + } } -/* Given a allocation_state for an api, get the "nonnull" state - for the corresponding allocator. */ +/* Given a allocation_state for a deallocator_set, get the "nonnull" state + for the corresponding allocator(s). */ const allocation_state * allocation_state::get_nonnull () const { - gcc_assert (m_api); - return as_a_allocation_state (m_api->m_nonnull); + gcc_assert (m_deallocators); + return as_a_allocation_state (m_deallocators->m_nonnull); } /* malloc_state_machine's ctor. */ malloc_state_machine::malloc_state_machine (logger *logger) : state_machine ("malloc", logger), - m_malloc (this, "malloc", "free", WORDING_FREED), - m_scalar_new (this, "new", "delete", WORDING_DELETED), - m_vector_new (this, "new[]", "delete[]", WORDING_DELETED) + m_free (this, "free", WORDING_FREED), + m_scalar_delete (this, "delete", WORDING_DELETED), + m_vector_delete (this, "delete[]", WORDING_DELETED) { gcc_assert (m_start->get_id () == 0); - m_null = add_state ("null", RS_FREED, NULL); - m_non_heap = add_state ("non-heap", RS_NON_HEAP, NULL); - m_stop = add_state ("stop", RS_STOP, NULL); + m_null = add_state ("null", RS_FREED, NULL, NULL); + m_non_heap = add_state ("non-heap", RS_NON_HEAP, NULL, NULL); + m_stop = add_state ("stop", RS_STOP, NULL, NULL); +} + +malloc_state_machine::~malloc_state_machine () +{ + unsigned i; + custom_deallocator_set *set; + FOR_EACH_VEC_ELT (m_dynamic_sets, i, set) + delete set; + custom_deallocator *d; + FOR_EACH_VEC_ELT (m_dynamic_deallocators, i, d) + delete d; } state_machine::state_t malloc_state_machine::add_state (const char *name, enum resource_state rs, - const api *a) + const deallocator_set *deallocators, + const deallocator *deallocator) { return add_custom_state (new allocation_state (name, alloc_state_id (), - rs, a)); + rs, deallocators, + deallocator)); +} + +/* If ALLOCATOR_FNDECL has any "__attribute__((malloc(FOO)))", + return a custom_deallocator_set for them, consolidating them + to ensure uniqueness of the sets. + + Return NULL if it has no such attributes. */ + +const custom_deallocator_set * +malloc_state_machine:: +get_or_create_custom_deallocator_set (tree allocator_fndecl) +{ + /* Early rejection of decls without attributes. */ + tree attrs = DECL_ATTRIBUTES (allocator_fndecl); + if (!attrs) + return NULL; + + /* Otherwise, call maybe_create_custom_deallocator_set, + memoizing the result. */ + if (custom_deallocator_set **slot + = m_custom_deallocator_set_cache.get (allocator_fndecl)) + return *slot; + custom_deallocator_set *set + = maybe_create_custom_deallocator_set (allocator_fndecl); + m_custom_deallocator_set_cache.put (allocator_fndecl, set); + return set; +} + +/* Given ALLOCATOR_FNDECL, a FUNCTION_DECL with attributes, + look for any "__attribute__((malloc(FOO)))" and return a + custom_deallocator_set for them, consolidating them + to ensure uniqueness of the sets. + + Return NULL if it has no such attributes. + + Subroutine of get_or_create_custom_deallocator_set which + memoizes the result. */ + +custom_deallocator_set * +malloc_state_machine:: +maybe_create_custom_deallocator_set (tree allocator_fndecl) +{ + tree attrs = DECL_ATTRIBUTES (allocator_fndecl); + gcc_assert (attrs); + + /* Look for instances of __attribute__((malloc(FOO))). */ + auto_vec<const deallocator *> deallocator_vec; + for (tree allocs = attrs; + (allocs = lookup_attribute ("malloc", allocs)); + allocs = TREE_CHAIN (allocs)) + { + tree args = TREE_VALUE (allocs); + if (!args) + continue; + if (TREE_VALUE (args)) + { + const deallocator *d + = get_or_create_deallocator (TREE_VALUE (args)); + deallocator_vec.safe_push (d); + } + } + + /* If there weren't any deallocators, bail. */ + if (deallocator_vec.length () == 0) + return NULL; + + /* Consolidate, so that we reuse existing deallocator_set + instances. */ + deallocator_vec.qsort (deallocator::cmp_ptr_ptr); + custom_deallocator_set **slot + = m_custom_deallocator_set_map.get (&deallocator_vec); + if (slot) + return *slot; + custom_deallocator_set *set + = new custom_deallocator_set (this, &deallocator_vec, WORDING_DEALLOCATED); + m_custom_deallocator_set_map.put (&set->m_deallocator_vec, set); + m_dynamic_sets.safe_push (set); + return set; +} + +/* Get the deallocator for DEALLOCATOR_FNDECL, creating it if necessary. */ + +const deallocator * +malloc_state_machine::get_or_create_deallocator (tree deallocator_fndecl) +{ + deallocator **slot = m_deallocator_map.get (deallocator_fndecl); + if (slot) + return *slot; + + /* Reuse "free". */ + deallocator *d; + if (is_named_call_p (deallocator_fndecl, "free") + || is_std_named_call_p (deallocator_fndecl, "free")) + d = &m_free.m_deallocator; + else + { + custom_deallocator *cd + = new custom_deallocator (this, deallocator_fndecl, + WORDING_DEALLOCATED); + m_dynamic_deallocators.safe_push (cd); + d = cd; + } + m_deallocator_map.put (deallocator_fndecl, d); + return d; } /* Implementation of state_machine::on_stmt vfunc for malloc_state_machine. */ @@ -1024,23 +1513,25 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, || is_named_call_p (callee_fndecl, "strdup", call, 1) || is_named_call_p (callee_fndecl, "strndup", call, 2)) { - on_allocator_call (sm_ctxt, call, m_malloc); + on_allocator_call (sm_ctxt, call, &m_free); return true; } if (is_named_call_p (callee_fndecl, "operator new", call, 1)) - on_allocator_call (sm_ctxt, call, m_scalar_new); + on_allocator_call (sm_ctxt, call, &m_scalar_delete); else if (is_named_call_p (callee_fndecl, "operator new []", call, 1)) - on_allocator_call (sm_ctxt, call, m_vector_new); + on_allocator_call (sm_ctxt, call, &m_vector_delete); else if (is_named_call_p (callee_fndecl, "operator delete", call, 1) || is_named_call_p (callee_fndecl, "operator delete", call, 2)) { - on_deallocator_call (sm_ctxt, node, call, m_scalar_new); + on_deallocator_call (sm_ctxt, node, call, + &m_scalar_delete.m_deallocator, 0); return true; } else if (is_named_call_p (callee_fndecl, "operator delete []", call, 1)) { - on_deallocator_call (sm_ctxt, node, call, m_vector_new); + on_deallocator_call (sm_ctxt, node, call, + &m_vector_delete.m_deallocator, 0); return true; } @@ -1057,10 +1548,26 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, || is_std_named_call_p (callee_fndecl, "free", call, 1) || is_named_call_p (callee_fndecl, "__builtin_free", call, 1)) { - on_deallocator_call (sm_ctxt, node, call, m_malloc); + on_deallocator_call (sm_ctxt, node, call, + &m_free.m_deallocator, 0); return true; } + /* Cast away const-ness for cache-like operations. */ + malloc_state_machine *mutable_this + = const_cast <malloc_state_machine *> (this); + + /* Handle "__attribute__((malloc(FOO)))". */ + if (const deallocator_set *deallocators + = mutable_this->get_or_create_custom_deallocator_set + (callee_fndecl)) + { + tree attrs = TYPE_ATTRIBUTES (TREE_TYPE (callee_fndecl)); + bool returns_nonnull + = lookup_attribute ("returns_nonnull", attrs); + on_allocator_call (sm_ctxt, call, deallocators, returns_nonnull); + } + /* Handle "__attribute__((nonnull))". */ { tree fntype = TREE_TYPE (callee_fndecl); @@ -1103,6 +1610,16 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, BITMAP_FREE (nonnull_args); } } + + /* Check for this after nonnull, so that if we have both + then we transition to "freed", rather than "checked". */ + unsigned dealloc_argno = fndecl_dealloc_argno (callee_fndecl); + if (dealloc_argno != UINT_MAX) + { + const deallocator *d + = mutable_this->get_or_create_deallocator (callee_fndecl); + on_deallocator_call (sm_ctxt, node, call, d, dealloc_argno); + } } if (tree lhs = sm_ctxt->is_zero_assignment (stmt)) @@ -1162,7 +1679,7 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, const allocation_state *astate = as_a_allocation_state (state); sm_ctxt->warn (node, stmt, arg, new use_after_free (*this, diag_arg, - astate->m_api)); + astate->m_deallocator)); sm_ctxt->set_next_state (stmt, arg, m_stop); } } @@ -1170,18 +1687,24 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, return false; } -/* Handle a call to an allocator. */ +/* Handle a call to an allocator. + RETURNS_NONNULL is true if CALL is to a fndecl known to have + __attribute__((returns_nonnull)). */ void malloc_state_machine::on_allocator_call (sm_context *sm_ctxt, const gcall *call, - const api &ap) const + const deallocator_set *deallocators, + bool returns_nonnull) const { tree lhs = gimple_call_lhs (call); if (lhs) { if (sm_ctxt->get_state (call, lhs) == m_start) - sm_ctxt->set_next_state (call, lhs, ap.m_unchecked); + sm_ctxt->set_next_state (call, lhs, + (returns_nonnull + ? deallocators->m_nonnull + : deallocators->m_unchecked)); } else { @@ -1193,40 +1716,42 @@ void malloc_state_machine::on_deallocator_call (sm_context *sm_ctxt, const supernode *node, const gcall *call, - const api &ap) const + const deallocator *d, + unsigned argno) const { - tree arg = gimple_call_arg (call, 0); + if (argno >= gimple_call_num_args (call)) + return; + tree arg = gimple_call_arg (call, argno); tree diag_arg = sm_ctxt->get_diagnostic_tree (arg); state_t state = sm_ctxt->get_state (call, arg); /* start/unchecked/nonnull -> freed. */ if (state == m_start) - sm_ctxt->set_next_state (call, arg, ap.m_freed); + sm_ctxt->set_next_state (call, arg, d->m_freed); else if (unchecked_p (state) || nonnull_p (state)) { const allocation_state *astate = as_a_allocation_state (state); - - if (astate->m_api != &ap) + gcc_assert (astate->m_deallocators); + if (!astate->m_deallocators->contains_p (d)) { /* Wrong allocator. */ - pending_diagnostic *d + pending_diagnostic *pd = new mismatching_deallocation (*this, diag_arg, - astate->m_api, &ap); - sm_ctxt->warn (node, call, arg, d); + astate->m_deallocators, + d); + sm_ctxt->warn (node, call, arg, pd); } - sm_ctxt->set_next_state (call, arg, ap.m_freed); + sm_ctxt->set_next_state (call, arg, d->m_freed); } /* Keep state "null" as-is, rather than transitioning to "freed"; we don't want to complain about double-free of NULL. */ - - else if (state == ap.m_freed) + else if (state == d->m_freed) { /* freed -> stop, with warning. */ sm_ctxt->warn (node, call, arg, - new double_free (*this, diag_arg, - ap.m_dealloc_funcname)); + new double_free (*this, diag_arg, d->m_name)); sm_ctxt->set_next_state (call, arg, m_stop); } else if (state == m_non_heap) @@ -1234,7 +1759,7 @@ malloc_state_machine::on_deallocator_call (sm_context *sm_ctxt, /* non-heap -> stop, with warning. */ sm_ctxt->warn (node, call, arg, new free_of_non_heap (*this, diag_arg, - ap.m_dealloc_funcname)); + d->m_name)); sm_ctxt->set_next_state (call, arg, m_stop); } } diff --git a/gcc/attribs.h b/gcc/attribs.h index 9e3e56a73fb..21d28a47f39 100644 --- a/gcc/attribs.h +++ b/gcc/attribs.h @@ -310,4 +310,6 @@ extern void init_attr_rdwr_indices (rdwr_map *, tree); extern attr_access *get_parm_access (rdwr_map &, tree, tree = current_function_decl); +extern unsigned fndecl_dealloc_argno (tree fndecl); + #endif // GCC_ATTRIBS_H diff --git a/gcc/builtins.c b/gcc/builtins.c index 02e7815aee5..c1115a32d91 100644 --- a/gcc/builtins.c +++ b/gcc/builtins.c @@ -13014,6 +13014,16 @@ call_dealloc_argno (tree exp) if (!fndecl) return UINT_MAX; + return fndecl_dealloc_argno (fndecl); +} + +/* Return the zero-based number corresponding to the argument being + deallocated if FNDECL is a deallocation function or UINT_MAX + if it isn't. */ + +unsigned +fndecl_dealloc_argno (tree fndecl) +{ /* A call to operator delete isn't recognized as one to a built-in. */ if (DECL_IS_OPERATOR_DELETE_P (fndecl)) return 0; diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi index 2748e98462e..6f8edc7b576 100644 --- a/gcc/doc/extend.texi +++ b/gcc/doc/extend.texi @@ -3291,6 +3291,58 @@ __attribute__ ((malloc, malloc (fclose (1)))) FILE* tmpfile (void); @end smallexample +The warnings guarded by @option{-fanalyzer} respect allocation and +deallocation pairs marked with the @code{malloc}. In particular: + +@itemize @bullet + +@item +The analyzer will emit a @option{-Wanalyzer-mismatching-deallocation} +diagnostic if there is an execution path in which the result of an +allocation call is passed to a different deallocator. + +@item +The analyzer will emit a @option{-Wanalyzer-double-free} +diagnostic if there is an execution path in which a value is passed +more than once to a deallocation call. + +@item +The analyzer will consider the possibility that an allocation function +could fail and return NULL. It will emit +@option{-Wanalyzer-possible-null-dereference} and +@option{-Wanalyzer-possible-null-argument} diagnostics if there are +execution paths in which an unchecked result of an allocation call is +dereferenced or passed to a function requiring a non-null argument. +If the allocator always returns non-null, use +@code{__attribute__ ((returns_nonnull))} to suppress these warnings. +For example: +@smallexample +char *xstrdup (const char *) + __attribute__((malloc (free), returns_nonnull)); +@end smallexample + +@item +The analyzer will emit a @option{-Wanalyzer-use-after-free} +diagnostic if there is an execution path in which the memory passed +by pointer to a deallocation call is used after the deallocation. + +@item +The analyzer will emit a @option{-Wanalyzer-malloc-leak} diagnostic if +there is an execution path in which the result of an allocation call +is leaked (without being passed to the deallocation function). + +@item +The analyzer will emit a @option{-Wanalyzer-free-of-non-heap} diagnostic +if a deallocation function is used on a global or on-stack variable. + +@end itemize + +The analyzer assumes that deallocators can gracefully handle the @code{NULL} +pointer. If this is not the case, the deallocator can be marked with +@code{__attribute__((nonnull))} so that @option{-fanalyzer} can emit +a @option{-Wanalyzer-possible-null-argument} diagnostic for code paths +in which the deallocator is called with NULL. + @item no_icf @cindex @code{no_icf} function attribute This function attribute prevents a functions from being merged with another diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index 298f1f873e3..e053d569aec 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -9155,7 +9155,8 @@ This warning requires @option{-fanalyzer}, which enables it; use @option{-Wno-analyzer-double-free} to disable it. This diagnostic warns for paths through the code in which a pointer -can have @code{free} called on it more than once. +can have a deallocator called on it more than once, either @code{free}, +or a deallocator referenced by attribute @code{malloc}. @item -Wno-analyzer-exposure-through-output-file @opindex Wanalyzer-exposure-through-output-file @@ -9196,7 +9197,8 @@ This warning requires @option{-fanalyzer}, which enables it; use to disable it. This diagnostic warns for paths through the code in which a -pointer allocated via @code{malloc} is leaked. +pointer allocated via an allocator is leaked: either @code{malloc}, +or a function marked with attribute @code{malloc}. @item -Wno-analyzer-mismatching-deallocation @opindex Wanalyzer-mismatching-deallocation @@ -9207,7 +9209,10 @@ to disable it. This diagnostic warns for paths through the code in which the wrong deallocation function is called on a pointer value, based on -which function was used to allocate the pointer value. +which function was used to allocate the pointer value. The diagnostic +will warn about mismatches between @code{free}, scalar @code{delete} +and vector @code{delete[]}, and those marked as allocator/deallocator +pairs using attribute @code{malloc}. @item -Wno-analyzer-possible-null-argument @opindex Wanalyzer-possible-null-argument @@ -9322,7 +9327,8 @@ This warning requires @option{-fanalyzer}, which enables it; use @option{-Wno-analyzer-use-after-free} to disable it. This diagnostic warns for paths through the code in which a -pointer is used after @code{free} is called on it. +pointer is used after a deallocator is called on it: either @code{free}, +or a deallocator referenced by attribute @code{malloc}. @item -Wno-analyzer-use-of-pointer-in-stale-stack-frame @opindex Wanalyzer-use-of-pointer-in-stale-stack-frame diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c new file mode 100644 index 00000000000..3de32b1b14b --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c @@ -0,0 +1,75 @@ +extern void free (void *); + +struct foo +{ + int m_int; +}; + +extern void foo_release (struct foo *); +extern struct foo *foo_acquire (void) + __attribute__ ((malloc (foo_release))); +extern void use_foo (const struct foo *) + __attribute__((nonnull)); + +void test_1 (void) +{ + struct foo *p = foo_acquire (); + foo_release (p); +} + +void test_2 (void) +{ + struct foo *p = foo_acquire (); /* { dg-message "this call could return NULL" } */ + p->m_int = 42; /* { dg-warning "dereference of possibly-NULL 'p'" } */ + foo_release (p); +} + +void test_2a (void) +{ + struct foo *p = foo_acquire (); /* { dg-message "this call could return NULL" } */ + use_foo (p); /* { dg-warning "use of possibly-NULL 'p' where non-null expected" } */ + foo_release (p); +} + +void test_3 (void) +{ + struct foo *p = foo_acquire (); /* { dg-message "allocated here" } */ +} /* { dg-warning "leak of 'p'" } */ + +void test_4 (struct foo *p) +{ + foo_release (p); + foo_release (p); /* { dg-warning "double-'foo_release' of 'p'" } */ +} + +void test_4a (void) +{ + struct foo *p = foo_acquire (); + foo_release (p); + foo_release (p); /* { dg-warning "double-'foo_release' of 'p'" } */ +} + +void test_5 (void) +{ + struct foo *p = foo_acquire (); /* { dg-message "allocated here \\(expects deallocation with 'foo_release'\\)" } */ + free (p); /* { dg-warning "'p' should have been deallocated with 'foo_release' but was deallocated with 'free'" } */ +} + +void test_6 (struct foo *p) +{ + foo_release (p); + free (p); // TODO: double-release warning! +} + +void test_7 () +{ + struct foo f; + foo_release (&f); /* { dg-warning "not on the heap" "analyzer" } */ + /* { dg-warning "'foo_release' called on unallocated object 'f'" "non-analyzer" { target *-*-* } .-1 } */ +} + +int test_8 (struct foo *p) +{ + foo_release (p); + return p->m_int; /* { dg-warning "use after 'foo_release' of 'p'" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-2.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-2.c new file mode 100644 index 00000000000..09d941fe082 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-2.c @@ -0,0 +1,24 @@ +extern void free (void *); +char *xstrdup (const char *) + __attribute__((malloc (free), returns_nonnull)); + +void test_1 (const char *s) +{ + char *p = xstrdup (s); + free (p); +} + +/* Verify that we don't issue -Wanalyzer-possible-null-dereference + when the allocator has __attribute__((returns_nonnull)). */ + +char *test_2 (const char *s) +{ + char *p = xstrdup (s); + p[0] = 'a'; /* { dg-bogus "possibly-NULL" } */ + return p; +} + +void test_3 (const char *s) +{ + char *p = xstrdup (s); /* { dg-message "allocated here" } */ +} /* { dg-warning "leak of 'p'" } */ diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-4.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-4.c new file mode 100644 index 00000000000..1517667dfb2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-4.c @@ -0,0 +1,21 @@ +/* An example where the deallocator requires non-NULL. */ + +struct foo; +extern void foo_release (struct foo *) + __attribute__((nonnull)); +extern struct foo *foo_acquire (void) + __attribute__ ((malloc (foo_release))); + +void test_1 (void) +{ + struct foo *p = foo_acquire (); /* { dg-message "this call could return NULL" } */ + foo_release (p); /* { dg-warning "use of possibly-NULL 'p' where non-null" } */ +} + +void test_2 (void) +{ + struct foo *p = foo_acquire (); + if (!p) + return; + foo_release (p); +} diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-5.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-5.c new file mode 100644 index 00000000000..7ff4e57fcfb --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-5.c @@ -0,0 +1,12 @@ +/* Example of extra argument to "malloc" attribute. */ + +struct foo; +extern void foo_release (int, struct foo *); +extern struct foo *foo_acquire (void) + __attribute__ ((malloc (foo_release, 2))); + +void test_1 (void) +{ + struct foo *p = foo_acquire (); + foo_release (0, p); +} diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c new file mode 100644 index 00000000000..bd28107d0d7 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c @@ -0,0 +1,228 @@ +/* Adapted from gcc.dg/Wmismatched-dealloc.c. */ + +#define A(...) __attribute__ ((malloc (__VA_ARGS__))) + +typedef struct FILE FILE; +typedef __SIZE_TYPE__ size_t; + +void free (void*); +void* malloc (size_t); +void* realloc (void*, size_t); + +int fclose (FILE*); +FILE* freopen (const char*, const char*, FILE*); +int pclose (FILE*); + +A (fclose) A (freopen, 3) + FILE* fdopen (int); +A (fclose) A (freopen, 3) + FILE* fopen (const char*, const char*); +A (fclose) A (freopen, 3) + FILE* fmemopen(void *, size_t, const char *); +A (fclose) A (freopen, 3) + FILE* freopen (const char*, const char*, FILE*); +A (pclose) A (freopen, 3) + FILE* popen (const char*, const char*); +A (fclose) A (freopen, 3) + FILE* tmpfile (void); + +void sink (FILE*); + + + void release (void*); +A (release) FILE* acquire (void); + +void nowarn_fdopen (void) +{ + { + FILE *q = fdopen (0); + if (!q) + return; + + fclose (q); + } + + { + FILE *q = fdopen (0); + if (!q) + return; + + q = freopen ("1", "r", q); + fclose (q); + } + + { + FILE *q = fdopen (0); + if (!q) + return; + + sink (q); + } +} + + +void warn_fdopen (void) +{ + { + FILE *q = fdopen (0); // { dg-message "allocated here" } + release (q); // { dg-warning "'release' called on 'q' returned from a mismatched allocation function" } + } + { + FILE *q = fdopen (0); // { dg-message "allocated here" } + free (q); // { dg-warning "'free' called on 'q' returned from a mismatched allocation function" } + } + + { + FILE *q = fdopen (0); // { dg-message "allocated here" } + q = realloc (q, 7); // { dg-warning "'realloc' called on 'q' returned from a mismatched allocation function" } + sink (q); + } +} + + +void nowarn_fopen (void) +{ + { + FILE *q = fopen ("1", "r"); + sink (q); + fclose (q); + } + + { + FILE *q = fopen ("2", "r"); + sink (q); + q = freopen ("3", "r", q); + sink (q); + fclose (q); + } + + { + FILE *q = fopen ("4", "r"); + sink (q); + } +} + + +void warn_fopen (void) +{ + { + FILE *q = fopen ("1", "r"); + release (q); // { dg-warning "'release' called on 'q' returned from a mismatched allocation function" } + fclose (q); + } + { + FILE *q = fdopen (0); + free (q); // { dg-warning "'free' called on 'q' returned from a mismatched allocation function" } + } + + { + FILE *q = fdopen (0); + q = realloc (q, 7); // { dg-warning "'realloc' called on 'q' returned from a mismatched allocation function" } + sink (q); + } +} + + +void test_popen (void) +{ + { + FILE *p = popen ("1", "r"); + sink (p); + pclose (p); + } + + { + FILE *p; + p = popen ("2", "r"); // { dg-message "allocated here" } + fclose (p); // { dg-warning "'fclose' called on 'p' returned from a mismatched allocation function" } + } + + { + /* freopen() can close a stream open by popen() but pclose() can't + close the stream returned from freopen(). */ + FILE *p = popen ("2", "r"); + p = freopen ("3", "r", p); // { dg-message "allocated here" } + pclose (p); // { dg-warning "'pclose' called on 'p' returned from a mismatched allocation function" } + } +} + + +void test_tmpfile (void) +{ + { + FILE *p = tmpfile (); + fclose (p); + } + + { + FILE *p = tmpfile (); + p = freopen ("1", "r", p); + fclose (p); + } + + { + FILE *p = tmpfile (); // { dg-message "allocated here" } + pclose (p); // { dg-warning "'pclose' called on 'p' returned from a mismatched allocation function" } + } +} + + +void warn_malloc (void) +{ + { + FILE *p = malloc (100); // { dg-message "allocated here" } + fclose (p); // { dg-warning "'p' should have been deallocated with 'free' but was deallocated with 'fclose'" } + } + + { + FILE *p = malloc (100); // { dg-message "allocated here" } + p = freopen ("1", "r", p);// { dg-warning "'p' should have been deallocated with 'free' but was deallocated with 'freopen'" } + fclose (p); + } + + { + FILE *p = malloc (100); // { dg-message "allocated here" } + pclose (p); // { dg-warning "'p' should have been deallocated with 'free' but was deallocated with 'pclose'" } + } +} + + +void test_acquire (void) +{ + { + FILE *p = acquire (); + release (p); + } + + { + FILE *p = acquire (); + release (p); + } + + { + FILE *p = acquire (); // { dg-message "allocated here \\(expects deallocation with 'release'\\)" } + fclose (p); // { dg-warning "'p' should have been deallocated with 'release' but was deallocated with 'fclose'" } + } + + { + FILE *p = acquire (); // { dg-message "allocated here \\(expects deallocation with 'release'\\)" } + pclose (p); // { dg-warning "'p' should have been deallocated with 'release' but was deallocated with 'pclose'" } + } + + { + FILE *p = acquire (); // { dg-message "allocated here \\(expects deallocation with 'release'\\)" } + p = freopen ("1", "r", p); // { dg-warning "'p' should have been deallocated with 'release' but was deallocated with 'freopen'" } + sink (p); + } + + { + FILE *p = acquire (); // { dg-message "allocated here \\(expects deallocation with 'release'\\)" } + free (p); // { dg-warning "'p' should have been deallocated with 'release' but was deallocated with 'free'" } + } + + { + FILE *p = acquire (); // { dg-message "allocated here \\(expects deallocation with 'release'\\)" } + p = realloc (p, 123); // { dg-warning "'p' should have been deallocated with 'release' but was deallocated with 'realloc'" } + sink (p); + } +} diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c new file mode 100644 index 00000000000..905d50ec3f9 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c @@ -0,0 +1,224 @@ +/* Adapted from linux 5.3.11: drivers/net/wireless/ath/ath10k/usb.c + Reduced reproducer for CVE-2019-19078 (leak of struct urb). */ + +typedef unsigned char u8; +typedef unsigned short u16; +typedef _Bool bool; + +#define ENOMEM 12 +#define EINVAL 22 + +/* The original file has this licence header. */ + +// SPDX-License-Identifier: ISC +/* + * Copyright (c) 2007-2011 Atheros Communications Inc. + * Copyright (c) 2011-2012,2017 Qualcomm Atheros, Inc. + * Copyright (c) 2016-2017 Erik Stromdahl <erik.stromdahl@gmail.com> + */ + +/* Adapted from include/linux/compiler_attributes.h. */ +#define __aligned(x) __attribute__((__aligned__(x))) +#define __printf(a, b) __attribute__((__format__(printf, a, b))) + +/* Possible macro for the new attribute. */ +#define __malloc(f) __attribute__((malloc(f))); + +/* From include/linux/types.h. */ + +typedef unsigned int gfp_t; + +/* Not the real value, which is in include/linux/gfp.h. */ +#define GFP_ATOMIC 32 + +/* From include/linux/usb.h. */ + +struct urb; +extern void usb_free_urb(struct urb *urb); +extern struct urb *usb_alloc_urb(int iso_packets, gfp_t mem_flags) + __malloc(usb_free_urb); +/* attribute added as part of testcase */ + +extern int usb_submit_urb(/*struct urb *urb, */gfp_t mem_flags); +extern void usb_unanchor_urb(struct urb *urb); + +/* From drivers/net/wireless/ath/ath10k/core.h. */ + +struct ath10k; + +struct ath10k { + /* [...many other fields removed...] */ + + /* must be last */ + u8 drv_priv[0] __aligned(sizeof(void *)); +}; + +/* From drivers/net/wireless/ath/ath10k/debug.h. */ + +enum ath10k_debug_mask { + /* [...other values removed...] */ + ATH10K_DBG_USB_BULK = 0x00080000, +}; + +extern unsigned int ath10k_debug_mask; + +__printf(3, 4) void __ath10k_dbg(struct ath10k *ar, + enum ath10k_debug_mask mask, + const char *fmt, ...); + +/* Simplified for now, to avoid pulling in tracepoint code. */ +static inline +bool trace_ath10k_log_dbg_enabled(void) { return 0; } + +#define ath10k_dbg(ar, dbg_mask, fmt, ...) \ +do { \ + if ((ath10k_debug_mask & dbg_mask) || \ + trace_ath10k_log_dbg_enabled()) \ + __ath10k_dbg(ar, dbg_mask, fmt, ##__VA_ARGS__); \ +} while (0) + +/* From drivers/net/wireless/ath/ath10k/hif.h. */ + +struct ath10k_hif_sg_item { + /* [...other fields removed...] */ + void *transfer_context; /* NULL = tx completion callback not called */ +}; + +struct ath10k_hif_ops { + /* send a scatter-gather list to the target */ + int (*tx_sg)(struct ath10k *ar, u8 pipe_id, + struct ath10k_hif_sg_item *items, int n_items); + /* [...other fields removed...] */ +}; + +/* From drivers/net/wireless/ath/ath10k/usb.h. */ + +/* tx/rx pipes for usb */ +enum ath10k_usb_pipe_id { + /* [...other values removed...] */ + ATH10K_USB_PIPE_MAX = 8 +}; + +struct ath10k_usb_pipe { + /* [...all fields removed...] */ +}; + +/* usb device object */ +struct ath10k_usb { + /* [...other fields removed...] */ + struct ath10k_usb_pipe pipes[ATH10K_USB_PIPE_MAX]; +}; + +/* usb urb object */ +struct ath10k_urb_context { + /* [...other fields removed...] */ + struct ath10k_usb_pipe *pipe; + struct sk_buff *skb; +}; + +static inline struct ath10k_usb *ath10k_usb_priv(struct ath10k *ar) +{ + return (struct ath10k_usb *)ar->drv_priv; +} + +/* The source file. */ + +static void ath10k_usb_post_recv_transfers(struct ath10k *ar, + struct ath10k_usb_pipe *recv_pipe); + +struct ath10k_urb_context * +ath10k_usb_alloc_urb_from_pipe(struct ath10k_usb_pipe *pipe); + +void ath10k_usb_free_urb_to_pipe(struct ath10k_usb_pipe *pipe, + struct ath10k_urb_context *urb_context); + +static int ath10k_usb_hif_tx_sg(struct ath10k *ar, u8 pipe_id, + struct ath10k_hif_sg_item *items, int n_items) +{ + struct ath10k_usb *ar_usb = ath10k_usb_priv(ar); + struct ath10k_usb_pipe *pipe = &ar_usb->pipes[pipe_id]; + struct ath10k_urb_context *urb_context; + struct sk_buff *skb; + struct urb *urb; + int ret, i; + + for (i = 0; i < n_items; i++) { + urb_context = ath10k_usb_alloc_urb_from_pipe(pipe); + if (!urb_context) { + ret = -ENOMEM; + goto err; + } + + skb = items[i].transfer_context; + urb_context->skb = skb; + + urb = usb_alloc_urb(0, GFP_ATOMIC); /* { dg-message "allocated here" } */ + if (!urb) { + ret = -ENOMEM; + goto err_free_urb_to_pipe; + } + + /* TODO: these are disabled, otherwise we conservatively + assume that they could free urb. */ +#if 0 + usb_fill_bulk_urb(urb, + ar_usb->udev, + pipe->usb_pipe_handle, + skb->data, + skb->len, + ath10k_usb_transmit_complete, urb_context); + if (!(skb->len % pipe->max_packet_size)) { + /* hit a max packet boundary on this pipe */ + urb->transfer_flags |= URB_ZERO_PACKET; + } + + usb_anchor_urb(urb, &pipe->urb_submitted); +#endif + /* TODO: initial argument disabled, otherwise we conservatively + assume that it could free urb. */ + ret = usb_submit_urb(/*urb, */GFP_ATOMIC); + if (ret) { /* TODO: why doesn't it show this condition at default verbosity? */ + ath10k_dbg(ar, ATH10K_DBG_USB_BULK, + "usb bulk transmit failed: %d\n", ret); + + /* TODO: this is disabled, otherwise we conservatively + assume that it could free urb. */ +#if 0 + usb_unanchor_urb(urb); +#endif + + ret = -EINVAL; + /* Leak of urb happens here. */ + goto err_free_urb_to_pipe; + } + + /* TODO: the loop confuses the double-free checker (another + instance of PR analyzer/93695). */ + usb_free_urb(urb); /* { dg-bogus "double-'usb_free_urb' of 'urb'" "" { xfail *-*-* } } */ + } + + return 0; + +err_free_urb_to_pipe: + ath10k_usb_free_urb_to_pipe(urb_context->pipe, urb_context); +err: + return ret; /* { dg-warning "leak of 'urb'" } */ +} + +static const struct ath10k_hif_ops ath10k_usb_hif_ops = { + .tx_sg = ath10k_usb_hif_tx_sg, +}; + +/* Simulate code to register the callback. */ +extern void callback_registration (const void *); +int ath10k_usb_probe(void) +{ + callback_registration(&ath10k_usb_hif_ops); +} + + +/* The original source file ends with: +MODULE_AUTHOR("Atheros Communications, Inc."); +MODULE_DESCRIPTION("Driver support for Qualcomm Atheros 802.11ac WLAN USB devices"); +MODULE_LICENSE("Dual BSD/GPL"); +*/ diff --git a/gcc/testsuite/gcc.dg/analyzer/attr-malloc-misuses.c b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-misuses.c new file mode 100644 index 00000000000..3c6c17bddde --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/attr-malloc-misuses.c @@ -0,0 +1,18 @@ +extern void free (void *); + +int not_a_fn __attribute__ ((malloc (free))); /* { dg-warning "'malloc' attribute ignored; valid only for functions" } */ + +void void_return (void) __attribute__ ((malloc(free))); /* { dg-warning "'malloc' attribute ignored on functions returning 'void'" } */ + +void test_void_return (void) +{ + void_return (); +} + +extern void void_args (void); /* { dg-message "declared here" } */ +void *has_malloc_with_void_args (void) + __attribute__ ((malloc(void_args))); /* { dg-error "'malloc' attribute argument 1 must take a pointer type as its first argument; have 'void'" } */ + +extern void no_args (); /* { dg-message "declared here" } */ +void *has_malloc_with_no_args (void) + __attribute__ ((malloc(no_args))); /* { dg-error "'malloc' attribute argument 1 must take a pointer type as its first argument" } */
This is an updated version of this patch from October: 'RFC: add "deallocated_by" attribute for use by analyzer' https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555544.html reworking it to build on top of Martin's work as noted below, reusing the existing attribute rather than adding a new one. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Also tested by hand with valgrind. Apart from a trivial change to attrib.h and to builtins.c, this is confined to the analyzer code and docs. The original patch was posted in stage 1. Is this OK for master? (I'm hoping for release manager permission to commit this code now; it's not clear to me whether the end of stage 3 was on the 16th or is today on the 17th). Thanks Dave In dce6c58db87ebf7f4477bd3126228e73e4eeee97 msebor extended the "malloc" attribute to support user-defined allocator/deallocator pairs. This patch extends the "malloc" checker within -fanalyzer to use these attributes. It is based on an earlier patch: 'RFC: add "deallocated_by" attribute for use by analyzer' https://gcc.gnu.org/pipermail/gcc-patches/2020-October/555544.html which added a different attribute. I mistakenly thought that it would be easy to merge our patches; the patch turned out to need a lot of reworking, to support multiple deallocators per allocator. My hope was that this would provide a minimal level of markup that would support library-checking without requiring lots of further markup. I attempted to use this to detect a memory leak within a Linux driver (CVE-2019-19078), by adding the attribute to mark these fns: extern struct urb *usb_alloc_urb(int iso_packets, gfp_t mem_flags); extern void usb_free_urb(struct urb *urb); where there is a leak of a "urb" on an error-handling path. Unfortunately I ran into the problem that there are various other fns that take "struct urb *" and the analyzer conservatively assumes that a urb passed to them might or might not be freed and thus stops tracking state for them. Hence this will only detect issues for the simplest cases (without adding another attribute). gcc/analyzer/ChangeLog: * analyzer.h (is_std_named_call_p): New decl. * diagnostic-manager.cc (path_builder::get_sm): New. (state_change_event_creator::state_change_event_creator): Add "pb" param. (state_change_event_creator::on_global_state_change): Don't consider state changes affecting other state_machines. (state_change_event_creator::on_state_change): Likewise. (state_change_event_creator::m_pb): New field. (diagnostic_manager::add_events_for_eedge): Pass pb to visitor ctor. * region-model-impl-calls.cc (region_model::impl_deallocation_call): New. * region-model.cc: Include "attribs.h". (region_model::on_call_post): Handle fndecls referenced by __attribute__((deallocated_by(FOO))). * region-model.h (region_model::impl_deallocation_call): New decl. * sm-malloc.cc: Include "stringpool.h" and "attribs.h". Add leading comment. (class api): Delete. (enum resource_state): Update comment for change from api to deallocator and deallocator_set. (allocation_state::allocation_state): Drop api param. Add "deallocators" and "deallocator". (allocation_state::m_api): Drop field in favor of... (allocation_state::m_deallocators): New field. (allocation_state::m_deallocator): New field. (enum wording): Add WORDING_DEALLOCATED. (struct deallocator): New. (struct standard_deallocator): New. (struct custom_deallocator): New. (struct deallocator_set): New. (struct custom_deallocator_set): New. (struct standard_deallocator_set): New. (struct deallocator_set_map_traits): New. (malloc_state_machine::m_malloc): Drop field (malloc_state_machine::m_scalar_new): Likewise. (malloc_state_machine::m_vector_new): Likewise. (malloc_state_machine::m_free): New field (malloc_state_machine::m_scalar_delete): Likewise. (malloc_state_machine::m_vector_delete): Likewise. (malloc_state_machine::deallocator_map_t): New typedef. (malloc_state_machine::m_deallocator_map): New field. (malloc_state_machine::deallocator_set_cache_t): New typedef. (malloc_state_machine::m_custom_deallocator_set_cache): New field. (malloc_state_machine::custom_deallocator_set_map_t): New typedef. (malloc_state_machine::m_custom_deallocator_set_map): New field. (malloc_state_machine::m_dynamic_sets): New field. (malloc_state_machine::m_dynamic_deallocators): New field. (api::api): Delete. (deallocator::deallocator): New ctor. (deallocator::hash): New. (deallocator::dump_to_pp): New. (deallocator::cmp): New. (deallocator::cmp_ptr_ptr): New. (standard_deallocator::standard_deallocator): New ctor. (deallocator_set::deallocator_set): New ctor. (deallocator_set::dump): New. (custom_deallocator_set::custom_deallocator_set): New ctor. (custom_deallocator_set::contains_p): New. (custom_deallocator_set::maybe_get_single): New. (custom_deallocator_set::dump_to_pp): New. (standard_deallocator_set::standard_deallocator_set): New ctor. (standard_deallocator_set::contains_p): New. (standard_deallocator_set::maybe_get_single): New. (standard_deallocator_set::dump_to_pp): New. (start_p): New. (class mismatching_deallocation): Update for conversion from api to deallocator_set and deallocator. (double_free::emit): Use %qs. (class use_after_free): Update for conversion from api to deallocator_set and deallocator. (malloc_leak::describe_state_change): Only emit "allocated here" on a start->nonnull transition, rather than on other transitions to nonnull. (allocation_state::dump_to_pp): Update for conversion from api to deallocator_set. (allocation_state::get_nonnull): Likewise. (malloc_state_machine::malloc_state_machine): Likewise. (malloc_state_machine::~malloc_state_machine): New. (malloc_state_machine::add_state): Update for conversion from api to deallocator_set. (malloc_state_machine::get_or_create_custom_deallocator_set): New. (malloc_state_machine::maybe_create_custom_deallocator_set): New. (malloc_state_machine::get_or_create_deallocator): New. (malloc_state_machine::on_stmt): Update for conversion from api to deallocator_set. Handle "__attribute__((malloc(FOO)))", and the special attribute set on FOO. (malloc_state_machine::on_allocator_call): Update for conversion from api to deallocator_set. Add "returns_nonnull" param and use it to affect which state to transition to. (malloc_state_machine::on_deallocator_call): Update for conversion from api to deallocator_set. gcc/ChangeLog: * attribs.h (fndecl_dealloc_argno): New decl. * builtins.c (call_dealloc_argno): Split out second half of function into... (fndecl_dealloc_argno): New. * doc/extend.texi (Common Function Attributes): Document the interaction between the analyzer and the malloc attribute. * doc/invoke.texi (Static Analyzer Options): Likewise. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/attr-malloc-1.c: New test. * gcc.dg/analyzer/attr-malloc-2.c: New test. * gcc.dg/analyzer/attr-malloc-4.c: New test. * gcc.dg/analyzer/attr-malloc-5.c: New test. * gcc.dg/analyzer/attr-malloc-6.c: New test. * gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c: New test. * gcc.dg/analyzer/attr-malloc-misuses.c: New test. --- gcc/analyzer/analyzer.h | 1 + gcc/analyzer/diagnostic-manager.cc | 15 +- gcc/analyzer/region-model-impl-calls.cc | 9 + gcc/analyzer/region-model.cc | 9 + gcc/analyzer/region-model.h | 1 + gcc/analyzer/sm-malloc.cc | 767 +++++++++++++++--- gcc/attribs.h | 2 + gcc/builtins.c | 10 + gcc/doc/extend.texi | 52 ++ gcc/doc/invoke.texi | 14 +- gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c | 75 ++ gcc/testsuite/gcc.dg/analyzer/attr-malloc-2.c | 24 + gcc/testsuite/gcc.dg/analyzer/attr-malloc-4.c | 21 + gcc/testsuite/gcc.dg/analyzer/attr-malloc-5.c | 12 + gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c | 228 ++++++ .../attr-malloc-CVE-2019-19078-usb-leak.c | 224 +++++ .../gcc.dg/analyzer/attr-malloc-misuses.c | 18 + 17 files changed, 1354 insertions(+), 128 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-2.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-4.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-5.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-6.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-CVE-2019-19078-usb-leak.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/attr-malloc-misuses.c