diff --git a/decompiler/IR2/ExpressionHelpers.cpp b/decompiler/IR2/ExpressionHelpers.cpp index d2a66df643..48f73a0d0d 100644 --- a/decompiler/IR2/ExpressionHelpers.cpp +++ b/decompiler/IR2/ExpressionHelpers.cpp @@ -2,6 +2,8 @@ #include "decompiler/IR2/Form.h" #include "decompiler/IR2/Env.h" #include "common/goal_constants.h" +#include "decompiler/IR2/GenericElementMatcher.h" +#include "decompiler/IR2/FormStack.h" namespace decompiler { @@ -200,4 +202,154 @@ FormElement* handle_get_property_value(const std::vector& forms, property_name, default_value, tag_pointer, time, TypeSpec("uint128")); } + +namespace { +Form* var_to_form(const RegisterAccess& var, FormPool& pool) { + return pool.alloc_single_element_form(nullptr, SimpleAtom::make_var(var)); +} + +} // namespace + +/*! + * Recognize the handle->process macro. + * If it occurs inside of another and, the part_of_longer_sc argument should be set. + */ +FormElement* last_two_in_and_to_handle_get_proc(Form* first, + Form* second, + const Env& env, + FormPool& pool, + FormStack& stack, + bool part_of_longer_sc) { + constexpr int reg_input_1 = 0; + constexpr int reg_input_2 = 1; + constexpr int reg_input_3 = 2; + constexpr int reg_temp_1 = 10; + constexpr int reg_temp_2 = 11; + constexpr int reg_temp_3 = 12; + + // only used if part of a longer sc. + Form* longer_sc_src = nullptr; // the source (can be found without repopping) + Form longer_sc_first_form; // the equivalent of the first form for a normal handle->proc + RegisterAccess longer_sc_var; // the second temp var that only appears in this case. + + // check first. + Matcher first_matcher = + Matcher::op(GenericOpMatcher::condition(IR2_Condition::Kind::NONZERO), + {Matcher::op(GenericOpMatcher::fixed(FixedOperatorKind::L32_NOT_FALSE_CBOOL), + {Matcher::any_reg(reg_input_1)})}); + + // if we're part of a longer, the first will actually be (being (set! foo <>) (... foo)) + // so let's strip out the set, then remember what was being set. If it all works out, + // we can just substitute the <> into the macro + if (part_of_longer_sc) { + if (first->size() != 2) { + return nullptr; + } + + auto as_var_set = dynamic_cast(first->elts().at(0)); + if (!as_var_set) { + return nullptr; + } + + longer_sc_var = as_var_set->dst(); + longer_sc_src = as_var_set->src(); + + longer_sc_first_form.elts().push_back(first->elts().at(1)); + first = &longer_sc_first_form; + } + + auto first_result = match(first_matcher, first); + if (!first_result.matched) { + return nullptr; + } + + // auto first_use_of_in = *first_result.maps.regs.at(reg_input_1); + // fmt::print("reg1: {}\n", first_use_of_in.to_string(env)); + + auto setup_matcher = Matcher::set_var( + Matcher::deref(Matcher::any_reg(reg_input_2), false, + {DerefTokenMatcher::string("process"), DerefTokenMatcher::integer(0)}), + reg_temp_1); + + auto if_matcher = Matcher::if_no_else( + Matcher::op( + GenericOpMatcher::fixed(FixedOperatorKind::EQ), + {Matcher::deref(Matcher::any_reg(reg_input_3), false, {DerefTokenMatcher::string("pid")}), + Matcher::deref(Matcher::any_reg(reg_temp_2), false, + {DerefTokenMatcher::string("pid")})}), + Matcher::any_reg(reg_temp_3)); + + auto second_matcher = Matcher::begin({setup_matcher, if_matcher}); + + auto second_result = match(second_matcher, second); + if (!second_result.matched) { + return nullptr; + } + + auto in1 = *first_result.maps.regs.at(reg_input_1); + auto in2 = *second_result.maps.regs.at(reg_input_2); + auto in3 = *second_result.maps.regs.at(reg_input_3); + + auto in_name = in1.to_string(env); + if (in_name != in2.to_string(env)) { + return nullptr; + } + + if (in_name != in3.to_string(env)) { + return nullptr; + } + + auto temp_name = second_result.maps.regs.at(reg_temp_1)->to_string(env); + if (temp_name != second_result.maps.regs.at(reg_temp_2)->to_string(env)) { + return nullptr; + } + + if (temp_name != second_result.maps.regs.at(reg_temp_3)->to_string(env)) { + return nullptr; + } + + const auto& temp_use_def = env.get_use_def_info(*second_result.maps.regs.at(reg_temp_1)); + if (temp_use_def.use_count() != 2 || temp_use_def.def_count() != 1) { + fmt::print("failed usedef: {} {}\n", temp_use_def.use_count(), temp_use_def.def_count()); + return nullptr; + } + + if (part_of_longer_sc) { + // check that our temporary name matches (it's the var used inside the macro) + if (in_name != longer_sc_var.to_string(env)) { + fmt::print("failed var name: {} vs {}\n", temp_name, longer_sc_var.to_string(env)); + return nullptr; + } + + // check that our temporary has the right usage pattern. + const auto& outer_temp_usedef = env.get_use_def_info(longer_sc_var); + if (outer_temp_usedef.use_count() != 3 || outer_temp_usedef.def_count() != 1) { + fmt::print("failed usedef2: {} {}\n", outer_temp_usedef.use_count(), + outer_temp_usedef.def_count()); + return nullptr; + } + + return pool.alloc_element( + GenericOperator::make_function( + pool.alloc_single_element_form(nullptr, "handle->process")), + longer_sc_src); + } else { + // modify use def: + auto* menv = const_cast(&env); + menv->disable_use(in2); + menv->disable_use(in3); + + auto repopped = stack.pop_reg(in1, {}, env, true); + // fmt::print("repopped: {}\n", repopped->to_string(env)); + + if (!repopped) { + repopped = var_to_form(in1, pool); + } + + return pool.alloc_element( + GenericOperator::make_function( + pool.alloc_single_element_form(nullptr, "handle->process")), + repopped); + } +} } // namespace decompiler \ No newline at end of file diff --git a/decompiler/IR2/ExpressionHelpers.h b/decompiler/IR2/ExpressionHelpers.h index 34bdf50996..8ce138a689 100644 --- a/decompiler/IR2/ExpressionHelpers.h +++ b/decompiler/IR2/ExpressionHelpers.h @@ -7,6 +7,7 @@ class FormElement; class Form; class Env; class FormPool; +class FormStack; FormElement* handle_get_property_value_float(const std::vector& forms, FormPool& pool, @@ -21,4 +22,11 @@ FormElement* handle_get_property_struct(const std::vector& forms, FormElement* handle_get_property_value(const std::vector& forms, FormPool& pool, const Env& env); + +FormElement* last_two_in_and_to_handle_get_proc(Form* first, + Form* second, + const Env& env, + FormPool& pool, + FormStack& stack, + bool part_of_longer_sc); } // namespace decompiler diff --git a/decompiler/IR2/FormExpressionAnalysis.cpp b/decompiler/IR2/FormExpressionAnalysis.cpp index 4693eaf17b..af66361bd3 100644 --- a/decompiler/IR2/FormExpressionAnalysis.cpp +++ b/decompiler/IR2/FormExpressionAnalysis.cpp @@ -3524,97 +3524,27 @@ FormElement* sc_to_handle_get_proc(ShortCircuitElement* elt, return nullptr; } - if (elt->entries.size() != 2) { + if (elt->entries.size() < 2) { return nullptr; } - // fmt::print("candidate: {}\n", elt->to_string(env)); + Form* last = elt->entries[elt->entries.size() - 1].condition; + Form* second_to_last = elt->entries[elt->entries.size() - 2].condition; - constexpr int reg_input_1 = 0; - constexpr int reg_input_2 = 1; - constexpr int reg_input_3 = 2; - constexpr int reg_temp_1 = 10; - constexpr int reg_temp_2 = 11; - constexpr int reg_temp_3 = 12; - - // check first. - auto first_matcher = - Matcher::op(GenericOpMatcher::condition(IR2_Condition::Kind::NONZERO), - {Matcher::op(GenericOpMatcher::fixed(FixedOperatorKind::L32_NOT_FALSE_CBOOL), - {Matcher::any_reg(reg_input_1)})}); - - auto first_result = match(first_matcher, elt->entries.at(0).condition); - if (!first_result.matched) { + auto result = last_two_in_and_to_handle_get_proc(second_to_last, last, env, pool, stack, + elt->entries.size() > 2); + if (!result) { return nullptr; } - // auto first_use_of_in = *first_result.maps.regs.at(reg_input_1); - // fmt::print("reg1: {}\n", first_use_of_in.to_string(env)); - - auto setup_matcher = Matcher::set_var( - Matcher::deref(Matcher::any_reg(reg_input_2), false, - {DerefTokenMatcher::string("process"), DerefTokenMatcher::integer(0)}), - reg_temp_1); - - auto if_matcher = Matcher::if_no_else( - Matcher::op( - GenericOpMatcher::fixed(FixedOperatorKind::EQ), - {Matcher::deref(Matcher::any_reg(reg_input_3), false, {DerefTokenMatcher::string("pid")}), - Matcher::deref(Matcher::any_reg(reg_temp_2), false, - {DerefTokenMatcher::string("pid")})}), - Matcher::any_reg(reg_temp_3)); - - auto second_matcher = Matcher::begin({setup_matcher, if_matcher}); - - auto second_result = match(second_matcher, elt->entries.at(1).condition); - if (!second_result.matched) { - return nullptr; + if (elt->entries.size() == 2) { + // just replace the whole thing + return result; + } else { + elt->entries.pop_back(); + elt->entries.back().condition = pool.alloc_single_form(elt, result); } - auto in1 = *first_result.maps.regs.at(reg_input_1); - auto in2 = *second_result.maps.regs.at(reg_input_2); - auto in3 = *second_result.maps.regs.at(reg_input_3); - - auto in_name = in1.to_string(env); - if (in_name != in2.to_string(env)) { - return nullptr; - } - - if (in_name != in3.to_string(env)) { - return nullptr; - } - - auto temp_name = second_result.maps.regs.at(reg_temp_1)->to_string(env); - if (temp_name != second_result.maps.regs.at(reg_temp_2)->to_string(env)) { - return nullptr; - } - - if (temp_name != second_result.maps.regs.at(reg_temp_3)->to_string(env)) { - return nullptr; - } - - const auto& temp_use_def = env.get_use_def_info(*second_result.maps.regs.at(reg_temp_1)); - if (temp_use_def.use_count() != 2 || temp_use_def.def_count() != 1) { - return nullptr; - } - - // modify use def: - auto* menv = const_cast(&env); - menv->disable_use(in2); - menv->disable_use(in3); - - auto repopped = stack.pop_reg(in1, {}, env, true); - // fmt::print("repopped: {}\n", repopped->to_string(env)); - - if (!repopped) { - repopped = var_to_form(in1, pool); - } - - return pool.alloc_element( - GenericOperator::make_function( - pool.alloc_single_element_form(nullptr, "handle->process")), - repopped); - return nullptr; } diff --git a/decompiler/IR2/FormStack.cpp b/decompiler/IR2/FormStack.cpp index 7538fd1121..58d803c6c9 100644 --- a/decompiler/IR2/FormStack.cpp +++ b/decompiler/IR2/FormStack.cpp @@ -314,6 +314,7 @@ std::vector FormStack::rewrite(FormPool& pool, const Env& env) con // we want to untangle coloring moves here auto simplified_source = e.source; + auto type = e.set_type; auto src_as_var = dynamic_cast(e.source->try_as_single_element()); if (src_as_var && src_as_var->expr().is_var() && e.is_compactable) { bool keep_going = true; @@ -331,14 +332,16 @@ std::vector FormStack::rewrite(FormPool& pool, const Env& env) con var_to_get = as_one->expr().var(); } simplified_source = last_op_as_set->src(); + // because we are eliminating, we need to use the source's cast. + // to make the code cleaner, we drop casts that would occur in the middle + type = last_op_as_set->src_type(); // result = last_op_as_set->src()->elts(); } } } - auto type = e.set_type; auto expected_type = env.get_variable_type(*e.destination, true); - if (!env.dts->ts.tc(expected_type, e.set_type)) { + if (!env.dts->ts.tc(expected_type, type)) { // we would cast. let's see if we can simplify the source to avoid this. auto casted = try_cast_simplify(simplified_source, expected_type, pool, env); if (casted) { diff --git a/docs/markdown/progress-notes/changelog.md b/docs/markdown/progress-notes/changelog.md index 2cf5e1739e..c9324c651b 100644 --- a/docs/markdown/progress-notes/changelog.md +++ b/docs/markdown/progress-notes/changelog.md @@ -202,4 +202,5 @@ - It is now an error to have two arguments with the same name. - It is now a warning to redefine a constant. - Fix a bug where the size of static boxed arrays was only `length` and not `allocated-length` -- It is now possible to call a method on a forward declared type. The forward declared type must be a basic. \ No newline at end of file +- It is now possible to call a method on a forward declared type. The forward declared type must be a basic. +- Using `->` on a plain `pointer` or `inline-array` now generates an error instead of crashing the compiler \ No newline at end of file diff --git a/goalc/compiler/compilation/Type.cpp b/goalc/compiler/compilation/Type.cpp index 3f3bc9edf0..3b1a567959 100644 --- a/goalc/compiler/compilation/Type.cpp +++ b/goalc/compiler/compilation/Type.cpp @@ -742,6 +742,10 @@ Val* Compiler::compile_deref(const goos::Object& form, const goos::Object& _rest } if (result->type().base_type() == "inline-array") { + if (!result->type().has_single_arg()) { + throw_compiler_error(form, "Cannot dereference an inline-array with type {}", + result->type().print()); + } auto di = m_ts.get_deref_info(result->type()); auto base_type = di.result_type; assert(di.can_deref); @@ -755,6 +759,10 @@ Val* Compiler::compile_deref(const goos::Object& form, const goos::Object& _rest result = fe->alloc_val(di.result_type, result, offset); } } else if (result->type().base_type() == "pointer") { + if (!result->type().has_single_arg()) { + throw_compiler_error(form, "Cannot dereference a pointer with type {}", + result->type().print()); + } auto di = m_ts.get_deref_info(result->type()); auto base_type = di.result_type; assert(di.mem_deref); diff --git a/test/decompiler/reference/engine/load/loader_REF.gc b/test/decompiler/reference/engine/load/loader_REF.gc index 355954d0f9..a83d2fec24 100644 --- a/test/decompiler/reference/engine/load/loader_REF.gc +++ b/test/decompiler/reference/engine/load/loader_REF.gc @@ -397,7 +397,6 @@ ;; definition for method 10 of type external-art-buffer ;; WARN: Found some very strange gotos. Check result carefully, this is not well tested. (defmethod update external-art-buffer ((obj external-art-buffer)) - (local-vars (v1-54 handle)) (when (or (not (name= (-> obj pending-load-file) (-> obj load-file))) @@ -552,16 +551,8 @@ ) ) (('locked) - (when (and (not (-> obj locked?)) (begin - (set! v1-54 (-> obj load-file-owner)) - (nonzero? (l32-false-check v1-54)) - ) - (let ((a0-46 (-> v1-54 process 0))) - (if (= (-> v1-54 pid) (-> a0-46 pid)) - a0-46 - ) - ) - ) + (when + (and (not (-> obj locked?)) (handle->process (-> obj load-file-owner))) (link-file obj (-> obj art-group)) (set! (-> obj other locked?) #t) (set! (-> obj status) 'active) diff --git a/test/offline/offline_test_main.cpp b/test/offline/offline_test_main.cpp index d81eda78b1..c554459709 100644 --- a/test/offline/offline_test_main.cpp +++ b/test/offline/offline_test_main.cpp @@ -171,8 +171,6 @@ const std::unordered_set g_functions_to_skip_compiling = { // camera "slave-set-rotation!", "v-slrp2!", "v-slrp3!", // vector-dot involving the stack - // loader - decompiler bug with detecting handle macros - "(method 10 external-art-buffer)", // function returning float with a weird cast. "debug-menu-item-var-make-float"};