From ae2666a7c5548b841be62ccf1e8b4c03fdbc9fc4 Mon Sep 17 00:00:00 2001 From: water111 <48171810+water111@users.noreply.github.com> Date: Sat, 26 Jun 2021 16:31:38 -0400 Subject: [PATCH] fix use def issues (#629) --- decompiler/IR2/Form.h | 16 +++++ decompiler/IR2/FormExpressionAnalysis.cpp | 22 +++++-- decompiler/IR2/FormStack.cpp | 20 ++++-- decompiler/IR2/FormStack.h | 11 ++-- decompiler/analysis/cfg_builder.cpp | 8 +++ .../reference/engine/load/file-io_REF.gc | 1 - test/decompiler/test_FormExpressionBuild2.cpp | 66 +++++++++++++++++++ 7 files changed, 126 insertions(+), 18 deletions(-) diff --git a/decompiler/IR2/Form.h b/decompiler/IR2/Form.h index cc9a368354..f841066626 100644 --- a/decompiler/IR2/Form.h +++ b/decompiler/IR2/Form.h @@ -1593,6 +1593,8 @@ class Form { std::vector m_elements; }; +class CfgVtx; + /*! * A FormPool is used to allocate forms and form elements. * It will clean up everything when it is destroyed. @@ -1640,11 +1642,25 @@ class FormPool { return form; } + Form* lookup_cached_conversion(const CfgVtx* vtx) const { + auto it = m_vtx_to_form_cache.find(vtx); + if (it == m_vtx_to_form_cache.end()) { + return nullptr; + } + return it->second; + } + + void cache_conversion(const CfgVtx* vtx, Form* form) { + assert(m_vtx_to_form_cache.find(vtx) == m_vtx_to_form_cache.end()); + m_vtx_to_form_cache[vtx] = form; + } + ~FormPool(); private: std::vector m_forms; std::vector m_elements; + std::unordered_map m_vtx_to_form_cache; }; std::optional form_element_as_atom(const FormElement* f); diff --git a/decompiler/IR2/FormExpressionAnalysis.cpp b/decompiler/IR2/FormExpressionAnalysis.cpp index b7a8252176..743ed05c2c 100644 --- a/decompiler/IR2/FormExpressionAnalysis.cpp +++ b/decompiler/IR2/FormExpressionAnalysis.cpp @@ -2229,6 +2229,7 @@ void CondNoElseElement::push_to_stack(const Env& env, FormPool& pool, FormStack& x->push_to_stack(env, pool, stack); } + RegisterAccess write_as_value = final_destination; bool first = true; for (auto& entry : entries) { for (auto form : {entry.condition, entry.body}) { @@ -2243,8 +2244,15 @@ void CondNoElseElement::push_to_stack(const Env& env, FormPool& pool, FormStack& } std::vector new_entries; + if (form == entry.body && used_as_value) { - new_entries = rewrite_to_get_var(temp_stack, pool, final_destination, env); + // try to advance us to the real write so we don't use the final_destination, + // which may contain the wrong variable, but right register. + std::optional written_var; + new_entries = rewrite_to_get_var(temp_stack, pool, final_destination, env, &written_var); + if (written_var) { + write_as_value = *written_var; + } } else { new_entries = temp_stack.rewrite(pool, env); } @@ -2259,7 +2267,7 @@ void CondNoElseElement::push_to_stack(const Env& env, FormPool& pool, FormStack& if (used_as_value) { // TODO - is this wrong? - stack.push_value_to_reg(final_destination, pool.alloc_single_form(nullptr, this), true, + stack.push_value_to_reg(write_as_value, pool.alloc_single_form(nullptr, this), true, env.get_variable_type(final_destination, false)); } else { stack.push_form_element(this, true); @@ -2386,10 +2394,12 @@ void CondWithElseElement::push_to_stack(const Env& env, FormPool& pool, FormStac if (rewrite_as_set && !set_unused) { // might not be the same if a set is eliminated by a coloring move. // assert(dest_sets.size() == write_output_forms.size()); - for (size_t i = 0; i < dest_sets.size() - 1; i++) { - auto var = dest_sets.at(i)->dst(); - auto* env2 = const_cast(&env); - env2->disable_def(var, env2->func->warnings); + if (!dest_sets.empty()) { + for (size_t i = 0; i < dest_sets.size() - 1; i++) { + auto var = dest_sets.at(i)->dst(); + auto* env2 = const_cast(&env); + env2->disable_def(var, env2->func->warnings); + } } } diff --git a/decompiler/IR2/FormStack.cpp b/decompiler/IR2/FormStack.cpp index ebdb715dd6..cd9f9f7a40 100644 --- a/decompiler/IR2/FormStack.cpp +++ b/decompiler/IR2/FormStack.cpp @@ -350,14 +350,15 @@ std::vector FormStack::rewrite(FormPool& pool, const Env& env) con return result; } -void rewrite_to_get_var(std::vector& default_result, - FormPool& pool, - const RegisterAccess& var, - const Env& env) { +std::optional rewrite_to_get_var(std::vector& default_result, + FormPool& pool, + const RegisterAccess& var, + const Env& env) { bool keep_going = true; RegisterAccess var_to_get = var; std::vector result; + std::optional result_access; bool first = true; while (keep_going && !default_result.empty()) { @@ -386,26 +387,33 @@ void rewrite_to_get_var(std::vector& default_result, } else { result = last_op_as_set->src()->elts(); } + result_access = last_op_as_set->dst(); } first = false; } if (result.empty()) { default_result.push_back(pool.alloc_element(SimpleAtom::make_var(var))); + return {}; } else { for (auto x : result) { x->parent_form = nullptr; default_result.push_back(x); } + return result_access; } } std::vector rewrite_to_get_var(FormStack& stack, FormPool& pool, const RegisterAccess& var, - const Env& env) { + const Env& env, + std::optional* used_var) { auto default_result = stack.rewrite(pool, env); - rewrite_to_get_var(default_result, pool, var, env); + auto uv = rewrite_to_get_var(default_result, pool, var, env); + if (used_var) { + *used_var = uv; + } return default_result; } diff --git a/decompiler/IR2/FormStack.h b/decompiler/IR2/FormStack.h index 1ee341aecd..fa5ea18784 100644 --- a/decompiler/IR2/FormStack.h +++ b/decompiler/IR2/FormStack.h @@ -68,12 +68,13 @@ class FormStack { bool m_is_root_stack = false; }; -void rewrite_to_get_var(std::vector& default_result, - FormPool& pool, - const RegisterAccess& var, - const Env& env); +std::optional rewrite_to_get_var(std::vector& default_result, + FormPool& pool, + const RegisterAccess& var, + const Env& env); std::vector rewrite_to_get_var(FormStack& stack, FormPool& pool, const RegisterAccess& var, - const Env& env); + const Env& env, + std::optional* used_var = nullptr); } // namespace decompiler diff --git a/decompiler/analysis/cfg_builder.cpp b/decompiler/analysis/cfg_builder.cpp index 7274e6b672..dea94079ac 100644 --- a/decompiler/analysis/cfg_builder.cpp +++ b/decompiler/analysis/cfg_builder.cpp @@ -1653,11 +1653,19 @@ Form* cfg_to_ir_helper(FormPool& pool, Function& f, const CfgVtx* vtx) { } Form* cfg_to_ir(FormPool& pool, Function& f, const CfgVtx* vtx) { + // we cache these because some functions will do a conversion, give up, and throw away the result. + // converting multiple times means that env-modifications will happen multiple times. + auto cached = pool.lookup_cached_conversion(vtx); + if (cached) { + return cached; + } Form* result = cfg_to_ir_helper(pool, f, vtx); if (vtx->needs_label) { result->elts().insert(result->elts().begin(), pool.alloc_element(vtx->get_first_block_id())); } + + pool.cache_conversion(vtx, result); return result; } diff --git a/test/decompiler/reference/engine/load/file-io_REF.gc b/test/decompiler/reference/engine/load/file-io_REF.gc index a1145eea0e..f40ab0e725 100644 --- a/test/decompiler/reference/engine/load/file-io_REF.gc +++ b/test/decompiler/reference/engine/load/file-io_REF.gc @@ -149,7 +149,6 @@ ) ;; definition for function file-info-correct-version? -;; WARN: disable def twice: 16. This may happen when a cond (no else) is nested inside of another conditional, but it should be rare. (defun file-info-correct-version? ((info file-info) (kind file-kind) (version-override int)) diff --git a/test/decompiler/test_FormExpressionBuild2.cpp b/test/decompiler/test_FormExpressionBuild2.cpp index 5a0fcf7d05..dc51c7c84c 100644 --- a/test/decompiler/test_FormExpressionBuild2.cpp +++ b/test/decompiler/test_FormExpressionBuild2.cpp @@ -1316,4 +1316,70 @@ TEST_F(FormRegressionTest, SoundNameEqual) { std::string type = "(function sound-name sound-name symbol)"; std::string expected = "(and (= arg0 arg1) (= (-> arg0 hi) (-> arg1 hi)))"; test_with_expr(func, type, expected); +} + +TEST_F(FormRegressionTest, DebugMenuFuncDecode) { + std::string func = + "sll r0, r0, 0\n" + " dsll32 v1, a0, 29\n" + " beql v1, r0, L203\n" + " lw v1, binteger(s7)\n" + + " bgtzl v1, L203\n" + " lw v1, pair(s7)\n" + + " lwu v1, -4(a0)\n" + + "L203:\n" + " lw a1, symbol(s7)\n" + " dsubu a1, v1, a1\n" + " daddiu a2, s7, 8\n" + " movn a2, s7, a1\n" + " bnel s7, a2, L204\n" + " or a1, a2, r0\n" + + " lw a1, type(s7)\n" + " dsubu a2, v1, a1\n" + " daddiu a1, s7, 8\n" + " movn a1, s7, a2\n" + "L204:\n" + " beq s7, a1, L205\n" + " sll r0, r0, 0\n" + + " lw v0, 0(a0)\n" + " beq r0, r0, L207\n" + " sll r0, r0, 0\n" + + "L205:\n" + " lw a1, function(s7)\n" + " bne v1, a1, L206\n" + " sll r0, r0, 0\n" + + " or v0, a0, r0\n" + " beq r0, r0, L207\n" + " sll r0, r0, 0\n" + + "L206:\n" + " lw v0, nothing(s7)\n" + + "L207:\n" + " jr ra\n" + " daddu sp, sp, r0"; + std::string type = "(function object function)"; + std::string expected = + "(let ((v1-1 (rtype-of arg0)))\n" + " (the-as function (cond\n" + " ((or (= v1-1 symbol) (= v1-1 type))\n" + " (-> (the-as symbol arg0) value)\n" + " )\n" + " ((= v1-1 function)\n" + " arg0\n" + " )\n" + " (else\n" + " nothing\n" + " )\n" + " )\n" + " )\n" + " )"; + test_with_expr(func, type, expected, false, "", {}, "[[13, \"a0\", \"symbol\"]]"); } \ No newline at end of file