* [PATCH 4/7] [analyzer] Support global states and custom transitions
2019-12-04 16:25 [PATCH 0/7] [analyzer] Add checking for unsafe calls within signal handlers David Malcolm
` (2 preceding siblings ...)
2019-12-04 16:25 ` [PATCH 7/7] [analyzer] Add -Wanalyzer-unsafe-call-within-signal-handler David Malcolm
@ 2019-12-04 16:25 ` David Malcolm
2019-12-04 16:25 ` [PATCH 2/7] [analyzer] More test coverage David Malcolm
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: David Malcolm @ 2019-12-04 16:25 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch generalizes the state machine code to support "global" states
as well as per-expression states. It also adds a custom_transition hook
for state machines that need to make non-standard transitions (e.g. calling
into a signal handler).
gcc/ChangeLog:
* analyzer/checker-path.cc (state_change_event::get_desc): Support
global state.
* analyzer/diagnostic-manager.cc (get_any_origin): Assert that
dst_rep is non-NULL.
(state_change_event_creator::on_global_state_change): New function.
(for_each_state_change): Handle global state changes.
(diagnostic_manager::prune_path): Handle global state changes, where
var is NULL.
* analyzer/engine.cc (impl_sm_context::warn_for_state): Support
global states.
(impl_sm_context::get_global_state): New vfunc implementation.
(impl_sm_context::set_global_state): New vfunc implementation.
(impl_sm_context::on_custom_transition): New vfunc implementation.
(exploded_node::get_dot_fillcolor): Support global states.
* analyzer/pending-diagnostic.h (state_change::is_global_p): New function.
* analyzer/program-state.cc (sm_state_map::sm_state_map): New ctor.
(sm_state_map::print): Support global states.
(sm_state_map::is_empty_p): Likewise.
(sm_state_map::hash): Likewise.
(sm_state_map::operator==): Likewise.
(sm_state_map::set_global_state): New.
(sm_state_map::get_global_state): New.
* analyzer/program-state.h (sm_state_map::sm_state_map): New ctor decl.
(sm_state_map::set_global_state): New decl.
(sm_state_map::get_global_state): New decl.
(sm_state_map::m_global_state): New field.
(state_change_visitor::on_global_state_change): New vfunc.
* analyzer/sm.h (class custom_transition): New abstract base class.
(sm_context::get_global_state): New vfunc.
(sm_context::set_global_state): New vfunc.
(sm_context::on_custom_transition): New vfunc.
---
gcc/analyzer/checker-path.cc | 40 +++++++----
gcc/analyzer/diagnostic-manager.cc | 112 ++++++++++++++++++++---------
gcc/analyzer/engine.cc | 39 ++++++++--
gcc/analyzer/pending-diagnostic.h | 2 +
gcc/analyzer/program-state.cc | 38 +++++++++-
gcc/analyzer/program-state.h | 11 +++
gcc/analyzer/sm.h | 23 ++++++
7 files changed, 209 insertions(+), 56 deletions(-)
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index d277c2806308..c0783df9e8e9 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -246,21 +246,33 @@ state_change_event::get_desc (bool can_colorize) const
}
/* Fallback description. */
- if (m_origin)
- return make_label_text
- (can_colorize,
- "state of %qE: %qs -> %qs (origin: %qE)",
- m_var,
- m_sm.get_state_name (m_from),
- m_sm.get_state_name (m_to),
- m_origin);
+ if (m_var)
+ {
+ if (m_origin)
+ return make_label_text
+ (can_colorize,
+ "state of %qE: %qs -> %qs (origin: %qE)",
+ m_var,
+ m_sm.get_state_name (m_from),
+ m_sm.get_state_name (m_to),
+ m_origin);
+ else
+ return make_label_text
+ (can_colorize,
+ "state of %qE: %qs -> %qs (origin: NULL)",
+ m_var,
+ m_sm.get_state_name (m_from),
+ m_sm.get_state_name (m_to));
+ }
else
- return make_label_text
- (can_colorize,
- "state of %qE: %qs -> %qs (origin: NULL)",
- m_var,
- m_sm.get_state_name (m_from),
- m_sm.get_state_name (m_to));
+ {
+ gcc_assert (m_origin == NULL_TREE);
+ return make_label_text
+ (can_colorize,
+ "global state: %qs -> %qs",
+ m_sm.get_state_name (m_from),
+ m_sm.get_state_name (m_to));
+ }
}
////////////////////////////////////////////////////////////////////////////
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 461cc3318b7f..694993cab10b 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -471,6 +471,8 @@ get_any_origin (const gimple *stmt,
if (!stmt)
return NULL_TREE;
+ gcc_assert (dst_rep);
+
if (const gassign *assign = dyn_cast <const gassign *> (stmt))
{
tree lhs = gimple_assign_lhs (assign);
@@ -523,6 +525,33 @@ public:
m_emission_path (emission_path)
{}
+ bool on_global_state_change (const state_machine &sm,
+ state_machine::state_t src_sm_val,
+ state_machine::state_t dst_sm_val)
+ FINAL OVERRIDE
+ {
+ const exploded_node *src_node = m_eedge.m_src;
+ const program_point &src_point = src_node->get_point ();
+ const int src_stack_depth = src_point.get_stack_depth ();
+ const exploded_node *dst_node = m_eedge.m_dest;
+ const gimple *stmt = src_point.get_stmt ();
+ const supernode *supernode = src_point.get_supernode ();
+ const program_state &dst_state = dst_node->get_state ();
+
+ int stack_depth = src_stack_depth;
+
+ m_emission_path->add_event (new state_change_event (supernode,
+ stmt,
+ stack_depth,
+ sm,
+ NULL_TREE,
+ src_sm_val,
+ dst_sm_val,
+ NULL_TREE,
+ dst_state));
+ return false;
+ }
+
bool on_state_change (const state_machine &sm,
state_machine::state_t src_sm_val,
state_machine::state_t dst_sm_val,
@@ -575,18 +604,19 @@ public:
/* Compare SRC_STATE and DST_STATE (which use EXT_STATE), and call
VISITOR's on_state_change for every sm-state change that occurs
- to a tree.
+ to a tree, and on_global_state_change for every global state change
+ that occurs.
This determines the state changes that ought to be reported to
the user: a combination of the effects of changes to sm_state_map
(which maps svalues to sm-states), and of region_model changes
(which map trees to svalues).
- Bail out early and return true if any call to on_state_change returns
- true, otherwise return false.
+ Bail out early and return true if any call to on_global_state_change
+ or on_state_change returns true, otherwise return false.
This is split out to make it easier to experiment with changes to
- exploded_node granulairy (so that we can observe what state changes
+ exploded_node granularity (so that we can observe what state changes
lead to state_change_events being emitted). */
bool
@@ -604,6 +634,15 @@ for_each_state_change (const program_state &src_state,
const state_machine &sm = ext_state.get_sm (i);
const sm_state_map &src_smap = *src_state.m_checker_states[i];
const sm_state_map &dst_smap = *dst_state.m_checker_states[i];
+
+ /* Add events for any global state changes. */
+ if (src_smap.get_global_state () != dst_smap.get_global_state ())
+ if (visitor->on_global_state_change (sm,
+ src_smap.get_global_state (),
+ dst_smap.get_global_state ()))
+ return true;
+
+ /* Add events for per-svalue state changes. */
for (sm_state_map::iterator_t iter = dst_smap.begin ();
iter != dst_smap.end ();
++iter)
@@ -856,8 +895,14 @@ diagnostic_manager::prune_path (checker_path *path,
if (get_logger ())
{
if (sm)
- log ("considering event %i, with var: %qE, state: %qs",
- idx, var, sm->get_state_name (state));
+ {
+ if (var)
+ log ("considering event %i, with var: %qE, state: %qs",
+ idx, var, sm->get_state_name (state));
+ else
+ log ("considering event %i, with global state: %qs",
+ idx, sm->get_state_name (state));
+ }
else
log ("considering event %i", idx);
}
@@ -877,14 +922,17 @@ diagnostic_manager::prune_path (checker_path *path,
case EK_STMT:
{
/* If this stmt is the origin of "var", update var. */
- statement_event *stmt_event = (statement_event *)base_event;
- tree new_var = get_any_origin (stmt_event->m_stmt, var,
- stmt_event->m_dst_state);
- if (new_var)
+ if (var)
{
- log ("event %i: switching var of interest from %qE to %qE",
- idx, var, new_var);
- var = new_var;
+ statement_event *stmt_event = (statement_event *)base_event;
+ tree new_var = get_any_origin (stmt_event->m_stmt, var,
+ stmt_event->m_dst_state);
+ if (new_var)
+ {
+ log ("event %i: switching var of interest from %qE to %qE",
+ idx, var, new_var);
+ var = new_var;
+ }
}
if (m_verbosity < 3)
{
@@ -1007,28 +1055,24 @@ diagnostic_manager::prune_path (checker_path *path,
// TODO: potentially update var/state based on return value,
// args etc
{
- return_event *event = (return_event *)base_event;
- const callgraph_superedge& cg_superedge
- = event->get_callgraph_superedge ();
- callsite_expr expr;
- tree callee_var
- = cg_superedge.map_expr_from_caller_to_callee (var, &expr);
- if (callee_var)
+ if (var)
{
- if (var)
- log ("event %i:"
- " switching var of interest"
- " from %qE in caller to %qE in callee",
- idx, var, callee_var);
- else
- // TODO: how does this happen?
- log ("event %i:"
- " switching var of interest"
- " from NULL in caller to %qE in callee",
- idx, callee_var);
- var = callee_var;
- if (expr.return_value_p ())
- event->record_critical_state (var, state);
+ return_event *event = (return_event *)base_event;
+ const callgraph_superedge& cg_superedge
+ = event->get_callgraph_superedge ();
+ callsite_expr expr;
+ tree callee_var
+ = cg_superedge.map_expr_from_caller_to_callee (var, &expr);
+ if (callee_var)
+ {
+ log ("event %i:"
+ " switching var of interest"
+ " from %qE in caller to %qE in callee",
+ idx, var, callee_var);
+ var = callee_var;
+ if (expr.return_value_p ())
+ event->record_critical_state (var, state);
+ }
}
}
break;
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index d327340c8c28..f739af779ef3 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -234,9 +234,16 @@ public:
impl_region_model_context old_ctxt
(m_eg, m_enode_for_diag, m_old_state, m_new_state, m_change, NULL);
- svalue_id var_old_sid
- = m_old_state->m_region_model->get_rvalue (var, &old_ctxt);
- state_machine::state_t current = m_old_smap->get_state (var_old_sid);
+ state_machine::state_t current;
+ if (var)
+ {
+ svalue_id var_old_sid
+ = m_old_state->m_region_model->get_rvalue (var, &old_ctxt);
+ current = m_old_smap->get_state (var_old_sid);
+ }
+ else
+ current = m_old_smap->get_global_state ();
+
if (state == current)
{
m_eg.get_diagnostic_manager ().add_diagnostic
@@ -275,6 +282,23 @@ public:
return pvs[0].m_tree;
}
+ state_machine::state_t get_global_state () const FINAL OVERRIDE
+ {
+ return m_old_state->m_checker_states[m_sm_idx]->get_global_state ();
+ }
+
+ void set_global_state (state_machine::state_t state) FINAL OVERRIDE
+ {
+ m_new_state->m_checker_states[m_sm_idx]->set_global_state (state);
+ }
+
+ void on_custom_transition (custom_transition *transition) FINAL OVERRIDE
+ {
+ transition->impl_transition (&m_eg,
+ const_cast<exploded_node *> (m_enode_for_diag),
+ m_sm_idx);
+ }
+
exploded_graph &m_eg;
const exploded_node *m_enode_for_diag;
const program_state *m_old_state;
@@ -718,10 +742,13 @@ exploded_node::get_dot_fillcolor () const
int i;
sm_state_map *smap;
FOR_EACH_VEC_ELT (state.m_checker_states, i, smap)
- for (sm_state_map::iterator_t iter = smap->begin ();
+ {
+ for (sm_state_map::iterator_t iter = smap->begin ();
iter != smap->end ();
- ++iter)
- total_sm_state += (*iter).second.m_state;
+ ++iter)
+ total_sm_state += (*iter).second.m_state;
+ total_sm_state += smap->get_global_state ();
+ }
if (total_sm_state > 0)
{
diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h
index f1ab484c0570..4103a91e2f56 100644
--- a/gcc/analyzer/pending-diagnostic.h
+++ b/gcc/analyzer/pending-diagnostic.h
@@ -57,6 +57,8 @@ struct state_change : public event_desc
m_event_id (event_id)
{}
+ bool is_global_p () const { return m_expr == NULL_TREE; }
+
tree m_expr;
tree m_origin;
state_machine::state_t m_old_state;
diff --git a/gcc/analyzer/program-state.cc b/gcc/analyzer/program-state.cc
index ae15795b58be..6313174e99ad 100644
--- a/gcc/analyzer/program-state.cc
+++ b/gcc/analyzer/program-state.cc
@@ -35,6 +35,13 @@ along with GCC; see the file COPYING3. If not see
/* class sm_state_map. */
+/* sm_state_map's ctor. */
+
+sm_state_map::sm_state_map ()
+: m_map (), m_global_state (0)
+{
+}
+
/* Clone the sm_state_map. */
sm_state_map *
@@ -80,13 +87,20 @@ sm_state_map::clone_with_remapping (const one_way_svalue_id_map &id_map) const
void
sm_state_map::print (const state_machine &sm, pretty_printer *pp) const
{
+ bool first = true;
pp_string (pp, "{");
+ if (m_global_state != 0)
+ {
+ pp_printf (pp, "global: %s", sm.get_state_name (m_global_state));
+ first = false;
+ }
for (typename map_t::iterator iter = m_map.begin ();
iter != m_map.end ();
++iter)
{
- if (iter != m_map.begin ())
+ if (!first)
pp_string (pp, ", ");
+ first = false;
svalue_id sid = (*iter).first;
sid.print (pp);
@@ -118,7 +132,7 @@ sm_state_map::dump (const state_machine &sm) const
bool
sm_state_map::is_empty_p () const
{
- return m_map.elements () == 0;
+ return m_map.elements () == 0 && m_global_state == 0;
}
/* Generate a hash value for this sm_state_map. */
@@ -142,6 +156,7 @@ sm_state_map::hash () const
inchash::add (e.m_origin, hstate);
result ^= hstate.end ();
}
+ result ^= m_global_state;
return result;
}
@@ -151,6 +166,9 @@ sm_state_map::hash () const
bool
sm_state_map::operator== (const sm_state_map &other) const
{
+ if (m_global_state != other.m_global_state)
+ return false;
+
if (m_map.elements () != other.m_map.elements ())
return false;
@@ -261,6 +279,22 @@ sm_state_map::impl_set_state (svalue_id sid, state_machine::state_t state,
m_map.put (sid, entry_t (state, origin));
}
+/* Set the "global" state within this state map to STATE. */
+
+void
+sm_state_map::set_global_state (state_machine::state_t state)
+{
+ m_global_state = state;
+}
+
+/* Get the "global" state within this state map. */
+
+state_machine::state_t
+sm_state_map::get_global_state () const
+{
+ return m_global_state;
+}
+
/* Handle CALL to unknown FNDECL with an unknown function body, which
could do anything to the states passed to it.
Clear any state for SM for the params and any LHS.
diff --git a/gcc/analyzer/program-state.h b/gcc/analyzer/program-state.h
index 9c87c1505c71..6e57257df74d 100644
--- a/gcc/analyzer/program-state.h
+++ b/gcc/analyzer/program-state.h
@@ -136,6 +136,8 @@ public:
typedef hash_map <svalue_id, entry_t> map_t;
typedef typename map_t::iterator iterator_t;
+ sm_state_map ();
+
sm_state_map *clone () const;
sm_state_map *
@@ -168,6 +170,9 @@ public:
state_machine::state_t state,
svalue_id origin);
+ void set_global_state (state_machine::state_t state);
+ state_machine::state_t get_global_state () const;
+
void purge_for_unknown_fncall (const exploded_graph &eg,
const state_machine &sm,
const gcall *call, tree fndecl,
@@ -194,6 +199,7 @@ public:
private:
map_t m_map;
+ state_machine::state_t m_global_state;
};
/* A class for representing the state of interest at a given path of
@@ -284,6 +290,11 @@ class state_change_visitor
public:
virtual ~state_change_visitor () {}
+ /* Return true for early exit, false to keep iterating. */
+ virtual bool on_global_state_change (const state_machine &sm,
+ state_machine::state_t src_sm_val,
+ state_machine::state_t dst_sm_val) = 0;
+
/* Return true for early exit, false to keep iterating. */
virtual bool on_state_change (const state_machine &sm,
state_machine::state_t src_sm_val,
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index fe67c6b4b979..5939a69d1ac9 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -104,6 +104,22 @@ start_start_p (state_machine::state_t state)
////////////////////////////////////////////////////////////////////////////
+/* Abstract base class for state machines to pass to
+ sm_context::on_custom_transition for handling non-standard transitions
+ (e.g. adding a node and edge to simulate registering a callback and having
+ the callback be called later). */
+
+class custom_transition
+{
+public:
+ virtual ~custom_transition () {}
+ virtual void impl_transition (exploded_graph *eg,
+ exploded_node *src_enode,
+ int sm_idx) = 0;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
/* Abstract base class giving an interface for the state machine to call
the checker engine, at a particular stmt. */
@@ -140,6 +156,13 @@ public:
return expr;
}
+ virtual state_machine::state_t get_global_state () const = 0;
+ virtual void set_global_state (state_machine::state_t) = 0;
+
+ /* A vfunc for handling custom transitions, such as when registering
+ a signal handler. */
+ virtual void on_custom_transition (custom_transition *transition) = 0;
+
protected:
sm_context (int sm_idx, const state_machine &sm)
: m_sm_idx (sm_idx), m_sm (sm) {}
--
2.21.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 6/7] [analyzer] Expose state_change_event in evdesc::state_change
2019-12-04 16:25 [PATCH 0/7] [analyzer] Add checking for unsafe calls within signal handlers David Malcolm
@ 2019-12-04 16:25 ` David Malcolm
2019-12-04 16:25 ` [PATCH 3/7] [analyzer] Generalize rewind_info_t to exploded_edge::custom_info_t David Malcolm
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: David Malcolm @ 2019-12-04 16:25 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
gcc/ChangeLog:
* analyzer/analyzer.h (class state_change_event): Forward decl.
* analyzer/checker-path.cc (state_change_event::get_desc): Pass *this
to evdesc::state_change ctor.
* analyzer/pending-diagnostic.h
(evdesc::state_change::state_change): Add a
const state_change_event & param and use it initialize...
(evdesc::state_change::m_event): ...this new field.
---
gcc/analyzer/analyzer.h | 1 +
gcc/analyzer/checker-path.cc | 2 +-
gcc/analyzer/pending-diagnostic.h | 6 ++++--
3 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
index 19bc2c6d8252..353285c35c36 100644
--- a/gcc/analyzer/analyzer.h
+++ b/gcc/analyzer/analyzer.h
@@ -50,6 +50,7 @@ struct model_merger;
struct svalue_id_merger_mapping;
struct canonicalization;
class pending_diagnostic;
+class state_change_event;
class checker_path;
class extrinsic_state;
class sm_state_map;
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index bc47380bc3f5..554a20f31bec 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -229,7 +229,7 @@ state_change_event::get_desc (bool can_colorize) const
label_text custom_desc
= m_pending_diagnostic->describe_state_change
(evdesc::state_change (can_colorize, m_var, m_origin,
- m_from, m_to, m_emission_id));
+ m_from, m_to, m_emission_id, *this));
if (custom_desc.m_buffer)
{
if (flag_analyzer_verbose_state_changes)
diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h
index 4103a91e2f56..15a1379e8fd1 100644
--- a/gcc/analyzer/pending-diagnostic.h
+++ b/gcc/analyzer/pending-diagnostic.h
@@ -50,11 +50,12 @@ struct state_change : public event_desc
tree origin,
state_machine::state_t old_state,
state_machine::state_t new_state,
- diagnostic_event_id_t event_id)
+ diagnostic_event_id_t event_id,
+ const state_change_event &event)
: event_desc (colorize),
m_expr (expr), m_origin (origin),
m_old_state (old_state), m_new_state (new_state),
- m_event_id (event_id)
+ m_event_id (event_id), m_event (event)
{}
bool is_global_p () const { return m_expr == NULL_TREE; }
@@ -64,6 +65,7 @@ struct state_change : public event_desc
state_machine::state_t m_old_state;
state_machine::state_t m_new_state;
diagnostic_event_id_t m_event_id;
+ const state_change_event &m_event;
};
/* For use by pending_diagnostic::describe_call_with_state. */
--
2.21.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 5/7] [analyzer] Support custom events
2019-12-04 16:25 [PATCH 0/7] [analyzer] Add checking for unsafe calls within signal handlers David Malcolm
` (4 preceding siblings ...)
2019-12-04 16:25 ` [PATCH 2/7] [analyzer] More test coverage David Malcolm
@ 2019-12-04 16:25 ` David Malcolm
2019-12-04 16:25 ` [PATCH 1/7] [analyzer] Support paths for callbacks David Malcolm
6 siblings, 0 replies; 8+ messages in thread
From: David Malcolm @ 2019-12-04 16:25 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch adds a custom_event subclass for diagnostics that need
non-standard events (e.g. signal handlers).
gcc/ChangeLog:
* analyzer/checker-path.cc (event_kind_to_string): Handle EK_CUSTOM.
(custom_event::get_desc): New vfunc implementation.
* analyzer/checker-path (class custom_event): New.
* analyzer/diagnostic-manager.cc (diagnostic_manager::prune_path):
Handle EK_CUSTOM.
---
gcc/analyzer/checker-path.cc | 16 ++++++++++++++++
gcc/analyzer/checker-path.h | 30 ++++++++++++++++++++++++++++++
gcc/analyzer/diagnostic-manager.cc | 4 ++++
3 files changed, 50 insertions(+)
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index c0783df9e8e9..bc47380bc3f5 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -44,6 +44,8 @@ event_kind_to_string (enum event_kind ek)
gcc_unreachable ();
case EK_DEBUG:
return "EK_DEBUG";
+ case EK_CUSTOM:
+ return "EK_CUSTOM";
case EK_STMT:
return "EK_STMT";
case EK_FUNCTION_ENTRY:
@@ -129,6 +131,20 @@ debug_event::get_desc (bool) const
////////////////////////////////////////////////////////////////////////////
+/* class custom_event : public checker_event. */
+
+/* Implementation of diagnostic_event::get_desc vfunc for
+ custom_event.
+ Use the saved string as the event's description. */
+
+label_text
+custom_event::get_desc (bool) const
+{
+ return label_text::borrow (m_desc);
+}
+
+////////////////////////////////////////////////////////////////////////////
+
/* class statement_event : public checker_event. */
/* statement_event's ctor. */
diff --git a/gcc/analyzer/checker-path.h b/gcc/analyzer/checker-path.h
index ccff8f2ea0bc..f5f27e7e3bba 100644
--- a/gcc/analyzer/checker-path.h
+++ b/gcc/analyzer/checker-path.h
@@ -32,6 +32,7 @@ along with GCC; see the file COPYING3. If not see
enum event_kind
{
EK_DEBUG,
+ EK_CUSTOM,
EK_STMT,
EK_FUNCTION_ENTRY,
EK_STATE_CHANGE,
@@ -57,6 +58,7 @@ extern const char *event_kind_to_string (enum event_kind ek);
diagnostic_event
checker_event
debug_event (EK_DEBUG)
+ custom_event (EK_CUSTOM)
statement_event (EK_STMT)
function_entry_event (EK_FUNCTION_ENTRY)
state_change_event (EK_STATE_CHANGE)
@@ -142,6 +144,34 @@ private:
char *m_desc;
};
+/* A concrete event subclass for custom events. These are not filtered,
+ as they are likely to be pertinent to the diagnostic. */
+
+class custom_event : public checker_event
+{
+public:
+ custom_event (location_t loc, tree fndecl, int depth,
+ const char *desc)
+ : checker_event (EK_CUSTOM, loc, fndecl, depth),
+ m_desc (xstrdup (desc))
+ {
+ }
+ ~custom_event ()
+ {
+ free (m_desc);
+ }
+
+ label_text get_desc (bool) const FINAL OVERRIDE;
+
+ checker_event *clone () const FINAL OVERRIDE
+ {
+ return new custom_event (m_loc, m_fndecl, m_depth, m_desc);
+ }
+
+private:
+ char *m_desc;
+};
+
/* A concrete event subclass describing the execution of a gimple statement,
for use at high verbosity levels when debugging paths. */
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 694993cab10b..cbbba4e8de40 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -919,6 +919,10 @@ diagnostic_manager::prune_path (checker_path *path,
}
break;
+ case EK_CUSTOM:
+ /* Don't filter custom events. */
+ break;
+
case EK_STMT:
{
/* If this stmt is the origin of "var", update var. */
--
2.21.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 3/7] [analyzer] Generalize rewind_info_t to exploded_edge::custom_info_t
2019-12-04 16:25 [PATCH 0/7] [analyzer] Add checking for unsafe calls within signal handlers David Malcolm
2019-12-04 16:25 ` [PATCH 6/7] [analyzer] Expose state_change_event in evdesc::state_change David Malcolm
@ 2019-12-04 16:25 ` David Malcolm
2019-12-04 16:25 ` [PATCH 7/7] [analyzer] Add -Wanalyzer-unsafe-call-within-signal-handler David Malcolm
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: David Malcolm @ 2019-12-04 16:25 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch generalizes the optional rewind_info_t associated with an
exploded_edge (for handling longjmp) into a subclass of a new
exploded_edge::custom_info_t abstract base class, so that other kinds
of custom edges can be supported (e.g. to support signal handlers being
called, or possibly to support C++ exceptions, etc)
gcc/ChangeLog:
* analyzer/analyzer.h (class rewind_info_t): New forward decl.
* analyzer/checker-path.cc (rewind_event::rewind_event): Update
assertion.
(rewind_to_setjmp_event::prepare_for_emission): Update call to
get_enode_origin.
* analyzer/checker-path.h
(rewind_to_setjmp_event::rewind_to_setjmp_event): Add rewind_info
param and use it to initializer m_rewind_event.
(rewind_to_setjmp_event::clone): Update for new field.
(rewind_to_setjmp_event::m_rewind_info): New field.
* analyzer/diagnostic-manager.cc
(diagnostic_manager::add_events_for_eedge): Drop src_stack_depth
local. Move injection of rewind_from/to_setjmp_event from here
to rewind_info_t::add_events_to_path, calling it via a vfunc.
* analyzer/engine.cc: Include "analyzer/checker-path.h".
(rewind_info_t::update_model): New vfunc.
(rewind_info_t::add_events_to_path): New vfunc.
(exploded_edge::exploded_edge): Generalize final param from
rewind_info_t * to custom_info_t *.
(exploded_edge::~exploded_edge): Update for renaming of
m_rewind_info to m_custom_info.
(exploded_edge::dump_dot): Likewise, replacing hardcode print
of "rewind" with a call to the custom_info_t::print vfunc.
(exploded_graph::add_edge): Update final param from
rewind_info_t * to exploded_edge::custom_info_t *.
(exploded_path::feasible_p): When logging a rejection due to the
region model, dump the model. Move update due to rewind info
to rewind_info_t::update_model and call it via a vfunc.
* analyzer/exploded-graph.h (class exploded_edge::custom_info_t):
New class.
(exploded_edge::m_rewind_info): Rename to...
(exploded_edge::m_custom_info): ...this, converting from a
rewind_info_t * to a custom_info_t *.
(class rewind_info_t): Make a subclass of
exploded_edge::custom_info_t.
(rewind_info_t::print): New vfunc.
(rewind_info_t::update_model): New vfunc.
(rewind_info_t::add_events_to_path): New vfunc.
(exploded_graph::add_edge): Convert final param from
rewind_info_t * to exploded_edge::custom_info_t *.
---
gcc/analyzer/analyzer.h | 1 +
gcc/analyzer/checker-path.cc | 4 +-
gcc/analyzer/checker-path.h | 10 ++-
gcc/analyzer/diagnostic-manager.cc | 19 ++----
gcc/analyzer/engine.cc | 104 +++++++++++++++++++++--------
gcc/analyzer/exploded-graph.h | 85 +++++++++++++++--------
6 files changed, 148 insertions(+), 75 deletions(-)
diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
index 90da44b1a00a..19bc2c6d8252 100644
--- a/gcc/analyzer/analyzer.h
+++ b/gcc/analyzer/analyzer.h
@@ -65,6 +65,7 @@ class analysis_plan;
class state_purge_map;
class state_purge_per_ssa_name;
class state_change;
+class rewind_info_t;
////////////////////////////////////////////////////////////////////////////
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index 0a1840682cd5..d277c2806308 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -705,7 +705,7 @@ rewind_event::rewind_event (const exploded_edge *eedge,
: checker_event (kind, loc, fndecl, depth),
m_eedge (eedge)
{
- gcc_assert (m_eedge->m_rewind_info);
+ gcc_assert (m_eedge->m_custom_info); // a rewind_info_t
}
////////////////////////////////////////////////////////////////////////////
@@ -789,7 +789,7 @@ rewind_to_setjmp_event::prepare_for_emission (checker_path *path,
diagnostic_event_id_t emission_id)
{
checker_event::prepare_for_emission (path, pd, emission_id);
- path->get_setjmp_event (get_eedge ()->m_rewind_info->get_enode_origin (),
+ path->get_setjmp_event (m_rewind_info->get_enode_origin (),
&m_original_setjmp_event_id);
}
diff --git a/gcc/analyzer/checker-path.h b/gcc/analyzer/checker-path.h
index 916e5a736e3e..ccff8f2ea0bc 100644
--- a/gcc/analyzer/checker-path.h
+++ b/gcc/analyzer/checker-path.h
@@ -428,8 +428,10 @@ class rewind_to_setjmp_event : public rewind_event
{
public:
rewind_to_setjmp_event (const exploded_edge *eedge,
- location_t loc, tree fndecl, int depth)
- : rewind_event (eedge, EK_REWIND_TO_SETJMP, loc, fndecl, depth)
+ location_t loc, tree fndecl, int depth,
+ const rewind_info_t *rewind_info)
+ : rewind_event (eedge, EK_REWIND_TO_SETJMP, loc, fndecl, depth),
+ m_rewind_info (rewind_info)
{
}
@@ -438,7 +440,8 @@ public:
rewind_to_setjmp_event *clone () const FINAL OVERRIDE
{
return new rewind_to_setjmp_event (get_eedge (),
- m_loc, m_fndecl, m_depth);
+ m_loc, m_fndecl, m_depth,
+ m_rewind_info);
}
void prepare_for_emission (checker_path *path,
@@ -447,6 +450,7 @@ public:
private:
diagnostic_event_id_t m_original_setjmp_event_id;
+ const rewind_info_t *m_rewind_info;
};
/* Concrete subclass of checker_event for use at the end of a path:
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index 8cd4507bc6e5..461cc3318b7f 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -655,7 +655,6 @@ diagnostic_manager::add_events_for_eedge (const exploded_edge &eedge,
{
const exploded_node *src_node = eedge.m_src;
const program_point &src_point = src_node->get_point ();
- const int src_stack_depth = src_point.get_stack_depth ();
const exploded_node *dst_node = eedge.m_dest;
const program_point &dst_point = dst_node->get_point ();
const int dst_stack_depth = dst_point.get_stack_depth ();
@@ -693,20 +692,10 @@ diagnostic_manager::add_events_for_eedge (const exploded_edge &eedge,
for_each_state_change (src_state, dst_state, ext_state,
&visitor);
- /* Add events for rewinding from a longjmp to a setjmp. */
- if (eedge.m_rewind_info)
- {
- emission_path->add_event
- (new rewind_from_longjmp_event
- (&eedge, src_point.get_supernode ()->get_end_location (),
- src_point.get_fndecl (),
- src_stack_depth));
- emission_path->add_event
- (new rewind_to_setjmp_event
- (&eedge, eedge.m_rewind_info->get_setjmp_call ()->location,
- dst_point.get_fndecl (),
- dst_stack_depth));
- }
+ /* Allow non-standard edges to add events, e.g. when rewinding from
+ longjmp to a setjmp. */
+ if (eedge.m_custom_info)
+ eedge.m_custom_info->add_events_to_path (emission_path, eedge);
/* Add events for superedges, function entries, and for statements. */
switch (dst_point.get_kind ())
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index eed2be091c93..d327340c8c28 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -26,6 +26,7 @@ along with GCC; see the file COPYING3. If not see
#include "gcc-rich-location.h"
#include "analyzer/exploded-graph.h"
#include "analyzer/analysis-plan.h"
+#include "analyzer/checker-path.h"
#include "analyzer/state-purge.h"
/* For an overview, see gcc/doc/analyzer.texi. */
@@ -1247,6 +1248,66 @@ exploded_node::dump_succs_and_preds (FILE *outf) const
////////////////////////////////////////////////////////////////////////////
+/* class rewind_info_t : public exploded_edge::custom_info_t. */
+
+/* Implementation of exploded_edge::custom_info_t::update_model vfunc
+ for rewind_info_t.
+
+ Update state for the special-case of a rewind of a longjmp
+ to a setjmp (which doesn't have a superedge, but does affect
+ state). */
+
+void
+rewind_info_t::update_model (region_model *model,
+ const exploded_edge &eedge)
+{
+ const exploded_node &src_enode = *eedge.m_src;
+ const program_point &src_point = src_enode.get_point ();
+
+ const gimple *last_stmt
+ = src_point.get_supernode ()->get_last_stmt ();
+ gcc_assert (last_stmt);
+ const gcall *longjmp_call = as_a <const gcall *> (last_stmt);
+
+ const program_point &longjmp_point = eedge.m_src->get_point ();
+ const program_point &setjmp_point = eedge.m_dest->get_point ();
+
+ gcc_assert (longjmp_point.get_stack_depth ()
+ >= setjmp_point.get_stack_depth ());
+
+ model->on_longjmp (longjmp_call,
+ get_setjmp_call (),
+ setjmp_point.get_stack_depth (), NULL);
+}
+
+/* Implementation of exploded_edge::custom_info_t::add_events_to_path vfunc
+ for rewind_info_t. */
+
+void
+rewind_info_t::add_events_to_path (checker_path *emission_path,
+ const exploded_edge &eedge)
+{
+ const exploded_node *src_node = eedge.m_src;
+ const program_point &src_point = src_node->get_point ();
+ const int src_stack_depth = src_point.get_stack_depth ();
+ const exploded_node *dst_node = eedge.m_dest;
+ const program_point &dst_point = dst_node->get_point ();
+ const int dst_stack_depth = dst_point.get_stack_depth ();
+
+ emission_path->add_event
+ (new rewind_from_longjmp_event
+ (&eedge, src_point.get_supernode ()->get_end_location (),
+ src_point.get_fndecl (),
+ src_stack_depth));
+ emission_path->add_event
+ (new rewind_to_setjmp_event
+ (&eedge, get_setjmp_call ()->location,
+ dst_point.get_fndecl (),
+ dst_stack_depth, this));
+}
+
+////////////////////////////////////////////////////////////////////////////
+
/* class exploded_edge : public dedge. */
/* exploded_edge's ctor. */
@@ -1254,9 +1315,9 @@ exploded_node::dump_succs_and_preds (FILE *outf) const
exploded_edge::exploded_edge (exploded_node *src, exploded_node *dest,
const superedge *sedge,
const state_change &change,
- rewind_info_t *rewind_info)
+ custom_info_t *custom_info)
: dedge (src, dest), m_sedge (sedge), m_change (change),
- m_rewind_info (rewind_info)
+ m_custom_info (custom_info)
{
change.validate (dest->get_state ());
}
@@ -1265,7 +1326,7 @@ exploded_edge::exploded_edge (exploded_node *src, exploded_node *dest,
exploded_edge::~exploded_edge ()
{
- delete m_rewind_info;
+ delete m_custom_info;
}
/* Implementation of dedge::dump_dot vfunc for exploded_edge.
@@ -1300,7 +1361,7 @@ exploded_edge::dump_dot (graphviz_out *gv, const dump_args_t &args) const
style = "\"dotted\"";
break;
}
- if (m_rewind_info)
+ if (m_custom_info)
{
color = "red";
style = "\"dotted\"";
@@ -1316,8 +1377,8 @@ exploded_edge::dump_dot (graphviz_out *gv, const dump_args_t &args) const
if (m_sedge)
m_sedge->dump_label_to_pp (pp, false);
- else if (m_rewind_info)
- pp_string (pp, "rewind");
+ else if (m_custom_info)
+ m_custom_info->print (pp);
m_change.dump (pp, args.m_eg.get_ext_state ());
//pp_write_text_as_dot_label_to_stream (pp, /*for_record=*/false);
@@ -1859,9 +1920,9 @@ exploded_edge *
exploded_graph::add_edge (exploded_node *src, exploded_node *dest,
const superedge *sedge,
const state_change &change,
- rewind_info_t *rewind_info)
+ exploded_edge::custom_info_t *custom_info)
{
- exploded_edge *e = new exploded_edge (src, dest, sedge, change, rewind_info);
+ exploded_edge *e = new exploded_edge (src, dest, sedge, change, custom_info);
digraph::add_edge (e);
return e;
}
@@ -2569,7 +2630,10 @@ exploded_path::feasible_p (logger *logger) const
if (!model.maybe_update_for_edge (*sedge, last_stmt, NULL))
{
if (logger)
- logger->log ("rejecting due to region model");
+ {
+ logger->log ("rejecting due to region model");
+ model.dump_to_pp (logger->get_printer (), false);
+ }
return false;
}
}
@@ -2589,26 +2653,8 @@ exploded_path::feasible_p (logger *logger) const
if (logger)
logger->log (" pushing frame for %qD", fun->decl);
}
- else if (eedge->m_rewind_info)
- {
- /* Update state for the special-case of a rewind of a longjmp
- to a setjmp (which doesn't have a superedge, but does affect
- state). */
- const gimple *last_stmt
- = src_point.get_supernode ()->get_last_stmt ();
- gcc_assert (last_stmt);
- const gcall *longjmp_call = as_a <const gcall *> (last_stmt);
-
- const program_point &longjmp_point = eedge->m_src->get_point ();
- const program_point &setjmp_point = eedge->m_dest->get_point ();
-
- gcc_assert (longjmp_point.get_stack_depth ()
- >= setjmp_point.get_stack_depth ());
-
- model.on_longjmp (longjmp_call,
- eedge->m_rewind_info->get_setjmp_call (),
- setjmp_point.get_stack_depth (), NULL);
- }
+ else if (eedge->m_custom_info)
+ eedge->m_custom_info->update_model (&model, *eedge);
}
/* Handle phi nodes on an edge leaving a PK_BEFORE_SUPERNODE (to
diff --git a/gcc/analyzer/exploded-graph.h b/gcc/analyzer/exploded-graph.h
index c8e36f25a575..26ae109628f4 100644
--- a/gcc/analyzer/exploded-graph.h
+++ b/gcc/analyzer/exploded-graph.h
@@ -252,16 +252,73 @@ public:
const int m_index;
};
+/* An edge within the exploded graph.
+ Some exploded_edges have an underlying superedge; others don't. */
+
+class exploded_edge : public dedge<eg_traits>
+{
+ public:
+ /* Abstract base class for associating custom data with an
+ exploded_edge, for handling non-standard edges such as
+ rewinding from a longjmp, signal handlers, etc. */
+ class custom_info_t
+ {
+ public:
+ virtual ~custom_info_t () {}
+
+ /* Hook for making .dot label more readable . */
+ virtual void print (pretty_printer *pp) = 0;
+
+ /* Hook for updating MODEL within exploded_path::feasible_p. */
+ virtual void update_model (region_model *model,
+ const exploded_edge &eedge) = 0;
+
+ virtual void add_events_to_path (checker_path *emission_path,
+ const exploded_edge &eedge) = 0;
+ };
+
+ exploded_edge (exploded_node *src, exploded_node *dest,
+ const superedge *sedge,
+ const state_change &change,
+ custom_info_t *custom_info);
+ ~exploded_edge ();
+ void dump_dot (graphviz_out *gv, const dump_args_t &args)
+ const FINAL OVERRIDE;
+
+ //private:
+ const superedge *const m_sedge;
+
+ const state_change m_change;
+
+ /* NULL for most edges; will be non-NULL for special cases
+ such as an unwind from a longjmp to a setjmp, or when
+ a signal is delivered to a signal-handler.
+
+ Owned by this class. */
+ custom_info_t *m_custom_info;
+};
+
/* Extra data for an exploded_edge that represents a rewind from a
longjmp to a setjmp. */
-class rewind_info_t
+class rewind_info_t : public exploded_edge::custom_info_t
{
public:
rewind_info_t (const exploded_node *enode_origin)
: m_enode_origin (enode_origin)
{}
+ void print (pretty_printer *pp) FINAL OVERRIDE
+ {
+ pp_string (pp, "rewind");
+ }
+
+ void update_model (region_model *model,
+ const exploded_edge &eedge) FINAL OVERRIDE;
+
+ void add_events_to_path (checker_path *emission_path,
+ const exploded_edge &eedge) FINAL OVERRIDE;
+
const program_point &get_setjmp_point () const
{
const program_point &origin_point = m_enode_origin->get_point ();
@@ -285,30 +342,6 @@ private:
const exploded_node *m_enode_origin;
};
-/* An edge within the exploded graph.
- Some exploded_edges have an underlying superedge; others don't. */
-
-class exploded_edge : public dedge<eg_traits>
-{
- public:
- exploded_edge (exploded_node *src, exploded_node *dest,
- const superedge *sedge,
- const state_change &change,
- rewind_info_t *rewind_info);
- ~exploded_edge ();
- void dump_dot (graphviz_out *gv, const dump_args_t &args)
- const FINAL OVERRIDE;
-
- //private:
- const superedge *const m_sedge;
-
- const state_change m_change;
-
- /* NULL for most edges; will be non-NULL for an unwind from a longjmp
- to a setjmp (owned by this class). */
- rewind_info_t *m_rewind_info;
-};
-
/* Statistics about aspects of an exploded_graph. */
struct stats
@@ -665,7 +698,7 @@ public:
exploded_edge *add_edge (exploded_node *src, exploded_node *dest,
const superedge *sedge,
const state_change &change,
- rewind_info_t *rewind_info = NULL);
+ exploded_edge::custom_info_t *custom = NULL);
per_program_point_data *
get_or_create_per_program_point_data (const program_point &);
--
2.21.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 0/7] [analyzer] Add checking for unsafe calls within signal handlers
@ 2019-12-04 16:25 David Malcolm
2019-12-04 16:25 ` [PATCH 6/7] [analyzer] Expose state_change_event in evdesc::state_change David Malcolm
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: David Malcolm @ 2019-12-04 16:25 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch kit adds a new warning:
-Wanalyzer-unsafe-call-within-signal-handler
to the analyzer branch.
A colorized example of the output can be seen here:
https://dmalcolm.fedorapeople.org/gcc/2019-12-04/signal.c.html
Currently it only detects calls to "fprintf".
This is a new feature beyond what I posted before the stage 1 deadline
- but what I posted already contained a couple of proof-of-concept
checkers.
For now I'm pushing it to the branch, since that seems better than having
it just on my hard drive; the supporting patches also contain
generalizations of the code that I think are likely to be useful for
future work (e.g. supporting C++ exceptions).
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Pushed to branch "dmalcolm/analyzer" on the GCC git mirror.
Dave
David Malcolm (7):
[analyzer] Support paths for callbacks
[analyzer] More test coverage
[analyzer] Generalize rewind_info_t to exploded_edge::custom_info_t
[analyzer] Support global states and custom transitions
[analyzer] Support custom events
[analyzer] Expose state_change_event in evdesc::state_change
[analyzer] Add -Wanalyzer-unsafe-call-within-signal-handler
gcc/analyzer/Make-plugin.in | 1 +
gcc/analyzer/analyzer.h | 2 +
gcc/analyzer/checker-path.cc | 62 +++-
gcc/analyzer/checker-path.h | 40 ++-
gcc/analyzer/diagnostic-manager.cc | 135 +++++---
gcc/analyzer/engine.cc | 143 ++++++--
gcc/analyzer/exploded-graph.h | 85 +++--
gcc/analyzer/pending-diagnostic.h | 8 +-
gcc/analyzer/plugin.opt | 4 +
gcc/analyzer/program-state.cc | 38 ++-
gcc/analyzer/program-state.h | 11 +
gcc/analyzer/sm-signal.cc | 304 ++++++++++++++++++
gcc/analyzer/sm.cc | 1 +
gcc/analyzer/sm.h | 24 ++
gcc/doc/invoke.texi | 13 +
gcc/testsuite/gcc.dg/analyzer/data-model-1.c | 6 +
gcc/testsuite/gcc.dg/analyzer/signal-1.c | 31 ++
gcc/testsuite/gcc.dg/analyzer/signal-2.c | 34 ++
gcc/testsuite/gcc.dg/analyzer/signal-3.c | 23 ++
gcc/testsuite/gcc.dg/analyzer/signal-4a.c | 74 +++++
gcc/testsuite/gcc.dg/analyzer/signal-4b.c | 89 +++++
.../gcc.dg/plugin/diagnostic-test-paths-4.c | 83 +++++
.../plugin/diagnostic_plugin_test_paths.c | 81 +++++
gcc/testsuite/gcc.dg/plugin/plugin.exp | 1 +
gcc/tree-diagnostic-path.cc | 75 +++--
25 files changed, 1201 insertions(+), 167 deletions(-)
create mode 100644 gcc/analyzer/sm-signal.cc
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-1.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-2.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-3.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-4a.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-4b.c
create mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
--
2.21.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 7/7] [analyzer] Add -Wanalyzer-unsafe-call-within-signal-handler
2019-12-04 16:25 [PATCH 0/7] [analyzer] Add checking for unsafe calls within signal handlers David Malcolm
2019-12-04 16:25 ` [PATCH 6/7] [analyzer] Expose state_change_event in evdesc::state_change David Malcolm
2019-12-04 16:25 ` [PATCH 3/7] [analyzer] Generalize rewind_info_t to exploded_edge::custom_info_t David Malcolm
@ 2019-12-04 16:25 ` David Malcolm
2019-12-04 16:25 ` [PATCH 4/7] [analyzer] Support global states and custom transitions David Malcolm
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: David Malcolm @ 2019-12-04 16:25 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch adds another new experimental state machine, this time for
tracking bad calls made from within a signal handler.
Example of output:
../../src/demos/signal.c: In function ‘custom_logger’:
../../src/demos/signal.c:15:5: warning: call to ‘fprintf’ from within signal handler [CWE-479] [-Wanalyzer-unsafe-call-within-signal-handler]
15 | fprintf(stderr, "LOG: %s", msg);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
‘main’: events 1-2
|
| 28 | int main(int argc, const char *argv)
| | ^~~~
| | |
| | (1) entry to ‘main’
| 29 | {
| 30 | register_handler ();
| | ~~~~~~~~~~~~~~~~~~~
| | |
| | (2) calling ‘register_handler’ from ‘main’
|
+--> ‘register_handler’: events 3-4
|
| 23 | static void register_handler ()
| | ^~~~~~~~~~~~~~~~
| | |
| | (3) entry to ‘register_handler’
| 24 | {
| 25 | signal(SIGINT, int_handler);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (4) registering ‘int_handler’ as signal handler
|
event 5
|
|cc1:
| (5): later on, when the signal is delivered to the process
|
+--> ‘int_handler’: events 6-7
|
| 18 | static void int_handler(int signum)
| | ^~~~~~~~~~~
| | |
| | (6) entry to ‘int_handler’
| 19 | {
| 20 | custom_logger("got signal");
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (7) calling ‘custom_logger’ from ‘int_handler’
|
+--> ‘custom_logger’: events 8-11
|
| 12 | void custom_logger(const char *msg)
| | ^~~~~~~~~~~~~
| | |
| | (8) entry to ‘custom_logger’
| 13 | {
| 14 | if (logging)
| | ~
| | |
| | (9) following ‘true’ branch...
| 15 | fprintf(stderr, "LOG: %s", msg);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (10) ...to here
| | (11) call to ‘fprintf’ from within signal handler
gcc/ChangeLog:
* analyzer/Make-plugin.in (analyzer_OBJS): Add
analyzer/sm-signal.o.
* analyzer/plugin.opt
(Wanalyzer-unsafe-call-within-signal-handler): New.
* analyzer/sm-signal.cc: New file.
* analyzer/sm.cc (make_checkers): Add result of
make_signal_state_machine.
* analyzer/sm.h (make_signal_state_machine): New decl.
* doc/invoke.texi (-Wanalyzer-unsafe-call-within-signal-handler):
New analyzer option.
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/signal-1.c: New test.
* gcc.dg/analyzer/signal-2.c: New test.
* gcc.dg/analyzer/signal-3.c: New test.
* gcc.dg/analyzer/signal-4a.c: New test.
* gcc.dg/analyzer/signal-4b.c: New test.
---
gcc/analyzer/Make-plugin.in | 1 +
gcc/analyzer/plugin.opt | 4 +
gcc/analyzer/sm-signal.cc | 304 ++++++++++++++++++++++
gcc/analyzer/sm.cc | 1 +
gcc/analyzer/sm.h | 1 +
gcc/doc/invoke.texi | 13 +
gcc/testsuite/gcc.dg/analyzer/signal-1.c | 31 +++
gcc/testsuite/gcc.dg/analyzer/signal-2.c | 34 +++
gcc/testsuite/gcc.dg/analyzer/signal-3.c | 23 ++
gcc/testsuite/gcc.dg/analyzer/signal-4a.c | 74 ++++++
gcc/testsuite/gcc.dg/analyzer/signal-4b.c | 89 +++++++
11 files changed, 575 insertions(+)
create mode 100644 gcc/analyzer/sm-signal.cc
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-1.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-2.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-3.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-4a.c
create mode 100644 gcc/testsuite/gcc.dg/analyzer/signal-4b.c
diff --git a/gcc/analyzer/Make-plugin.in b/gcc/analyzer/Make-plugin.in
index 08f58f467a2a..f679686783fa 100644
--- a/gcc/analyzer/Make-plugin.in
+++ b/gcc/analyzer/Make-plugin.in
@@ -76,6 +76,7 @@ analyzer_OBJS = \
analyzer/sm-malloc.o \
analyzer/sm-pattern-test.o \
analyzer/sm-sensitive.o \
+ analyzer/sm-signal.o \
analyzer/sm-taint.o \
analyzer/state-purge.o \
analyzer/supergraph.o \
diff --git a/gcc/analyzer/plugin.opt b/gcc/analyzer/plugin.opt
index 8408f1c7db97..d722667dea09 100644
--- a/gcc/analyzer/plugin.opt
+++ b/gcc/analyzer/plugin.opt
@@ -70,6 +70,10 @@ Wanalyzer-possible-null-dereference
Common Var(warn_analyzer_possible_null_dereference) Init(1) Warning
Warn about code paths in which a possibly-NULL pointer is dereferenced.
+Wanalyzer-unsafe-call-within-signal-handler
+Common Var(warn_analyzer_unsafe_call_within_signal_handler) Init(1) Warning
+Warn about code paths in which an async-signal-unsafe function is called from a signal handler.
+
Wanalyzer-null-argument
Common Var(warn_analyzer_null_argument) Init(1) Warning
Warn about code paths in which NULL is passed to a must-not-be-NULL function argument.
diff --git a/gcc/analyzer/sm-signal.cc b/gcc/analyzer/sm-signal.cc
new file mode 100644
index 000000000000..c9130b243b0a
--- /dev/null
+++ b/gcc/analyzer/sm-signal.cc
@@ -0,0 +1,304 @@
+/* An experimental state machine, for tracking bad calls from within
+ signal handlers.
+
+ Copyright (C) 2019 Free Software Foundation, Inc.
+ Contributed by David Malcolm <dmalcolm@redhat.com>.
+
+This file is part of GCC.
+
+GCC is free software; you can redistribute it and/or modify it
+under the terms of the GNU General Public License as published by
+the Free Software Foundation; either version 3, or (at your option)
+any later version.
+
+GCC is distributed in the hope that it will be useful, but
+WITHOUT ANY WARRANTY; without even the implied warranty of
+MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+General Public License for more details.
+
+You should have received a copy of the GNU General Public License
+along with GCC; see the file COPYING3. If not see
+<http://www.gnu.org/licenses/>. */
+
+#include "config.h"
+#include "gcc-plugin.h"
+#include "system.h"
+#include "coretypes.h"
+#include "tree.h"
+#include "gimple.h"
+#include "diagnostic-path.h"
+#include "diagnostic-metadata.h"
+#include "analyzer/analyzer.h"
+#include "analyzer/checker-path.h"
+#include "analyzer/exploded-graph.h"
+#include "analyzer/pending-diagnostic.h"
+#include "analyzer/sm.h"
+
+namespace {
+
+/* An experimental state machine, for tracking calls to async-signal-unsafe
+ functions from within signal handlers. */
+
+class signal_state_machine : public state_machine
+{
+public:
+ signal_state_machine (logger *logger);
+
+ bool inherited_state_p () const FINAL OVERRIDE { return false; }
+
+ bool on_stmt (sm_context *sm_ctxt,
+ const supernode *node,
+ const gimple *stmt) const FINAL OVERRIDE;
+
+ void on_condition (sm_context *sm_ctxt,
+ const supernode *node,
+ const gimple *stmt,
+ tree lhs,
+ enum tree_code op,
+ tree rhs) const FINAL OVERRIDE;
+
+ bool can_purge_p (state_t s) const FINAL OVERRIDE;
+
+ /* These states are "global", rather than per-expression. */
+
+ /* Start state. */
+ state_t m_start;
+
+ /* State for when we're in a signal handler. */
+ state_t m_in_signal_handler;
+
+ /* Stop state. */
+ state_t m_stop;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
+/* Concrete subclass for describing call to an async-signal-unsafe function
+ from a signal handler. */
+
+class signal_unsafe_call
+ : public pending_diagnostic_subclass<signal_unsafe_call>
+{
+public:
+ signal_unsafe_call (const signal_state_machine &sm, const gcall *unsafe_call,
+ tree unsafe_fndecl)
+ : m_sm (sm), m_unsafe_call (unsafe_call), m_unsafe_fndecl (unsafe_fndecl)
+ {
+ gcc_assert (m_unsafe_fndecl);
+ }
+
+ const char *get_kind () const FINAL OVERRIDE { return "signal_unsafe_call"; }
+
+ bool operator== (const signal_unsafe_call &other) const
+ {
+ return m_unsafe_call == other.m_unsafe_call;
+ }
+
+ bool emit (rich_location *rich_loc) FINAL OVERRIDE
+ {
+ diagnostic_metadata m;
+ /* CWE-479: Signal Handler Use of a Non-reentrant Function. */
+ m.add_cwe (479);
+ return warning_at (rich_loc, m,
+ OPT_Wanalyzer_unsafe_call_within_signal_handler,
+ "call to %qD from within signal handler",
+ m_unsafe_fndecl);
+ }
+
+ label_text describe_state_change (const evdesc::state_change &change)
+ FINAL OVERRIDE
+ {
+ if (change.is_global_p ()
+ && change.m_new_state == m_sm.m_in_signal_handler)
+ {
+ function *handler
+ = change.m_event.m_dst_state.m_region_model->get_current_function ();
+ return change.formatted_print ("registering %qD as signal handler",
+ handler->decl);
+ }
+ return label_text ();
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+ {
+ return ev.formatted_print ("call to %qD from within signal handler",
+ m_unsafe_fndecl);
+ }
+
+private:
+ const signal_state_machine &m_sm;
+ const gcall *m_unsafe_call;
+ tree m_unsafe_fndecl;
+};
+
+////////////////////////////////////////////////////////////////////////////
+
+/* signal_state_machine's ctor. */
+
+signal_state_machine::signal_state_machine (logger *logger)
+: state_machine ("signal", logger)
+{
+ m_start = add_state ("start");
+ m_in_signal_handler = add_state ("in_signal_handler");
+ m_stop = add_state ("stop");
+}
+
+/* Update MODEL for edges that simulate HANDLER_FUN being called as
+ an signal-handler in response to a signal. */
+
+static void
+update_model_for_signal_handler (region_model *model,
+ function *handler_fun)
+{
+ /* Purge all state within MODEL. */
+ *model = region_model ();
+ model->push_frame (handler_fun, NULL, NULL);
+}
+
+/* Custom exploded_edge info: entry into a signal-handler. */
+
+class signal_delivery_edge_info_t : public exploded_edge::custom_info_t
+{
+public:
+ void print (pretty_printer *pp) FINAL OVERRIDE
+ {
+ pp_string (pp, "signal delivered");
+ }
+
+ void update_model (region_model *model,
+ const exploded_edge &eedge) FINAL OVERRIDE
+ {
+ update_model_for_signal_handler (model, eedge.m_dest->get_function ());
+ }
+
+ void add_events_to_path (checker_path *emission_path,
+ const exploded_edge &eedge ATTRIBUTE_UNUSED)
+ {
+ emission_path->add_event
+ (new custom_event (UNKNOWN_LOCATION, NULL_TREE, 0,
+ "later on,"
+ " when the signal is delivered to the process"));
+ }
+};
+
+/* Concrete subclass of custom_transition for modeling registration of a
+ signal handler and the signal handler later being called. */
+
+class register_signal_handler : public custom_transition
+{
+public:
+ register_signal_handler (const signal_state_machine &sm,
+ tree fndecl)
+ : m_sm (sm), m_fndecl (fndecl) {}
+
+ /* Model a signal-handler FNDECL being called at some later point
+ by injecting an edge to a new function-entry node with an empty
+ callstring, setting the 'in-signal-handler' global state
+ on the node. */
+ void impl_transition (exploded_graph *eg,
+ exploded_node *src_enode,
+ int sm_idx) FINAL OVERRIDE
+ {
+ function *handler_fun = DECL_STRUCT_FUNCTION (m_fndecl);
+ if (!handler_fun)
+ return;
+ program_point entering_handler
+ = program_point::from_function_entry (eg->get_supergraph (),
+ handler_fun);
+
+ program_state state_entering_handler (eg->get_ext_state ());
+ update_model_for_signal_handler (state_entering_handler.m_region_model,
+ handler_fun);
+ state_entering_handler.m_checker_states[sm_idx]->set_global_state
+ (m_sm.m_in_signal_handler);
+
+ exploded_node *dst_enode = eg->get_or_create_node (entering_handler,
+ state_entering_handler,
+ NULL);
+ if (dst_enode)
+ eg->add_edge (src_enode, dst_enode, NULL, state_change (),
+ new signal_delivery_edge_info_t ());
+ }
+
+ const signal_state_machine &m_sm;
+ tree m_fndecl;
+};
+
+/* Return true if CALL is known to be unsafe to call from a signal handler. */
+
+static bool
+signal_unsafe_p (tree callee_fndecl)
+{
+ // TODO: maintain a list of known unsafe functions
+ if (is_named_call_p (callee_fndecl, "fprintf"))
+ return true;
+
+ return false;
+}
+
+/* Implementation of state_machine::on_stmt vfunc for signal_state_machine. */
+
+bool
+signal_state_machine::on_stmt (sm_context *sm_ctxt,
+ const supernode *node,
+ const gimple *stmt) const
+{
+ const state_t global_state = sm_ctxt->get_global_state ();
+ if (global_state == m_start)
+ {
+ if (const gcall *call = dyn_cast <const gcall *> (stmt))
+ if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
+ if (is_named_call_p (callee_fndecl, "signal", call, 2))
+ {
+ tree handler = gimple_call_arg (call, 1);
+ if (TREE_CODE (handler) == ADDR_EXPR
+ && TREE_CODE (TREE_OPERAND (handler, 0)) == FUNCTION_DECL)
+ {
+ tree fndecl = TREE_OPERAND (handler, 0);
+ register_signal_handler rsh (*this, fndecl);
+ sm_ctxt->on_custom_transition (&rsh);
+ }
+ }
+ }
+ else if (global_state == m_in_signal_handler)
+ {
+ if (const gcall *call = dyn_cast <const gcall *> (stmt))
+ if (tree callee_fndecl = sm_ctxt->get_fndecl_for_call (call))
+ if (signal_unsafe_p (callee_fndecl))
+ sm_ctxt->warn_for_state (node, stmt, NULL_TREE, m_in_signal_handler,
+ new signal_unsafe_call (*this, call,
+ callee_fndecl));
+ }
+
+ return false;
+}
+
+/* Implementation of state_machine::on_condition vfunc for
+ signal_state_machine. */
+
+void
+signal_state_machine::on_condition (sm_context *sm_ctxt ATTRIBUTE_UNUSED,
+ const supernode *node ATTRIBUTE_UNUSED,
+ const gimple *stmt ATTRIBUTE_UNUSED,
+ tree lhs ATTRIBUTE_UNUSED,
+ enum tree_code op ATTRIBUTE_UNUSED,
+ tree rhs ATTRIBUTE_UNUSED) const
+{
+ // Empty
+}
+
+bool
+signal_state_machine::can_purge_p (state_t s ATTRIBUTE_UNUSED) const
+{
+ return true;
+}
+
+} // anonymous namespace
+
+/* Internal interface to this file. */
+
+state_machine *
+make_signal_state_machine (logger *logger)
+{
+ return new signal_state_machine (logger);
+}
diff --git a/gcc/analyzer/sm.cc b/gcc/analyzer/sm.cc
index eda93506ab2a..39360c03f6cc 100644
--- a/gcc/analyzer/sm.cc
+++ b/gcc/analyzer/sm.cc
@@ -115,6 +115,7 @@ make_checkers (auto_delete_vec <state_machine> &out, logger *logger)
out.safe_push (make_fileptr_state_machine (logger));
out.safe_push (make_taint_state_machine (logger));
out.safe_push (make_sensitive_state_machine (logger));
+ out.safe_push (make_signal_state_machine (logger));
/* We only attempt to run the pattern tests if it might have been manually
enabled (for DejaGnu purposes). */
diff --git a/gcc/analyzer/sm.h b/gcc/analyzer/sm.h
index 5939a69d1ac9..905e64407df6 100644
--- a/gcc/analyzer/sm.h
+++ b/gcc/analyzer/sm.h
@@ -184,6 +184,7 @@ extern state_machine *make_malloc_state_machine (logger *logger);
extern state_machine *make_fileptr_state_machine (logger *logger);
extern state_machine *make_taint_state_machine (logger *logger);
extern state_machine *make_sensitive_state_machine (logger *logger);
+extern state_machine *make_signal_state_machine (logger *logger);
extern state_machine *make_pattern_test_state_machine (logger *logger);
#endif /* GCC_ANALYZER_SM_H */
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 286c1623aaa7..caa57d658ac9 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -308,6 +308,7 @@ Objective-C and Objective-C++ Dialects}.
-Wno-analyzer-null-dereference @gol
-Wno-analyzer-stale-setjmp-buffer @gol
-Wno-analyzer-tainted-array-index @gol
+-Wno-analyzer-unsafe-call-within-signal-handler @gol
-Wno-analyzer-use-after-free @gol
-Wno-analyzer-use-of-pointer-in-stale-stack-frame @gol
-Wno-analyzer-use-of-uninitialized-value @gol
@@ -406,6 +407,7 @@ Objective-C and Objective-C++ Dialects}.
-Wanalyzer-possible-null-dereference @gol
-Wanalyzer-stale-setjmp-buffer @gol
-Wanalyzer-tainted-array-index @gol
+-Wanalyzer-unsafe-call-within-signal-handler @gol
-Wanalyzer-use-after-free @gol
-Wanalyzer-use-of-pointer-in-stale-stack-frame @gol
-Wanalyzer-use-of-uninitialized-value @gol
@@ -6550,6 +6552,16 @@ This diagnostic warns for paths through the code in which a value
that could be under an attacker's control is used as the index
of an array access without being sanitized.
+@item -Wno-analyzer-unsafe-call-within-signal-handler
+@opindex Wanalyzer-unsafe-call-within-signal-handler
+@opindex Wno-analyzer-unsafe-call-within-signal-handler
+This warning requires @option{-fanalyzer}, which enables it; use
+@option{-Wno-analyzer-unsafe-call-within-signal-handler} to disable it.
+
+This diagnostic warns for paths through the code in which a
+function known to be async-signal-unsafe (such as @code{fprintf}) is
+called from a signal handler.
+
@item -Wno-analyzer-use-after-free
@opindex Wanalyzer-use-after-free
@opindex Wno-analyzer-use-after-free
@@ -8262,6 +8274,7 @@ Enabling this option effectively enables the following warnings:
-Wanalyzer-null-argument @gol
-Wanalyzer-null-dereference @gol
-Wanalyzer-tainted-array-index @gol
+-Wanalyzer-unsafe-call-within-signal-handler @gol
-Wanalyzer-use-after-free @gol
-Wanalyzer-use-of-uninitialized-value @gol
-Wanalyzer-use-of-pointer-in-stale-stack-frame @gol
diff --git a/gcc/testsuite/gcc.dg/analyzer/signal-1.c b/gcc/testsuite/gcc.dg/analyzer/signal-1.c
new file mode 100644
index 000000000000..4dcbcc0fc6bd
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/signal-1.c
@@ -0,0 +1,31 @@
+/* Example of a bad call within a signal handler.
+ 'handler' calls 'custom_logger' which calls 'fprintf', and 'fprintf' is
+ not allowed from a signal handler. */
+
+#include <stdio.h>
+#include <signal.h>
+
+extern void body_of_program(void);
+
+void custom_logger(const char *msg)
+{
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+int main(int argc, const char *argv)
+{
+ custom_logger("started");
+
+ signal(SIGINT, handler); /* { dg-message "registering 'handler' as signal handler" } */
+
+ body_of_program();
+
+ custom_logger("stopped");
+
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/signal-2.c b/gcc/testsuite/gcc.dg/analyzer/signal-2.c
new file mode 100644
index 000000000000..a56acb060ec8
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/signal-2.c
@@ -0,0 +1,34 @@
+/* Example of a bad call within a signal handler.
+ 'handler' calls 'custom_logger' which calls 'fprintf', and 'fprintf' is
+ not allowed from a signal handler. */
+
+#include <stdio.h>
+#include <signal.h>
+
+extern void body_of_program(void);
+
+int logging = 1;
+
+void custom_logger(const char *msg)
+{
+ if (logging)
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+int main(int argc, const char *argv)
+{
+ custom_logger("started");
+
+ signal(SIGINT, handler); /* { dg-message "registering 'handler' as signal handler" } */
+
+ body_of_program();
+
+ custom_logger("stopped");
+
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/signal-3.c b/gcc/testsuite/gcc.dg/analyzer/signal-3.c
new file mode 100644
index 000000000000..5b3088887771
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/signal-3.c
@@ -0,0 +1,23 @@
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+
+extern void body_of_program(void);
+
+void custom_logger(const char *msg)
+{
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+void test (void)
+{
+ void *ptr = malloc (1024);
+ signal(SIGINT, handler); /* { dg-message "registering 'handler' as signal handler" } */
+ body_of_program();
+ free (ptr);
+}
diff --git a/gcc/testsuite/gcc.dg/analyzer/signal-4a.c b/gcc/testsuite/gcc.dg/analyzer/signal-4a.c
new file mode 100644
index 000000000000..19e6b83b032a
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/signal-4a.c
@@ -0,0 +1,74 @@
+/* Verify how paths are printed for signal-handler diagnostics. */
+
+/* { dg-options "-fanalyzer -fdiagnostics-show-line-numbers -fdiagnostics-nn-line-numbers -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+
+extern void body_of_program(void);
+
+void custom_logger(const char *msg)
+{
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void int_handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+void test (void)
+{
+ void *ptr = malloc (1024);
+ signal(SIGINT, int_handler);
+ body_of_program();
+ free (ptr);
+}
+
+/* "call to 'fprintf' from within signal handler [CWE-479]". */
+/* { dg-begin-multiline-output "" }
+ NN | fprintf(stderr, "LOG: %s", msg);
+ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 'test': events 1-2
+ |
+ | NN | void test (void)
+ | | ^~~~
+ | | |
+ | | (1) entry to 'test'
+ |......
+ | NN | signal(SIGINT, int_handler);
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (2) registering 'int_handler' as signal handler
+ |
+ event 3
+ |
+ |cc1:
+ | (3): later on, when the signal is delivered to the process
+ |
+ +--> 'int_handler': events 4-5
+ |
+ | NN | static void int_handler(int signum)
+ | | ^~~~~~~~~~~
+ | | |
+ | | (4) entry to 'int_handler'
+ | NN | {
+ | NN | custom_logger("got signal");
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (5) calling 'custom_logger' from 'int_handler'
+ |
+ +--> 'custom_logger': events 6-7
+ |
+ | NN | void custom_logger(const char *msg)
+ | | ^~~~~~~~~~~~~
+ | | |
+ | | (6) entry to 'custom_logger'
+ | NN | {
+ | NN | fprintf(stderr, "LOG: %s", msg);
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (7) call to 'fprintf' from within signal handler
+ |
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/analyzer/signal-4b.c b/gcc/testsuite/gcc.dg/analyzer/signal-4b.c
new file mode 100644
index 000000000000..764af10b1b64
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/analyzer/signal-4b.c
@@ -0,0 +1,89 @@
+/* Verify how paths are printed for signal-handler diagnostics. */
+
+/* { dg-options "-fanalyzer -fdiagnostics-show-line-numbers -fdiagnostics-nn-line-numbers -fdiagnostics-path-format=inline-events -fdiagnostics-show-caret" } */
+
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+
+extern void body_of_program(void);
+
+void custom_logger(const char *msg)
+{
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void int_handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+static void register_handler ()
+{
+ signal(SIGINT, int_handler);
+}
+
+void test (void)
+{
+ register_handler ();
+ body_of_program();
+}
+
+/* "call to 'fprintf' from within signal handler [CWE-479]". */
+/* { dg-begin-multiline-output "" }
+ NN | fprintf(stderr, "LOG: %s", msg);
+ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 'test': events 1-2
+ |
+ | NN | void test (void)
+ | | ^~~~
+ | | |
+ | | (1) entry to 'test'
+ | NN | {
+ | NN | register_handler ();
+ | | ~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (2) calling 'register_handler' from 'test'
+ |
+ +--> 'register_handler': events 3-4
+ |
+ | NN | static void register_handler ()
+ | | ^~~~~~~~~~~~~~~~
+ | | |
+ | | (3) entry to 'register_handler'
+ | NN | {
+ | NN | signal(SIGINT, int_handler);
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (4) registering 'int_handler' as signal handler
+ |
+ event 5
+ |
+ |cc1:
+ | (5): later on, when the signal is delivered to the process
+ |
+ +--> 'int_handler': events 6-7
+ |
+ | NN | static void int_handler(int signum)
+ | | ^~~~~~~~~~~
+ | | |
+ | | (6) entry to 'int_handler'
+ | NN | {
+ | NN | custom_logger("got signal");
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (7) calling 'custom_logger' from 'int_handler'
+ |
+ +--> 'custom_logger': events 8-9
+ |
+ | NN | void custom_logger(const char *msg)
+ | | ^~~~~~~~~~~~~
+ | | |
+ | | (8) entry to 'custom_logger'
+ | NN | {
+ | NN | fprintf(stderr, "LOG: %s", msg);
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (9) call to 'fprintf' from within signal handler
+ |
+ { dg-end-multiline-output "" } */
--
2.21.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/7] [analyzer] Support paths for callbacks
2019-12-04 16:25 [PATCH 0/7] [analyzer] Add checking for unsafe calls within signal handlers David Malcolm
` (5 preceding siblings ...)
2019-12-04 16:25 ` [PATCH 5/7] [analyzer] Support custom events David Malcolm
@ 2019-12-04 16:25 ` David Malcolm
6 siblings, 0 replies; 8+ messages in thread
From: David Malcolm @ 2019-12-04 16:25 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
This patch extends the path-printing support to cope with paths
for signal-handlers, popping the stack to a new entrypoint, with
events outside any function.
gcc/testsuite/ChangeLog:
* gcc.dg/plugin/diagnostic-test-paths-4.c: New test.
* gcc.dg/plugin/diagnostic_plugin_test_paths.c (example_3): New.
(pass_test_show_path::execute): Call it.
* gcc.dg/plugin/plugin.exp (plugin_test_list): Add
diagnostic-test-paths-4.c.
gcc/ChangeLog:
* tree-diagnostic-path.cc (path_summary::print): Support NULL_TREE
for range->fndecl. Handle disjoint paths in which the stack pops
below the start point.
---
.../gcc.dg/plugin/diagnostic-test-paths-4.c | 83 +++++++++++++++++++
.../plugin/diagnostic_plugin_test_paths.c | 81 ++++++++++++++++++
gcc/testsuite/gcc.dg/plugin/plugin.exp | 1 +
gcc/tree-diagnostic-path.cc | 75 +++++++++--------
4 files changed, 207 insertions(+), 33 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
new file mode 100644
index 000000000000..41cbaaaaa976
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic-test-paths-4.c
@@ -0,0 +1,83 @@
+/* { dg-do compile } */
+/* { dg-options "-fdiagnostics-path-format=inline-events -fdiagnostics-show-caret -fdiagnostics-show-line-numbers -fdiagnostics-nn-line-numbers" } */
+
+#include <stdio.h>
+#include <signal.h>
+#include <stdlib.h>
+
+extern void body_of_program(void);
+
+void custom_logger(const char *msg)
+{
+ fprintf(stderr, "LOG: %s", msg); /* { dg-warning "call to 'fprintf' from within signal handler" } */
+}
+
+static void int_handler(int signum)
+{
+ custom_logger("got signal");
+}
+
+static void register_handler ()
+{
+ signal(SIGINT, int_handler);
+}
+
+void test (void)
+{
+ register_handler ();
+ body_of_program();
+}
+
+/* { dg-begin-multiline-output "" }
+ NN | fprintf(stderr, "LOG: %s", msg);
+ | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ 'test': events 1-2
+ |
+ | NN | {
+ | | ^
+ | | |
+ | | (1) entering 'test'
+ | NN | register_handler ();
+ | | ~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (2) calling 'register_handler'
+ |
+ +--> 'register_handler': events 3-4
+ |
+ | NN | {
+ | | ^
+ | | |
+ | | (3) entering 'register_handler'
+ | NN | signal(SIGINT, int_handler);
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (4) registering 'int_handler' as signal handler
+ |
+ event 5
+ |
+ |cc1:
+ | (5): later on, when the signal is delivered to the process
+ |
+ +--> 'int_handler': events 6-7
+ |
+ | NN | {
+ | | ^
+ | | |
+ | | (6) entering 'int_handler'
+ | NN | custom_logger("got signal");
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (7) calling 'custom_logger'
+ |
+ +--> 'custom_logger': events 8-9
+ |
+ | NN | {
+ | | ^
+ | | |
+ | | (8) entering 'custom_logger'
+ | NN | fprintf(stderr, "LOG: %s", msg);
+ | | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ | | |
+ | | (9) calling 'fprintf'
+ |
+ { dg-end-multiline-output "" } */
diff --git a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
index 823981550333..cf05ca3a5d32 100644
--- a/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
+++ b/gcc/testsuite/gcc.dg/plugin/diagnostic_plugin_test_paths.c
@@ -341,11 +341,92 @@ example_2 ()
}
}
+/* Example 3: an interprocedural path with a callback. */
+
+static void
+example_3 ()
+{
+ gimple_stmt_iterator gsi;
+ basic_block bb;
+
+ event_location_t entry_to_custom_logger;
+ event_location_t call_to_fprintf;
+
+ event_location_t entry_to_int_handler;
+ event_location_t call_to_custom_logger;
+
+ event_location_t entry_to_register_handler;
+ event_location_t call_to_signal;
+
+ event_location_t entry_to_test;
+ event_location_t call_to_register_handler;
+
+ cgraph_node *node;
+ FOR_EACH_FUNCTION_WITH_GIMPLE_BODY (node)
+ {
+ function *fun = node->get_fun ();
+ FOR_EACH_BB_FN (bb, fun)
+ {
+ check_for_named_function (fun, "custom_logger",
+ &entry_to_custom_logger);
+ check_for_named_function (fun, "int_handler",
+ &entry_to_int_handler);
+ check_for_named_function (fun, "register_handler",
+ &entry_to_register_handler);
+ check_for_named_function (fun, "test",
+ &entry_to_test);
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple *stmt = gsi_stmt (gsi);
+ if (gcall *call = check_for_named_call (stmt, "fprintf", 3))
+ call_to_fprintf.set (call, fun);
+ if (gcall *call = check_for_named_call (stmt, "custom_logger", 1))
+ call_to_custom_logger.set (call, fun);
+ if (gcall *call = check_for_named_call (stmt, "register_handler",
+ 0))
+ call_to_register_handler.set (call, fun);
+ if (gcall *call = check_for_named_call (stmt, "signal", 2))
+ call_to_signal.set (call, fun);
+ }
+ }
+ }
+
+ if (call_to_fprintf.m_fun)
+ {
+ auto_diagnostic_group d;
+
+ gcc_rich_location richloc (call_to_fprintf.m_loc);
+ test_diagnostic_path path (global_dc->printer);
+ path.add_entry (entry_to_test, 1, "test");
+ path.add_call (call_to_register_handler, 1,
+ entry_to_register_handler, "register_handler");
+ path.add_event (call_to_signal.m_loc, call_to_signal.m_fun->decl,
+ 2, "registering 'int_handler' as signal handler");
+ path.add_event (UNKNOWN_LOCATION, NULL_TREE, 0,
+ "later on, when the signal is delivered to the process");
+ path.add_entry (entry_to_int_handler, 1, "int_handler");
+ path.add_call (call_to_custom_logger, 1,
+ entry_to_custom_logger, "custom_logger");
+ path.add_leaf_call (call_to_fprintf, 2, "fprintf");
+
+ richloc.set_path (&path);
+
+ diagnostic_metadata m;
+ /* CWE-479: Signal Handler Use of a Non-reentrant Function. */
+ m.add_cwe (479);
+
+ warning_at (&richloc, m, 0,
+ "call to %qs from within signal handler",
+ "fprintf");
+ }
+}
+
unsigned int
pass_test_show_path::execute (function *)
{
example_1 ();
example_2 ();
+ example_3 ();
return 0;
}
diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index 4ae30481fb88..6fdb1c67b9e6 100644
--- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
+++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
@@ -98,6 +98,7 @@ set plugin_test_list [list \
diagnostic-test-paths-1.c \
diagnostic-test-paths-2.c \
diagnostic-test-paths-3.c \
+ diagnostic-test-paths-4.c \
diagnostic-path-format-default.c \
diagnostic-path-format-none.c \
diagnostic-path-format-separate-events.c \
diff --git a/gcc/tree-diagnostic-path.cc b/gcc/tree-diagnostic-path.cc
index abb418dd2fa9..a6b51fd9228b 100644
--- a/gcc/tree-diagnostic-path.cc
+++ b/gcc/tree-diagnostic-path.cc
@@ -339,12 +339,16 @@ path_summary::print (diagnostic_context *dc, bool show_depths) const
cur_indent += strlen (push_prefix);
}
}
- print_fndecl (pp, range->m_fndecl, true);
+ if (range->m_fndecl)
+ {
+ print_fndecl (pp, range->m_fndecl, true);
+ pp_string (pp, ": ");
+ }
if (range->m_start_idx == range->m_end_idx)
- pp_printf (pp, ": event %i",
+ pp_printf (pp, "event %i",
range->m_start_idx + 1);
else
- pp_printf (pp, ": events %i-%i",
+ pp_printf (pp, "events %i-%i",
range->m_start_idx + 1, range->m_end_idx + 1);
if (show_depths)
pp_printf (pp, " (depth %i)", range->m_stack_depth);
@@ -387,36 +391,41 @@ path_summary::print (diagnostic_context *dc, bool show_depths) const
if (range->m_stack_depth > next_range->m_stack_depth)
{
- /* Show returning from stack frame(s), by printing
- something like:
- " |\n"
- " <------------ +\n"
- " |\n". */
-
- gcc_assert (vbar_column_for_depth.get
- (next_range->m_stack_depth));
-
- int vbar_for_next_frame
- = *vbar_column_for_depth.get (next_range->m_stack_depth);
-
- int indent_for_next_frame
- = vbar_for_next_frame - per_frame_indent;
- write_indent (pp, vbar_for_next_frame);
- pp_string (pp, start_line_color);
- pp_character (pp, '<');
- for (int i = indent_for_next_frame + per_frame_indent;
- i < cur_indent + per_frame_indent - 1; i++)
- pp_character (pp, '-');
- pp_character (pp, '+');
- pp_string (pp, end_line_color);
- pp_newline (pp);
- cur_indent = indent_for_next_frame;
-
- write_indent (pp, vbar_for_next_frame);
- pp_string (pp, start_line_color);
- pp_printf (pp, "|");
- pp_string (pp, end_line_color);
- pp_newline (pp);
+ if (vbar_column_for_depth.get (next_range->m_stack_depth))
+ {
+ /* Show returning from stack frame(s), by printing
+ something like:
+ " |\n"
+ " <------------ +\n"
+ " |\n". */
+ int vbar_for_next_frame
+ = *vbar_column_for_depth.get (next_range->m_stack_depth);
+
+ int indent_for_next_frame
+ = vbar_for_next_frame - per_frame_indent;
+ write_indent (pp, vbar_for_next_frame);
+ pp_string (pp, start_line_color);
+ pp_character (pp, '<');
+ for (int i = indent_for_next_frame + per_frame_indent;
+ i < cur_indent + per_frame_indent - 1; i++)
+ pp_character (pp, '-');
+ pp_character (pp, '+');
+ pp_string (pp, end_line_color);
+ pp_newline (pp);
+ cur_indent = indent_for_next_frame;
+
+ write_indent (pp, vbar_for_next_frame);
+ pp_string (pp, start_line_color);
+ pp_printf (pp, "|");
+ pp_string (pp, end_line_color);
+ pp_newline (pp);
+ }
+ else
+ {
+ /* Handle disjoint paths (e.g. a callback at some later
+ time). */
+ cur_indent = base_indent;
+ }
}
else if (range->m_stack_depth < next_range->m_stack_depth)
{
--
2.21.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/7] [analyzer] More test coverage
2019-12-04 16:25 [PATCH 0/7] [analyzer] Add checking for unsafe calls within signal handlers David Malcolm
` (3 preceding siblings ...)
2019-12-04 16:25 ` [PATCH 4/7] [analyzer] Support global states and custom transitions David Malcolm
@ 2019-12-04 16:25 ` David Malcolm
2019-12-04 16:25 ` [PATCH 5/7] [analyzer] Support custom events David Malcolm
2019-12-04 16:25 ` [PATCH 1/7] [analyzer] Support paths for callbacks David Malcolm
6 siblings, 0 replies; 8+ messages in thread
From: David Malcolm @ 2019-12-04 16:25 UTC (permalink / raw)
To: gcc-patches; +Cc: David Malcolm
gcc/testsuite/ChangeLog:
* gcc.dg/analyzer/data-model-1.c: Include <stdio.h>.
(test_53): New function.
---
gcc/testsuite/gcc.dg/analyzer/data-model-1.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
index c195b74edc72..43d08474283e 100644
--- a/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
+++ b/gcc/testsuite/gcc.dg/analyzer/data-model-1.c
@@ -1,5 +1,6 @@
#include <stdlib.h>
#include <string.h>
+#include <stdio.h>
struct foo
{
@@ -1076,3 +1077,8 @@ void test_52 (struct big b)
__analyzer_eval (b.ia[0] == d.ia[0]); /* { dg-warning "TRUE" "" { xfail *-*-* } } */
/* { dg-warning "UNKNOWN" "" { target *-*-* } .-1 } */
}
+
+void test_53 (const char *msg)
+{
+ (void)fprintf(stderr, "LOG: %s", msg);
+}
--
2.21.0
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-12-04 16:25 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-04 16:25 [PATCH 0/7] [analyzer] Add checking for unsafe calls within signal handlers David Malcolm
2019-12-04 16:25 ` [PATCH 6/7] [analyzer] Expose state_change_event in evdesc::state_change David Malcolm
2019-12-04 16:25 ` [PATCH 3/7] [analyzer] Generalize rewind_info_t to exploded_edge::custom_info_t David Malcolm
2019-12-04 16:25 ` [PATCH 7/7] [analyzer] Add -Wanalyzer-unsafe-call-within-signal-handler David Malcolm
2019-12-04 16:25 ` [PATCH 4/7] [analyzer] Support global states and custom transitions David Malcolm
2019-12-04 16:25 ` [PATCH 2/7] [analyzer] More test coverage David Malcolm
2019-12-04 16:25 ` [PATCH 5/7] [analyzer] Support custom events David Malcolm
2019-12-04 16:25 ` [PATCH 1/7] [analyzer] Support paths for callbacks David Malcolm
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).