analyzer: new warning: -Wanalyzer-deref-before-check [PR99671]

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)
    |

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 <dmalcolm@redhat.com>
This commit is contained in:
David Malcolm 2022-11-10 13:23:56 -05:00
parent 7e3ce73849
commit 5c6546ca7d
17 changed files with 860 additions and 16 deletions

View file

@ -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.

View file

@ -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;

View file

@ -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. */

View file

@ -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;

View file

@ -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<const svalue *> 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;

View file

@ -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;

View file

@ -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;

View file

@ -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);

View file

@ -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<pending_diagnostic> 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<const frame_region *, state_t> 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 <malloc_state_machine *> (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 <const gcond *> (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<deref_before_check> (*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<const svalue *> 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

View file

@ -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')"];

View file

@ -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;

View file

@ -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|

View file

@ -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;
}

View file

@ -0,0 +1,130 @@
#include <stdio.h>
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'" } */
}

View file

@ -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;
}

View file

@ -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" } */

View file

@ -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; }