diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h index bfd098b861340bfd258f156fa19dcb449944ab18..8f79e4b5df5133f2aa8e27a46f9b8267e8fed3e4 100644 --- a/gcc/analyzer/analyzer.h +++ b/gcc/analyzer/analyzer.h @@ -93,7 +93,9 @@ class bounded_ranges_manager; class pending_diagnostic; class pending_note; struct event_loc_info; -class state_change_event; +class checker_event; + class state_change_event; + class warning_event; class checker_path; class extrinsic_state; class sm_state_map; diff --git a/gcc/analyzer/checker-event.cc b/gcc/analyzer/checker-event.cc index b0128e56d50de71aa0fe579590a31cddb1f2152e..3612df7bd1d36914cf5f00a0c0e3f87a503a5290 100644 --- a/gcc/analyzer/checker-event.cc +++ b/gcc/analyzer/checker-event.cc @@ -410,7 +410,8 @@ state_change_event::state_change_event (const supernode *node, state_machine::state_t from, state_machine::state_t to, const svalue *origin, - const program_state &dst_state) + const program_state &dst_state, + const exploded_node *enode) : checker_event (EK_STATE_CHANGE, event_loc_info (stmt->location, node->m_fun->decl, @@ -418,7 +419,8 @@ state_change_event::state_change_event (const supernode *node, m_node (node), m_stmt (stmt), m_sm (sm), m_sval (sval), m_from (from), m_to (to), m_origin (origin), - m_dst_state (dst_state) + m_dst_state (dst_state), + m_enode (enode) { } @@ -1159,7 +1161,7 @@ warning_event::get_desc (bool can_colorize) const tree var = fixup_tree_for_diagnostic (m_var); label_text ev_desc = m_pending_diagnostic->describe_final_event - (evdesc::final_event (can_colorize, var, m_state)); + (evdesc::final_event (can_colorize, var, m_state, *this)); if (ev_desc.get ()) { if (m_sm && flag_analyzer_verbose_state_changes) diff --git a/gcc/analyzer/checker-event.h b/gcc/analyzer/checker-event.h index 429d4cbca817a7f39e5e51c2163b2ab5014c10e2..5dd25cb0775e62222d485cf5f573827b418ca72c 100644 --- a/gcc/analyzer/checker-event.h +++ b/gcc/analyzer/checker-event.h @@ -357,7 +357,8 @@ public: state_machine::state_t from, state_machine::state_t to, const svalue *origin, - const program_state &dst_state); + const program_state &dst_state, + const exploded_node *enode); label_text get_desc (bool can_colorize) const final override; meaning get_meaning () const override; @@ -367,6 +368,8 @@ public: return m_dst_state.get_current_function (); } + const exploded_node *get_exploded_node () const { return m_enode; } + const supernode *m_node; const gimple *m_stmt; const state_machine &m_sm; @@ -375,6 +378,7 @@ public: state_machine::state_t m_to; const svalue *m_origin; program_state m_dst_state; + const exploded_node *m_enode; }; /* Subclass of checker_event; parent class for subclasses that relate to @@ -668,9 +672,11 @@ class warning_event : public checker_event { public: warning_event (const event_loc_info &loc_info, + const exploded_node *enode, const state_machine *sm, tree var, state_machine::state_t state) : checker_event (EK_WARNING, loc_info), + m_enode (enode), m_sm (sm), m_var (var), m_state (state) { } @@ -678,7 +684,10 @@ public: label_text get_desc (bool can_colorize) const final override; meaning get_meaning () const override; + const exploded_node *get_exploded_node () const { return m_enode; } + private: + const exploded_node *m_enode; const state_machine *m_sm; tree m_var; state_machine::state_t m_state; diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index f4d35617ae3deb96cd135f6ac917e0e4349aa25c..1227d6c1151295d850ac41d8a2fbfc2f771d14fa 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -1572,7 +1572,8 @@ public: src_sm_val, dst_sm_val, NULL, - dst_state)); + dst_state, + src_node)); return false; } @@ -1616,7 +1617,8 @@ public: src_sm_val, dst_sm_val, dst_origin_sval, - dst_state)); + dst_state, + src_node)); return false; } @@ -1760,7 +1762,8 @@ struct null_assignment_sm_context : public sm_context var_new_sval, from, to, NULL, - *m_new_state)); + *m_new_state, + NULL)); } void set_next_state (const gimple *stmt, @@ -1785,7 +1788,8 @@ struct null_assignment_sm_context : public sm_context sval, from, to, NULL, - *m_new_state)); + *m_new_state, + NULL)); } void warn (const supernode *, const gimple *, diff --git a/gcc/analyzer/infinite-recursion.cc b/gcc/analyzer/infinite-recursion.cc index a49ad2d71184e513a370e2251c59c8d2b5c80d89..c4e85bfac18312df1f1de6b459b6235adf4472df 100644 --- a/gcc/analyzer/infinite-recursion.cc +++ b/gcc/analyzer/infinite-recursion.cc @@ -182,7 +182,7 @@ public: /* Customize the location where the warning_event appears, putting it at the topmost entrypoint to the function. */ void add_final_event (const state_machine *, - const exploded_node *, + const exploded_node *enode, const gimple *, tree, state_machine::state_t, @@ -195,6 +195,7 @@ public: ()->get_start_location (), m_callee_fndecl, m_new_entry_enode->get_stack_depth ()), + enode, NULL, NULL, NULL)); } diff --git a/gcc/analyzer/pending-diagnostic.cc b/gcc/analyzer/pending-diagnostic.cc index c5a0e9ce3060ee7d5b7afedc3db8503763087756..79e6c5528ebcd8ba54db35bbad2bf701c294aced 100644 --- a/gcc/analyzer/pending-diagnostic.cc +++ b/gcc/analyzer/pending-diagnostic.cc @@ -232,6 +232,7 @@ pending_diagnostic::add_final_event (const state_machine *sm, (event_loc_info (get_stmt_location (stmt, enode->get_function ()), enode->get_function ()->decl, enode->get_stack_depth ()), + enode, sm, var, state)); } diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h index 6016bf69c9fcad03d2e100b685bc3f7962f62958..de79890b0e07bcfa66900e76981954123935ddc9 100644 --- a/gcc/analyzer/pending-diagnostic.h +++ b/gcc/analyzer/pending-diagnostic.h @@ -131,13 +131,15 @@ struct return_of_state : public event_desc struct final_event : public event_desc { final_event (bool colorize, - tree expr, state_machine::state_t state) + tree expr, state_machine::state_t state, + const warning_event &event) : event_desc (colorize), - m_expr (expr), m_state (state) + m_expr (expr), m_state (state), m_event (event) {} tree m_expr; state_machine::state_t m_state; + const warning_event &m_event; }; } /* end of namespace evdesc */ diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc index 212e9c2c1e25d237dc7c128a5de50f9eddca170f..9aee810f818b4c744bc97c0ec2abe6ae6ee32d13 100644 --- a/gcc/analyzer/sm-malloc.cc +++ b/gcc/analyzer/sm-malloc.cc @@ -45,6 +45,8 @@ along with GCC; see the file COPYING3. If not see #include "attribs.h" #include "analyzer/function-set.h" #include "analyzer/program-state.h" +#include "analyzer/checker-event.h" +#include "analyzer/exploded-graph.h" #if ENABLE_ANALYZER @@ -1490,7 +1492,9 @@ class deref_before_check : public malloc_diagnostic { public: deref_before_check (const malloc_state_machine &sm, tree arg) - : malloc_diagnostic (sm, arg) + : malloc_diagnostic (sm, arg), + m_deref_enode (NULL), + m_check_enode (NULL) {} const char *get_kind () const final override { return "deref_before_check"; } @@ -1502,6 +1506,31 @@ public: bool emit (rich_location *rich_loc) final override { + /* Don't emit the warning if we can't show where the deref + and the check occur. */ + if (!m_deref_enode) + return false; + if (!m_check_enode) + return false; + /* Only emit the warning for intraprocedural cases. */ + if (m_deref_enode->get_function () != m_check_enode->get_function ()) + return false; + if (&m_deref_enode->get_point ().get_call_string () + != &m_check_enode->get_point ().get_call_string ()) + return false; + + /* Reject the warning if the deref's BB doesn't dominate that + of the check, so that we don't warn e.g. for shared cleanup + code that checks a pointer for NULL, when that code is sometimes + used before a deref and sometimes after. + Using the dominance code requires setting cfun. */ + auto_cfun sentinel (m_deref_enode->get_function ()); + calculate_dominance_info (CDI_DOMINATORS); + if (!dominated_by_p (CDI_DOMINATORS, + m_check_enode->get_supernode ()->m_bb, + m_deref_enode->get_supernode ()->m_bb)) + return false; + if (m_arg) return warning_at (rich_loc, get_controlling_option (), "check of %qE for NULL after already" @@ -1520,6 +1549,7 @@ public: && assumed_non_null_p (change.m_new_state)) { m_first_deref_event = change.m_event_id; + m_deref_enode = change.m_event.get_exploded_node (); if (m_arg) return change.formatted_print ("pointer %qE is dereferenced here", m_arg); @@ -1531,6 +1561,7 @@ public: label_text describe_final_event (const evdesc::final_event &ev) final override { + m_check_enode = ev.m_event.get_exploded_node (); if (m_first_deref_event.known_p ()) { if (m_arg) @@ -1556,6 +1587,8 @@ public: private: diagnostic_event_id_t m_first_deref_event; + const exploded_node *m_deref_enode; + const exploded_node *m_check_enode; }; /* struct allocation_state : public state_machine::state. */ diff --git a/gcc/testsuite/gcc.dg/analyzer/deref-before-check-1.c b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-1.c index e1c7a00b97cca10bf3e3d36437c7d267b5d40dc3..1b11da5d8e15a59832c784daf8b45001bb4578ad 100644 --- a/gcc/testsuite/gcc.dg/analyzer/deref-before-check-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-1.c @@ -167,3 +167,39 @@ int test_checking_ptr_after_calling_deref (int *q) return 0; return v; } + +extern void foo (); +extern void bar (); +extern void baz (); + +int test_cfg_diamond_1 (int *p, int flag) +{ + int x; + x = *p; /* { dg-message "pointer 'p' is dereferenced here" } */ + if (flag) + foo (); + else + bar (); + if (p) /* { dg-warning "check of 'p' for NULL after already dereferencing it" } */ + { + baz (); + } + return x; +} + +int test_cfg_diamond_2 (int *p, int flag) +{ + int x = 0; + if (flag) + foo (); + else + { + x = *p; + bar (); + } + if (p) /* { dg-bogus "check of 'p' for NULL after already dereferencing it" } */ + { + baz (); + } + return x; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr108455-1.c b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr108455-1.c new file mode 100644 index 0000000000000000000000000000000000000000..d7d873edc51dbe9811a1c065126d9de1a8fc7c98 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr108455-1.c @@ -0,0 +1,36 @@ +extern int could_fail_1 (void); +extern void *could_fail_2 (int); +extern void cleanup (void *); + +struct header { + int signature; +}; + +int test_1 (void) { + int fd, ret = 0; + void *data = ((void *)0); + struct header *hdr; + + fd = could_fail_1 (); + + if (fd < 0) { + ret = -1; + goto cleanup; + } + + data = could_fail_2 (fd); + hdr = data; + + if (hdr->signature != 42) { + ret = -2; + goto cleanup; + } + +cleanup: + if (ret) { + if (data) /* { dg-bogus "check of 'data' for NULL after already dereferencing it" } */ + cleanup (data); + } + + return ret; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr108455-git-pack-revindex.c b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr108455-git-pack-revindex.c new file mode 100644 index 0000000000000000000000000000000000000000..7553f86051d678d21f0ca382de245a8427085027 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr108455-git-pack-revindex.c @@ -0,0 +1,133 @@ +/* Reduced from git-2.39.0's pack-revindex.c */ + +typedef unsigned int __uint32_t; +typedef unsigned long int __uintmax_t; +typedef long int __off_t; +typedef long int __off64_t; +typedef __SIZE_TYPE__ size_t; +typedef __off64_t off_t; +typedef __uint32_t uint32_t; +typedef __uintmax_t uintmax_t; + +struct stat { + /* [...snip...] */ + __off_t st_size; + /* [...snip...] */ +}; + +extern int close(int __fd); +extern int fstat(int __fd, struct stat *__buf) + __attribute__((__nothrow__, __leaf__)) __attribute__((__nonnull__(2))); +extern uint32_t default_swab32(uint32_t val); +extern uint32_t git_bswap32(uint32_t x); +__attribute__((__noreturn__)) void die(const char *err, ...) + __attribute__((format(printf, 1, 2))); +int error(const char *err, ...) __attribute__((format(printf, 1, 2))); +int error_errno(const char *err, ...) __attribute__((format(printf, 1, 2))); +static inline int const_error(void) { return -1; } +extern int munmap(void *__addr, size_t __len) + __attribute__((__nothrow__, __leaf__)); +extern size_t st_mult(size_t a, size_t b); +extern void *xmmap(void *start, size_t length, int prot, int flags, int fd, + off_t offset); +extern size_t xsize_t(off_t len); + +extern char *gettext(const char *__msgid) __attribute__((__nothrow__, __leaf__)) +__attribute__((__format_arg__(1))); +static inline __attribute__((format_arg(1))) const char *_(const char *msgid) { + if (!*msgid) + return ""; + return gettext(msgid); +} + +struct repository { + /* [...snip...] */ + const struct git_hash_algo *hash_algo; + /* [...snip...] */ +}; +extern struct repository *the_repository; +struct git_hash_algo { + /* [...snip...] */ + size_t rawsz; + /* [...snip...] */ +}; + +int git_open_cloexec(const char *name, int flags); + +struct revindex_header { + uint32_t signature; + uint32_t version; + uint32_t hash_id; +}; + +int load_revindex_from_disk(char *revindex_name, uint32_t num_objects, + const uint32_t **data_p, size_t *len_p) { + int fd, ret = 0; + struct stat st; + void *data = ((void *)0); + size_t revindex_size; + struct revindex_header *hdr; + + fd = git_open_cloexec(revindex_name, 00); + + if (fd < 0) { + ret = -1; + goto cleanup; + } + if (fstat(fd, &st)) { + ret = (error_errno(_("failed to read %s"), revindex_name), const_error()); + goto cleanup; + } + + revindex_size = xsize_t(st.st_size); + + if (revindex_size < ((12) + (2 * the_repository->hash_algo->rawsz))) { + ret = (error(_("reverse-index file %s is too small"), revindex_name), + const_error()); + goto cleanup; + } + + if (revindex_size - ((12) + (2 * the_repository->hash_algo->rawsz)) != + st_mult(sizeof(uint32_t), num_objects)) { + ret = (error(_("reverse-index file %s is corrupt"), revindex_name), + const_error()); + goto cleanup; + } + + data = xmmap(((void *)0), revindex_size, 0x1, 0x02, fd, 0); + hdr = data; + + if (git_bswap32(hdr->signature) != 0x52494458) { + ret = + (error(_("reverse-index file %s has unknown signature"), revindex_name), + const_error()); + goto cleanup; + } + if (git_bswap32(hdr->version) != 1) { + ret = (error(_("reverse-index file %s has unsupported version %" + "u"), + revindex_name, git_bswap32(hdr->version)), + const_error()); + goto cleanup; + } + if (!(git_bswap32(hdr->hash_id) == 1 || git_bswap32(hdr->hash_id) == 2)) { + ret = (error(_("reverse-index file %s has unsupported hash id %" + "u"), + revindex_name, git_bswap32(hdr->hash_id)), + const_error()); + goto cleanup; + } + +cleanup: + if (ret) { + if (data) /* { dg-bogus "check of 'data' for NULL after already dereferencing it" } */ + munmap(data, revindex_size); + } else { + *len_p = revindex_size; + *data_p = (const uint32_t *)data; + } + + if (fd >= 0) + close(fd); + return ret; +}