Commit fd9982bb by David Malcolm

analyzer: fix setjmp handling with -g (PR 93378)

PR analyzer/93378 reports an ICE at -O1 -g when analyzing a rewind via
longjmp to a setjmp call with.

The root cause is that the rewind_info_t::get_setjmp_call attempts to
locate the setjmp GIMPLE_CALL via within the exploded_node containing
it, but the exploded_node has two stmts: a GIMPLE_DEBUG, then the
GIMPLE_CALL, and so erroneously picks the GIMPLE_DEBUG, leading to
a failed as_a <const gcall *>.

This patch reworks how the analyzer stores information about a setjmp
so that instead of storing an exploded_node *, it instead introduces
a "setjmp_record" struct, for use by both setjmp_svalue and
rewind_info_t.  Hence we store the information directly, rather than
attempting to reconstruct it, fixing the bug.

gcc/analyzer/ChangeLog:
	PR analyzer/93378
	* engine.cc (setjmp_svalue::compare_fields): Update for
	replacement of m_enode with m_setjmp_record.
	(setjmp_svalue::add_to_hash): Likewise.
	(setjmp_svalue::get_index): Rename...
	(setjmp_svalue::get_enode_index): ...to this.
	(setjmp_svalue::print_details): Update for replacement of m_enode
	with m_setjmp_record.
	(exploded_node::on_longjmp): Likewise.
	* exploded-graph.h (rewind_info_t::m_enode_origin): Replace...
	(rewind_info_t::m_setjmp_record): ...with this.
	(rewind_info_t::rewind_info_t): Update for replacement of m_enode
	with m_setjmp_record.
	(rewind_info_t::get_setjmp_point): Likewise.
	(rewind_info_t::get_setjmp_call): Likewise.
	* region-model.cc (region_model::dump_summary_of_map): Likewise.
	(region_model::on_setjmp): Likewise.
	* region-model.h (struct setjmp_record): New struct.
	(setjmp_svalue::m_enode): Replace...
	(setjmp_svalue::m_setjmp_record): ...with this.
	(setjmp_svalue::setjmp_svalue): Update for replacement of m_enode
	with m_setjmp_record.
	(setjmp_svalue::clone): Likewise.
	(setjmp_svalue::get_index): Rename...
	(setjmp_svalue::get_enode_index): ...to this.
	(setjmp_svalue::get_exploded_node): Replace...
	(setjmp_svalue::get_setjmp_record): ...with this.

gcc/testsuite/ChangeLog:
	PR analyzer/93378
	* gcc.dg/analyzer/setjmp-pr93378.c: New test.
parent da7cf663
2020-01-22 David Malcolm <dmalcolm@redhat.com> 2020-01-22 David Malcolm <dmalcolm@redhat.com>
PR analyzer/93378
* engine.cc (setjmp_svalue::compare_fields): Update for
replacement of m_enode with m_setjmp_record.
(setjmp_svalue::add_to_hash): Likewise.
(setjmp_svalue::get_index): Rename...
(setjmp_svalue::get_enode_index): ...to this.
(setjmp_svalue::print_details): Update for replacement of m_enode
with m_setjmp_record.
(exploded_node::on_longjmp): Likewise.
* exploded-graph.h (rewind_info_t::m_enode_origin): Replace...
(rewind_info_t::m_setjmp_record): ...with this.
(rewind_info_t::rewind_info_t): Update for replacement of m_enode
with m_setjmp_record.
(rewind_info_t::get_setjmp_point): Likewise.
(rewind_info_t::get_setjmp_call): Likewise.
* region-model.cc (region_model::dump_summary_of_map): Likewise.
(region_model::on_setjmp): Likewise.
* region-model.h (struct setjmp_record): New struct.
(setjmp_svalue::m_enode): Replace...
(setjmp_svalue::m_setjmp_record): ...with this.
(setjmp_svalue::setjmp_svalue): Update for replacement of m_enode
with m_setjmp_record.
(setjmp_svalue::clone): Likewise.
(setjmp_svalue::get_index): Rename...
(setjmp_svalue::get_enode_index): ...to this.
(setjmp_svalue::get_exploded_node): Replace...
(setjmp_svalue::get_setjmp_record): ...with this.
2020-01-22 David Malcolm <dmalcolm@redhat.com>
PR analyzer/93316 PR analyzer/93316
* analyzer.cc (is_setjmp_call_p): Check for "setjmp" as well as * analyzer.cc (is_setjmp_call_p): Check for "setjmp" as well as
"_setjmp". "_setjmp".
......
...@@ -155,7 +155,7 @@ impl_region_model_context::on_unknown_change (svalue_id sid) ...@@ -155,7 +155,7 @@ impl_region_model_context::on_unknown_change (svalue_id sid)
bool bool
setjmp_svalue::compare_fields (const setjmp_svalue &other) const setjmp_svalue::compare_fields (const setjmp_svalue &other) const
{ {
return m_enode == other.m_enode; return m_setjmp_record == other.m_setjmp_record;
} }
/* Implementation of svalue::add_to_hash vfunc for setjmp_svalue. */ /* Implementation of svalue::add_to_hash vfunc for setjmp_svalue. */
...@@ -163,15 +163,15 @@ setjmp_svalue::compare_fields (const setjmp_svalue &other) const ...@@ -163,15 +163,15 @@ setjmp_svalue::compare_fields (const setjmp_svalue &other) const
void void
setjmp_svalue::add_to_hash (inchash::hash &hstate) const setjmp_svalue::add_to_hash (inchash::hash &hstate) const
{ {
hstate.add_int (m_enode->m_index); hstate.add_int (m_setjmp_record.m_enode->m_index);
} }
/* Get the index of the stored exploded_node. */ /* Get the index of the stored exploded_node. */
int int
setjmp_svalue::get_index () const setjmp_svalue::get_enode_index () const
{ {
return m_enode->m_index; return m_setjmp_record.m_enode->m_index;
} }
/* Implementation of svalue::print_details vfunc for setjmp_svalue. */ /* Implementation of svalue::print_details vfunc for setjmp_svalue. */
...@@ -181,7 +181,7 @@ setjmp_svalue::print_details (const region_model &model ATTRIBUTE_UNUSED, ...@@ -181,7 +181,7 @@ setjmp_svalue::print_details (const region_model &model ATTRIBUTE_UNUSED,
svalue_id this_sid ATTRIBUTE_UNUSED, svalue_id this_sid ATTRIBUTE_UNUSED,
pretty_printer *pp) const pretty_printer *pp) const
{ {
pp_printf (pp, "setjmp: EN: %i", m_enode->m_index); pp_printf (pp, "setjmp: EN: %i", get_enode_index ());
} }
/* Concrete implementation of sm_context, wiring it up to the rest of this /* Concrete implementation of sm_context, wiring it up to the rest of this
...@@ -1172,11 +1172,11 @@ exploded_node::on_longjmp (exploded_graph &eg, ...@@ -1172,11 +1172,11 @@ exploded_node::on_longjmp (exploded_graph &eg,
if (!setjmp_sval) if (!setjmp_sval)
return; return;
const setjmp_record tmp_setjmp_record = setjmp_sval->get_setjmp_record ();
/* Build a custom enode and eedge for rewinding from the longjmp /* Build a custom enode and eedge for rewinding from the longjmp
call back to the setjmp. */ call back to the setjmp. */
rewind_info_t rewind_info (tmp_setjmp_record);
const exploded_node *enode_origin = setjmp_sval->get_exploded_node ();
rewind_info_t rewind_info (enode_origin);
const gcall *setjmp_call = rewind_info.get_setjmp_call (); const gcall *setjmp_call = rewind_info.get_setjmp_call ();
const program_point &setjmp_point = rewind_info.get_setjmp_point (); const program_point &setjmp_point = rewind_info.get_setjmp_point ();
...@@ -1217,7 +1217,7 @@ exploded_node::on_longjmp (exploded_graph &eg, ...@@ -1217,7 +1217,7 @@ exploded_node::on_longjmp (exploded_graph &eg,
exploded_edge *eedge exploded_edge *eedge
= eg.add_edge (const_cast<exploded_node *> (this), next, NULL, = eg.add_edge (const_cast<exploded_node *> (this), next, NULL,
change, change,
new rewind_info_t (enode_origin)); new rewind_info_t (tmp_setjmp_record));
/* For any diagnostics that were queued here (such as leaks) we want /* For any diagnostics that were queued here (such as leaks) we want
the checker_path to show the rewinding events after the "final event" the checker_path to show the rewinding events after the "final event"
......
...@@ -307,8 +307,8 @@ private: ...@@ -307,8 +307,8 @@ private:
class rewind_info_t : public exploded_edge::custom_info_t class rewind_info_t : public exploded_edge::custom_info_t
{ {
public: public:
rewind_info_t (const exploded_node *enode_origin) rewind_info_t (const setjmp_record &setjmp_record)
: m_enode_origin (enode_origin) : m_setjmp_record (setjmp_record)
{} {}
void print (pretty_printer *pp) FINAL OVERRIDE void print (pretty_printer *pp) FINAL OVERRIDE
...@@ -324,7 +324,7 @@ public: ...@@ -324,7 +324,7 @@ public:
const program_point &get_setjmp_point () const const program_point &get_setjmp_point () const
{ {
const program_point &origin_point = m_enode_origin->get_point (); const program_point &origin_point = get_enode_origin ()->get_point ();
/* "origin_point" ought to be before the call to "setjmp". */ /* "origin_point" ought to be before the call to "setjmp". */
gcc_assert (origin_point.get_kind () == PK_BEFORE_STMT); gcc_assert (origin_point.get_kind () == PK_BEFORE_STMT);
...@@ -336,13 +336,16 @@ public: ...@@ -336,13 +336,16 @@ public:
const gcall *get_setjmp_call () const const gcall *get_setjmp_call () const
{ {
return as_a <const gcall *> (get_setjmp_point ().get_stmt ()); return m_setjmp_record.m_setjmp_call;
} }
const exploded_node *get_enode_origin () const { return m_enode_origin; } const exploded_node *get_enode_origin () const
{
return m_setjmp_record.m_enode;
}
private: private:
const exploded_node *m_enode_origin; setjmp_record m_setjmp_record;
}; };
/* Statistics about aspects of an exploded_graph. */ /* Statistics about aspects of an exploded_graph. */
......
...@@ -3660,7 +3660,7 @@ region_model::dump_summary_of_map (pretty_printer *pp, ...@@ -3660,7 +3660,7 @@ region_model::dump_summary_of_map (pretty_printer *pp,
case SK_SETJMP: case SK_SETJMP:
dump_separator (pp, is_first); dump_separator (pp, is_first);
pp_printf (pp, "setjmp: EN: %i", pp_printf (pp, "setjmp: EN: %i",
sval->dyn_cast_setjmp_svalue ()->get_index ()); sval->dyn_cast_setjmp_svalue ()->get_enode_index ());
break; break;
} }
} }
...@@ -4493,10 +4493,11 @@ region_model::on_setjmp (const gcall *call, const exploded_node *enode, ...@@ -4493,10 +4493,11 @@ region_model::on_setjmp (const gcall *call, const exploded_node *enode,
region_id buf_rid = deref_rvalue (gimple_call_arg (call, 0), ctxt); region_id buf_rid = deref_rvalue (gimple_call_arg (call, 0), ctxt);
region *buf = get_region (buf_rid); region *buf = get_region (buf_rid);
/* Create a setjmp_svalue for ENODE and store it in BUF_RID's region. */ /* Create a setjmp_svalue for this call and store it in BUF_RID's region. */
if (buf) if (buf)
{ {
svalue *sval = new setjmp_svalue (enode, buf->get_type ()); setjmp_record r (enode, call);
svalue *sval = new setjmp_svalue (r, buf->get_type ());
svalue_id new_sid = add_svalue (sval); svalue_id new_sid = add_svalue (sval);
set_value (buf_rid, new_sid, ctxt); set_value (buf_rid, new_sid, ctxt);
} }
......
...@@ -718,20 +718,42 @@ is_a_helper <poisoned_svalue *>::test (svalue *sval) ...@@ -718,20 +718,42 @@ is_a_helper <poisoned_svalue *>::test (svalue *sval)
namespace ana { namespace ana {
/* A bundle of information recording a setjmp call, corresponding roughly
to a jmp_buf. */
struct setjmp_record
{
setjmp_record (const exploded_node *enode,
const gcall *setjmp_call)
: m_enode (enode), m_setjmp_call (setjmp_call)
{
}
bool operator== (const setjmp_record &other) const
{
return (m_enode == other.m_enode
&& m_setjmp_call == other.m_setjmp_call);
}
const exploded_node *m_enode;
const gcall *m_setjmp_call;
};
/* Concrete subclass of svalue representing setjmp buffers, so that /* Concrete subclass of svalue representing setjmp buffers, so that
longjmp can potentially "return" to an entirely different function. */ longjmp can potentially "return" to an entirely different function. */
class setjmp_svalue : public svalue class setjmp_svalue : public svalue
{ {
public: public:
setjmp_svalue (const exploded_node *enode, tree type) setjmp_svalue (const setjmp_record &setjmp_record,
: svalue (type), m_enode (enode) tree type)
: svalue (type), m_setjmp_record (setjmp_record)
{} {}
bool compare_fields (const setjmp_svalue &other) const; bool compare_fields (const setjmp_svalue &other) const;
svalue *clone () const FINAL OVERRIDE svalue *clone () const FINAL OVERRIDE
{ return new setjmp_svalue (m_enode, get_type ()); } { return new setjmp_svalue (m_setjmp_record, get_type ()); }
enum svalue_kind get_kind () const FINAL OVERRIDE { return SK_SETJMP; } enum svalue_kind get_kind () const FINAL OVERRIDE { return SK_SETJMP; }
...@@ -739,9 +761,9 @@ public: ...@@ -739,9 +761,9 @@ public:
setjmp_svalue *dyn_cast_setjmp_svalue () FINAL OVERRIDE { return this; } setjmp_svalue *dyn_cast_setjmp_svalue () FINAL OVERRIDE { return this; }
int get_index () const; int get_enode_index () const;
const exploded_node *get_exploded_node () const { return m_enode; } const setjmp_record &get_setjmp_record () const { return m_setjmp_record; }
private: private:
void print_details (const region_model &model, void print_details (const region_model &model,
...@@ -749,7 +771,7 @@ public: ...@@ -749,7 +771,7 @@ public:
pretty_printer *pp) const pretty_printer *pp) const
FINAL OVERRIDE; FINAL OVERRIDE;
const exploded_node *m_enode; setjmp_record m_setjmp_record;
}; };
/* An enum for discriminating between the different concrete subclasses /* An enum for discriminating between the different concrete subclasses
......
2020-01-22 David Malcolm <dmalcolm@redhat.com> 2020-01-22 David Malcolm <dmalcolm@redhat.com>
PR analyzer/93378
* gcc.dg/analyzer/setjmp-pr93378.c: New test.
2020-01-22 David Malcolm <dmalcolm@redhat.com>
PR analyzer/93316 PR analyzer/93316
* gcc.dg/analyzer/data-model-1.c: Include <alloca.h>. * gcc.dg/analyzer/data-model-1.c: Include <alloca.h>.
* gcc.dg/analyzer/malloc-1.c: Likewise. * gcc.dg/analyzer/malloc-1.c: Likewise.
......
/* { dg-additional-options "-O1 -g" } */
#include <setjmp.h>
jmp_buf buf;
int
test (void)
{
if (_setjmp (buf) != 0)
return 0;
longjmp (buf, 1);
return 1;
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment