From d26de26d218ac1492375d49cf205dbc538285948 Mon Sep 17 00:00:00 2001 From: water111 <48171810+water111@users.noreply.github.com> Date: Wed, 16 Jun 2021 21:11:21 -0400 Subject: [PATCH] [decompiler] Small bitfield fixes (#599) * fix a bunch of small bitfield related things * fix up test * format --- decompiler/IR2/AtomicOpTypeAnalysis.cpp | 15 ++-- decompiler/IR2/Form.h | 1 + decompiler/IR2/FormExpressionAnalysis.cpp | 84 +++++++++++-------- decompiler/IR2/FormStack.cpp | 14 +++- decompiler/IR2/bitfields.cpp | 16 +++- decompiler/util/data_decompile.cpp | 25 ++++-- decompiler/util/data_decompile.h | 6 ++ .../reference/engine/dma/dma-disasm_REF.gc | 20 ++--- .../reference/engine/game/fact-h_REF.gc | 2 +- .../reference/engine/gfx/hw/display-h_REF.gc | 2 +- .../reference/engine/gfx/hw/display_REF.gc | 6 +- test/decompiler/test_DisasmVifDecompile.cpp | 15 ++-- test/decompiler/test_FormExpressionBuild2.cpp | 2 +- 13 files changed, 125 insertions(+), 83 deletions(-) diff --git a/decompiler/IR2/AtomicOpTypeAnalysis.cpp b/decompiler/IR2/AtomicOpTypeAnalysis.cpp index caad15da8b..8ab8513282 100644 --- a/decompiler/IR2/AtomicOpTypeAnalysis.cpp +++ b/decompiler/IR2/AtomicOpTypeAnalysis.cpp @@ -328,8 +328,7 @@ TP_Type SimpleExpression::get_type_int2(const TypeState& input, int size = shift_size - m_args[1].get_int(); int start_bit = shift_size - size; auto field = find_field(dts.ts, bf, start_bit, size, is_unsigned); - return TP_Type::make_from_ts(field.type()); - // auto field = find_field(arg0_type.typespec(), bf, 64) + return TP_Type::make_from_ts(coerce_to_reg_type(field.type())); } } @@ -350,7 +349,7 @@ TP_Type SimpleExpression::get_type_int2(const TypeState& input, auto as_bitfield = dynamic_cast(type); assert(as_bitfield); auto field = find_field(dts.ts, as_bitfield, start_bit, size, is_unsigned); - return TP_Type::make_from_ts(field.type()); + return TP_Type::make_from_ts(coerce_to_reg_type(field.type())); } if (m_kind == Kind::RIGHT_SHIFT_ARITH) { @@ -1138,12 +1137,12 @@ TypeState StackSpillLoadOp::propagate_types_internal(const TypeState& input, // stack slot load auto info = env.stack_spills().lookup(m_offset); if (info.size != m_size) { - throw std::runtime_error(fmt::format( - "Stack slot load mismatch: defined as size {}, got size {}\n", info.size, m_size)); + env.func->warnings.general_warning( + "Stack slot load mismatch: defined as size {}, got size {}\n", info.size, m_size); } if (info.is_signed != m_is_signed) { - throw std::runtime_error("Stack slot signed mismatch"); + env.func->warnings.general_warning("Stack slot offset {} signed mismatch", m_offset); } auto& loaded_type = input.get_slot(m_offset); @@ -1157,8 +1156,8 @@ TypeState StackSpillStoreOp::propagate_types_internal(const TypeState& input, DecompilerTypeSystem& dts) { auto info = env.stack_spills().lookup(m_offset); if (info.size != m_size) { - throw std::runtime_error(fmt::format( - "Stack slot load mismatch: defined as size {}, got size {}\n", info.size, m_size)); + env.func->warnings.general_warning( + "Stack slot store mismatch: defined as size {}, got size {}\n", info.size, m_size); } auto stored_type = m_value.get_type(input, env, dts); diff --git a/decompiler/IR2/Form.h b/decompiler/IR2/Form.h index a18eb09760..9c7c7a2553 100644 --- a/decompiler/IR2/Form.h +++ b/decompiler/IR2/Form.h @@ -1652,4 +1652,5 @@ GenericElement* alloc_generic_token_op(const std::string& name, const std::vector& args, FormPool& pool); Form* alloc_var_form(const RegisterAccess& var, FormPool& pool); +Form* try_cast_simplify(Form* in, const TypeSpec& new_type, FormPool& pool, const Env& env); } // namespace decompiler diff --git a/decompiler/IR2/FormExpressionAnalysis.cpp b/decompiler/IR2/FormExpressionAnalysis.cpp index 7b3624e698..fe4ce01f6b 100644 --- a/decompiler/IR2/FormExpressionAnalysis.cpp +++ b/decompiler/IR2/FormExpressionAnalysis.cpp @@ -53,6 +53,47 @@ namespace decompiler { +namespace { +Form* strip_pcypld_64(Form* in) { + auto m = match(Matcher::op(GenericOpMatcher::fixed(FixedOperatorKind::PCPYLD), + {Matcher::integer(0), Matcher::any(0)}), + in); + if (m.matched) { + return m.maps.forms.at(0); + } else { + return in; + } +} +} // namespace + +Form* try_cast_simplify(Form* in, const TypeSpec& new_type, FormPool& pool, const Env& env) { + auto in_as_cast = dynamic_cast(in->try_as_single_element()); + if (in_as_cast && in_as_cast->type() == new_type) { + return in; // no need to cast again, it already has it! + } + + auto type_info = env.dts->ts.lookup_type(new_type); + auto bitfield_info = dynamic_cast(type_info); + if (bitfield_info) { + // todo remove this. + if (bitfield_info->get_load_size() == 8) { + in = strip_pcypld_64(in); + } + return cast_to_bitfield(bitfield_info, new_type, pool, env, in); + } + + auto enum_info = dynamic_cast(type_info); + if (enum_info) { + if (enum_info->is_bitfield()) { + return cast_to_bitfield_enum(enum_info, new_type, pool, env, in); + } else { + return cast_to_int_enum(enum_info, new_type, pool, env, in); + } + } + + return nullptr; +} + bool Form::has_side_effects() { bool has_side_effect = false; apply([&](FormElement* elt) { @@ -209,44 +250,13 @@ void pop_helper(const std::vector& vars, } } -namespace { -Form* strip_pcypld_64(Form* in) { - auto m = match(Matcher::op(GenericOpMatcher::fixed(FixedOperatorKind::PCPYLD), - {Matcher::integer(0), Matcher::any(0)}), - in); - if (m.matched) { - return m.maps.forms.at(0); - } else { - return in; - } -} -} // namespace - /*! * This should be used to generate all casts. */ Form* cast_form(Form* in, const TypeSpec& new_type, FormPool& pool, const Env& env) { - auto in_as_cast = dynamic_cast(in->try_as_single_element()); - if (in_as_cast && in_as_cast->type() == new_type) { - return in; - } - - auto type_info = env.dts->ts.lookup_type(new_type); - auto bitfield_info = dynamic_cast(type_info); - if (bitfield_info) { - if (bitfield_info->get_load_size() == 8) { - in = strip_pcypld_64(in); - } - return cast_to_bitfield(bitfield_info, new_type, pool, env, in); - } - - auto enum_info = dynamic_cast(type_info); - if (enum_info) { - if (enum_info->is_bitfield()) { - return cast_to_bitfield_enum(enum_info, new_type, pool, env, in); - } else { - return cast_to_int_enum(enum_info, new_type, pool, env, in); - } + auto result = try_cast_simplify(in, new_type, pool, env); + if (result) { + return result; } return pool.alloc_single_element_form(nullptr, new_type, in); @@ -3439,9 +3449,13 @@ void ArrayFieldAccess::update_with_val(Form* new_val, // (+ v0-0 (the-as uint (* 12 (+ a3-0 -1)))) auto mult_matcher = Matcher::op(GenericOpMatcher::fixed(FixedOperatorKind::MULTIPLICATION), {Matcher::integer(m_expected_stride), Matcher::any(0)}); - mult_matcher = Matcher::match_or({Matcher::cast("uint", mult_matcher), mult_matcher}); + mult_matcher = Matcher::match_or( + {Matcher::cast("uint", mult_matcher), Matcher::cast("int", mult_matcher), mult_matcher}); auto add_matcher = Matcher::op(GenericOpMatcher::fixed(FixedOperatorKind::ADDITION), {Matcher::any(1), mult_matcher}); + add_matcher = Matcher::match_or( + {add_matcher, Matcher::op(GenericOpMatcher::fixed(FixedOperatorKind::ADDITION), + {mult_matcher, Matcher::any(1)})}); auto mr = match(add_matcher, new_val); if (!mr.matched) { diff --git a/decompiler/IR2/FormStack.cpp b/decompiler/IR2/FormStack.cpp index b279a6b11f..ebdb715dd6 100644 --- a/decompiler/IR2/FormStack.cpp +++ b/decompiler/IR2/FormStack.cpp @@ -3,6 +3,7 @@ #include "Form.h" #include "GenericElementMatcher.h" #include "decompiler/Function/Function.h" +#include "decompiler/util/DecompilerTypeSystem.h" namespace decompiler { std::string FormStack::StackEntry::print(const Env& env) const { @@ -321,8 +322,19 @@ std::vector FormStack::rewrite(FormPool& pool, const Env& env) con } } + 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)) { + // 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) { + simplified_source = casted; + type = expected_type; + } + } + auto elt = pool.alloc_element(*e.destination, simplified_source, - e.sequence_point, e.set_type, e.set_info); + e.sequence_point, type, e.set_info); e.source->parent_element = elt; auto final_elt = try_rewrites_in_place(elt, env, pool); diff --git a/decompiler/IR2/bitfields.cpp b/decompiler/IR2/bitfields.cpp index 24c07a1eea..514d40a4a5 100644 --- a/decompiler/IR2/bitfields.cpp +++ b/decompiler/IR2/bitfields.cpp @@ -660,8 +660,12 @@ Form* cast_to_bitfield(const BitFieldType* type_info, // check if it's just a constant: auto in_as_atom = form_as_atom(in); if (in_as_atom && in_as_atom->is_int()) { - auto fields = decompile_bitfield_from_int(typespec, env.dts->ts, in_as_atom->get_int()); - return pool.alloc_single_element_form(nullptr, typespec, fields, + auto fields = + try_decompile_bitfield_from_int(typespec, env.dts->ts, in_as_atom->get_int(), false); + if (!fields) { + return pool.alloc_single_element_form(nullptr, typespec, in); + } + return pool.alloc_single_element_form(nullptr, typespec, *fields, pool); } @@ -679,8 +683,12 @@ Form* cast_to_bitfield(const BitFieldType* type_info, for (auto it = args.begin(); it != args.end(); it++) { auto constant = get_goal_integer_constant(*it, env); if (constant) { - auto constant_defs = decompile_bitfield_from_int(typespec, env.dts->ts, *constant); - for (auto& x : constant_defs) { + auto constant_defs = + try_decompile_bitfield_from_int(typespec, env.dts->ts, *constant, false); + if (!constant_defs) { + return pool.alloc_single_element_form(nullptr, typespec, in); + } + for (auto& x : *constant_defs) { field_defs.push_back(BitFieldDef::from_constant(x, pool)); } diff --git a/decompiler/util/data_decompile.cpp b/decompiler/util/data_decompile.cpp index f7687ae91f..7a03da91a5 100644 --- a/decompiler/util/data_decompile.cpp +++ b/decompiler/util/data_decompile.cpp @@ -899,9 +899,11 @@ goos::Object decompile_bitfield(const TypeSpec& type, return bitfield_defs_print(type, defs); } -std::vector decompile_bitfield_from_int(const TypeSpec& type, - const TypeSystem& ts, - u64 value) { +std::optional> try_decompile_bitfield_from_int( + const TypeSpec& type, + const TypeSystem& ts, + u64 value, + bool require_success) { u64 touched_bits = 0; std::vector result; @@ -941,14 +943,23 @@ std::vector decompile_bitfield_from_int(const TypeSpec& typ u64 untouched_but_set = value & (~touched_bits); if (untouched_but_set) { - throw std::runtime_error( - fmt::format("Failed to decompile static bitfield of type {}. Original value is 0x{:x} but " - "we didn't touch", - type.print(), value, untouched_but_set)); + if (require_success) { + throw std::runtime_error(fmt::format( + "Failed to decompile static bitfield of type {}. Original value is 0x{:x} but " + "we didn't touch", + type.print(), value, untouched_but_set)); + } + return {}; } return result; } +std::vector decompile_bitfield_from_int(const TypeSpec& type, + const TypeSystem& ts, + u64 value) { + return *try_decompile_bitfield_from_int(type, ts, value, true); +} + std::vector decompile_bitfield_enum_from_int(const TypeSpec& type, const TypeSystem& ts, u64 value) { diff --git a/decompiler/util/data_decompile.h b/decompiler/util/data_decompile.h index a57df18198..a6a3f3fbab 100644 --- a/decompiler/util/data_decompile.h +++ b/decompiler/util/data_decompile.h @@ -78,6 +78,12 @@ std::vector decompile_bitfield_from_int(const TypeSpec& typ const TypeSystem& ts, u64 value); +std::optional> try_decompile_bitfield_from_int( + const TypeSpec& type, + const TypeSystem& ts, + u64 value, + bool require_success); + std::vector decompile_bitfield_enum_from_int(const TypeSpec& type, const TypeSystem& ts, u64 value); diff --git a/test/decompiler/reference/engine/dma/dma-disasm_REF.gc b/test/decompiler/reference/engine/dma/dma-disasm_REF.gc index f0175ebd37..068bd258da 100644 --- a/test/decompiler/reference/engine/dma/dma-disasm_REF.gc +++ b/test/decompiler/reference/engine/dma/dma-disasm_REF.gc @@ -519,7 +519,7 @@ (+ (* (-> *vif-disasm-table* cmd-template-idx val) - (the-as uint (-> first-tag num)) + (-> first-tag num) ) 3 ) @@ -621,19 +621,19 @@ ) ) ) - (if (> (the-as uint (-> arg0 addr)) 0) + (if (> (-> arg0 addr) 0) (format arg1 " :addr #x~8x" (-> arg0 addr)) ) - (if (> (the-as uint (-> arg0 qwc)) 0) + (if (> (-> arg0 qwc) 0) (format arg1 " :qwc ~d" (-> arg0 qwc)) ) - (if (> (the-as uint (-> arg0 spr)) 0) + (if (> (-> arg0 spr) 0) (format arg1 " :spr ~d" (-> arg0 spr)) ) (if (> (the-as uint (-> arg0 irq)) 0) (format arg1 " :irq ~d" (-> arg0 irq)) ) - (if (> (the-as uint (-> arg0 pce)) 0) + (if (> (-> arg0 pce) 0) (format arg1 " :pce ~d" (-> arg0 pce)) ) (format arg1 ")~%") @@ -739,10 +739,7 @@ ) ) (disasm-vif-tag - (the-as - (pointer vif-tag) - (+ (the-as uint addr) (the-as uint v0-10)) - ) + (the-as (pointer vif-tag) (+ addr (the-as uint v0-10))) (the-as int (- (* qwc 4) (the-as uint (/ v0-10 4)))) stream-2 (= mode-2 'details) @@ -777,10 +774,7 @@ data-2 (the-as dma-packet - (+ - (the-as uint data-2) - (the-as uint (* (+ (the-as uint qwc) (the-as uint 1)) 16)) - ) + (+ (the-as uint data-2) (the-as uint (* (+ qwc 1) 16))) ) ) (let ((v1-68 data-2)) diff --git a/test/decompiler/reference/engine/game/fact-h_REF.gc b/test/decompiler/reference/engine/game/fact-h_REF.gc index 6378626c2d..ef98967f61 100644 --- a/test/decompiler/reference/engine/game/fact-h_REF.gc +++ b/test/decompiler/reference/engine/game/fact-h_REF.gc @@ -287,7 +287,7 @@ (let ((a0-6 (-> tag elt-count))) (set! (-> obj pickup-type) (the-as pickup-type (-> v1-6 0))) (set! pkup-amount (cond - ((< (the-as uint 1) (the-as uint a0-6)) + ((< (the-as uint 1) a0-6) (the float (-> v1-6 1)) ) (else diff --git a/test/decompiler/reference/engine/gfx/hw/display-h_REF.gc b/test/decompiler/reference/engine/gfx/hw/display-h_REF.gc index df3263bda3..907986d8c6 100644 --- a/test/decompiler/reference/engine/gfx/hw/display-h_REF.gc +++ b/test/decompiler/reference/engine/gfx/hw/display-h_REF.gc @@ -78,7 +78,7 @@ (dma-send (the-as dma-bank #x1000a000) (the-as uint arg0) - (the-as uint (+ (the-as uint (-> arg0 0 nloop)) (the-as uint 1))) + (+ (-> arg0 0 nloop) 1) ) (none) ) diff --git a/test/decompiler/reference/engine/gfx/hw/display_REF.gc b/test/decompiler/reference/engine/gfx/hw/display_REF.gc index 3833b9b74c..17de952346 100644 --- a/test/decompiler/reference/engine/gfx/hw/display_REF.gc +++ b/test/decompiler/reference/engine/gfx/hw/display_REF.gc @@ -141,8 +141,8 @@ (set! (-> env xyoffset1) (new 'static 'gs-xy-offset - :ofx (* (- x (the-as int (shr (the-as int (+ (the-as uint (-> env scissor1 scax1)) (the-as uint 1))) 1))) 16) - :ofy (+ (* (- y (the-as int (shr (the-as int (+ (the-as uint (-> env scissor1 scay1)) (the-as uint 1))) 1))) 16) (if (zero? arg3) 0 8)) + :ofx (* (- x (the-as int (shr (+ (-> env scissor1 scax1) 1) 1))) 16) + :ofy (+ (* (- y (the-as int (shr (+ (-> env scissor1 scay1) 1) 1))) 16) (if (zero? arg3) 0 8)) ) ) env @@ -1000,7 +1000,7 @@ (set! (-> (the-as (pointer gs-reg64) gif-data) 3) (gs-reg64 xyoffset-1)) (set! (-> (the-as (pointer gs-frame) gif-data) 4) - (new 'static 'gs-frame :fbw #x8 :fbp (the-as int fbp)) + (new 'static 'gs-frame :fbw #x8 :fbp fbp) ) (set! (-> (the-as (pointer gs-reg64) gif-data) 5) (gs-reg64 frame-1)) (set! diff --git a/test/decompiler/test_DisasmVifDecompile.cpp b/test/decompiler/test_DisasmVifDecompile.cpp index a8064084ab..055c86be27 100644 --- a/test/decompiler/test_DisasmVifDecompile.cpp +++ b/test/decompiler/test_DisasmVifDecompile.cpp @@ -553,8 +553,8 @@ TEST_F(FormRegressionTest, ExprDisasmVif) { " \" (~s :irq ~D :wl ~D :cl ~D)~%\"\n" " (-> *vif-disasm-table* v1-0 string1)\n" " (-> s1-0 irq)\n" - " (shr (shl (the-as int t1-1) 48) 56)\n" - " (shr (shl (the-as int t1-1) 56) 56)\n" + " (shr (shl t1-1 48) 56)\n" + " (shr (shl t1-1 56) 56)\n" " )\n" " )\n" " )\n" @@ -636,10 +636,7 @@ TEST_F(FormRegressionTest, ExprDisasmVif) { " (the-as\n" " int\n" " (+\n" - " (*\n" - " (-> *vif-disasm-table* v1-0 val)\n" - " (the-as uint (-> s1-0 num))\n" - " )\n" + " (* (-> *vif-disasm-table* v1-0 val) (-> s1-0 num))\n" " 3\n" " )\n" " )\n" @@ -655,14 +652,14 @@ TEST_F(FormRegressionTest, ExprDisasmVif) { " (-> *vif-disasm-table* v1-0 string1)\n" " (-> s1-0 irq)\n" " (-> s1-0 num)\n" - " (shr (shl (the-as int sv-64) 54) 54)\n" + " (shr (shl sv-64 54) 54)\n" " )\n" " (format\n" " arg2\n" " \":msk ~D :flg ~D :usn ~D [skip ~d])~%\"\n" " (-> s1-0 msk)\n" - " (shr (shl (the-as int sv-64) 48) 63)\n" - " (shr (shl (the-as int sv-64) 49) 63)\n" + " (shr (shl sv-64 48) 63)\n" + " (shr (shl sv-64 49) 63)\n" " (the-as uint s0-0)\n" " )\n" " (if arg3\n" diff --git a/test/decompiler/test_FormExpressionBuild2.cpp b/test/decompiler/test_FormExpressionBuild2.cpp index 7fc8811db0..5a0fcf7d05 100644 --- a/test/decompiler/test_FormExpressionBuild2.cpp +++ b/test/decompiler/test_FormExpressionBuild2.cpp @@ -1168,7 +1168,7 @@ TEST_F(FormRegressionTest, Method4ResTag) { std::string expected = "(the-as int (if (zero? (-> arg0 inlined?))\n" " (* (-> arg0 elt-count) 4)\n" - " (* (the-as uint (-> arg0 elt-count)) (-> arg0 elt-type size))\n" + " (* (-> arg0 elt-count) (-> arg0 elt-type size))\n" " )\n" " )"; test_with_expr(func, type, expected);