In my PyPy libgccjit experiments I managed to get various crashes deep inside the compile (when expanding gimple -> RTL).
Investigation showed that I was erroneously using params and locals from one function in an entirely different function. The API checks individual rvalues and the tree-like structures they make as they are constructed, but was not checking rvalue trees within a specific statement (which are within a particular block and hence within a particular function). This patch adds validation of rvalues from the POV of statements. It checks that every rvalue that has a "scope" is within the correct scope when used within a statement. This has to be checked for all subexpressions within an rvalue (e.g. the two rvalues making up a binary_op), so doing so requires implementing a way of walking the subexpressions of the recording::rvalue subclasses. This is done by adding a "visit_children" virtual function to recording::rvalue, which calls into a new rvalue_visitor abstract class, with a concrete rvalue_usage_validator implementation of the latter to do the actual checking. In an earlier version of the patch the checking was done during pre-compilation validation in function::validate, by walking the statements in each block, validating the expression trees within them. I then realized that this validation could be moved much earlier, to the API entrypoints that create the statements themselves. It's better to generate an error as close as possible to the point of failure, so this version of the patch does the validation there (after the stmts are created, so we can use get_debug_string on them to get more readable error messages). This takes jit.sum's # of expected passes from 7272 to 7362. Committed to trunk as r219498. gcc/jit/ChangeLog: * jit-recording.c (class gcc::jit::rvalue_usage_validator): New. (gcc::jit::rvalue_usage_validator::rvalue_usage_validator): New ctor. (gcc::jit::rvalue_usage_validator::visit): New function. (gcc::jit::recording::rvalue::verify_valid_within_stmt): New function. (gcc::jit::recording::rvalue::set_scope): New function. (gcc::jit::recording::function::function): Call set_scope on each param, issuing errors for any params that already have a function. (gcc::jit::recording::block::add_eval): Return the new statement; update the comment given that some error-checking now happens after this returns. (gcc::jit::recording::block::add_assignment): Likewise. (gcc::jit::recording::block::add_assignment_op): Likewise. (gcc::jit::recording::block::add_comment): Likewise. (gcc::jit::recording::block::end_with_conditional): Likewise. (gcc::jit::recording::block::end_with_jump): Likewise. (gcc::jit::recording::block::end_with_return): Likewise. (gcc::jit::recording::block::validate): Add a comment. (gcc::jit::recording::unary_op::visit_children): New function. (gcc::jit::recording::binary_op::visit_children): New function. (gcc::jit::recording::comparison::visit_children): New function. (gcc::jit::recording::cast::visit_children): New function. (gcc::jit::recording::call::visit_children): New function. (gcc::jit::recording::call_through_ptr::visit_children): New function. (gcc::jit::recording::array_access::visit_children): New function. (gcc::jit::recording::access_field_of_lvalue::visit_children): New function. (gcc::jit::recording::access_field_rvalue::visit_children): New function. (gcc::jit::recording::dereference_field_rvalue::visit_children): New function. (gcc::jit::recording::dereference_rvalue::visit_children): New function. (gcc::jit::recording::get_address_of_lvalue::visit_children): New function. * jit-recording.h: Within namespace gcc::jit::recording... (class rvalue_visitor): New. (rvalue::rvalue): Initialize m_scope. (rvalue::get_loc): New accessor. (rvalue::verify_valid_within_stmt): New function. (rvalue::visit_children): New pure virtual function. (rvalue::set_scope): New function. (rvalue::get_scope): New function. (rvalue::dyn_cast_param): New function. (rvalue::m_scope): New field. (param::visit_children): New empty function. (param::dyn_cast_param): New function. (function::get_loc): New function. (block::add_eval): Return the new statement. (block::add_assignment): Likewise. (block::add_assignment_op): Likewise. (block::add_comment): Likewise. (block::end_with_conditional): Likewise. (block::end_with_jump): Likewise. (block::end_with_return): Likewise. (global::visit_children): New function. (memento_of_new_rvalue_from_const<HOST_TYPE>::visit_children): New function. (memento_of_new_string_literal::visit_children): New function. (unary_op::visit_children): New function. (binary_op::visit_children): New function. (comparison::visit_children): New function. (cast::visit_children): New function. (call::visit_children): New function. (call_through_ptr::visit_children): New function. (array_access::visit_children): New function. (access_field_of_lvalue::visit_children): New function. (access_field_rvalue::visit_children): New function. (dereference_field_rvalue::visit_children): New function. (dereference_rvalue::visit_children): New function. (get_address_of_lvalue::visit_children): New function. (local::local): Call set_scope. (local::visit_children): New function. (statement::get_block): Make public. * libgccjit.c (RETURN_VAL_IF_FAIL_PRINTF5): New macro. (RETURN_NULL_IF_FAIL_PRINTF5): New macro. (gcc_jit_context_new_function): Verify that each param has not yet been used for creating another function. (gcc_jit_block_add_eval): After creating the stmt, verify that the rvalue expression tree is valid to use within it. (gcc_jit_block_add_assignment): Likewise for the lvalue and rvalue expression trees. (gcc_jit_block_add_assignment_op): Likewise. (gcc_jit_block_end_with_conditional): Likewise for the boolval expression tree. (gcc_jit_block_end_with_return): Likewise for the rvalue expression tree. (gcc_jit_block_end_with_void_return): Remove return of "void", now that block::end_with_return is now non-void. gcc/testsuite/ChangeLog: * jit.dg/test-error-local-used-from-other-function.c: New test case. * jit.dg/test-error-param-reuse.c: New test case. * jit.dg/test-error-param-sharing.c: New test case. * jit.dg/test-error-param-used-from-other-function.c: New test case. * jit.dg/test-error-param-used-without-a-function.c: New test case. --- gcc/jit/jit-recording.c | 295 +++++++++++++++++++-- gcc/jit/jit-recording.h | 90 ++++++- gcc/jit/libgccjit.c | 83 +++++- .../test-error-local-used-from-other-function.c | 70 +++++ gcc/testsuite/jit.dg/test-error-param-reuse.c | 48 ++++ gcc/testsuite/jit.dg/test-error-param-sharing.c | 61 +++++ .../test-error-param-used-from-other-function.c | 74 ++++++ .../test-error-param-used-without-a-function.c | 56 ++++ 8 files changed, 736 insertions(+), 41 deletions(-) create mode 100644 gcc/testsuite/jit.dg/test-error-local-used-from-other-function.c create mode 100644 gcc/testsuite/jit.dg/test-error-param-reuse.c create mode 100644 gcc/testsuite/jit.dg/test-error-param-sharing.c create mode 100644 gcc/testsuite/jit.dg/test-error-param-used-from-other-function.c create mode 100644 gcc/testsuite/jit.dg/test-error-param-used-without-a-function.c diff --git a/gcc/jit/jit-recording.c b/gcc/jit/jit-recording.c index 20fd2d2..ec247e5 100644 --- a/gcc/jit/jit-recording.c +++ b/gcc/jit/jit-recording.c @@ -2034,6 +2034,115 @@ recording::rvalue::dereference (recording::location *loc) return result; } +/* An rvalue visitor, for validating that every rvalue within an expression + trees within "STMT" has the correct scope (e.g. no access to locals + of a different function). */ + +class rvalue_usage_validator : public recording::rvalue_visitor +{ + public: + rvalue_usage_validator (const char *api_funcname, + recording::context *ctxt, + recording::statement *stmt); + + void + visit (recording::rvalue *rvalue); + + private: + const char *m_api_funcname; + recording::context *m_ctxt; + recording::statement *m_stmt; +}; + +/* The trivial constructor for rvalue_usage_validator. */ + +rvalue_usage_validator::rvalue_usage_validator (const char *api_funcname, + recording::context *ctxt, + recording::statement *stmt) + : m_api_funcname (api_funcname), + m_ctxt (ctxt), + m_stmt (stmt) +{ +} + +/* Verify that the given rvalue is in the correct scope. */ + +void +rvalue_usage_validator::visit (recording::rvalue *rvalue) +{ + gcc_assert (m_stmt->get_block ()); + recording::function *stmt_scope = m_stmt->get_block ()->get_function (); + + /* Most rvalues don't have a scope (only locals and params). */ + if (rvalue->get_scope ()) + { + if (rvalue->get_scope () != stmt_scope) + m_ctxt->add_error + (rvalue->get_loc (), + "%s:" + " rvalue %s (type: %s)" + " has scope limited to function %s" + " but was used within function %s" + " (in statement: %s)", + m_api_funcname, + rvalue->get_debug_string (), + rvalue->get_type ()->get_debug_string (), + rvalue->get_scope ()->get_debug_string (), + stmt_scope->get_debug_string (), + m_stmt->get_debug_string ()); + } + else + { + if (rvalue->dyn_cast_param ()) + m_ctxt->add_error + (rvalue->get_loc (), + "%s:" + " param %s (type: %s)" + " was used within function %s" + " (in statement: %s)" + " but is not associated with any function", + m_api_funcname, + rvalue->get_debug_string (), + rvalue->get_type ()->get_debug_string (), + stmt_scope->get_debug_string (), + m_stmt->get_debug_string ()); + } +} + +/* Verify that it's valid to use this rvalue (and all expressions + in the tree below it) within the given statement. + + For example, we must reject attempts to use a local from one + function within a different function here, or we'll get + an ICE deep inside toplev::main. */ + +void +recording::rvalue::verify_valid_within_stmt (const char *api_funcname, statement *s) +{ + rvalue_usage_validator v (api_funcname, + s->get_context (), + s); + + /* Verify that it's OK to use this rvalue within s. */ + v.visit (this); + + /* Traverse the expression tree below "this", verifying all rvalues + within it. */ + visit_children (&v); +} + +/* Set the scope of this rvalue to be the given function. This can only + be done once on a given rvalue. */ + +void +recording::rvalue::set_scope (function *scope) +{ + gcc_assert (scope); + gcc_assert (NULL == m_scope); + m_scope = scope; +} + + /* The implementation of class gcc::jit::recording::lvalue. */ /* Create a recording::new_access_field_of_lvalue instance and add it to @@ -2106,7 +2215,40 @@ recording::function::function (context *ctxt, m_blocks () { for (int i = 0; i< num_params; i++) - m_params.safe_push (params[i]); + { + param *param = params[i]; + gcc_assert (param); + + /* Associate each param with this function. + + Verify that the param doesn't already have a function. */ + if (param->get_scope ()) + { + /* We've already rejected attempts to reuse a param between + different functions (within gcc_jit_context_new_function), so + if the param *does* already have a function, it must be being + reused within the params array for this function. We must + produce an error for this reuse (blocking the compile), since + otherwise we'd have an ICE later on. */ + gcc_assert (this == param->get_scope ()); + ctxt->add_error + (loc, + "gcc_jit_context_new_function:" + " parameter %s (type: %s)" + " is used more than once when creating function %s", + param->get_debug_string (), + param->get_type ()->get_debug_string (), + name->c_str ()); + } + else + { + /* The normal, non-error case: associate this function with the + param. */ + param->set_scope (this); + } + + m_params.safe_push (param); + } } /* Implementation of pure virtual hook recording::memento::replay_into @@ -2366,26 +2508,25 @@ recording::function::make_debug_string () the block's context's list of mementos, and to the block's list of statements. - Implements the post-error-checking part of - gcc_jit_block_add_eval. */ + Implements the heart of gcc_jit_block_add_eval. */ -void +recording::statement * recording::block::add_eval (recording::location *loc, recording::rvalue *rvalue) { statement *result = new eval (this, loc, rvalue); m_ctxt->record (result); m_statements.safe_push (result); + return result; } /* Create a recording::assignment instance and add it to the block's context's list of mementos, and to the block's list of statements. - Implements the post-error-checking part of - gcc_jit_block_add_assignment. */ + Implements the heart of gcc_jit_block_add_assignment. */ -void +recording::statement * recording::block::add_assignment (recording::location *loc, recording::lvalue *lvalue, recording::rvalue *rvalue) @@ -2393,16 +2534,16 @@ recording::block::add_assignment (recording::location *loc, statement *result = new assignment (this, loc, lvalue, rvalue); m_ctxt->record (result); m_statements.safe_push (result); + return result; } /* Create a recording::assignment_op instance and add it to the block's context's list of mementos, and to the block's list of statements. - Implements the post-error-checking part of - gcc_jit_block_add_assignment_op. */ + Implements the heart of gcc_jit_block_add_assignment_op. */ -void +recording::statement * recording::block::add_assignment_op (recording::location *loc, recording::lvalue *lvalue, enum gcc_jit_binary_op op, @@ -2411,32 +2552,32 @@ recording::block::add_assignment_op (recording::location *loc, statement *result = new assignment_op (this, loc, lvalue, op, rvalue); m_ctxt->record (result); m_statements.safe_push (result); + return result; } /* Create a recording::comment instance and add it to the block's context's list of mementos, and to the block's list of statements. - Implements the post-error-checking part of - gcc_jit_block_add_comment. */ + Implements the heart of gcc_jit_block_add_comment. */ -void +recording::statement * recording::block::add_comment (recording::location *loc, const char *text) { statement *result = new comment (this, loc, new_string (text)); m_ctxt->record (result); m_statements.safe_push (result); + return result; } /* Create a recording::end_with_conditional instance and add it to the block's context's list of mementos, and to the block's list of statements. - Implements the post-error-checking part of - gcc_jit_block_end_with_conditional. */ + Implements the heart of gcc_jit_block_end_with_conditional. */ -void +recording::statement * recording::block::end_with_conditional (recording::location *loc, recording::rvalue *boolval, recording::block *on_true, @@ -2446,16 +2587,16 @@ recording::block::end_with_conditional (recording::location *loc, m_ctxt->record (result); m_statements.safe_push (result); m_has_been_terminated = true; + return result; } /* Create a recording::end_with_jump instance and add it to the block's context's list of mementos, and to the block's list of statements. - Implements the post-error-checking part of - gcc_jit_block_end_with_jump. */ + Implements the heart of gcc_jit_block_end_with_jump. */ -void +recording::statement * recording::block::end_with_jump (recording::location *loc, recording::block *target) { @@ -2463,6 +2604,7 @@ recording::block::end_with_jump (recording::location *loc, m_ctxt->record (result); m_statements.safe_push (result); m_has_been_terminated = true; + return result; } /* Create a recording::end_with_return instance and add it to @@ -2473,7 +2615,7 @@ recording::block::end_with_jump (recording::location *loc, gcc_jit_block_end_with_return and gcc_jit_block_end_with_void_return. */ -void +recording::statement * recording::block::end_with_return (recording::location *loc, recording::rvalue *rvalue) { @@ -2484,6 +2626,7 @@ recording::block::end_with_return (recording::location *loc, m_ctxt->record (result); m_statements.safe_push (result); m_has_been_terminated = true; + return result; } /* Override the default implementation of @@ -2513,6 +2656,7 @@ recording::block::write_to_dump (dump &d) bool recording::block::validate () { + /* Check for termination. */ if (!has_been_terminated ()) { statement *stmt = get_last_statement (); @@ -2856,6 +3000,14 @@ recording::unary_op::replay_into (replayer *r) m_a->playback_rvalue ())); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::unary_op. */ +void +recording::unary_op::visit_children (rvalue_visitor *v) +{ + v->visit (m_a); +} + /* Implementation of recording::memento::make_debug_string for unary ops. */ @@ -2890,6 +3042,15 @@ recording::binary_op::replay_into (replayer *r) m_b->playback_rvalue ())); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::binary_op. */ +void +recording::binary_op::visit_children (rvalue_visitor *v) +{ + v->visit (m_a); + v->visit (m_b); +} + /* Implementation of recording::memento::make_debug_string for binary ops. */ @@ -2955,6 +3116,16 @@ recording::comparison::replay_into (replayer *r) m_b->playback_rvalue ())); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::comparison. */ + +void +recording::comparison::visit_children (rvalue_visitor *v) +{ + v->visit (m_a); + v->visit (m_b); +} + /* Implementation of pure virtual hook recording::memento::replay_into for recording::cast. */ @@ -2966,6 +3137,14 @@ recording::cast::replay_into (replayer *r) get_type ()->playback_type ())); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::cast. */ +void +recording::cast::visit_children (rvalue_visitor *v) +{ + v->visit (m_rvalue); +} + /* Implementation of recording::memento::make_debug_string for casts. */ @@ -3011,6 +3190,16 @@ recording::call::replay_into (replayer *r) &playback_args)); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::call. */ + +void +recording::call::visit_children (rvalue_visitor *v) +{ + for (unsigned i = 0; i< m_args.length (); i++) + v->visit (m_args[i]); +} + /* Implementation of recording::memento::make_debug_string for function calls. */ @@ -3088,6 +3277,17 @@ recording::call_through_ptr::replay_into (replayer *r) &playback_args)); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::call_through_ptr. */ + +void +recording::call_through_ptr::visit_children (rvalue_visitor *v) +{ + v->visit (m_fn_ptr); + for (unsigned i = 0; i< m_args.length (); i++) + v->visit (m_args[i]); +} + /* Implementation of recording::memento::make_debug_string for calls through function ptrs. */ @@ -3144,6 +3344,16 @@ recording::array_access::replay_into (replayer *r) m_index->playback_rvalue ())); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::array_access. */ + +void +recording::array_access::visit_children (rvalue_visitor *v) +{ + v->visit (m_ptr); + v->visit (m_index); +} + /* Implementation of recording::memento::make_debug_string for array accesses. */ @@ -3171,6 +3381,15 @@ recording::access_field_of_lvalue::replay_into (replayer *r) } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::access_field_of_lvalue. */ + +void +recording::access_field_of_lvalue::visit_children (rvalue_visitor *v) +{ + v->visit (m_lvalue); +} + /* Implementation of recording::memento::make_debug_string for accessing a field of an lvalue. */ @@ -3197,6 +3416,15 @@ recording::access_field_rvalue::replay_into (replayer *r) m_field->playback_field ())); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::access_field_rvalue. */ + +void +recording::access_field_rvalue::visit_children (rvalue_visitor *v) +{ + v->visit (m_rvalue); +} + /* Implementation of recording::memento::make_debug_string for accessing a field of an rvalue. */ @@ -3224,6 +3452,15 @@ recording::dereference_field_rvalue::replay_into (replayer *r) m_field->playback_field ())); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::dereference_field_rvalue. */ + +void +recording::dereference_field_rvalue::visit_children (rvalue_visitor *v) +{ + v->visit (m_rvalue); +} + /* Implementation of recording::memento::make_debug_string for dereferencing a field of an rvalue. */ @@ -3249,6 +3486,15 @@ recording::dereference_rvalue::replay_into (replayer *r) dereference (playback_location (r, m_loc))); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::dereference_rvalue. */ + +void +recording::dereference_rvalue::visit_children (rvalue_visitor *v) +{ + v->visit (m_rvalue); +} + /* Implementation of recording::memento::make_debug_string for dereferencing an rvalue. */ @@ -3273,6 +3519,15 @@ recording::get_address_of_lvalue::replay_into (replayer *r) get_address (playback_location (r, m_loc))); } +/* Implementation of pure virtual hook recording::rvalue::visit_children + for recording::get_address_of_lvalue. */ + +void +recording::get_address_of_lvalue::visit_children (rvalue_visitor *v) +{ + v->visit (m_lvalue); +} + /* Implementation of recording::memento::make_debug_string for getting the address of an lvalue. */ diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h index 43e99ba..812205c 100644 --- a/gcc/jit/jit-recording.h +++ b/gcc/jit/jit-recording.h @@ -800,6 +800,18 @@ private: fields *m_fields; }; +/* An abstract base class for operations that visit all rvalues within an + expression tree. + Currently the only implementation is class rvalue_usage_validator within + jit-recording.c. */ + +class rvalue_visitor +{ + public: + virtual ~rvalue_visitor () {} + virtual void visit (rvalue *rvalue) = 0; +}; + class rvalue : public memento { public: @@ -808,11 +820,14 @@ public: type *type_) : memento (ctxt), m_loc (loc), - m_type (type_) + m_type (type_), + m_scope (NULL) { gcc_assert (type_); } + location * get_loc () const { return m_loc; } + /* Get the recording::type of this rvalue. Implements the post-error-checking part of @@ -835,9 +850,23 @@ public: lvalue * dereference (location *loc); + void + verify_valid_within_stmt (const char *api_funcname, statement *s); + + virtual void visit_children (rvalue_visitor *v) = 0; + + void set_scope (function *scope); + function *get_scope () const { return m_scope; } + + /* Dynamic cast. */ + virtual param *dyn_cast_param () { return NULL; } + protected: location *m_loc; type *m_type; + + private: + function *m_scope; /* NULL for globals, non-NULL for locals/params */ }; class lvalue : public rvalue @@ -881,12 +910,16 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *) {} + playback::param * playback_param () const { return static_cast <playback::param *> (m_playback_obj); } + param *dyn_cast_param () { return this; } + private: string * make_debug_string () { return m_name; } @@ -925,6 +958,7 @@ public: block* new_block (const char *name); + location *get_loc () const { return m_loc; } type *get_return_type () const { return m_return_type; } string * get_name () const { return m_name; } const vec<param *> &get_params () const { return m_params; } @@ -979,36 +1013,36 @@ public: bool has_been_terminated () { return m_has_been_terminated; } bool is_reachable () { return m_is_reachable; } - void + statement * add_eval (location *loc, rvalue *rvalue); - void + statement * add_assignment (location *loc, lvalue *lvalue, rvalue *rvalue); - void + statement * add_assignment_op (location *loc, lvalue *lvalue, enum gcc_jit_binary_op op, rvalue *rvalue); - void + statement * add_comment (location *loc, const char *text); - void + statement * end_with_conditional (location *loc, rvalue *boolval, block *on_true, block *on_false); - void + statement * end_with_jump (location *loc, block *target); - void + statement * end_with_return (location *loc, rvalue *rvalue); @@ -1063,6 +1097,8 @@ public: void replay_into (replayer *); + void visit_children (rvalue_visitor *) {} + void write_to_dump (dump &d); private: @@ -1086,6 +1122,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *) {} + private: string * make_debug_string (); @@ -1104,6 +1142,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *) {} + private: string * make_debug_string (); @@ -1126,6 +1166,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1149,6 +1191,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1173,6 +1217,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1195,6 +1241,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1213,6 +1261,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1232,6 +1282,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1254,6 +1306,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1276,6 +1330,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1298,6 +1354,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1320,6 +1378,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1339,6 +1399,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1358,6 +1420,8 @@ public: void replay_into (replayer *r); + void visit_children (rvalue_visitor *v); + private: string * make_debug_string (); @@ -1371,10 +1435,15 @@ public: local (function *func, location *loc, type *type_, string *name) : lvalue (func->m_ctxt, loc, type_), m_func (func), - m_name (name) {} + m_name (name) + { + set_scope (func); + } void replay_into (replayer *r); + void visit_children (rvalue_visitor *) {} + void write_to_dump (dump &d); private: @@ -1393,6 +1462,7 @@ public: void write_to_dump (dump &d); + block *get_block () const { return m_block; } location *get_loc () const { return m_loc; } protected: @@ -1401,8 +1471,6 @@ protected: m_block (b), m_loc (loc) {} - block *get_block () const { return m_block; } - playback::location * playback_location (replayer *r) const { diff --git a/gcc/jit/libgccjit.c b/gcc/jit/libgccjit.c index d596d08..ad8ee75 100644 --- a/gcc/jit/libgccjit.c +++ b/gcc/jit/libgccjit.c @@ -172,6 +172,16 @@ struct gcc_jit_param : public gcc::jit::recording::param } \ JIT_END_STMT +#define RETURN_VAL_IF_FAIL_PRINTF5(TEST_EXPR, RETURN_EXPR, CTXT, LOC, ERR_FMT, A0, A1, A2, A3, A4) \ + JIT_BEGIN_STMT \ + if (!(TEST_EXPR)) \ + { \ + jit_error ((CTXT), (LOC), "%s: " ERR_FMT, \ + __func__, (A0), (A1), (A2), (A3), (A4)); \ + return (RETURN_EXPR); \ + } \ + JIT_END_STMT + #define RETURN_VAL_IF_FAIL_PRINTF6(TEST_EXPR, RETURN_EXPR, CTXT, LOC, ERR_FMT, A0, A1, A2, A3, A4, A5) \ JIT_BEGIN_STMT \ if (!(TEST_EXPR)) \ @@ -197,6 +207,9 @@ struct gcc_jit_param : public gcc::jit::recording::param #define RETURN_NULL_IF_FAIL_PRINTF4(TEST_EXPR, CTXT, LOC, ERR_FMT, A0, A1, A2, A3) \ RETURN_VAL_IF_FAIL_PRINTF4 (TEST_EXPR, NULL, CTXT, LOC, ERR_FMT, A0, A1, A2, A3) +#define RETURN_NULL_IF_FAIL_PRINTF5(TEST_EXPR, CTXT, LOC, ERR_FMT, A0, A1, A2, A3, A4) \ + RETURN_VAL_IF_FAIL_PRINTF5 (TEST_EXPR, NULL, CTXT, LOC, ERR_FMT, A0, A1, A2, A3, A4) + #define RETURN_NULL_IF_FAIL_PRINTF6(TEST_EXPR, CTXT, LOC, ERR_FMT, A0, A1, A2, A3, A4, A5) \ RETURN_VAL_IF_FAIL_PRINTF6 (TEST_EXPR, NULL, CTXT, LOC, ERR_FMT, A0, A1, A2, A3, A4, A5) @@ -844,10 +857,23 @@ gcc_jit_context_new_function (gcc_jit_context *ctxt, ctxt, loc, "NULL params creating function %s", name); for (int i = 0; i < num_params; i++) - RETURN_NULL_IF_FAIL_PRINTF2 ( - params[i], - ctxt, loc, - "NULL parameter %i creating function %s", i, name); + { + RETURN_NULL_IF_FAIL_PRINTF2 ( + params[i], + ctxt, loc, + "NULL parameter %i creating function %s", i, name); + RETURN_NULL_IF_FAIL_PRINTF5 ( + (NULL == params[i]->get_scope ()), + ctxt, loc, + "parameter %i \"%s\"" + " (type: %s)" + " for function %s" + " was already used for function %s", + i, params[i]->get_debug_string (), + params[i]->get_type ()->get_debug_string (), + name, + params[i]->get_scope ()->get_debug_string ()); + } return (gcc_jit_function*) ctxt->new_function (loc, kind, return_type, name, @@ -1800,7 +1826,14 @@ gcc_jit_block_add_eval (gcc_jit_block *block, /* LOC can be NULL. */ RETURN_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue"); - return block->add_eval (loc, rvalue); + gcc::jit::recording::statement *stmt = block->add_eval (loc, rvalue); + + /* "stmt" should be good enough to be usable in error-messages, + but might still not be compilable; perform some more + error-checking here. We do this here so that the error messages + can contain a stringified version of "stmt", whilst appearing + as close as possible to the point of failure. */ + rvalue->verify_valid_within_stmt (__func__, stmt); } /* Public entrypoint. See description in libgccjit.h. @@ -1832,7 +1865,15 @@ gcc_jit_block_add_assignment (gcc_jit_block *block, rvalue->get_debug_string (), rvalue->get_type ()->get_debug_string ()); - return block->add_assignment (loc, lvalue, rvalue); + gcc::jit::recording::statement *stmt = block->add_assignment (loc, lvalue, rvalue); + + /* "stmt" should be good enough to be usable in error-messages, + but might still not be compilable; perform some more + error-checking here. We do this here so that the error messages + can contain a stringified version of "stmt", whilst appearing + as close as possible to the point of failure. */ + lvalue->verify_valid_within_stmt (__func__, stmt); + rvalue->verify_valid_within_stmt (__func__, stmt); } /* Public entrypoint. See description in libgccjit.h. @@ -1860,7 +1901,15 @@ gcc_jit_block_add_assignment_op (gcc_jit_block *block, op); RETURN_IF_FAIL (rvalue, ctxt, loc, "NULL rvalue"); - return block->add_assignment_op (loc, lvalue, op, rvalue); + gcc::jit::recording::statement *stmt = block->add_assignment_op (loc, lvalue, op, rvalue); + + /* "stmt" should be good enough to be usable in error-messages, + but might still not be compilable; perform some more + error-checking here. We do this here so that the error messages + can contain a stringified version of "stmt", whilst appearing + as close as possible to the point of failure. */ + lvalue->verify_valid_within_stmt (__func__, stmt); + rvalue->verify_valid_within_stmt (__func__, stmt); } /* Internal helper function for determining if rvalue BOOLVAL is of @@ -1921,7 +1970,14 @@ gcc_jit_block_end_with_conditional (gcc_jit_block *block, on_false->get_debug_string (), on_false->get_function ()->get_debug_string ()); - return block->end_with_conditional (loc, boolval, on_true, on_false); + gcc::jit::recording::statement *stmt = block->end_with_conditional (loc, boolval, on_true, on_false); + + /* "stmt" should be good enough to be usable in error-messages, + but might still not be compilable; perform some more + error-checking here. We do this here so that the error messages + can contain a stringified version of "stmt", whilst appearing + as close as possible to the point of failure. */ + boolval->verify_valid_within_stmt (__func__, stmt); } /* Public entrypoint. See description in libgccjit.h. @@ -2003,7 +2059,14 @@ gcc_jit_block_end_with_return (gcc_jit_block *block, func->get_debug_string (), func->get_return_type ()->get_debug_string ()); - return block->end_with_return (loc, rvalue); + gcc::jit::recording::statement *stmt = block->end_with_return (loc, rvalue); + + /* "stmt" should be good enough to be usable in error-messages, + but might still not be compilable; perform some more + error-checking here. We do this here so that the error messages + can contain a stringified version of "stmt", whilst appearing + as close as possible to the point of failure. */ + rvalue->verify_valid_within_stmt (__func__, stmt); } /* Public entrypoint. See description in libgccjit.h. @@ -2029,7 +2092,7 @@ gcc_jit_block_end_with_void_return (gcc_jit_block *block, func->get_debug_string (), func->get_return_type ()->get_debug_string ()); - return block->end_with_return (loc, NULL); + block->end_with_return (loc, NULL); } /********************************************************************** diff --git a/gcc/testsuite/jit.dg/test-error-local-used-from-other-function.c b/gcc/testsuite/jit.dg/test-error-local-used-from-other-function.c new file mode 100644 index 0000000..fac47c8 --- /dev/null +++ b/gcc/testsuite/jit.dg/test-error-local-used-from-other-function.c @@ -0,0 +1,70 @@ +#include <stdlib.h> +#include <stdio.h> + +#include "libgccjit.h" + +#include "harness.h" + +void +create_code (gcc_jit_context *ctxt, void *user_data) +{ + /* Let's try to inject the equivalent of: + + void + fn_one () + { + int i; + } + + int + fn_two () + { + return i; + } + + and verify that the API complains about the use of the local + from the other function. */ + gcc_jit_type *void_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID); + gcc_jit_type *int_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); + + gcc_jit_function *fn_one = + gcc_jit_context_new_function (ctxt, NULL, + GCC_JIT_FUNCTION_EXPORTED, + void_type, + "fn_one", + 0, NULL, + 0); + gcc_jit_lvalue *local = + gcc_jit_function_new_local (fn_one, NULL, int_type, "i"); + + gcc_jit_function *fn_two = + gcc_jit_context_new_function (ctxt, NULL, + GCC_JIT_FUNCTION_EXPORTED, + int_type, + "fn_two", + 0, NULL, + 0); + + gcc_jit_block *block = gcc_jit_function_new_block (fn_two, NULL); + /* "return i;", using local i from the wrong function. */ + gcc_jit_block_end_with_return (block, + NULL, + gcc_jit_lvalue_as_rvalue (local)); +} + +void +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) +{ + CHECK_VALUE (result, NULL); + + /* Verify that the correct error message was emitted. */ + CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), + ("gcc_jit_block_end_with_return:" + " rvalue i (type: int)" + " has scope limited to function fn_one" + " but was used within function fn_two" + " (in statement: return i;)")); +} + diff --git a/gcc/testsuite/jit.dg/test-error-param-reuse.c b/gcc/testsuite/jit.dg/test-error-param-reuse.c new file mode 100644 index 0000000..32cb0c0 --- /dev/null +++ b/gcc/testsuite/jit.dg/test-error-param-reuse.c @@ -0,0 +1,48 @@ +#include <stdlib.h> +#include <stdio.h> + +#include "libgccjit.h" + +#include "harness.h" + +void +create_code (gcc_jit_context *ctxt, void *user_data) +{ + /* Verify that we get an error (rather than a crash) + if the client code reuses a gcc_jit_param * within + a function. */ + gcc_jit_type *void_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID); + gcc_jit_type *int_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); + + /* Create a param. */ + gcc_jit_param *param = + gcc_jit_context_new_param (ctxt, NULL, int_type, "i"); + + /* Try to use it twice when creating "fn". */ + gcc_jit_param *params[2]; + params[0] = param; + params[1] = param; + + gcc_jit_context_new_function (ctxt, NULL, + GCC_JIT_FUNCTION_IMPORTED, + void_type, + "fn", + 2, params, + 0); +} + +void +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) +{ + CHECK_VALUE (result, NULL); + + /* Verify that the correct error message was emitted. */ + CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), + ("gcc_jit_context_new_function:" + " parameter i (type: int)" + " is used more than once when creating function" + " fn")) +} + diff --git a/gcc/testsuite/jit.dg/test-error-param-sharing.c b/gcc/testsuite/jit.dg/test-error-param-sharing.c new file mode 100644 index 0000000..036049c --- /dev/null +++ b/gcc/testsuite/jit.dg/test-error-param-sharing.c @@ -0,0 +1,61 @@ +#include <stdlib.h> +#include <stdio.h> + +#include "libgccjit.h" + +#include "harness.h" + +#ifdef __cplusplus +extern "C" { +#endif + + extern void + called_function (void *ptr); + +#ifdef __cplusplus +} +#endif + +void +create_code (gcc_jit_context *ctxt, void *user_data) +{ + /* Verify that we get an error (rather than a crash) + if the client code reuses a gcc_jit_param * for + two different functions. */ + gcc_jit_type *void_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID); + gcc_jit_type *int_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); + + /* Create a param. */ + gcc_jit_param *param = + gcc_jit_context_new_param (ctxt, NULL, int_type, "i"); + + /* Try to use it for two different functions. */ + gcc_jit_context_new_function (ctxt, NULL, + GCC_JIT_FUNCTION_EXPORTED, + void_type, + "fn_one", + 1, ¶m, + 0); + gcc_jit_context_new_function (ctxt, NULL, + GCC_JIT_FUNCTION_EXPORTED, + void_type, + "fn_two", + 1, ¶m, + 0); +} + +void +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) +{ + CHECK_VALUE (result, NULL); + + /* Verify that the correct error message was emitted. */ + CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), + ("gcc_jit_context_new_function:" + " parameter 0 \"i\" (type: int)" + " for function fn_two" + " was already used for function fn_one")) +} + diff --git a/gcc/testsuite/jit.dg/test-error-param-used-from-other-function.c b/gcc/testsuite/jit.dg/test-error-param-used-from-other-function.c new file mode 100644 index 0000000..84e898b --- /dev/null +++ b/gcc/testsuite/jit.dg/test-error-param-used-from-other-function.c @@ -0,0 +1,74 @@ +#include <stdlib.h> +#include <stdio.h> + +#include "libgccjit.h" + +#include "harness.h" + +void +create_code (gcc_jit_context *ctxt, void *user_data) +{ + /* Let's try to inject the equivalent of: + + void + fn_one (int i) + { + } + + int + fn_two () + { + return i * 2; + } + + and verify that the API complains about the use of the param + from the other function. */ + gcc_jit_type *void_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID); + gcc_jit_type *int_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); + + gcc_jit_param *param = + gcc_jit_context_new_param (ctxt, NULL, int_type, "i"); + + gcc_jit_context_new_function (ctxt, NULL, + GCC_JIT_FUNCTION_EXPORTED, + void_type, + "fn_one", + 1, ¶m, + 0); + gcc_jit_function *fn_two = + gcc_jit_context_new_function (ctxt, NULL, + GCC_JIT_FUNCTION_EXPORTED, + int_type, + "fn_two", + 0, NULL, + 0); + + gcc_jit_block *block = gcc_jit_function_new_block (fn_two, NULL); + /* "return i * 2;", using param i from the wrong function. */ + gcc_jit_block_end_with_return ( + block, + NULL, + gcc_jit_context_new_binary_op ( + ctxt, NULL, + GCC_JIT_BINARY_OP_MULT, + int_type, + gcc_jit_param_as_rvalue (param), + gcc_jit_context_new_rvalue_from_int (ctxt, int_type, 2))); +} + +void +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) +{ + CHECK_VALUE (result, NULL); + + /* Verify that the correct error message was emitted. */ + CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), + ("gcc_jit_block_end_with_return:" + " rvalue i (type: int)" + " has scope limited to function fn_one" + " but was used within function fn_two" + " (in statement: return i * (int)2;)")); +} + diff --git a/gcc/testsuite/jit.dg/test-error-param-used-without-a-function.c b/gcc/testsuite/jit.dg/test-error-param-used-without-a-function.c new file mode 100644 index 0000000..f369c6b --- /dev/null +++ b/gcc/testsuite/jit.dg/test-error-param-used-without-a-function.c @@ -0,0 +1,56 @@ +#include <stdlib.h> +#include <stdio.h> + +#include "libgccjit.h" + +#include "harness.h" + +void +create_code (gcc_jit_context *ctxt, void *user_data) +{ + /* Let's try to inject the equivalent of: + + int + test_fn () + { + return i; + } + + where "i" is a param that isn't associated with any function, + and verify that the API complains. */ + + gcc_jit_type *int_type = + gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_INT); + + gcc_jit_param *param = + gcc_jit_context_new_param (ctxt, NULL, int_type, "i"); + + gcc_jit_function *test_fn = + gcc_jit_context_new_function (ctxt, NULL, + GCC_JIT_FUNCTION_EXPORTED, + int_type, + "test_fn", + 0, NULL, + 0); + + gcc_jit_block *block = gcc_jit_function_new_block (test_fn, NULL); + /* "return i;", using param i from the wrong function. */ + gcc_jit_block_end_with_return (block, + NULL, + gcc_jit_param_as_rvalue (param)); +} + +void +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result) +{ + CHECK_VALUE (result, NULL); + + /* Verify that the correct error message was emitted. */ + CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt), + ("gcc_jit_block_end_with_return:" + " param i (type: int)" + " was used within function test_fn" + " (in statement: return i;)" + " but is not associated with any function")) +} + -- 1.8.5.3