* [PATCH] Add analyzer plugin support and CPython GIL example
@ 2020-10-15 1:21 David Malcolm
2020-11-10 21:51 ` PING: " David Malcolm
0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2020-10-15 1:21 UTC (permalink / raw)
To: gcc-patches
This patch adds a new GCC plugin event: PLUGIN_ANALYZER_INIT, called
when -fanalyzer is starting, allowing for GCC plugins to register
additional state-machine-based checks within -fanalyzer. The idea
is that 3rd-party code might want to add domain-specific checks for
its own APIs - with the caveat that the analyzer is itself still
rather experimental.
As an example, the patch adds a proof-of-concept plugin to the testsuite
for checking CPython code: verifying that code that relinquishes
CPython's global interpreter lock doesn't attempt to do anything with
PyObjects in the sections where the lock isn't held. It also adds a
warning about nested releases of the lock, which is forbidden.
For example:
demo.c: In function 'foo':
demo.c:11:3: warning: use of PyObject '*(obj)' without the GIL
11 | Py_INCREF (obj);
| ^~~~~~~~~
'test': events 1-3
|
| 15 | void test (PyObject *obj)
| | ^~~~
| | |
| | (1) entry to 'test'
| 16 | {
| 17 | Py_BEGIN_ALLOW_THREADS
| | ~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) releasing the GIL here
| 18 | foo (obj);
| | ~~~~~~~~~
| | |
| | (3) calling 'foo' from 'test'
|
+--> 'foo': events 4-5
|
| 9 | foo (PyObject *obj)
| | ^~~
| | |
| | (4) entry to 'foo'
| 10 | {
| 11 | Py_INCREF (obj);
| | ~~~~~~~~~
| | |
| | (5) PyObject '*(obj)' used here without the GIL
|
Doing so requires adding some logic for ignoring macro expansions in
analyzer diagnostics, since the insides of Py_INCREF and
Py_BEGIN_ALLOW_THREADS are not of interest to the user for these cases.
Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
Are the non-analyzer parts OK for master?
gcc/analyzer/ChangeLog:
* analyzer-pass.cc (pass_analyzer::execute): Move sorry call to...
(sorry_no_analyzer): New.
* analyzer.h (class state_machine): New forward decl.
(class logger): New forward decl.
(class plugin_analyzer_init_iface): New.
(sorry_no_analyzer): New decl.
* checker-path.cc (checker_path::fixup_locations): New.
* checker-path.h (checker_event::set_location): New.
(checker_path::fixup_locations): New decl.
* diagnostic-manager.cc
(diagnostic_manager::emit_saved_diagnostic): Call
checker_path::fixup_locations, and call fixup_location
on the primary location.
* engine.cc: Include "plugin.h".
(class plugin_analyzer_init_impl): New.
(impl_run_checkers): Invoke PLUGIN_ANALYZER_INIT callbacks.
* pending-diagnostic.h (pending_diagnostic::fixup_location): New
vfunc.
gcc/ChangeLog:
* doc/plugins.texi (Plugin callbacks): Add PLUGIN_ANALYZER_INIT.
* plugin.c (register_callback): Likewise.
(invoke_plugin_callbacks_full): Likewise.
* plugin.def (PLUGIN_ANALYZER_INIT): New event.
gcc/testsuite/ChangeLog:
* gcc.dg/plugin/analyzer_gil_plugin.c: New test.
* gcc.dg/plugin/gil-1.c: New test.
* gcc.dg/plugin/gil.h: New header.
* gcc.dg/plugin/plugin.exp (plugin_test_list): Add the new plugin
and test.
---
gcc/analyzer/analyzer-pass.cc | 18 +-
gcc/analyzer/analyzer.h | 15 +
gcc/analyzer/checker-path.cc | 9 +
gcc/analyzer/checker-path.h | 4 +
gcc/analyzer/diagnostic-manager.cc | 9 +-
gcc/analyzer/engine.cc | 31 ++
gcc/analyzer/pending-diagnostic.h | 8 +
gcc/doc/plugins.texi | 4 +
gcc/plugin.c | 2 +
gcc/plugin.def | 4 +
.../gcc.dg/plugin/analyzer_gil_plugin.c | 436 ++++++++++++++++++
gcc/testsuite/gcc.dg/plugin/gil-1.c | 90 ++++
gcc/testsuite/gcc.dg/plugin/gil.h | 32 ++
gcc/testsuite/gcc.dg/plugin/plugin.exp | 2 +
14 files changed, 660 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
create mode 100644 gcc/testsuite/gcc.dg/plugin/gil-1.c
create mode 100644 gcc/testsuite/gcc.dg/plugin/gil.h
diff --git a/gcc/analyzer/analyzer-pass.cc b/gcc/analyzer/analyzer-pass.cc
index a27421e46d4..1f65bf8b154 100644
--- a/gcc/analyzer/analyzer-pass.cc
+++ b/gcc/analyzer/analyzer-pass.cc
@@ -83,9 +83,7 @@ pass_analyzer::execute (function *)
#if ENABLE_ANALYZER
ana::run_checkers ();
#else
- sorry ("%qs was not enabled in this build of GCC"
- " (missing configure-time option %qs)",
- "-fanalyzer", "--enable-analyzer");
+ sorry_no_analyzer ();
#endif
return 0;
@@ -100,3 +98,17 @@ make_pass_analyzer (gcc::context *ctxt)
{
return new pass_analyzer (ctxt);
}
+
+#if !ENABLE_ANALYZER
+
+/* Issue a "sorry" diagnostic that the analyzer was not enabled. */
+
+void
+sorry_no_analyzer ()
+{
+ sorry ("%qs was not enabled in this build of GCC"
+ " (missing configure-time option %qs)",
+ "-fanalyzer", "--enable-analyzer");
+}
+
+#endif /* #if !ENABLE_ANALYZER */
diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
index aa43b7f66a9..5d01d022f3c 100644
--- a/gcc/analyzer/analyzer.h
+++ b/gcc/analyzer/analyzer.h
@@ -97,6 +97,8 @@ class state_change;
class rewind_info_t;
class engine;
+class state_machine;
+class logger;
/* Forward decls of functions. */
@@ -186,6 +188,15 @@ private:
extern location_t get_stmt_location (const gimple *stmt, function *fun);
+/* Passed by pointer to PLUGIN_ANALYZER_INIT callbacks. */
+
+class plugin_analyzer_init_iface
+{
+public:
+ virtual void register_state_machine (state_machine *) = 0;
+ virtual logger *get_logger () const = 0;
+};
+
} // namespace ana
extern bool is_special_named_call_p (const gcall *call, const char *funcname,
@@ -306,4 +317,8 @@ private:
#pragma GCC diagnostic ignored "-Wformat-diag"
#endif
+#if !ENABLE_ANALYZER
+extern void sorry_no_analyzer ();
+#endif /* #if !ENABLE_ANALYZER */
+
#endif /* GCC_ANALYZER_ANALYZER_H */
diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-path.cc
index 1f6d6a86001..062730cc789 100644
--- a/gcc/analyzer/checker-path.cc
+++ b/gcc/analyzer/checker-path.cc
@@ -982,6 +982,15 @@ checker_path::add_final_event (const state_machine *sm,
add_event (end_of_path);
}
+void
+checker_path::fixup_locations (pending_diagnostic *pd)
+{
+ checker_event *e;
+ int i;
+ FOR_EACH_VEC_ELT (m_events, i, e)
+ e->set_location (pd->fixup_location (e->get_location ()));
+}
+
} // namespace ana
#endif /* #if ENABLE_ANALYZER */
diff --git a/gcc/analyzer/checker-path.h b/gcc/analyzer/checker-path.h
index 7b86d48e983..53a7a4efea3 100644
--- a/gcc/analyzer/checker-path.h
+++ b/gcc/analyzer/checker-path.h
@@ -99,6 +99,8 @@ public:
void dump (pretty_printer *pp) const;
+ void set_location (location_t loc) { m_loc = loc; }
+
public:
const enum event_kind m_kind;
protected:
@@ -498,6 +500,8 @@ public:
e->prepare_for_emission (this, pd, diagnostic_event_id_t (i));
}
+ void fixup_locations (pending_diagnostic *pd);
+
void record_setjmp_event (const exploded_node *enode,
diagnostic_event_id_t setjmp_emission_id)
{
diff --git a/gcc/analyzer/diagnostic-manager.cc b/gcc/analyzer/diagnostic-manager.cc
index cb95a95ff0b..baad14c6c4d 100644
--- a/gcc/analyzer/diagnostic-manager.cc
+++ b/gcc/analyzer/diagnostic-manager.cc
@@ -648,7 +648,14 @@ diagnostic_manager::emit_saved_diagnostic (const exploded_graph &eg,
emission_path.prepare_for_emission (sd.m_d);
- gcc_rich_location rich_loc (get_stmt_location (stmt, sd.m_snode->m_fun));
+ location_t loc = get_stmt_location (stmt, sd.m_snode->m_fun);
+
+ /* Allow the pending_diagnostic to fix up the primary location
+ and any locations for events. */
+ loc = sd.m_d->fixup_location (loc);
+ emission_path.fixup_locations (sd.m_d);
+
+ gcc_rich_location rich_loc (loc);
rich_loc.set_path (&emission_path);
auto_diagnostic_group d;
diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
index 65d7495f26f..de772c77b2a 100644
--- a/gcc/analyzer/engine.cc
+++ b/gcc/analyzer/engine.cc
@@ -63,6 +63,7 @@ along with GCC; see the file COPYING3. If not see
#include "analyzer/state-purge.h"
#include "analyzer/bar-chart.h"
#include <zlib.h>
+#include "plugin.h"
/* For an overview, see gcc/doc/analyzer.texi. */
@@ -4510,6 +4511,33 @@ dump_analyzer_json (const supergraph &sg,
free (filename);
}
+/* Concrete subclass of plugin_analyzer_init_iface, allowing plugins
+ to register new state machines. */
+
+class plugin_analyzer_init_impl : public plugin_analyzer_init_iface
+{
+public:
+ plugin_analyzer_init_impl (auto_delete_vec <state_machine> *checkers,
+ logger *logger)
+ : m_checkers (checkers),
+ m_logger (logger)
+ {}
+
+ void register_state_machine (state_machine *sm) FINAL OVERRIDE
+ {
+ m_checkers->safe_push (sm);
+ }
+
+ logger *get_logger () const FINAL OVERRIDE
+ {
+ return m_logger;
+ }
+
+private:
+ auto_delete_vec <state_machine> *m_checkers;
+ logger *m_logger;
+};
+
/* Run the analysis "engine". */
void
@@ -4555,6 +4583,9 @@ impl_run_checkers (logger *logger)
auto_delete_vec <state_machine> checkers;
make_checkers (checkers, logger);
+ plugin_analyzer_init_impl data (&checkers, logger);
+ invoke_plugin_callbacks (PLUGIN_ANALYZER_INIT, &data);
+
if (logger)
{
int i;
diff --git a/gcc/analyzer/pending-diagnostic.h b/gcc/analyzer/pending-diagnostic.h
index b1ff2688bcc..fa6c9781044 100644
--- a/gcc/analyzer/pending-diagnostic.h
+++ b/gcc/analyzer/pending-diagnostic.h
@@ -175,6 +175,14 @@ class pending_diagnostic
diagnostic deduplication. */
static bool same_tree_p (tree t1, tree t2);
+ /* A vfunc for fixing up locations (both the primary location for the
+ diagnostic, and for events in their paths), e.g. to avoid unwinding
+ inside specific macros. */
+ virtual location_t fixup_location (location_t loc) const
+ {
+ return loc;
+ }
+
/* For greatest precision-of-wording, the various following "describe_*"
virtual functions give the pending diagnostic a way to describe events
in a diagnostic_path in terms that make sense for that diagnostic.
diff --git a/gcc/doc/plugins.texi b/gcc/doc/plugins.texi
index 23a8b410ebb..1c211bbacf1 100644
--- a/gcc/doc/plugins.texi
+++ b/gcc/doc/plugins.texi
@@ -218,6 +218,10 @@ enum plugin_event
as a const char* pointer. */
PLUGIN_INCLUDE_FILE,
+ /* Called when -fanalyzer starts. The event data is an
+ ana::plugin_analyzer_init_iface *. */
+ PLUGIN_ANALYZER_INIT,
+
PLUGIN_EVENT_FIRST_DYNAMIC /* Dummy event used for indexing callback
array. */
@};
diff --git a/gcc/plugin.c b/gcc/plugin.c
index 69b6f5b8b6a..76069e64798 100644
--- a/gcc/plugin.c
+++ b/gcc/plugin.c
@@ -497,6 +497,7 @@ register_callback (const char *plugin_name,
case PLUGIN_EARLY_GIMPLE_PASSES_END:
case PLUGIN_NEW_PASS:
case PLUGIN_INCLUDE_FILE:
+ case PLUGIN_ANALYZER_INIT:
{
struct callback_info *new_callback;
if (!callback)
@@ -577,6 +578,7 @@ invoke_plugin_callbacks_full (int event, void *gcc_data)
case PLUGIN_EARLY_GIMPLE_PASSES_END:
case PLUGIN_NEW_PASS:
case PLUGIN_INCLUDE_FILE:
+ case PLUGIN_ANALYZER_INIT:
{
/* Iterate over every callback registered with this event and
call it. */
diff --git a/gcc/plugin.def b/gcc/plugin.def
index 3482ed71906..b8110190d3a 100644
--- a/gcc/plugin.def
+++ b/gcc/plugin.def
@@ -99,6 +99,10 @@ DEFEVENT (PLUGIN_NEW_PASS)
as a const char* pointer. */
DEFEVENT (PLUGIN_INCLUDE_FILE)
+/* Called when -fanalyzer starts. The event data is an
+ ana::plugin_analyzer_init_iface *. */
+DEFEVENT (PLUGIN_ANALYZER_INIT)
+
/* When adding a new hard-coded plugin event, don't forget to edit in
file plugin.c the functions register_callback and
invoke_plugin_callbacks_full accordingly! */
diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c b/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
new file mode 100644
index 00000000000..05133d5250e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
@@ -0,0 +1,436 @@
+/* Proof-of-concept of a -fanalyzer plugin.
+ Detect (some) uses of CPython API outside of the Global Interpreter Lock.
+ https://docs.python.org/3/c-api/init.html#thread-state-and-the-global-interpreter-lock
+*/
+/* { dg-options "-g" } */
+
+#include "gcc-plugin.h"
+#include "config.h"
+#include "system.h"
+#include "coretypes.h"
+#include "diagnostic.h"
+#include "tree.h"
+#include "gimple.h"
+#include "gimple-iterator.h"
+#include "gimple-walk.h"
+#include "diagnostic-event-id.h"
+#include "analyzer/analyzer.h"
+#include "analyzer/analyzer-logging.h"
+#include "json.h"
+#include "analyzer/sm.h"
+#include "analyzer/pending-diagnostic.h"
+
+int plugin_is_GPL_compatible;
+
+#if ENABLE_ANALYZER
+
+namespace ana {
+
+static bool
+type_based_on_pyobject_p (tree type)
+{
+ /* Ideally we'd also check for "subclasses" here by iterating up the
+ first field of each struct. */
+ if (TREE_CODE (type) != RECORD_TYPE)
+ return false;
+ tree name = TYPE_IDENTIFIER (type);
+ if (!name)
+ return false;
+ return id_equal (name, "PyObject");
+}
+
+/* An experimental state machine, for tracking whether the GIL is held,
+ as global state.. */
+
+class gil_state_machine : public state_machine
+{
+public:
+ gil_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;
+
+ void check_for_pyobject_usage_without_gil (sm_context *sm_ctxt,
+ const supernode *node,
+ const gimple *stmt,
+ tree op) const;
+
+ private:
+ void check_for_pyobject_in_call (sm_context *sm_ctxt,
+ const supernode *node,
+ const gcall *call,
+ tree callee_fndecl) const;
+
+ public:
+ /* These states are "global", rather than per-expression. */
+
+ /* State for when we've released the GIL. */
+ state_t m_released_gil;
+
+ /* Stop state. */
+ state_t m_stop;
+};
+
+/* Subclass for diagnostics involving the GIL. */
+
+class gil_diagnostic : public pending_diagnostic
+{
+public:
+ location_t fixup_location (location_t loc) const FINAL OVERRIDE
+ {
+ /* Ideally we'd check for specific macros here, and only
+ resolve certain macros. */
+ if (linemap_location_from_macro_expansion_p (line_table, loc))
+ loc = linemap_resolve_location (line_table, loc,
+ LRK_MACRO_EXPANSION_POINT, NULL);
+ return loc;
+ }
+
+ label_text describe_state_change (const evdesc::state_change &change)
+ FINAL OVERRIDE
+ {
+ if (change.is_global_p ()
+ && change.m_new_state == m_sm.m_released_gil)
+ return change.formatted_print ("releasing the GIL here");
+ if (change.is_global_p ()
+ && change.m_new_state == m_sm.get_start_state ())
+ return change.formatted_print ("acquiring the GIL here");
+ return label_text ();
+ }
+
+ protected:
+ gil_diagnostic (const gil_state_machine &sm) : m_sm (sm)
+ {
+ }
+
+ private:
+ const gil_state_machine &m_sm;
+};
+
+class double_save_thread : public gil_diagnostic
+{
+ public:
+ double_save_thread (const gil_state_machine &sm, const gcall *call)
+ : gil_diagnostic (sm), m_call (call)
+ {}
+
+ const char *get_kind () const FINAL OVERRIDE
+ {
+ return "double_save_thread";
+ }
+
+ bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE
+ {
+ const double_save_thread &sub_other
+ = (const double_save_thread &)base_other;
+ return m_call == sub_other.m_call;
+ }
+
+ bool emit (rich_location *rich_loc) FINAL OVERRIDE
+ {
+ return warning_at (rich_loc, 0,
+ "nested usage of %qs", "Py_BEGIN_ALLOW_THREADS");
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+ {
+ return ev.formatted_print ("nested usage of %qs here",
+ "Py_BEGIN_ALLOW_THREADS");
+ }
+
+ private:
+ const gcall *m_call;
+};
+
+class fncall_without_gil : public gil_diagnostic
+{
+ public:
+ fncall_without_gil (const gil_state_machine &sm, const gcall *call,
+ tree callee_fndecl, unsigned arg_idx)
+ : gil_diagnostic (sm), m_call (call), m_callee_fndecl (callee_fndecl),
+ m_arg_idx (arg_idx)
+ {}
+
+ const char *get_kind () const FINAL OVERRIDE
+ {
+ return "fncall_without_gil";
+ }
+
+ bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE
+ {
+ const fncall_without_gil &sub_other
+ = (const fncall_without_gil &)base_other;
+ return (m_call == sub_other.m_call
+ && m_callee_fndecl == sub_other.m_callee_fndecl
+ && m_arg_idx == sub_other.m_arg_idx);
+ }
+
+ bool emit (rich_location *rich_loc) FINAL OVERRIDE
+ {
+ auto_diagnostic_group d;
+ /* There isn't a warning ID for use to use. */
+ if (m_callee_fndecl)
+ return warning_at (rich_loc, 0,
+ "use of PyObject as argument %i of %qE"
+ " without the GIL",
+ m_arg_idx + 1, m_callee_fndecl);
+ else
+ return warning_at (rich_loc, 0,
+ "use of PyObject as argument %i of call"
+ " without the GIL",
+ m_arg_idx + 1, m_callee_fndecl);
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+ {
+ if (m_callee_fndecl)
+ return ev.formatted_print ("use of PyObject as argument %i of %qE here"
+ " without the GIL",
+ m_arg_idx + 1, m_callee_fndecl);
+ else
+ return ev.formatted_print ("use of PyObject as argument %i of call here"
+ " without the GIL",
+ m_arg_idx + 1, m_callee_fndecl);
+ }
+
+ private:
+ const gcall *m_call;
+ tree m_callee_fndecl;
+ unsigned m_arg_idx;
+};
+
+class pyobject_usage_without_gil : public gil_diagnostic
+{
+ public:
+ pyobject_usage_without_gil (const gil_state_machine &sm, tree expr)
+ : gil_diagnostic (sm), m_expr (expr)
+ {}
+
+ const char *get_kind () const FINAL OVERRIDE
+ {
+ return "pyobject_usage_without_gil";
+ }
+
+ bool subclass_equal_p (const pending_diagnostic &base_other) const OVERRIDE
+ {
+ return same_tree_p (m_expr,
+ ((const pyobject_usage_without_gil&)base_other).m_expr);
+ }
+
+ bool emit (rich_location *rich_loc) FINAL OVERRIDE
+ {
+ auto_diagnostic_group d;
+ /* There isn't a warning ID for use to use. */
+ return warning_at (rich_loc, 0,
+ "use of PyObject %qE without the GIL", m_expr);
+ }
+
+ label_text describe_final_event (const evdesc::final_event &ev) FINAL OVERRIDE
+ {
+ return ev.formatted_print ("PyObject %qE used here without the GIL",
+ m_expr);
+ }
+
+ private:
+ tree m_expr;
+};
+
+/* gil_state_machine's ctor. */
+
+gil_state_machine::gil_state_machine (logger *logger)
+: state_machine ("gil", logger)
+{
+ m_released_gil = add_state ("released_gil");
+ m_stop = add_state ("stop");
+}
+
+struct cb_data
+{
+ cb_data (const gil_state_machine &sm, sm_context *sm_ctxt,
+ const supernode *snode, const gimple *stmt)
+ : m_sm (sm), m_sm_ctxt (sm_ctxt), m_snode (snode), m_stmt (stmt)
+ {
+ }
+
+ const gil_state_machine &m_sm;
+ sm_context *m_sm_ctxt;
+ const supernode *m_snode;
+ const gimple *m_stmt;
+};
+
+static bool
+check_for_pyobject (gimple *, tree op, tree, void *data)
+{
+ cb_data *d = (cb_data *)data;
+ d->m_sm.check_for_pyobject_usage_without_gil (d->m_sm_ctxt, d->m_snode,
+ d->m_stmt, op);
+ return true;
+}
+
+/* Assuming that the GIL has been released, complain about any
+ PyObject * arguments passed to CALL. */
+
+void
+gil_state_machine::check_for_pyobject_in_call (sm_context *sm_ctxt,
+ const supernode *node,
+ const gcall *call,
+ tree callee_fndecl) const
+{
+ for (unsigned i = 0; i < gimple_call_num_args (call); i++)
+ {
+ tree arg = gimple_call_arg (call, i);
+ if (TREE_CODE (TREE_TYPE (arg)) != POINTER_TYPE)
+ continue;
+ tree type = TREE_TYPE (TREE_TYPE (arg));
+ if (type_based_on_pyobject_p (type))
+ {
+ sm_ctxt->warn (node, call, NULL_TREE,
+ new fncall_without_gil (*this, call,
+ callee_fndecl,
+ i));
+ sm_ctxt->set_global_state (m_stop);
+ }
+ }
+}
+
+/* Implementation of state_machine::on_stmt vfunc for gil_state_machine. */
+
+bool
+gil_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 (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, "PyEval_SaveThread", call, 0))
+ {
+ if (0)
+ inform (input_location, "found call to %qs",
+ "PyEval_SaveThread");
+ if (global_state == m_released_gil)
+ {
+ sm_ctxt->warn (node, stmt, NULL_TREE,
+ new double_save_thread (*this, call));
+ sm_ctxt->set_global_state (m_stop);
+ }
+ else
+ sm_ctxt->set_global_state (m_released_gil);
+ return true;
+ }
+ else if (is_named_call_p (callee_fndecl, "PyEval_RestoreThread",
+ call, 1))
+ {
+ if (0)
+ inform (input_location, "found call to %qs",
+ "PyEval_SaveThread");
+ if (global_state == m_released_gil)
+ sm_ctxt->set_global_state (m_start);
+ return true;
+ }
+ else if (global_state == m_released_gil)
+ {
+ /* Find PyObject * args of calls to fns with unknown bodies. */
+ if (!fndecl_has_gimple_body_p (callee_fndecl))
+ check_for_pyobject_in_call (sm_ctxt, node, call, callee_fndecl);
+ }
+ }
+ else if (global_state == m_released_gil)
+ check_for_pyobject_in_call (sm_ctxt, node, call, NULL);
+ }
+ else
+ if (global_state == m_released_gil)
+ {
+ /* Walk the stmt, finding uses of PyObject (or "subclasses"). */
+ cb_data d (*this, sm_ctxt, node, stmt);
+ walk_stmt_load_store_addr_ops (const_cast <gimple *> (stmt), &d,
+ check_for_pyobject,
+ check_for_pyobject,
+ check_for_pyobject);
+ }
+ return false;
+}
+
+/* Implementation of state_machine::on_condition vfunc for
+ gil_state_machine. */
+
+void
+gil_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
+gil_state_machine::can_purge_p (state_t s ATTRIBUTE_UNUSED) const
+{
+ return true;
+}
+
+void
+gil_state_machine::check_for_pyobject_usage_without_gil (sm_context *sm_ctxt,
+ const supernode *node,
+ const gimple *stmt,
+ tree op) const
+{
+ tree type = TREE_TYPE (op);
+ if (type_based_on_pyobject_p (type))
+ {
+ sm_ctxt->warn (node, stmt, NULL_TREE,
+ new pyobject_usage_without_gil (*this, op));
+ sm_ctxt->set_global_state (m_stop);
+ }
+}
+
+/* Callback handler for the PLUGIN_ANALYZER_INIT event. */
+
+static void
+gil_analyzer_init_cb (void *gcc_data, void */*user_data*/)
+{
+ ana::plugin_analyzer_init_iface *iface
+ = (ana::plugin_analyzer_init_iface *)gcc_data;
+ LOG_SCOPE (iface->get_logger ());
+ if (0)
+ inform (input_location, "got here: gil_analyzer_init_cb");
+ iface->register_state_machine (new gil_state_machine (iface->get_logger ()));
+}
+
+} // namespace ana
+
+#endif /* #if ENABLE_ANALYZER */
+
+int
+plugin_init (struct plugin_name_args *plugin_info,
+ struct plugin_gcc_version *version)
+{
+#if ENABLE_ANALYZER
+ const char *plugin_name = plugin_info->base_name;
+ if (0)
+ inform (input_location, "got here; %qs", plugin_name);
+ register_callback (plugin_info->base_name,
+ PLUGIN_ANALYZER_INIT,
+ ana::gil_analyzer_init_cb,
+ NULL); /* void *user_data */
+#else
+ sorry_no_analyzer ();
+#endif
+ return 0;
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/gil-1.c b/gcc/testsuite/gcc.dg/plugin/gil-1.c
new file mode 100644
index 00000000000..4e8f535ba85
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/gil-1.c
@@ -0,0 +1,90 @@
+/* { dg-do compile } */
+/* { dg-options "-fanalyzer" } */
+
+#include "gil.h"
+
+void test_1 (void)
+{
+ Py_BEGIN_ALLOW_THREADS
+ Py_END_ALLOW_THREADS
+}
+
+void test_2 (PyObject *obj)
+{
+ Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" } */
+
+ Py_INCREF (obj); /* { dg-warning "use of PyObject '\\*\\(obj\\)' without the GIL" } */
+ Py_DECREF (obj);
+
+ Py_END_ALLOW_THREADS
+}
+
+void test_3 (PyObject *obj)
+{
+ Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" } */
+
+ Py_BEGIN_ALLOW_THREADS /* { dg-warning "nested usage of 'Py_BEGIN_ALLOW_THREADS'" } */
+ Py_END_ALLOW_THREADS
+
+ Py_END_ALLOW_THREADS
+}
+
+void test_4 (PyObject *obj)
+{
+ /* These aren't nested, so should be OK. */
+ Py_BEGIN_ALLOW_THREADS
+ Py_END_ALLOW_THREADS
+
+ Py_BEGIN_ALLOW_THREADS
+ Py_END_ALLOW_THREADS
+}
+
+/* Interprocedural example of erroneously nested usage. */
+
+static void __attribute__((noinline))
+called_by_test_5 (void)
+{
+ Py_BEGIN_ALLOW_THREADS /* { dg-warning "nested usage of 'Py_BEGIN_ALLOW_THREADS'" } */
+ Py_END_ALLOW_THREADS
+}
+
+void test_5 (PyObject *obj)
+{
+ Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" } */
+ called_by_test_5 ();
+ Py_END_ALLOW_THREADS
+}
+
+/* Interprocedural example of bogusly using a PyObject outside of GIL. */
+
+static void __attribute__((noinline))
+called_by_test_6 (PyObject *obj)
+{
+ Py_INCREF (obj); /* { dg-warning "use of PyObject '\\*\\(obj\\)' without the GIL" } */
+ Py_DECREF (obj);
+}
+
+void test_6 (PyObject *obj)
+{
+ Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" } */
+ called_by_test_6 (obj);
+ Py_END_ALLOW_THREADS
+}
+
+extern void called_by_test_7 (PyObject *obj);
+
+void test_7 (PyObject *obj)
+{
+ Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" } */
+ called_by_test_7 (obj); /* { dg-warning "use of PyObject as argument 1 of 'called_by_test_7' without the GIL" } */
+ Py_END_ALLOW_THREADS
+}
+
+typedef void (*callback_t) (PyObject *);
+
+void test_8 (PyObject *obj, callback_t cb)
+{
+ Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" } */
+ cb (obj); /* { dg-warning "use of PyObject as argument 1 of call without the GIL" } */
+ Py_END_ALLOW_THREADS
+}
diff --git a/gcc/testsuite/gcc.dg/plugin/gil.h b/gcc/testsuite/gcc.dg/plugin/gil.h
new file mode 100644
index 00000000000..b0610cd749e
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/plugin/gil.h
@@ -0,0 +1,32 @@
+/* Adapted from CPython 3.8's ceval.h. */
+typedef struct PyThreadState PyThreadState;
+extern PyThreadState * PyEval_SaveThread(void);
+extern void PyEval_RestoreThread(PyThreadState *);
+
+#define Py_BEGIN_ALLOW_THREADS { \
+ PyThreadState *_save; \
+ _save = PyEval_SaveThread();
+#define Py_BLOCK_THREADS PyEval_RestoreThread(_save);
+#define Py_UNBLOCK_THREADS _save = PyEval_SaveThread();
+#define Py_END_ALLOW_THREADS PyEval_RestoreThread(_save); \
+ }
+
+/* Adapted/hacked up from CPython 3.8's object.h. */
+
+typedef struct _object {
+ int ob_refcnt;
+} PyObject;
+
+#define _PyObject_CAST(op) ((PyObject*)(op))
+
+extern void _Py_Dealloc(PyObject *);
+
+#define _Py_INCREF(OP) do { (OP)->ob_refcnt++; } while (0);
+#define _Py_DECREF(OP) do { \
+ if (--(OP)->ob_refcnt == 0) { \
+ _Py_Dealloc(OP); \
+ } \
+ } while (0)
+
+#define Py_INCREF(op) _Py_INCREF(_PyObject_CAST(op))
+#define Py_DECREF(op) _Py_DECREF(_PyObject_CAST(op))
diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp b/gcc/testsuite/gcc.dg/plugin/plugin.exp
index 5dd102ae05c..7f0ffd68f8b 100644
--- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
+++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
@@ -118,6 +118,8 @@ set plugin_test_list [list \
{ dump_plugin.c \
dump-1.c \
dump-2.c } \
+ { analyzer_gil_plugin.c \
+ gil-1.c } \
]
foreach plugin_test $plugin_test_list {
--
2.26.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* PING: Re: [PATCH] Add analyzer plugin support and CPython GIL example
2020-10-15 1:21 [PATCH] Add analyzer plugin support and CPython GIL example David Malcolm
@ 2020-11-10 21:51 ` David Malcolm
2020-11-23 21:10 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2020-11-10 21:51 UTC (permalink / raw)
To: gcc-patches
I'd like to ping this patch:
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/556214.html
Are the non-analyzer parts OK for master?
Thanks
Dave
On Wed, 2020-10-14 at 21:21 -0400, David Malcolm wrote:
> This patch adds a new GCC plugin event: PLUGIN_ANALYZER_INIT, called
> when -fanalyzer is starting, allowing for GCC plugins to register
> additional state-machine-based checks within -fanalyzer. The idea
> is that 3rd-party code might want to add domain-specific checks for
> its own APIs - with the caveat that the analyzer is itself still
> rather experimental.
>
> As an example, the patch adds a proof-of-concept plugin to the
> testsuite
> for checking CPython code: verifying that code that relinquishes
> CPython's global interpreter lock doesn't attempt to do anything with
> PyObjects in the sections where the lock isn't held. It also adds a
> warning about nested releases of the lock, which is forbidden.
> For example:
>
> demo.c: In function 'foo':
> demo.c:11:3: warning: use of PyObject '*(obj)' without the GIL
> 11 | Py_INCREF (obj);
> | ^~~~~~~~~
> 'test': events 1-3
> |
> | 15 | void test (PyObject *obj)
> | | ^~~~
> | | |
> | | (1) entry to 'test'
> | 16 | {
> | 17 | Py_BEGIN_ALLOW_THREADS
> | | ~~~~~~~~~~~~~~~~~~~~~~
> | | |
> | | (2) releasing the GIL here
> | 18 | foo (obj);
> | | ~~~~~~~~~
> | | |
> | | (3) calling 'foo' from 'test'
> |
> +--> 'foo': events 4-5
> |
> | 9 | foo (PyObject *obj)
> | | ^~~
> | | |
> | | (4) entry to 'foo'
> | 10 | {
> | 11 | Py_INCREF (obj);
> | | ~~~~~~~~~
> | | |
> | | (5) PyObject '*(obj)' used here without the GIL
> |
>
> Doing so requires adding some logic for ignoring macro expansions in
> analyzer diagnostics, since the insides of Py_INCREF and
> Py_BEGIN_ALLOW_THREADS are not of interest to the user for these
> cases.
>
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
>
> Are the non-analyzer parts OK for master?
>
> gcc/analyzer/ChangeLog:
> * analyzer-pass.cc (pass_analyzer::execute): Move sorry call
> to...
> (sorry_no_analyzer): New.
> * analyzer.h (class state_machine): New forward decl.
> (class logger): New forward decl.
> (class plugin_analyzer_init_iface): New.
> (sorry_no_analyzer): New decl.
> * checker-path.cc (checker_path::fixup_locations): New.
> * checker-path.h (checker_event::set_location): New.
> (checker_path::fixup_locations): New decl.
> * diagnostic-manager.cc
> (diagnostic_manager::emit_saved_diagnostic): Call
> checker_path::fixup_locations, and call fixup_location
> on the primary location.
> * engine.cc: Include "plugin.h".
> (class plugin_analyzer_init_impl): New.
> (impl_run_checkers): Invoke PLUGIN_ANALYZER_INIT callbacks.
> * pending-diagnostic.h (pending_diagnostic::fixup_location):
> New
> vfunc.
>
> gcc/ChangeLog:
> * doc/plugins.texi (Plugin callbacks): Add
> PLUGIN_ANALYZER_INIT.
> * plugin.c (register_callback): Likewise.
> (invoke_plugin_callbacks_full): Likewise.
> * plugin.def (PLUGIN_ANALYZER_INIT): New event.
>
> gcc/testsuite/ChangeLog:
> * gcc.dg/plugin/analyzer_gil_plugin.c: New test.
> * gcc.dg/plugin/gil-1.c: New test.
> * gcc.dg/plugin/gil.h: New header.
> * gcc.dg/plugin/plugin.exp (plugin_test_list): Add the new
> plugin
> and test.
> ---
> gcc/analyzer/analyzer-pass.cc | 18 +-
> gcc/analyzer/analyzer.h | 15 +
> gcc/analyzer/checker-path.cc | 9 +
> gcc/analyzer/checker-path.h | 4 +
> gcc/analyzer/diagnostic-manager.cc | 9 +-
> gcc/analyzer/engine.cc | 31 ++
> gcc/analyzer/pending-diagnostic.h | 8 +
> gcc/doc/plugins.texi | 4 +
> gcc/plugin.c | 2 +
> gcc/plugin.def | 4 +
> .../gcc.dg/plugin/analyzer_gil_plugin.c | 436
> ++++++++++++++++++
> gcc/testsuite/gcc.dg/plugin/gil-1.c | 90 ++++
> gcc/testsuite/gcc.dg/plugin/gil.h | 32 ++
> gcc/testsuite/gcc.dg/plugin/plugin.exp | 2 +
> 14 files changed, 660 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
> create mode 100644 gcc/testsuite/gcc.dg/plugin/gil-1.c
> create mode 100644 gcc/testsuite/gcc.dg/plugin/gil.h
>
> diff --git a/gcc/analyzer/analyzer-pass.cc b/gcc/analyzer/analyzer-
> pass.cc
> index a27421e46d4..1f65bf8b154 100644
> --- a/gcc/analyzer/analyzer-pass.cc
> +++ b/gcc/analyzer/analyzer-pass.cc
> @@ -83,9 +83,7 @@ pass_analyzer::execute (function *)
> #if ENABLE_ANALYZER
> ana::run_checkers ();
> #else
> - sorry ("%qs was not enabled in this build of GCC"
> - " (missing configure-time option %qs)",
> - "-fanalyzer", "--enable-analyzer");
> + sorry_no_analyzer ();
> #endif
>
> return 0;
> @@ -100,3 +98,17 @@ make_pass_analyzer (gcc::context *ctxt)
> {
> return new pass_analyzer (ctxt);
> }
> +
> +#if !ENABLE_ANALYZER
> +
> +/* Issue a "sorry" diagnostic that the analyzer was not enabled. */
> +
> +void
> +sorry_no_analyzer ()
> +{
> + sorry ("%qs was not enabled in this build of GCC"
> + " (missing configure-time option %qs)",
> + "-fanalyzer", "--enable-analyzer");
> +}
> +
> +#endif /* #if !ENABLE_ANALYZER */
> diff --git a/gcc/analyzer/analyzer.h b/gcc/analyzer/analyzer.h
> index aa43b7f66a9..5d01d022f3c 100644
> --- a/gcc/analyzer/analyzer.h
> +++ b/gcc/analyzer/analyzer.h
> @@ -97,6 +97,8 @@ class state_change;
> class rewind_info_t;
>
> class engine;
> +class state_machine;
> +class logger;
>
> /* Forward decls of functions. */
>
> @@ -186,6 +188,15 @@ private:
>
> extern location_t get_stmt_location (const gimple *stmt, function
> *fun);
>
> +/* Passed by pointer to PLUGIN_ANALYZER_INIT callbacks. */
> +
> +class plugin_analyzer_init_iface
> +{
> +public:
> + virtual void register_state_machine (state_machine *) = 0;
> + virtual logger *get_logger () const = 0;
> +};
> +
> } // namespace ana
>
> extern bool is_special_named_call_p (const gcall *call, const char
> *funcname,
> @@ -306,4 +317,8 @@ private:
> #pragma GCC diagnostic ignored "-Wformat-diag"
> #endif
>
> +#if !ENABLE_ANALYZER
> +extern void sorry_no_analyzer ();
> +#endif /* #if !ENABLE_ANALYZER */
> +
> #endif /* GCC_ANALYZER_ANALYZER_H */
> diff --git a/gcc/analyzer/checker-path.cc b/gcc/analyzer/checker-
> path.cc
> index 1f6d6a86001..062730cc789 100644
> --- a/gcc/analyzer/checker-path.cc
> +++ b/gcc/analyzer/checker-path.cc
> @@ -982,6 +982,15 @@ checker_path::add_final_event (const
> state_machine *sm,
> add_event (end_of_path);
> }
>
> +void
> +checker_path::fixup_locations (pending_diagnostic *pd)
> +{
> + checker_event *e;
> + int i;
> + FOR_EACH_VEC_ELT (m_events, i, e)
> + e->set_location (pd->fixup_location (e->get_location ()));
> +}
> +
> } // namespace ana
>
> #endif /* #if ENABLE_ANALYZER */
> diff --git a/gcc/analyzer/checker-path.h b/gcc/analyzer/checker-
> path.h
> index 7b86d48e983..53a7a4efea3 100644
> --- a/gcc/analyzer/checker-path.h
> +++ b/gcc/analyzer/checker-path.h
> @@ -99,6 +99,8 @@ public:
>
> void dump (pretty_printer *pp) const;
>
> + void set_location (location_t loc) { m_loc = loc; }
> +
> public:
> const enum event_kind m_kind;
> protected:
> @@ -498,6 +500,8 @@ public:
> e->prepare_for_emission (this, pd, diagnostic_event_id_t (i));
> }
>
> + void fixup_locations (pending_diagnostic *pd);
> +
> void record_setjmp_event (const exploded_node *enode,
> diagnostic_event_id_t setjmp_emission_id)
> {
> diff --git a/gcc/analyzer/diagnostic-manager.cc
> b/gcc/analyzer/diagnostic-manager.cc
> index cb95a95ff0b..baad14c6c4d 100644
> --- a/gcc/analyzer/diagnostic-manager.cc
> +++ b/gcc/analyzer/diagnostic-manager.cc
> @@ -648,7 +648,14 @@ diagnostic_manager::emit_saved_diagnostic (const
> exploded_graph &eg,
>
> emission_path.prepare_for_emission (sd.m_d);
>
> - gcc_rich_location rich_loc (get_stmt_location (stmt, sd.m_snode-
> >m_fun));
> + location_t loc = get_stmt_location (stmt, sd.m_snode->m_fun);
> +
> + /* Allow the pending_diagnostic to fix up the primary location
> + and any locations for events. */
> + loc = sd.m_d->fixup_location (loc);
> + emission_path.fixup_locations (sd.m_d);
> +
> + gcc_rich_location rich_loc (loc);
> rich_loc.set_path (&emission_path);
>
> auto_diagnostic_group d;
> diff --git a/gcc/analyzer/engine.cc b/gcc/analyzer/engine.cc
> index 65d7495f26f..de772c77b2a 100644
> --- a/gcc/analyzer/engine.cc
> +++ b/gcc/analyzer/engine.cc
> @@ -63,6 +63,7 @@ along with GCC; see the file COPYING3. If not see
> #include "analyzer/state-purge.h"
> #include "analyzer/bar-chart.h"
> #include <zlib.h>
> +#include "plugin.h"
>
> /* For an overview, see gcc/doc/analyzer.texi. */
>
> @@ -4510,6 +4511,33 @@ dump_analyzer_json (const supergraph &sg,
> free (filename);
> }
>
> +/* Concrete subclass of plugin_analyzer_init_iface, allowing plugins
> + to register new state machines. */
> +
> +class plugin_analyzer_init_impl : public plugin_analyzer_init_iface
> +{
> +public:
> + plugin_analyzer_init_impl (auto_delete_vec <state_machine>
> *checkers,
> + logger *logger)
> + : m_checkers (checkers),
> + m_logger (logger)
> + {}
> +
> + void register_state_machine (state_machine *sm) FINAL OVERRIDE
> + {
> + m_checkers->safe_push (sm);
> + }
> +
> + logger *get_logger () const FINAL OVERRIDE
> + {
> + return m_logger;
> + }
> +
> +private:
> + auto_delete_vec <state_machine> *m_checkers;
> + logger *m_logger;
> +};
> +
> /* Run the analysis "engine". */
>
> void
> @@ -4555,6 +4583,9 @@ impl_run_checkers (logger *logger)
> auto_delete_vec <state_machine> checkers;
> make_checkers (checkers, logger);
>
> + plugin_analyzer_init_impl data (&checkers, logger);
> + invoke_plugin_callbacks (PLUGIN_ANALYZER_INIT, &data);
> +
> if (logger)
> {
> int i;
> diff --git a/gcc/analyzer/pending-diagnostic.h
> b/gcc/analyzer/pending-diagnostic.h
> index b1ff2688bcc..fa6c9781044 100644
> --- a/gcc/analyzer/pending-diagnostic.h
> +++ b/gcc/analyzer/pending-diagnostic.h
> @@ -175,6 +175,14 @@ class pending_diagnostic
> diagnostic deduplication. */
> static bool same_tree_p (tree t1, tree t2);
>
> + /* A vfunc for fixing up locations (both the primary location for
> the
> + diagnostic, and for events in their paths), e.g. to avoid
> unwinding
> + inside specific macros. */
> + virtual location_t fixup_location (location_t loc) const
> + {
> + return loc;
> + }
> +
> /* For greatest precision-of-wording, the various following
> "describe_*"
> virtual functions give the pending diagnostic a way to describe
> events
> in a diagnostic_path in terms that make sense for that
> diagnostic.
> diff --git a/gcc/doc/plugins.texi b/gcc/doc/plugins.texi
> index 23a8b410ebb..1c211bbacf1 100644
> --- a/gcc/doc/plugins.texi
> +++ b/gcc/doc/plugins.texi
> @@ -218,6 +218,10 @@ enum plugin_event
> as a const char* pointer. */
> PLUGIN_INCLUDE_FILE,
>
> + /* Called when -fanalyzer starts. The event data is an
> + ana::plugin_analyzer_init_iface *. */
> + PLUGIN_ANALYZER_INIT,
> +
> PLUGIN_EVENT_FIRST_DYNAMIC /* Dummy event used for indexing
> callback
> array. */
> @};
> diff --git a/gcc/plugin.c b/gcc/plugin.c
> index 69b6f5b8b6a..76069e64798 100644
> --- a/gcc/plugin.c
> +++ b/gcc/plugin.c
> @@ -497,6 +497,7 @@ register_callback (const char *plugin_name,
> case PLUGIN_EARLY_GIMPLE_PASSES_END:
> case PLUGIN_NEW_PASS:
> case PLUGIN_INCLUDE_FILE:
> + case PLUGIN_ANALYZER_INIT:
> {
> struct callback_info *new_callback;
> if (!callback)
> @@ -577,6 +578,7 @@ invoke_plugin_callbacks_full (int event, void
> *gcc_data)
> case PLUGIN_EARLY_GIMPLE_PASSES_END:
> case PLUGIN_NEW_PASS:
> case PLUGIN_INCLUDE_FILE:
> + case PLUGIN_ANALYZER_INIT:
> {
> /* Iterate over every callback registered with this event
> and
> call it. */
> diff --git a/gcc/plugin.def b/gcc/plugin.def
> index 3482ed71906..b8110190d3a 100644
> --- a/gcc/plugin.def
> +++ b/gcc/plugin.def
> @@ -99,6 +99,10 @@ DEFEVENT (PLUGIN_NEW_PASS)
> as a const char* pointer. */
> DEFEVENT (PLUGIN_INCLUDE_FILE)
>
> +/* Called when -fanalyzer starts. The event data is an
> + ana::plugin_analyzer_init_iface *. */
> +DEFEVENT (PLUGIN_ANALYZER_INIT)
> +
> /* When adding a new hard-coded plugin event, don't forget to edit
> in
> file plugin.c the functions register_callback and
> invoke_plugin_callbacks_full accordingly! */
> diff --git a/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
> b/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
> new file mode 100644
> index 00000000000..05133d5250e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/plugin/analyzer_gil_plugin.c
> @@ -0,0 +1,436 @@
> +/* Proof-of-concept of a -fanalyzer plugin.
> + Detect (some) uses of CPython API outside of the Global
> Interpreter Lock.
> +
> https://docs.python.org/3/c-api/init.html#thread-state-and-the-global-interpreter-lock
> +*/
> +/* { dg-options "-g" } */
> +
> +#include "gcc-plugin.h"
> +#include "config.h"
> +#include "system.h"
> +#include "coretypes.h"
> +#include "diagnostic.h"
> +#include "tree.h"
> +#include "gimple.h"
> +#include "gimple-iterator.h"
> +#include "gimple-walk.h"
> +#include "diagnostic-event-id.h"
> +#include "analyzer/analyzer.h"
> +#include "analyzer/analyzer-logging.h"
> +#include "json.h"
> +#include "analyzer/sm.h"
> +#include "analyzer/pending-diagnostic.h"
> +
> +int plugin_is_GPL_compatible;
> +
> +#if ENABLE_ANALYZER
> +
> +namespace ana {
> +
> +static bool
> +type_based_on_pyobject_p (tree type)
> +{
> + /* Ideally we'd also check for "subclasses" here by iterating up
> the
> + first field of each struct. */
> + if (TREE_CODE (type) != RECORD_TYPE)
> + return false;
> + tree name = TYPE_IDENTIFIER (type);
> + if (!name)
> + return false;
> + return id_equal (name, "PyObject");
> +}
> +
> +/* An experimental state machine, for tracking whether the GIL is
> held,
> + as global state.. */
> +
> +class gil_state_machine : public state_machine
> +{
> +public:
> + gil_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;
> +
> + void check_for_pyobject_usage_without_gil (sm_context *sm_ctxt,
> + const supernode *node,
> + const gimple *stmt,
> + tree op) const;
> +
> + private:
> + void check_for_pyobject_in_call (sm_context *sm_ctxt,
> + const supernode *node,
> + const gcall *call,
> + tree callee_fndecl) const;
> +
> + public:
> + /* These states are "global", rather than per-expression. */
> +
> + /* State for when we've released the GIL. */
> + state_t m_released_gil;
> +
> + /* Stop state. */
> + state_t m_stop;
> +};
> +
> +/* Subclass for diagnostics involving the GIL. */
> +
> +class gil_diagnostic : public pending_diagnostic
> +{
> +public:
> + location_t fixup_location (location_t loc) const FINAL OVERRIDE
> + {
> + /* Ideally we'd check for specific macros here, and only
> + resolve certain macros. */
> + if (linemap_location_from_macro_expansion_p (line_table, loc))
> + loc = linemap_resolve_location (line_table, loc,
> + LRK_MACRO_EXPANSION_POINT, NULL);
> + return loc;
> + }
> +
> + label_text describe_state_change (const evdesc::state_change
> &change)
> + FINAL OVERRIDE
> + {
> + if (change.is_global_p ()
> + && change.m_new_state == m_sm.m_released_gil)
> + return change.formatted_print ("releasing the GIL here");
> + if (change.is_global_p ()
> + && change.m_new_state == m_sm.get_start_state ())
> + return change.formatted_print ("acquiring the GIL here");
> + return label_text ();
> + }
> +
> + protected:
> + gil_diagnostic (const gil_state_machine &sm) : m_sm (sm)
> + {
> + }
> +
> + private:
> + const gil_state_machine &m_sm;
> +};
> +
> +class double_save_thread : public gil_diagnostic
> +{
> + public:
> + double_save_thread (const gil_state_machine &sm, const gcall
> *call)
> + : gil_diagnostic (sm), m_call (call)
> + {}
> +
> + const char *get_kind () const FINAL OVERRIDE
> + {
> + return "double_save_thread";
> + }
> +
> + bool subclass_equal_p (const pending_diagnostic &base_other) const
> OVERRIDE
> + {
> + const double_save_thread &sub_other
> + = (const double_save_thread &)base_other;
> + return m_call == sub_other.m_call;
> + }
> +
> + bool emit (rich_location *rich_loc) FINAL OVERRIDE
> + {
> + return warning_at (rich_loc, 0,
> + "nested usage of %qs",
> "Py_BEGIN_ALLOW_THREADS");
> + }
> +
> + label_text describe_final_event (const evdesc::final_event &ev)
> FINAL OVERRIDE
> + {
> + return ev.formatted_print ("nested usage of %qs here",
> + "Py_BEGIN_ALLOW_THREADS");
> + }
> +
> + private:
> + const gcall *m_call;
> +};
> +
> +class fncall_without_gil : public gil_diagnostic
> +{
> + public:
> + fncall_without_gil (const gil_state_machine &sm, const gcall
> *call,
> + tree callee_fndecl, unsigned arg_idx)
> + : gil_diagnostic (sm), m_call (call), m_callee_fndecl
> (callee_fndecl),
> + m_arg_idx (arg_idx)
> + {}
> +
> + const char *get_kind () const FINAL OVERRIDE
> + {
> + return "fncall_without_gil";
> + }
> +
> + bool subclass_equal_p (const pending_diagnostic &base_other) const
> OVERRIDE
> + {
> + const fncall_without_gil &sub_other
> + = (const fncall_without_gil &)base_other;
> + return (m_call == sub_other.m_call
> + && m_callee_fndecl == sub_other.m_callee_fndecl
> + && m_arg_idx == sub_other.m_arg_idx);
> + }
> +
> + bool emit (rich_location *rich_loc) FINAL OVERRIDE
> + {
> + auto_diagnostic_group d;
> + /* There isn't a warning ID for use to use. */
> + if (m_callee_fndecl)
> + return warning_at (rich_loc, 0,
> + "use of PyObject as argument %i of %qE"
> + " without the GIL",
> + m_arg_idx + 1, m_callee_fndecl);
> + else
> + return warning_at (rich_loc, 0,
> + "use of PyObject as argument %i of call"
> + " without the GIL",
> + m_arg_idx + 1, m_callee_fndecl);
> + }
> +
> + label_text describe_final_event (const evdesc::final_event &ev)
> FINAL OVERRIDE
> + {
> + if (m_callee_fndecl)
> + return ev.formatted_print ("use of PyObject as argument %i of
> %qE here"
> + " without the GIL",
> + m_arg_idx + 1, m_callee_fndecl);
> + else
> + return ev.formatted_print ("use of PyObject as argument %i of
> call here"
> + " without the GIL",
> + m_arg_idx + 1, m_callee_fndecl);
> + }
> +
> + private:
> + const gcall *m_call;
> + tree m_callee_fndecl;
> + unsigned m_arg_idx;
> +};
> +
> +class pyobject_usage_without_gil : public gil_diagnostic
> +{
> + public:
> + pyobject_usage_without_gil (const gil_state_machine &sm, tree
> expr)
> + : gil_diagnostic (sm), m_expr (expr)
> + {}
> +
> + const char *get_kind () const FINAL OVERRIDE
> + {
> + return "pyobject_usage_without_gil";
> + }
> +
> + bool subclass_equal_p (const pending_diagnostic &base_other) const
> OVERRIDE
> + {
> + return same_tree_p (m_expr,
> + ((const
> pyobject_usage_without_gil&)base_other).m_expr);
> + }
> +
> + bool emit (rich_location *rich_loc) FINAL OVERRIDE
> + {
> + auto_diagnostic_group d;
> + /* There isn't a warning ID for use to use. */
> + return warning_at (rich_loc, 0,
> + "use of PyObject %qE without the GIL", m_expr);
> + }
> +
> + label_text describe_final_event (const evdesc::final_event &ev)
> FINAL OVERRIDE
> + {
> + return ev.formatted_print ("PyObject %qE used here without the
> GIL",
> + m_expr);
> + }
> +
> + private:
> + tree m_expr;
> +};
> +
> +/* gil_state_machine's ctor. */
> +
> +gil_state_machine::gil_state_machine (logger *logger)
> +: state_machine ("gil", logger)
> +{
> + m_released_gil = add_state ("released_gil");
> + m_stop = add_state ("stop");
> +}
> +
> +struct cb_data
> +{
> + cb_data (const gil_state_machine &sm, sm_context *sm_ctxt,
> + const supernode *snode, const gimple *stmt)
> + : m_sm (sm), m_sm_ctxt (sm_ctxt), m_snode (snode), m_stmt (stmt)
> + {
> + }
> +
> + const gil_state_machine &m_sm;
> + sm_context *m_sm_ctxt;
> + const supernode *m_snode;
> + const gimple *m_stmt;
> +};
> +
> +static bool
> +check_for_pyobject (gimple *, tree op, tree, void *data)
> +{
> + cb_data *d = (cb_data *)data;
> + d->m_sm.check_for_pyobject_usage_without_gil (d->m_sm_ctxt, d-
> >m_snode,
> + d->m_stmt, op);
> + return true;
> +}
> +
> +/* Assuming that the GIL has been released, complain about any
> + PyObject * arguments passed to CALL. */
> +
> +void
> +gil_state_machine::check_for_pyobject_in_call (sm_context *sm_ctxt,
> + const supernode *node,
> + const gcall *call,
> + tree callee_fndecl)
> const
> +{
> + for (unsigned i = 0; i < gimple_call_num_args (call); i++)
> + {
> + tree arg = gimple_call_arg (call, i);
> + if (TREE_CODE (TREE_TYPE (arg)) != POINTER_TYPE)
> + continue;
> + tree type = TREE_TYPE (TREE_TYPE (arg));
> + if (type_based_on_pyobject_p (type))
> + {
> + sm_ctxt->warn (node, call, NULL_TREE,
> + new fncall_without_gil (*this, call,
> + callee_fndecl,
> + i));
> + sm_ctxt->set_global_state (m_stop);
> + }
> + }
> +}
> +
> +/* Implementation of state_machine::on_stmt vfunc for
> gil_state_machine. */
> +
> +bool
> +gil_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 (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, "PyEval_SaveThread",
> call, 0))
> + {
> + if (0)
> + inform (input_location, "found call to %qs",
> + "PyEval_SaveThread");
> + if (global_state == m_released_gil)
> + {
> + sm_ctxt->warn (node, stmt, NULL_TREE,
> + new double_save_thread (*this, call));
> + sm_ctxt->set_global_state (m_stop);
> + }
> + else
> + sm_ctxt->set_global_state (m_released_gil);
> + return true;
> + }
> + else if (is_named_call_p (callee_fndecl,
> "PyEval_RestoreThread",
> + call, 1))
> + {
> + if (0)
> + inform (input_location, "found call to %qs",
> + "PyEval_SaveThread");
> + if (global_state == m_released_gil)
> + sm_ctxt->set_global_state (m_start);
> + return true;
> + }
> + else if (global_state == m_released_gil)
> + {
> + /* Find PyObject * args of calls to fns with unknown
> bodies. */
> + if (!fndecl_has_gimple_body_p (callee_fndecl))
> + check_for_pyobject_in_call (sm_ctxt, node, call,
> callee_fndecl);
> + }
> + }
> + else if (global_state == m_released_gil)
> + check_for_pyobject_in_call (sm_ctxt, node, call, NULL);
> + }
> + else
> + if (global_state == m_released_gil)
> + {
> + /* Walk the stmt, finding uses of PyObject (or
> "subclasses"). */
> + cb_data d (*this, sm_ctxt, node, stmt);
> + walk_stmt_load_store_addr_ops (const_cast <gimple *> (stmt),
> &d,
> + check_for_pyobject,
> + check_for_pyobject,
> + check_for_pyobject);
> + }
> + return false;
> +}
> +
> +/* Implementation of state_machine::on_condition vfunc for
> + gil_state_machine. */
> +
> +void
> +gil_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
> +gil_state_machine::can_purge_p (state_t s ATTRIBUTE_UNUSED) const
> +{
> + return true;
> +}
> +
> +void
> +gil_state_machine::check_for_pyobject_usage_without_gil (sm_context
> *sm_ctxt,
> + const
> supernode *node,
> + const gimple
> *stmt,
> + tree op) const
> +{
> + tree type = TREE_TYPE (op);
> + if (type_based_on_pyobject_p (type))
> + {
> + sm_ctxt->warn (node, stmt, NULL_TREE,
> + new pyobject_usage_without_gil (*this, op));
> + sm_ctxt->set_global_state (m_stop);
> + }
> +}
> +
> +/* Callback handler for the PLUGIN_ANALYZER_INIT event. */
> +
> +static void
> +gil_analyzer_init_cb (void *gcc_data, void */*user_data*/)
> +{
> + ana::plugin_analyzer_init_iface *iface
> + = (ana::plugin_analyzer_init_iface *)gcc_data;
> + LOG_SCOPE (iface->get_logger ());
> + if (0)
> + inform (input_location, "got here: gil_analyzer_init_cb");
> + iface->register_state_machine (new gil_state_machine (iface-
> >get_logger ()));
> +}
> +
> +} // namespace ana
> +
> +#endif /* #if ENABLE_ANALYZER */
> +
> +int
> +plugin_init (struct plugin_name_args *plugin_info,
> + struct plugin_gcc_version *version)
> +{
> +#if ENABLE_ANALYZER
> + const char *plugin_name = plugin_info->base_name;
> + if (0)
> + inform (input_location, "got here; %qs", plugin_name);
> + register_callback (plugin_info->base_name,
> + PLUGIN_ANALYZER_INIT,
> + ana::gil_analyzer_init_cb,
> + NULL); /* void *user_data */
> +#else
> + sorry_no_analyzer ();
> +#endif
> + return 0;
> +}
> diff --git a/gcc/testsuite/gcc.dg/plugin/gil-1.c
> b/gcc/testsuite/gcc.dg/plugin/gil-1.c
> new file mode 100644
> index 00000000000..4e8f535ba85
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/plugin/gil-1.c
> @@ -0,0 +1,90 @@
> +/* { dg-do compile } */
> +/* { dg-options "-fanalyzer" } */
> +
> +#include "gil.h"
> +
> +void test_1 (void)
> +{
> + Py_BEGIN_ALLOW_THREADS
> + Py_END_ALLOW_THREADS
> +}
> +
> +void test_2 (PyObject *obj)
> +{
> + Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" }
> */
> +
> + Py_INCREF (obj); /* { dg-warning "use of PyObject '\\*\\(obj\\)'
> without the GIL" } */
> + Py_DECREF (obj);
> +
> + Py_END_ALLOW_THREADS
> +}
> +
> +void test_3 (PyObject *obj)
> +{
> + Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" }
> */
> +
> + Py_BEGIN_ALLOW_THREADS /* { dg-warning "nested usage of
> 'Py_BEGIN_ALLOW_THREADS'" } */
> + Py_END_ALLOW_THREADS
> +
> + Py_END_ALLOW_THREADS
> +}
> +
> +void test_4 (PyObject *obj)
> +{
> + /* These aren't nested, so should be OK. */
> + Py_BEGIN_ALLOW_THREADS
> + Py_END_ALLOW_THREADS
> +
> + Py_BEGIN_ALLOW_THREADS
> + Py_END_ALLOW_THREADS
> +}
> +
> +/* Interprocedural example of erroneously nested usage. */
> +
> +static void __attribute__((noinline))
> +called_by_test_5 (void)
> +{
> + Py_BEGIN_ALLOW_THREADS /* { dg-warning "nested usage of
> 'Py_BEGIN_ALLOW_THREADS'" } */
> + Py_END_ALLOW_THREADS
> +}
> +
> +void test_5 (PyObject *obj)
> +{
> + Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" }
> */
> + called_by_test_5 ();
> + Py_END_ALLOW_THREADS
> +}
> +
> +/* Interprocedural example of bogusly using a PyObject outside of
> GIL. */
> +
> +static void __attribute__((noinline))
> +called_by_test_6 (PyObject *obj)
> +{
> + Py_INCREF (obj); /* { dg-warning "use of PyObject '\\*\\(obj\\)'
> without the GIL" } */
> + Py_DECREF (obj);
> +}
> +
> +void test_6 (PyObject *obj)
> +{
> + Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" }
> */
> + called_by_test_6 (obj);
> + Py_END_ALLOW_THREADS
> +}
> +
> +extern void called_by_test_7 (PyObject *obj);
> +
> +void test_7 (PyObject *obj)
> +{
> + Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" }
> */
> + called_by_test_7 (obj); /* { dg-warning "use of PyObject as
> argument 1 of 'called_by_test_7' without the GIL" } */
> + Py_END_ALLOW_THREADS
> +}
> +
> +typedef void (*callback_t) (PyObject *);
> +
> +void test_8 (PyObject *obj, callback_t cb)
> +{
> + Py_BEGIN_ALLOW_THREADS /* { dg-message "releasing the GIL here" }
> */
> + cb (obj); /* { dg-warning "use of PyObject as argument 1 of call
> without the GIL" } */
> + Py_END_ALLOW_THREADS
> +}
> diff --git a/gcc/testsuite/gcc.dg/plugin/gil.h
> b/gcc/testsuite/gcc.dg/plugin/gil.h
> new file mode 100644
> index 00000000000..b0610cd749e
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/plugin/gil.h
> @@ -0,0 +1,32 @@
> +/* Adapted from CPython 3.8's ceval.h. */
> +typedef struct PyThreadState PyThreadState;
> +extern PyThreadState * PyEval_SaveThread(void);
> +extern void PyEval_RestoreThread(PyThreadState *);
> +
> +#define Py_BEGIN_ALLOW_THREADS { \
> + PyThreadState *_save; \
> + _save = PyEval_SaveThread();
> +#define Py_BLOCK_THREADS PyEval_RestoreThread(_save);
> +#define Py_UNBLOCK_THREADS _save = PyEval_SaveThread();
> +#define Py_END_ALLOW_THREADS PyEval_RestoreThread(_save); \
> + }
> +
> +/* Adapted/hacked up from CPython 3.8's object.h. */
> +
> +typedef struct _object {
> + int ob_refcnt;
> +} PyObject;
> +
> +#define _PyObject_CAST(op) ((PyObject*)(op))
> +
> +extern void _Py_Dealloc(PyObject *);
> +
> +#define _Py_INCREF(OP) do { (OP)->ob_refcnt++; } while (0);
> +#define _Py_DECREF(OP) do { \
> + if (--(OP)->ob_refcnt == 0) { \
> + _Py_Dealloc(OP); \
> + } \
> + } while (0)
> +
> +#define Py_INCREF(op) _Py_INCREF(_PyObject_CAST(op))
> +#define Py_DECREF(op) _Py_DECREF(_PyObject_CAST(op))
> diff --git a/gcc/testsuite/gcc.dg/plugin/plugin.exp
> b/gcc/testsuite/gcc.dg/plugin/plugin.exp
> index 5dd102ae05c..7f0ffd68f8b 100644
> --- a/gcc/testsuite/gcc.dg/plugin/plugin.exp
> +++ b/gcc/testsuite/gcc.dg/plugin/plugin.exp
> @@ -118,6 +118,8 @@ set plugin_test_list [list \
> { dump_plugin.c \
> dump-1.c \
> dump-2.c } \
> + { analyzer_gil_plugin.c \
> + gil-1.c } \
> ]
>
> foreach plugin_test $plugin_test_list {
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: PING: Re: [PATCH] Add analyzer plugin support and CPython GIL example
2020-11-10 21:51 ` PING: " David Malcolm
@ 2020-11-23 21:10 ` Jeff Law
2020-11-30 21:19 ` [PATCH] Unbreak build with --disable-analyzer David Malcolm
0 siblings, 1 reply; 6+ messages in thread
From: Jeff Law @ 2020-11-23 21:10 UTC (permalink / raw)
To: David Malcolm, gcc-patches
On 11/10/20 2:51 PM, David Malcolm via Gcc-patches wrote:
> I'd like to ping this patch:
> https://gcc.gnu.org/pipermail/gcc-patches/2020-October/556214.html
> Are the non-analyzer parts OK for master?
Just looking for an ACK on the analyzer_init notification bits in the
generic parts of the compiler? If so, yea, that's fine. Sorry for the
delay.
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] Unbreak build with --disable-analyzer
2020-11-23 21:10 ` Jeff Law
@ 2020-11-30 21:19 ` David Malcolm
2020-11-30 22:39 ` David Malcolm
0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2020-11-30 21:19 UTC (permalink / raw)
To: gcc-patches
I broke the build with --disable-analyzer with
g:66dde7bc64b75d4a338266333c9c490b12d49825, due to:
../../src/gcc/analyzer/analyzer-pass.cc: In member function 'virtual unsigned int {anonymous}::pass_analyzer::execute(function*)':
../../src/gcc/analyzer/analyzer-pass.cc:86:3: error: 'sorry_no_analyzer' was not declared in this scope
86 | sorry_no_analyzer ();
| ^~~~~~~~~~~~~~~~~
Fixed by including the relevant header file.
Successfully built stage 1 compiler with and without --disable-analyzer;
full bootstrap in progress; I'll commit it if/when that succeeds.
Spotted by Jeff [CCed]
Sorry about the breakage.
gcc/analyzer/ChangeLog:
* analyzer-pass.cc: Include "analyzer/analyzer.h" for the
declaration of sorry_no_analyzer; include "tree.h" and
"function.h" as these are needed by it.
---
gcc/analyzer/analyzer-pass.cc | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gcc/analyzer/analyzer-pass.cc b/gcc/analyzer/analyzer-pass.cc
index 1f65bf8b154..333f87b7897 100644
--- a/gcc/analyzer/analyzer-pass.cc
+++ b/gcc/analyzer/analyzer-pass.cc
@@ -25,6 +25,9 @@ along with GCC; see the file COPYING3. If not see
#include "tree-pass.h"
#include "diagnostic.h"
#include "options.h"
+#include "tree.h"
+#include "function.h"
+#include "analyzer/analyzer.h"
#include "analyzer/engine.h"
namespace {
--
2.26.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Unbreak build with --disable-analyzer
2020-11-30 21:19 ` [PATCH] Unbreak build with --disable-analyzer David Malcolm
@ 2020-11-30 22:39 ` David Malcolm
2020-11-30 22:41 ` Jeff Law
0 siblings, 1 reply; 6+ messages in thread
From: David Malcolm @ 2020-11-30 22:39 UTC (permalink / raw)
To: gcc-patches
On Mon, 2020-11-30 at 16:19 -0500, David Malcolm wrote:
> I broke the build with --disable-analyzer with
> g:66dde7bc64b75d4a338266333c9c490b12d49825, due to:
>
> ../../src/gcc/analyzer/analyzer-pass.cc: In member function 'virtual
> unsigned int {anonymous}::pass_analyzer::execute(function*)':
> ../../src/gcc/analyzer/analyzer-pass.cc:86:3: error:
> 'sorry_no_analyzer' was not declared in this scope
> 86 | sorry_no_analyzer ();
> | ^~~~~~~~~~~~~~~~~
>
> Fixed by including the relevant header file.
>
> Successfully built stage 1 compiler with and without --disable-
> analyzer;
> full bootstrap in progress; I'll commit it if/when that succeeds.
The full bootstrap succeeded, so I've pushed this.
Dave
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Unbreak build with --disable-analyzer
2020-11-30 22:39 ` David Malcolm
@ 2020-11-30 22:41 ` Jeff Law
0 siblings, 0 replies; 6+ messages in thread
From: Jeff Law @ 2020-11-30 22:41 UTC (permalink / raw)
To: David Malcolm, gcc-patches
On 11/30/20 3:39 PM, David Malcolm wrote:
> On Mon, 2020-11-30 at 16:19 -0500, David Malcolm wrote:
>> I broke the build with --disable-analyzer with
>> g:66dde7bc64b75d4a338266333c9c490b12d49825, due to:
>>
>> ../../src/gcc/analyzer/analyzer-pass.cc: In member function 'virtual
>> unsigned int {anonymous}::pass_analyzer::execute(function*)':
>> ../../src/gcc/analyzer/analyzer-pass.cc:86:3: error:
>> 'sorry_no_analyzer' was not declared in this scope
>> 86 | sorry_no_analyzer ();
>> | ^~~~~~~~~~~~~~~~~
>>
>> Fixed by including the relevant header file.
>>
>> Successfully built stage 1 compiler with and without --disable-
>> analyzer;
>> full bootstrap in progress; I'll commit it if/when that succeeds.
> The full bootstrap succeeded, so I've pushed this.
Thanks. I'll un-pause the tester and resubmit the failed builds ;-)
jeff
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-11-30 22:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-15 1:21 [PATCH] Add analyzer plugin support and CPython GIL example David Malcolm
2020-11-10 21:51 ` PING: " David Malcolm
2020-11-23 21:10 ` Jeff Law
2020-11-30 21:19 ` [PATCH] Unbreak build with --disable-analyzer David Malcolm
2020-11-30 22:39 ` David Malcolm
2020-11-30 22:41 ` Jeff Law
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).