From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 51A983858D1E for ; Thu, 10 Nov 2022 18:37:21 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 51A983858D1E Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1668105441; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=HCt/i/pxNw/EOFONYTNrtIuHyFycB0Vft2o7llphVeE=; b=NJ6HjXz4A90vB7x8A8V6yS/+lHWzj3tNJ5rUcPNrLeqR36q63IkSCJUkjdhY9p2QopbR4K ztv7E0+46uleWBKfnHN4zArcJ8BqCu6FNPkP+hIJV8oUEyPusKBBsCFMT1VPKeVo4UhQ5o i7zxkAqe4Gw2iCvcCiBG28s2tTpIyN8= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-426-f_QbMZr1OLqu4LN8rIgPCw-1; Thu, 10 Nov 2022 13:37:19 -0500 X-MC-Unique: f_QbMZr1OLqu4LN8rIgPCw-1 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.rdu2.redhat.com [10.11.54.3]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 2930A185A78B for ; Thu, 10 Nov 2022 18:37:19 +0000 (UTC) Received: from t14s.localdomain.com (unknown [10.2.17.189]) by smtp.corp.redhat.com (Postfix) with ESMTP id D9F8C112131B; Thu, 10 Nov 2022 18:37:18 +0000 (UTC) From: David Malcolm To: gcc-patches@gcc.gnu.org Cc: David Malcolm Subject: [committed] analyzer: new warning: -Wanalyzer-deref-before-check [PR99671] Date: Thu, 10 Nov 2022 13:37:15 -0500 Message-Id: <20221110183715.2644564-1-dmalcolm@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.1 on 10.11.54.3 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII"; x-default=true X-Spam-Status: No, score=-11.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,GIT_PATCH_0,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_NONE,SPF_NONE,TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: This patch implements a new -Wanalyzer-deref-before-check within -fanalyzer. It complains about code paths in which a pointer is checked for NULL after it has already been dereferenced. For example, for the testcase in PR 77432 the diagnostic emits: deref-before-check-1.c: In function 'test_from_pr77432': deref-before-check-1.c:6:8: warning: check of 'a' for NULL after already dereferencing it [-Wanalyzer-deref-before-check] 6 | if (a) | ^ 'test_from_pr77432': events 1-2 | | 5 | int b = *a; | | ^ | | | | | (1) pointer 'a' is dereferenced here | 6 | if (a) | | ~ | | | | | (2) pointer 'a' is checked for NULL here but it was already dereferenced at (1) | and in PR 77425 we had an instance of this hidden behind a macro, which the diagnostic complains about as follows: deref-before-check-pr77425.c: In function 'get_odr_type': deref-before-check-pr77425.c:35:10: warning: check of 'odr_types_ptr' for NULL after already dereferencing it [-Wanalyzer-deref-before-check] 35 | if (odr_types_ptr) | ^ 'get_odr_type': events 1-3 | | 27 | if (cond) | | ^ | | | | | (1) following 'false' branch... |...... | 31 | else if (other_cond) | | ~~~~~~~~~~~ | | || | | |(2) ...to here | | (3) following 'true' branch... | 'get_odr_type': event 4 | | 11 | #define odr_types (*odr_types_ptr) | | ~^~~~~~~~~~~~~~~ | | | | | (4) ...to here deref-before-check-pr77425.c:33:7: note: in expansion of macro 'odr_types' | 33 | odr_types[val->id] = 0; | | ^~~~~~~~~ | 'get_odr_type': event 5 | | 11 | #define odr_types (*odr_types_ptr) | | ~^~~~~~~~~~~~~~~ | | | | | (5) pointer 'odr_types_ptr' is dereferenced here deref-before-check-pr77425.c:33:7: note: in expansion of macro 'odr_types' | 33 | odr_types[val->id] = 0; | | ^~~~~~~~~ | 'get_odr_type': event 6 | | 35 | if (odr_types_ptr) | | ^ | | | | | (6) pointer 'odr_types_ptr' is checked for NULL here but it was already dereferenced at (5) | Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. Pushed to trunk as r13-3884-g5c6546ca7d8cab. gcc/analyzer/ChangeLog: PR analyzer/99671 * analyzer.opt (Wanalyzer-deref-before-check): New warning. * diagnostic-manager.cc (null_assignment_sm_context::set_next_state): Only add state change events for transition to "null" state. (null_assignment_sm_context::is_transition_to_null): New. * engine.cc (impl_region_model_context::on_pop_frame): New. * exploded-graph.h (impl_region_model_context::on_pop_frame): New decl. * program-state.cc (sm_state_map::clear_any_state): New. (sm_state_map::can_merge_with_p): New. (program_state::can_merge_with_p): Replace requirement that sm-states be equal in favor of an attempt to merge them. * program-state.h (sm_state_map::clear_any_state): New decl. (sm_state_map::can_merge_with_p): New decl. * region-model.cc (region_model::eval_condition): Make const. (region_model::pop_frame): Call ctxt->on_pop_frame. * region-model.h (region_model::eval_condition): Make const. (region_model_context::on_pop_frame): New vfunc. (noop_region_model_context::on_pop_frame): New. (region_model_context_decorator::on_pop_frame): New. * sm-malloc.cc (enum resource_state): Add RS_ASSUMED_NON_NULL. (allocation_state::dump_to_pp): Drop "final". (struct assumed_non_null_state): New subclass. (malloc_state_machine::m_assumed_non_null): New. (assumed_non_null_p): New. (class deref_before_check): New. (assumed_non_null_state::dump_to_pp): New. (malloc_state_machine::get_or_create_assumed_non_null_state_for_frame): New. (malloc_state_machine::maybe_assume_non_null): New. (malloc_state_machine::on_stmt): Transition from start state to "assumed-non-null" state for pointers passed to __attribute__((nonnull)) arguments, and for pointers explicitly dereferenced. Call maybe_complain_about_deref_before_check for pointers explicitly compared against NULL. (malloc_state_machine::maybe_complain_about_deref_before_check): New. (malloc_state_machine::on_deallocator_call): Also transition "assumed-non-null" states to "freed". (malloc_state_machine::on_pop_frame): New. (malloc_state_machine::maybe_get_merged_states_nonequal): New. * sm-malloc.dot: Update for changes to sm-malloc.cc. * sm.h (state_machine::on_pop_frame): New. (state_machine::maybe_get_merged_state): New. (state_machine::maybe_get_merged_states_nonequal): New. gcc/ChangeLog: * doc/gcc/gcc-command-options/options-that-control-static-analysis.rst: Add -Wanalyzer-deref-before-check. gcc/testsuite/ChangeLog: * gcc.dg/analyzer/deref-before-check-1.c: New test. * gcc.dg/analyzer/deref-before-check-2.c: New test. * gcc.dg/analyzer/deref-before-check-pr77425.c: New test. * gcc.dg/analyzer/malloc-1.c (test_51): New test. gcc/ChangeLog: PR analyzer/99671 * tristate.h (tristate::is_unknown): New. Signed-off-by: David Malcolm --- gcc/analyzer/analyzer.opt | 4 + gcc/analyzer/diagnostic-manager.cc | 12 + gcc/analyzer/engine.cc | 16 + gcc/analyzer/exploded-graph.h | 2 + gcc/analyzer/program-state.cc | 83 ++++- gcc/analyzer/program-state.h | 6 + gcc/analyzer/region-model.cc | 9 +- gcc/analyzer/region-model.h | 11 +- gcc/analyzer/sm-malloc.cc | 314 +++++++++++++++++- gcc/analyzer/sm-malloc.dot | 7 + gcc/analyzer/sm.h | 31 ++ .../options-that-control-static-analysis.rst | 29 ++ .../gcc.dg/analyzer/deref-before-check-1.c | 169 ++++++++++ .../gcc.dg/analyzer/deref-before-check-2.c | 130 ++++++++ .../analyzer/deref-before-check-pr77425.c | 43 +++ gcc/testsuite/gcc.dg/analyzer/malloc-1.c | 9 + gcc/tristate.h | 1 + 17 files changed, 860 insertions(+), 16 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/analyzer/deref-before-check-1.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/deref-before-check-2.c create mode 100644 gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr77425.c diff --git a/gcc/analyzer/analyzer.opt b/gcc/analyzer/analyzer.opt index dbab3b82deb..58ad3262ebf 100644 --- a/gcc/analyzer/analyzer.opt +++ b/gcc/analyzer/analyzer.opt @@ -58,6 +58,10 @@ Wanalyzer-allocation-size Common Var(warn_analyzer_allocation_size) Init(1) Warning Warn about code paths in which a pointer to a buffer is assigned to an incompatible type. +Wanalyzer-deref-before-check +Common Var(warn_analyzer_deref_before_check) Init(1) Warning +Warn about code paths in which a pointer is checked for NULL after it has already been dereferenced. + Wanalyzer-double-fclose Common Var(warn_analyzer_double_fclose) Init(1) Warning Warn about code paths in which a stdio FILE can be closed more than once. diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc index 74cc7369d77..f82dd3a434a 100644 --- a/gcc/analyzer/diagnostic-manager.cc +++ b/gcc/analyzer/diagnostic-manager.cc @@ -1739,9 +1739,12 @@ struct null_assignment_sm_context : public sm_context state_machine::state_t from = get_state (stmt, var); if (from != m_sm.get_start_state ()) return; + if (!is_transition_to_null (to)) + return; const svalue *var_new_sval = m_new_state->m_region_model->get_rvalue (var, NULL); + const supernode *supernode = m_point->get_supernode (); int stack_depth = m_point->get_stack_depth (); @@ -1764,6 +1767,8 @@ struct null_assignment_sm_context : public sm_context state_machine::state_t from = get_state (stmt, sval); if (from != m_sm.get_start_state ()) return; + if (!is_transition_to_null (to)) + return; const supernode *supernode = m_point->get_supernode (); int stack_depth = m_point->get_stack_depth (); @@ -1834,6 +1839,13 @@ struct null_assignment_sm_context : public sm_context return m_new_state; } + /* We only care about transitions to the "null" state + within sm-malloc. Special-case this. */ + static bool is_transition_to_null (state_machine::state_t s) + { + return !strcmp (s->get_name (), "null"); + } + const program_state *m_old_state; const program_state *m_new_state; const gimple *m_stmt; diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc index 9c32afc6c71..fc90b49d041 100644 --- a/gcc/analyzer/engine.cc +++ b/gcc/analyzer/engine.cc @@ -921,6 +921,22 @@ impl_region_model_context::on_bounded_ranges (const svalue &sval, } } +/* Implementation of region_model_context::on_pop_frame vfunc. + Notify all state machines about the frame being popped, which + could lead to states being discarded. */ + +void +impl_region_model_context::on_pop_frame (const frame_region *frame_reg) +{ + int sm_idx; + sm_state_map *smap; + FOR_EACH_VEC_ELT (m_new_state->m_checker_states, sm_idx, smap) + { + const state_machine &sm = m_ext_state.get_sm (sm_idx); + sm.on_pop_frame (smap, frame_reg); + } +} + /* Implementation of region_model_context::on_phi vfunc. Notify all state machines about the phi, which could lead to state transitions. */ diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h index 8e3c160189c..de927e6c1b7 100644 --- a/gcc/analyzer/exploded-graph.h +++ b/gcc/analyzer/exploded-graph.h @@ -77,6 +77,8 @@ class impl_region_model_context : public region_model_context void on_bounded_ranges (const svalue &sval, const bounded_ranges &ranges) final override; + void on_pop_frame (const frame_region *frame_reg) final override; + void on_unknown_change (const svalue *sval, bool is_mutable) final override; void on_phi (const gphi *phi, tree rhs) final override; diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc index d00fd5ebe0b..a0585f5ceef 100644 --- a/gcc/analyzer/program-state.cc +++ b/gcc/analyzer/program-state.cc @@ -518,6 +518,14 @@ sm_state_map::impl_set_state (const svalue *sval, return true; } +/* Clear any state for SVAL from this state map. */ + +void +sm_state_map::clear_any_state (const svalue *sval) +{ + m_map.remove (sval); +} + /* Set the "global" state within this state map to STATE. */ void @@ -719,6 +727,67 @@ sm_state_map::canonicalize_svalue (const svalue *sval, return sval; } +/* Attempt to merge this state map with OTHER, writing the result + into *OUT. + Return true if the merger was possible, false otherwise. + + Normally, only identical state maps can be merged, so that + differences between state maps lead to different enodes + + However some state machines may support merging states to + allow for discarding of less important states, and thus avoid + blow-up of the exploded graph. */ + +bool +sm_state_map::can_merge_with_p (const sm_state_map &other, + const state_machine &sm, + const extrinsic_state &ext_state, + sm_state_map **out) const +{ + /* If identical, then they merge trivially, with a copy. */ + if (*this == other) + { + delete *out; + *out = clone (); + return true; + } + + delete *out; + *out = new sm_state_map (sm); + + /* Otherwise, attempt to merge element by element. */ + + /* Try to merge global state. */ + if (state_machine::state_t merged_global_state + = sm.maybe_get_merged_state (get_global_state (), + other.get_global_state ())) + (*out)->set_global_state (merged_global_state); + else + return false; + + /* Try to merge state each svalue's state (for the union + of svalues represented by each smap). + Ignore the origin information. */ + hash_set svals; + for (auto kv : *this) + svals.add (kv.first); + for (auto kv : other) + svals.add (kv.first); + for (auto sval : svals) + { + state_machine::state_t this_state = get_state (sval, ext_state); + state_machine::state_t other_state = other.get_state (sval, ext_state); + if (state_machine::state_t merged_state + = sm.maybe_get_merged_state (this_state, other_state)) + (*out)->impl_set_state (sval, merged_state, NULL, ext_state); + else + return false; + } + + /* Successfully merged all elements. */ + return true; +} + /* class program_state. */ /* program_state's ctor. */ @@ -1283,11 +1352,14 @@ program_state::can_merge_with_p (const program_state &other, gcc_assert (out); gcc_assert (m_region_model); - /* Early reject if there are sm-differences between the states. */ + /* Attempt to merge the sm-states. */ int i; sm_state_map *smap; FOR_EACH_VEC_ELT (out->m_checker_states, i, smap) - if (*m_checker_states[i] != *other.m_checker_states[i]) + if (!m_checker_states[i]->can_merge_with_p (*other.m_checker_states[i], + ext_state.get_sm (i), + ext_state, + &out->m_checker_states[i])) return false; /* Attempt to merge the region_models. */ @@ -1298,13 +1370,6 @@ program_state::can_merge_with_p (const program_state &other, this, &other)) return false; - /* Copy m_checker_states to OUT. */ - FOR_EACH_VEC_ELT (out->m_checker_states, i, smap) - { - delete smap; - out->m_checker_states[i] = m_checker_states[i]->clone (); - } - out->m_region_model->canonicalize (); return true; diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h index ad40578c703..79643278ee8 100644 --- a/gcc/analyzer/program-state.h +++ b/gcc/analyzer/program-state.h @@ -145,6 +145,7 @@ public: state_machine::state_t state, const svalue *origin, const extrinsic_state &ext_state); + void clear_any_state (const svalue *sval); void set_global_state (state_machine::state_t state); state_machine::state_t get_global_state () const; @@ -174,6 +175,11 @@ public: bool replay_call_summary (call_summary_replay &r, const sm_state_map &summary); + bool can_merge_with_p (const sm_state_map &other, + const state_machine &sm, + const extrinsic_state &ext_state, + sm_state_map **out) const; + private: const state_machine &m_sm; map_t m_map; diff --git a/gcc/analyzer/region-model.cc b/gcc/analyzer/region-model.cc index 5ffad64a9c5..b91434d7db4 100644 --- a/gcc/analyzer/region-model.cc +++ b/gcc/analyzer/region-model.cc @@ -4690,7 +4690,7 @@ tristate region_model::eval_condition (tree lhs, enum tree_code op, tree rhs, - region_model_context *ctxt) + region_model_context *ctxt) const { /* For now, make no attempt to model constraints on floating-point values. */ @@ -5415,8 +5415,13 @@ region_model::pop_frame (tree result_lvalue, { gcc_assert (m_current_frame); - /* Evaluate the result, within the callee frame. */ const frame_region *frame_reg = m_current_frame; + + /* Notify state machines. */ + if (ctxt) + ctxt->on_pop_frame (frame_reg); + + /* Evaluate the result, within the callee frame. */ tree fndecl = m_current_frame->get_function ()->decl; tree result = DECL_RESULT (fndecl); const svalue *retval = NULL; diff --git a/gcc/analyzer/region-model.h b/gcc/analyzer/region-model.h index 70c808f4973..50790596726 100644 --- a/gcc/analyzer/region-model.h +++ b/gcc/analyzer/region-model.h @@ -458,7 +458,7 @@ class region_model tristate eval_condition (tree lhs, enum tree_code op, tree rhs, - region_model_context *ctxt); + region_model_context *ctxt) const; bool add_constraint (tree lhs, enum tree_code op, tree rhs, region_model_context *ctxt); bool add_constraint (tree lhs, enum tree_code op, tree rhs, @@ -708,6 +708,9 @@ class region_model_context virtual void on_bounded_ranges (const svalue &sval, const bounded_ranges &ranges) = 0; + /* Hook for clients to be notified when a frame is popped from the stack. */ + virtual void on_pop_frame (const frame_region *) = 0; + /* Hooks for clients to be notified when an unknown change happens to SVAL (in response to a call to an unknown function). */ virtual void on_unknown_change (const svalue *sval, bool is_mutable) = 0; @@ -789,6 +792,7 @@ public: const bounded_ranges &) override { } + void on_pop_frame (const frame_region *) override {} void on_unknown_change (const svalue *sval ATTRIBUTE_UNUSED, bool is_mutable ATTRIBUTE_UNUSED) override { @@ -886,6 +890,11 @@ class region_model_context_decorator : public region_model_context m_inner->on_bounded_ranges (sval, ranges); } + void on_pop_frame (const frame_region *frame_reg) override + { + m_inner->on_pop_frame (frame_reg); + } + void on_unknown_change (const svalue *sval, bool is_mutable) override { m_inner->on_unknown_change (sval, is_mutable); diff --git a/gcc/analyzer/sm-malloc.cc b/gcc/analyzer/sm-malloc.cc index d050ef83eeb..dd10356155c 100644 --- a/gcc/analyzer/sm-malloc.cc +++ b/gcc/analyzer/sm-malloc.cc @@ -87,6 +87,9 @@ enum resource_state /* The start state. */ RS_START, + /* State for a pointer that's been unconditionally dereferenced. */ + RS_ASSUMED_NON_NULL, + /* State for a pointer that's known to be NULL. */ RS_NULL, @@ -126,7 +129,7 @@ struct allocation_state : public state_machine::state m_deallocator (deallocator) {} - void dump_to_pp (pretty_printer *pp) const final override; + void dump_to_pp (pretty_printer *pp) const override; const allocation_state *get_nonnull () const; @@ -135,6 +138,25 @@ struct allocation_state : public state_machine::state const deallocator *m_deallocator; }; +/* Custom state subclass, for the "assumed-non-null" state + where the assumption happens in a particular frame. */ + +struct assumed_non_null_state : public allocation_state +{ + assumed_non_null_state (const char *name, unsigned id, + const frame_region *frame) + : allocation_state (name, id, RS_ASSUMED_NON_NULL, + NULL, NULL), + m_frame (frame) + { + gcc_assert (m_frame); + } + + void dump_to_pp (pretty_printer *pp) const final override; + + const frame_region *m_frame; +}; + /* An enum for choosing which wording to use in various diagnostics when describing deallocations. */ @@ -384,14 +406,25 @@ public: enum tree_code op, const svalue *rhs) const final override; + void on_pop_frame (sm_state_map *smap, + const frame_region *) const final override; + bool can_purge_p (state_t s) const final override; std::unique_ptr on_leak (tree var) const final override; bool reset_when_passed_to_unknown_fn_p (state_t s, bool is_mutable) const final override; + state_t + maybe_get_merged_states_nonequal (state_t state_a, + state_t state_b) const final override; + static bool unaffected_by_call_p (tree fndecl); + void maybe_assume_non_null (sm_context *sm_ctxt, + tree ptr, + const gimple *stmt) const; + void on_realloc_with_move (region_model *model, sm_state_map *smap, const svalue *old_ptr_sval, @@ -406,6 +439,10 @@ public: /* States that are independent of api. */ + /* States for a pointer that's been unconditionally dereferenced + in a particular stack frame. */ + hash_map m_assumed_non_null; + /* State for a pointer that's known to be NULL. */ state_t m_null; @@ -425,6 +462,16 @@ private: const deallocator * get_or_create_deallocator (tree deallocator_fndecl); + state_t + get_or_create_assumed_non_null_state_for_frame (const frame_region *frame); + + void + maybe_complain_about_deref_before_check (sm_context *sm_ctxt, + const supernode *node, + const gimple *stmt, + const assumed_non_null_state *, + tree ptr) const; + void on_allocator_call (sm_context *sm_ctxt, const gcall *call, const deallocator_set *deallocators, @@ -678,6 +725,14 @@ freed_p (state_machine::state_t state) return get_rs (state) == RS_FREED; } +/* Return true if STATE is a value that has been assumed to be non-NULL. */ + +static bool +assumed_non_null_p (state_machine::state_t state) +{ + return get_rs (state) == RS_ASSUMED_NON_NULL; +} + /* Class for diagnostics relating to malloc_state_machine. */ class malloc_diagnostic : public pending_diagnostic @@ -1428,6 +1483,80 @@ private: const char *m_funcname; }; +/* Concrete pending_diagnostic subclass for -Wanalyzer-deref-before-check. */ + +class deref_before_check : public malloc_diagnostic +{ +public: + deref_before_check (const malloc_state_machine &sm, tree arg) + : malloc_diagnostic (sm, arg) + {} + + const char *get_kind () const final override { return "deref_before_check"; } + + int get_controlling_option () const final override + { + return OPT_Wanalyzer_deref_before_check; + } + + bool emit (rich_location *rich_loc) final override + { + if (m_arg) + return warning_at (rich_loc, get_controlling_option (), + "check of %qE for NULL after already" + " dereferencing it", + m_arg); + else + return warning_at (rich_loc, get_controlling_option (), + "check of pointer for NULL after already" + " dereferencing it"); + } + + label_text describe_state_change (const evdesc::state_change &change) + final override + { + if (change.m_old_state == m_sm.get_start_state () + && assumed_non_null_p (change.m_new_state)) + { + m_first_deref_event = change.m_event_id; + if (m_arg) + return change.formatted_print ("pointer %qE is dereferenced here", + m_arg); + else + return label_text::borrow ("pointer is dereferenced here"); + } + return malloc_diagnostic::describe_state_change (change); + } + + label_text describe_final_event (const evdesc::final_event &ev) final override + { + if (m_first_deref_event.known_p ()) + { + if (m_arg) + return ev.formatted_print ("pointer %qE is checked for NULL here but" + " it was already dereferenced at %@", + m_arg, &m_first_deref_event); + else + return ev.formatted_print ("pointer is checked for NULL here but" + " it was already dereferenced at %@", + &m_first_deref_event); + } + else + { + if (m_arg) + return ev.formatted_print ("pointer %qE is checked for NULL here but" + " it was already dereferenced", + m_arg); + else + return ev.formatted_print ("pointer is checked for NULL here but" + " it was already dereferenced"); + } + } + +private: + diagnostic_event_id_t m_first_deref_event; +}; + /* struct allocation_state : public state_machine::state. */ /* Implementation of state_machine::state::dump_to_pp vfunc @@ -1456,6 +1585,17 @@ allocation_state::get_nonnull () const return as_a_allocation_state (m_deallocators->m_nonnull); } +/* struct assumed_non_null_state : public allocation_state. */ + +void +assumed_non_null_state::dump_to_pp (pretty_printer *pp) const +{ + allocation_state::dump_to_pp (pp); + pp_string (pp, " (in "); + m_frame->dump_to_pp (pp, true); + pp_character (pp, ')'); +} + /* malloc_state_machine's ctor. */ malloc_state_machine::malloc_state_machine (logger *logger) @@ -1597,6 +1737,22 @@ malloc_state_machine::get_or_create_deallocator (tree deallocator_fndecl) return d; } +/* Get the "assumed-non-null" state for assumptions made within FRAME, + creating it if necessary. */ + +state_machine::state_t +malloc_state_machine:: +get_or_create_assumed_non_null_state_for_frame (const frame_region *frame) +{ + if (state_t *slot = m_assumed_non_null.get (frame)) + return *slot; + state_machine::state *new_state + = new assumed_non_null_state ("assumed-non-null", alloc_state_id (), frame); + add_custom_state (new_state); + m_assumed_non_null.put (frame, new_state); + return new_state; +} + /* Try to identify the function declaration either by name or as a known malloc builtin. */ @@ -1629,6 +1785,33 @@ known_allocator_p (const_tree fndecl, const gcall *call) return false; } +/* If PTR's nullness is not known, transition it to the "assumed-non-null" + state for the current frame. */ + +void +malloc_state_machine::maybe_assume_non_null (sm_context *sm_ctxt, + tree ptr, + const gimple *stmt) const +{ + const region_model *old_model = sm_ctxt->get_old_region_model (); + if (!old_model) + return; + + tree null_ptr_cst = build_int_cst (TREE_TYPE (ptr), 0); + tristate known_non_null + = old_model->eval_condition (ptr, NE_EXPR, null_ptr_cst, NULL); + if (known_non_null.is_unknown ()) + { + /* Cast away const-ness for cache-like operations. */ + malloc_state_machine *mut_this + = const_cast (this); + state_t next_state + = mut_this->get_or_create_assumed_non_null_state_for_frame + (old_model->get_current_frame ()); + sm_ctxt->set_next_state (stmt, ptr, next_state); + } +} + /* Implementation of state_machine::on_stmt vfunc for malloc_state_machine. */ bool @@ -1743,6 +1926,8 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, (*this, diag_arg, callee_fndecl, i)); sm_ctxt->set_next_state (stmt, arg, m_stop); } + else if (state == m_start) + maybe_assume_non_null (sm_ctxt, arg, stmt); } } BITMAP_FREE (nonnull_args); @@ -1760,6 +1945,36 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, } } + /* Look for pointers explicitly being compared against zero + that are in state assumed_non_null i.e. we already defererenced + them. + We have to do this check here, rather than in on_condition + because we add a constraint that the pointer is non-null when + dereferencing it, and this makes the apply_constraints_for_gcond + find known-true and known-false conditions; on_condition is only + called when adding new constraints. */ + if (const gcond *cond_stmt = dyn_cast (stmt)) + { + enum tree_code op = gimple_cond_code (cond_stmt); + if (op == EQ_EXPR || op == NE_EXPR) + { + tree lhs = gimple_cond_lhs (cond_stmt); + tree rhs = gimple_cond_rhs (cond_stmt); + if (any_pointer_p (lhs) + && any_pointer_p (rhs) + && zerop (rhs)) + { + state_t state = sm_ctxt->get_state (stmt, lhs); + if (assumed_non_null_p (state)) + maybe_complain_about_deref_before_check + (sm_ctxt, node, + stmt, + (const assumed_non_null_state *)state, + lhs); + } + } + } + if (tree lhs = sm_ctxt->is_zero_assignment (stmt)) if (any_pointer_p (lhs)) on_zero_assignment (sm_ctxt, stmt,lhs); @@ -1778,7 +1993,9 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, tree arg = TREE_OPERAND (op, 0); state_t state = sm_ctxt->get_state (stmt, arg); - if (unchecked_p (state)) + if (state == m_start) + maybe_assume_non_null (sm_ctxt, arg, stmt); + else if (unchecked_p (state)) { tree diag_arg = sm_ctxt->get_diagnostic_tree (arg); sm_ctxt->warn (node, stmt, arg, @@ -1808,6 +2025,53 @@ malloc_state_machine::on_stmt (sm_context *sm_ctxt, return false; } +/* Given a check against null of PTR in assumed-non-null state STATE, + potentially add a deref_before_check warning to SM_CTXT. */ + +void +malloc_state_machine:: +maybe_complain_about_deref_before_check (sm_context *sm_ctxt, + const supernode *node, + const gimple *stmt, + const assumed_non_null_state *state, + tree ptr) const +{ + const region_model *model = sm_ctxt->get_old_region_model (); + if (!model) + return; + + /* Don't complain if the current frame (where the check is occurring) is + deeper than the frame in which the "not null" assumption was made. + This suppress false positives for cases like: + + void foo (struct s *p) + { + int val = s->some_field; // deref here + shared_helper (p); + } + + where "shared_helper" has: + + void shared_helper (struct s *p) + { + if (!p) // check here + return; + // etc + } + + since the check in "shared_helper" is OK. */ + const frame_region *checked_in_frame = model->get_current_frame (); + const frame_region *assumed_nonnull_in_frame = state->m_frame; + if (checked_in_frame->get_index () > assumed_nonnull_in_frame->get_index ()) + return; + + tree diag_ptr = sm_ctxt->get_diagnostic_tree (ptr); + sm_ctxt->warn + (node, stmt, ptr, + make_unique (*this, diag_ptr)); + sm_ctxt->set_next_state (stmt, ptr, m_stop); +} + /* Handle a call to an allocator. RETURNS_NONNULL is true if CALL is to a fndecl known to have __attribute__((returns_nonnull)). */ @@ -1870,8 +2134,8 @@ malloc_state_machine::on_deallocator_call (sm_context *sm_ctxt, state_t state = sm_ctxt->get_state (call, arg); - /* start/unchecked/nonnull -> freed. */ - if (state == m_start) + /* start/assumed_non_null/unchecked/nonnull -> freed. */ + if (state == m_start || assumed_non_null_p (state)) sm_ctxt->set_next_state (call, arg, d->m_freed); else if (unchecked_p (state) || nonnull_p (state)) { @@ -2016,6 +2280,31 @@ malloc_state_machine::on_condition (sm_context *sm_ctxt, } } +/* Implementation of state_machine::on_pop_frame vfunc for malloc_state_machine. + Clear any "assumed-non-null" state where the assumption happened in + FRAME_REG. */ + +void +malloc_state_machine::on_pop_frame (sm_state_map *smap, + const frame_region *frame_reg) const +{ + hash_set svals_to_clear; + for (auto kv : *smap) + { + const svalue *sval = kv.first; + state_t state = kv.second.m_state; + if (assumed_non_null_p (state)) + { + const assumed_non_null_state *assumed_state + = (const assumed_non_null_state *)state; + if (frame_reg == assumed_state->m_frame) + svals_to_clear.add (sval); + } + } + for (auto sval : svals_to_clear) + smap->clear_any_state (sval); +} + /* Implementation of state_machine::can_purge_p vfunc for malloc_state_machine. Don't allow purging of pointers in state 'unchecked' or 'nonnull' (to avoid false leak reports). */ @@ -2053,6 +2342,23 @@ malloc_state_machine::reset_when_passed_to_unknown_fn_p (state_t s, return is_mutable; } +/* Implementation of state_machine::maybe_get_merged_states_nonequal vfunc + for malloc_state_machine. + + Support discarding "assumed-non-null" states when merging with + start state. */ + +state_machine::state_t +malloc_state_machine::maybe_get_merged_states_nonequal (state_t state_a, + state_t state_b) const +{ + if (assumed_non_null_p (state_a) && state_b == m_start) + return m_start; + if (state_a == m_start && assumed_non_null_p (state_b)) + return m_start; + return NULL; +} + /* Return true if calls to FNDECL are known to not affect this sm-state. */ bool diff --git a/gcc/analyzer/sm-malloc.dot b/gcc/analyzer/sm-malloc.dot index c8f4f9c06ba..cb1c54aa351 100644 --- a/gcc/analyzer/sm-malloc.dot +++ b/gcc/analyzer/sm-malloc.dot @@ -32,6 +32,9 @@ digraph "malloc" { It could be a pointer to heap-allocated memory, or could be NULL. */ unchecked; + /* State for a pointer that's been unconditionally dereferenced. */ + assumed_non_null; + /* State for a pointer that's known to be NULL. */ null; @@ -58,6 +61,7 @@ digraph "malloc" { /* On "free". */ start -> freed [label="on 'free(X);'"]; + assumed_non_null -> freed [label="on 'free(X);'"]; unchecked -> freed [label="on 'free(X);'"]; nonnull -> freed [label="on 'free(X);'"]; freed -> stop [label="on 'free(X);':\n Warn('double-free')"]; @@ -66,6 +70,7 @@ digraph "malloc" { /* Handle "__attribute__((nonnull))". */ unchecked -> nonnull [label="on 'FN(X)' with __attribute__((nonnull)):\nWarn('possible NULL arg')"]; null -> stop [label="on 'FN(X)' with __attribute__((nonnull)):\nWarn('NULL arg')"]; + start -> assumed_non_null [label="on 'FN(X)' with __attribute__((nonnull))"]; /* is_zero_assignment. */ start -> null [label="on 'X = 0;'"]; @@ -76,6 +81,7 @@ digraph "malloc" { start -> non_heap [label="on 'X = &EXPR;'"]; /* Handle dereferences. */ + start -> assumed_non_null [label="on '*X'"]; unchecked -> nonnull [label="on '*X':\nWarn('possible NULL deref')"]; null -> stop [label="on '*X':\nWarn('NULL deref')"]; freed -> stop [label="on '*X':\nWarn('use after free')"]; @@ -83,6 +89,7 @@ digraph "malloc" { /* on_condition. */ unchecked -> nonnull [label="on 'X != 0'"]; unchecked -> null [label="on 'X == 0'"]; + assumed_non_null -> stop [label="on 'if (X)':\nWarn('deref-before-check')"]; unchecked -> stop [label="on leak:\nWarn('leak')"]; nonnull -> stop [label="on leak:\nWarn('leak')"]; diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h index 0171474e4b9..085a3a1514b 100644 --- a/gcc/analyzer/sm.h +++ b/gcc/analyzer/sm.h @@ -117,6 +117,12 @@ public: { } + virtual void + on_pop_frame (sm_state_map *smap ATTRIBUTE_UNUSED, + const frame_region *frame_reg ATTRIBUTE_UNUSED) const + { + } + /* Return true if it safe to discard the given state (to help when simplifying state objects). States that need leak detection should return false. */ @@ -139,6 +145,31 @@ public: return is_mutable; } + /* Attempt to get a state for the merger of STATE_A and STATE_B, + or return NULL if merging shouldn't occur, so that differences + between sm-state will lead to separate exploded nodes. + + Most state machines will only merge equal states, but can + override maybe_get_merged_states_nonequal to support mergers + of certain non-equal states. */ + state_t maybe_get_merged_state (state_t state_a, + state_t state_b) const + { + if (state_a == state_b) + return state_a; + return maybe_get_merged_states_nonequal (state_a, state_b); + } + + /* Base implementation of hook for maybe_get_merged_state on non-equal + states. */ + virtual state_t + maybe_get_merged_states_nonequal (state_t state_a ATTRIBUTE_UNUSED, + state_t state_b ATTRIBUTE_UNUSED) const + { + /* By default, non-equal sm states should inhibit merger of enodes. */ + return NULL; + } + void validate (state_t s) const; void dump_to_pp (pretty_printer *pp) const; diff --git a/gcc/doc/gcc/gcc-command-options/options-that-control-static-analysis.rst b/gcc/doc/gcc/gcc-command-options/options-that-control-static-analysis.rst index c0e06ea5b2d..09bf049036f 100644 --- a/gcc/doc/gcc/gcc-command-options/options-that-control-static-analysis.rst +++ b/gcc/doc/gcc/gcc-command-options/options-that-control-static-analysis.rst @@ -19,6 +19,7 @@ Options That Control Static Analysis Enabling this option effectively enables the following warnings: :option:`-Wanalyzer-allocation-size` |gol| + :option:`-Wanalyzer-deref-before-check` |gol| :option:`-Wanalyzer-double-fclose` |gol| :option:`-Wanalyzer-double-free` |gol| :option:`-Wanalyzer-exposure-through-output-file` |gol| @@ -88,6 +89,33 @@ Options That Control Static Analysis Default setting; overrides :option:`-Wno-analyzer-allocation-size`. +.. option:: -Wno-analyzer-deref-before-check + + This warning requires :option:`-fanalyzer`, which enables it; use + :option:`-Wno-analyzer-deref-before-check` + to disable it. + + This diagnostic warns for paths through the code in which a pointer + is checked for ``NULL`` *after* it has already been + dereferenced, suggesting that the pointer could have been NULL. + Such cases suggest that the check for NULL is either redundant, + or that it needs to be moved to before the pointer is dereferenced. + + This diagnostic also considers values passed to a function argument + marked with ``__attribute__((nonnull))`` as requiring a non-NULL + value, and thus will complain if such values are checked for ``NULL`` + after returning from such a function call. + + This diagnostic is unlikely to be reported when any level of optimization + is enabled, as GCC's optimization logic will typically consider such + checks for NULL as being redundant, and optimize them away before the + analyzer "sees" them. Hence optimization should be disabled when + attempting to trigger this diagnostic. + +.. option:: -Wanalyzer-deref-before-check + + Default setting; overrides :option:`-Wno-analyzer-deref-before-check`. + .. option:: -Wno-analyzer-double-fclose This warning requires :option:`-fanalyzer`, which enables it; use @@ -825,6 +853,7 @@ The following options control the analyzer. Currently, :option:`-fanalyzer-checker=taint` disables the following warnings from :option:`-fanalyzer` : + :option:`-Wanalyzer-deref-before-check` |gol| :option:`-Wanalyzer-double-fclose` |gol| :option:`-Wanalyzer-double-free` |gol| :option:`-Wanalyzer-exposure-through-output-file` |gol| diff --git a/gcc/testsuite/gcc.dg/analyzer/deref-before-check-1.c b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-1.c new file mode 100644 index 00000000000..e1c7a00b97c --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-1.c @@ -0,0 +1,169 @@ +#define NULL ((void *)0) + +int test_from_pr77432 (int *a) +{ + int b = *a; /* { dg-message "pointer 'a' is dereferenced here" } */ + if (a) /* { dg-warning "check of 'a' for NULL after already dereferencing it \\\[-Wanalyzer-deref-before-check\\\]" "warning" } */ + /* { dg-message "pointer 'a' is checked for NULL here but it was already dereferenced at \\(1\\)" "final event" { target *-*-* } .-1 } */ + return b; + return 0; +} + +int test_1a (int *p, int x) +{ + *p = x; /* { dg-message "pointer 'p' is dereferenced here" } */ + + if (p) /* { dg-warning "check of 'p' for NULL after already dereferencing it \\\[-Wanalyzer-deref-before-check\\\]" "warning" } */ + /* { dg-message "pointer 'p' is checked for NULL here but it was already dereferenced at \\(1\\)" "final event" { target *-*-* } .-1 } */ + return 1; + else + return 0; +} + +int test_1b (int *p, int *q) +{ + *q = *p; /* { dg-message "8: pointer 'p' is dereferenced here" } */ + + if (p) /* { dg-warning "check of 'p' for NULL after already dereferencing it \\\[-Wanalyzer-deref-before-check\\\]" "warning" } */ + /* { dg-message "pointer 'p' is checked for NULL here but it was already dereferenced at \\(1\\)" "final event" { target *-*-* } .-1 } */ + return 1; + else + return 0; +} + +struct s2 +{ + int x; + int y; +}; + +int test_2a (struct s2 *p) +{ + int sum = p->x + p->y; /* { dg-message "pointer 'p' is dereferenced here" } */ + if (!p) /* { dg-warning "check of 'p' for NULL after already dereferencing it" } */ + __builtin_abort (); + return sum; +} + +int test_2b (struct s2 *p) +{ + if (!p) + __builtin_abort (); + int sum = p->x + p->y; + return sum; +} + +struct s3 +{ + int flag; +}; + +extern void err (const char *); + +void test_3 (struct s3 *p) +{ + if (p->flag) /* { dg-message "pointer 'p' is dereferenced here" } */ + err ("p->flag"); + if (!p) /* { dg-warning "check of 'p' for NULL after already dereferencing it" } */ + err ("p was NULL"); +} + +struct s4 +{ + struct s4 *m_next; + int m_val; +}; + +int test_4 (struct s4 *p) +{ + if (p->m_next->m_val > 0) /* { dg-message "pointer '\\*p.m_next' is dereferenced here" } */ + return -1; + if (!p->m_next) /* { dg-warning "check of '\\*p.m_next' for NULL after already dereferencing it" } */ + return -2; + return p->m_next->m_val; +} + +struct s5 +{ + const char *str; + int val; +}; + +int test_5 (struct s5 *p) +{ + __builtin_printf ("%s: %i\n", p->str, p->val); /* { dg-message "pointer 'p' is dereferenced here" } */ + if (p != NULL) /* { dg-warning "check of 'p' for NULL after already dereferencing it" } */ + return p->val; + return -1; +} + +static int __attribute__((noinline)) +__analyzer_check_ptr (int *p) +{ + if (p) + return *p; + else + return 42; +} + +int test_calling_check_ptr_after_deref_1 (int *q) +{ + int v = *q; /* { dg-bogus "dereferenced here" } */ + v += __analyzer_check_ptr (q); + return v; +} + +int test_calling_check_ptr_after_deref_2 (int *q) +{ + int v = *q; /* { dg-bogus "dereferenced here" } */ + v += __analyzer_check_ptr (q); + *q = 17; + return v; +} + +int test_calling_check_ptr_after_deref_3 (int *q) +{ + int v = *q; /* { dg-message "pointer 'q' is dereferenced here" } */ + v += __analyzer_check_ptr (q); + if (q) /* { dg-warning "check of 'q' for NULL after already dereferencing it" } */ + *q = 17; + return v; +} + +static int __attribute__((noinline)) +__analyzer_deref_ptr (int *p) +{ + return *p; +} + +int test_calling_check_ptr_after_calling_deref_1 (int *q) +{ + int v = __analyzer_deref_ptr (q); + v += __analyzer_check_ptr (q); + return v; +} + +int test_calling_check_ptr_after_calling_deref_2 (int *q) +{ + int v = __analyzer_deref_ptr (q); + v += __analyzer_check_ptr (q); + *q = 17; + return v; +} + +int test_calling_check_ptr_after_calling_deref_3 (int *q) +{ + int v = __analyzer_deref_ptr (q); + v += __analyzer_check_ptr (q); + if (q) + *q = 17; + return v; +} + +int test_checking_ptr_after_calling_deref (int *q) +{ + int v = __analyzer_deref_ptr (q); + if (q) + return 0; + return v; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/deref-before-check-2.c b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-2.c new file mode 100644 index 00000000000..c0409c42d89 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-2.c @@ -0,0 +1,130 @@ +#include + +struct st +{ + char *str; + int i; +}; + +int test_1 (struct st *p) +{ + fprintf (stderr, "str: %s\n", p->str); /* { dg-message "pointer 'p' is dereferenced here" } */ + if (!p) /* { dg-warning "check of 'p' for NULL after already dereferencing it" } */ + return -1; + return p->i; +} + +int test_2 (int flag_a, int flag_b, struct st *p) +{ + if (flag_a) + { + int j = p->i; /* { dg-message "pointer 'p' is dereferenced here" } */ + if (flag_b && p) /* { dg-warning "check of 'p' for NULL after already dereferencing it" } */ + return 1; + return j; + } + return 2; +} + +int test_3 (struct st *a, struct st *b) +{ + if (!a) + return b->i; + if (!b) + return a->i; + return 0; +} + +int test_4 (struct st *p) +{ + int *q = &p->i; + if (!p) + return -1; + return *q; +} + +void test_check_after_strlen (const char *str) +{ + size_t len_a = __builtin_strlen (str); /* { dg-message "pointer 'str' is dereferenced here" } */ + size_t len_b = str ? __builtin_strlen (str) : 0; /* { dg-warning "check of 'str' for NULL after already dereferencing it" } */ +} + +void test_6 (struct st *a, struct st *b) +{ + int diff = a->i - b->i; /* { dg-message "pointer 'b' is dereferenced here" } */ + + /* ... */ + + if (b) /* { dg-warning "check of 'b' for NULL after already dereferencing it" } */ + fprintf (stderr, "str: %s\n", b->str); +} + +void test_check_after_strcmp (const char *s1, const char *s2) +{ + if (!__builtin_strcmp (s1, s2)) /* { dg-message "pointer 's1' is dereferenced here" } */ + return; + + /* ... */ + + if (s1) /* { dg-warning "check of 's1' for NULL after already dereferencing it" } */ + return; +} + +void test_more_than_one_deref (struct st *p) +{ + char *str = p->str; /* { dg-message "pointer 'p' is dereferenced here" } */ + int i = p->i; + + /* ... */ + + if (p) /* { dg-warning "check of 'p' for NULL after already dereferencing it" } */ + return; + + /* ... */ +} + +void test_deref_under_another_name (struct st *p) +{ + struct st *q = p; + int i = q->i; /* { dg-message "pointer 'p' is dereferenced here" } */ + + /* ... */ + + if (p) /* { dg-warning "check of 'p' for NULL after already dereferencing it" } */ + return; + + /* ... */ +} + +void test_check_after_memcpy_src (struct st *dst, struct st *src) +{ + __builtin_memcpy (dst, src, sizeof (struct st)); /* { dg-message "pointer 'src' is dereferenced here" } */ + + /* ... */ + + if (!src) /* { dg-warning "check of 'src' for NULL after already dereferencing it" } */ + return; + + /* ... */ +} + +void test_check_after_memcpy_dst (struct st *dst, struct st *src) +{ + __builtin_memcpy (dst, src, sizeof (struct st)); /* { dg-message "pointer 'dst' is dereferenced here" } */ + + /* ... */ + + if (!dst) /* { dg-warning "check of 'dst' for NULL after already dereferencing it" } */ + return; + + /* ... */ +} + +void test_merger (int *p, int flag) +{ + int x = *p; + if (flag) + __builtin_free (p); + if (!flag) + __builtin_free (p); /* { dg-bogus "double-'free'" } */ +} diff --git a/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr77425.c b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr77425.c new file mode 100644 index 00000000000..1ceea97b422 --- /dev/null +++ b/gcc/testsuite/gcc.dg/analyzer/deref-before-check-pr77425.c @@ -0,0 +1,43 @@ +/* Fixed in r7-2945-g61f46d0e6dd568. + Simplified from gcc/ipa-devirt.c. */ + +#define NULL ((void *)0) +typedef struct odr_type_d { + /* .... */ + int id; + /* .... */ +} *odr_type; +static odr_type **odr_types_ptr; +#define odr_types (*odr_types_ptr) /* { dg-message "pointer 'odr_types_ptr' is dereferenced here" } */ + +int cond, other_cond; + +odr_type some_logic (); + +odr_type +get_odr_type (/* ... */) +{ + /* .... */ + odr_type val = NULL; + /* .... */ + + val = some_logic (); + + /* .... */ + if (cond) + { + /* .... */ + } + else if (other_cond) + { + odr_types[val->id] = 0; /* { dg-message "in expansion of macro 'odr_types'" } */ + /* .... */ + if (odr_types_ptr) /* { dg-warning "check of 'odr_types_ptr' for NULL after already dereferencing it" } */ + { + /* .... */ + val->id = 42; + } + /* .... */ + } + return val; +} diff --git a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c index d69a60530eb..6b5590a433a 100644 --- a/gcc/testsuite/gcc.dg/analyzer/malloc-1.c +++ b/gcc/testsuite/gcc.dg/analyzer/malloc-1.c @@ -625,5 +625,14 @@ void test_50c (void) free (&&my_label); /* { dg-warning "'free' of '&my_label' which points to memory not on the heap \\\[CWE-590\\\]" } */ } +/* Double free after unconditional dereference. */ + +int test_51 (int *p) +{ + int result = *p; + free (p); /* { dg-message "first 'free' here" } */ + free (p); /* { dg-warning "double-'free' of 'p'" } */ + return result; +} /* { dg-prune-output "\\\[-Wfree-nonheap-object" } */ diff --git a/gcc/tristate.h b/gcc/tristate.h index 8476b9ab370..bccfa8bf523 100644 --- a/gcc/tristate.h +++ b/gcc/tristate.h @@ -38,6 +38,7 @@ class tristate { const char *as_string () const; bool is_known () const { return m_value != TS_UNKNOWN; } + bool is_unknown () const { return m_value == TS_UNKNOWN; } bool is_true () const { return m_value == TS_TRUE; } bool is_false () const { return m_value == TS_FALSE; } -- 2.26.3