mirror/jak-project
1
0
Fork 0
mirror of https://github.com/open-goal/jak-project synced 2026-08-04 01:04:11 -04:00
Code Issues Packages Projects Releases Wiki Activity

[decompiler] bug fixes related to infinite loop CFGs, argument registers (#622)

Browse Source 
* temp

* clean up

* more clean
This commit is contained in:
water111
2021-06-23 22:09:02 -04:00
committed by GitHub
parent 997d5b57db
commit 44f8ff6d25
10 changed files with 115 additions and 48 deletions
Download Patch File Download Diff File Expand all files Collapse all files
decompiler/Function/CfgVtx.cpp
+29 -16
View File
@@ -912,7 +912,7 @@ bool ControlFlowGraph::find_goto_not_end() {
return replaced;
}
bool ControlFlowGraph::is_sequence(CfgVtx* b0, CfgVtx* b1) {
bool ControlFlowGraph::is_sequence(CfgVtx* b0, CfgVtx* b1, bool allow_self_loops) {
if (!b0 || !b1)
return false;
@@ -939,40 +939,49 @@ bool ControlFlowGraph::is_sequence(CfgVtx* b0, CfgVtx* b1) {
return false;
if (!b1->has_pred(b0))
return false;
if (b1->succ_branch == b0)
if (!allow_self_loops && b1->succ_branch == b0)
return false;
return true;
}
bool ControlFlowGraph::is_sequence_of_non_sequences(CfgVtx* b0, CfgVtx* b1) {
bool ControlFlowGraph::is_sequence_of_non_sequences(CfgVtx* b0, CfgVtx* b1, bool allow_self_loops) {
if (!b0 || !b1)
return false;
if (dynamic_cast<SequenceVtx*>(b0) || dynamic_cast<SequenceVtx*>(b1))
return false;
return is_sequence(b0, b1);
return is_sequence(b0, b1, allow_self_loops);
}
bool ControlFlowGraph::is_sequence_of_sequence_and_non_sequence(CfgVtx* b0, CfgVtx* b1) {
bool ControlFlowGraph::is_sequence_of_sequence_and_non_sequence(CfgVtx* b0,
CfgVtx* b1,
bool allow_self_loops) {
if (!b0 || !b1)
return false;
if (!dynamic_cast<SequenceVtx*>(b0))
return false;
if (dynamic_cast<SequenceVtx*>(b1))
return false;
return is_sequence(b0, b1);
return is_sequence(b0, b1, allow_self_loops);
}
bool ControlFlowGraph::is_sequence_of_sequence_and_sequence(CfgVtx* b0, CfgVtx* b1) {
bool ControlFlowGraph::is_sequence_of_sequence_and_sequence(CfgVtx* b0,
CfgVtx* b1,
bool allow_self_loops) {
if (!b0 || !b1)
return false;
if (!dynamic_cast<SequenceVtx*>(b0))
return false;
if (!dynamic_cast<SequenceVtx*>(b1))
return false;
return is_sequence(b0, b1);
return is_sequence(b0, b1, allow_self_loops);
}
bool ControlFlowGraph::is_sequence_of_non_sequence_and_sequence(CfgVtx* b0, CfgVtx* b1) {
bool ControlFlowGraph::is_sequence_of_non_sequence_and_sequence(CfgVtx* b0,
CfgVtx* b1,
bool allow_self_loops) {
if (!b0 || !b1) {
return false;
}
@@ -981,7 +990,7 @@ bool ControlFlowGraph::is_sequence_of_non_sequence_and_sequence(CfgVtx* b0, CfgV
return false;
if (!dynamic_cast<SequenceVtx*>(b1))
return false;
return is_sequence(b0, b1);
return is_sequence(b0, b1, allow_self_loops);
}
/*!
@@ -989,13 +998,13 @@ bool ControlFlowGraph::is_sequence_of_non_sequence_and_sequence(CfgVtx* b0, CfgV
* To generate more readable debug output, we should aim to run this as infrequent and as
* late as possible, to avoid condition vertices with tons of extra junk packed in.
*/
bool ControlFlowGraph::find_seq_top_level() {
bool ControlFlowGraph::find_seq_top_level(bool allow_self_loops) {
bool replaced = false;
for_each_top_level_vtx([&](CfgVtx* vtx) {
auto* b0 = vtx;
auto* b1 = vtx->next;
if (is_sequence_of_non_sequences(b0, b1)) { // todo, avoid nesting sequences.
if (is_sequence_of_non_sequences(b0, b1, allow_self_loops)) { // todo, avoid nesting sequences.
replaced = true;
auto* new_seq = alloc<SequenceVtx>();
@@ -1028,7 +1037,7 @@ bool ControlFlowGraph::find_seq_top_level() {
return false;
}
if (is_sequence_of_sequence_and_non_sequence(b0, b1)) {
if (is_sequence_of_sequence_and_non_sequence(b0, b1, allow_self_loops)) {
// printf("make seq type 2 %s %s\n", b0->to_string().c_str(), b1->to_string().c_str());
replaced = true;
auto* seq = dynamic_cast<SequenceVtx*>(b0);
@@ -1051,7 +1060,7 @@ bool ControlFlowGraph::find_seq_top_level() {
return false;
}
if (is_sequence_of_non_sequence_and_sequence(b0, b1)) {
if (is_sequence_of_non_sequence_and_sequence(b0, b1, allow_self_loops)) {
replaced = true;
auto* seq = dynamic_cast<SequenceVtx*>(b1);
assert(seq);
@@ -1070,7 +1079,7 @@ bool ControlFlowGraph::find_seq_top_level() {
return false;
}
if (is_sequence_of_sequence_and_sequence(b0, b1)) {
if (is_sequence_of_sequence_and_sequence(b0, b1, allow_self_loops)) {
// printf("make seq type 3 %s %s\n", b0->to_string().c_str(), b1->to_string().c_str());
replaced = true;
auto* seq = dynamic_cast<SequenceVtx*>(b0);
@@ -2134,7 +2143,7 @@ std::shared_ptr<ControlFlowGraph> build_cfg(const LinkedObjectFile& file,
changed = changed || cfg->find_cond_w_else(cond_with_else_hack);
changed = changed || cfg->find_while_loop_top_level();
changed = changed || cfg->find_seq_top_level();
changed = changed || cfg->find_seq_top_level(false);
changed = changed || cfg->find_short_circuits();
changed = changed || cfg->find_cond_n_else();
@@ -2145,6 +2154,10 @@ std::shared_ptr<ControlFlowGraph> build_cfg(const LinkedObjectFile& file,
changed = changed || cfg->find_infinite_loop();
};
if (!changed) {
changed = changed || cfg->find_seq_top_level(true);
}
if (!changed) {
changed = changed || cfg->find_goto_not_end();
}
decompiler/Function/CfgVtx.h
+6 -6
View File
@@ -322,7 +322,7 @@ class ControlFlowGraph {
bool find_cond_n_else();
// bool find_if_else_top_level();
bool find_seq_top_level();
bool find_seq_top_level(bool allow_self_loops);
bool find_while_loop_top_level();
bool find_until_loop();
bool find_until1_loop();
@@ -363,11 +363,11 @@ class ControlFlowGraph {
private:
// bool compact_one_in_top_level();
// bool is_if_else(CfgVtx* b0, CfgVtx* b1, CfgVtx* b2, CfgVtx* b3);
bool is_sequence(CfgVtx* b0, CfgVtx* b1);
bool is_sequence_of_non_sequences(CfgVtx* b0, CfgVtx* b1);
bool is_sequence_of_sequence_and_non_sequence(CfgVtx* b0, CfgVtx* b1);
bool is_sequence_of_sequence_and_sequence(CfgVtx* b0, CfgVtx* b1);
bool is_sequence_of_non_sequence_and_sequence(CfgVtx* b0, CfgVtx* b1);
bool is_sequence(CfgVtx* b0, CfgVtx* b1, bool allow_self_loops);
bool is_sequence_of_non_sequences(CfgVtx* b0, CfgVtx* b1, bool allow_self_loops);
bool is_sequence_of_sequence_and_non_sequence(CfgVtx* b0, CfgVtx* b1, bool allow_self_loops);
bool is_sequence_of_sequence_and_sequence(CfgVtx* b0, CfgVtx* b1, bool allow_self_loops);
bool is_sequence_of_non_sequence_and_sequence(CfgVtx* b0, CfgVtx* b1, bool allow_self_loops);
bool is_while_loop(CfgVtx* b0, CfgVtx* b1, CfgVtx* b2);
bool is_until_loop(CfgVtx* b1, CfgVtx* b2);
bool is_goto_end_and_unreachable(CfgVtx* b0, CfgVtx* b1);
decompiler/IR2/FormExpressionAnalysis.cpp
+2 -1
View File
@@ -1846,7 +1846,8 @@ void FunctionCallElement::update_from_stack(const Env& env,
function_type = tp_type.typespec();
}
bool swap_function = tp_type.kind == TP_Type::Kind::NON_VIRTUAL_METHOD && true;
bool swap_function =
tp_type.kind == TP_Type::Kind::NON_VIRTUAL_METHOD && all_pop_vars.size() >= 2;
if (tp_type.kind == TP_Type::Kind::NON_VIRTUAL_METHOD) {
// this is a hack to make some weird macro for calling res-lump methods work
if (env.dts->ts.tc(TypeSpec("res-lump"), tp_type.method_from_type())) {
decompiler/analysis/stack_spill.cpp
+2
View File
@@ -78,6 +78,8 @@ struct StackInstrInfo {
constexpr StackInstrInfo stack_instrs[] = {{InstructionKind::SQ, false, 16, false},
{InstructionKind::LQ, true, 16, false},
{InstructionKind::SW, false, 4, false},
{InstructionKind::SB, false, 1, false},
{InstructionKind::LBU, true, 1, false},
//{InstructionKind::LWU, true, 4, false}
{InstructionKind::SD, false, 8, false},
{InstructionKind::SWC1, false, 4, false},
decompiler/analysis/variable_naming.cpp
+36 -2
View File
@@ -279,6 +279,16 @@ bool is_possible_coloring_move(Register dst, Register src) {
return false;
}
namespace {
int arg_count(const Function& f) {
if (f.type.arg_count() > 0) {
return f.type.arg_count() - 1;
} else {
return 0;
}
}
} // namespace
/*!
* Create a "really crude" SSA, as described in
* "Aycock and Horspool Simple Generation of Static Single-Assignment Form"
@@ -310,6 +320,8 @@ SSA make_rc_ssa(const Function& function, const RegUsageInfo& rui, const Functio
// local map: current register names at the current op.
std::unordered_map<Register, VarSSA, Register::hash> current_regs;
// if we're block zero, write function arguments:
// initialize phis. this is only done on:
// - variables live out at the first op
// - variables read by the first op
@@ -336,6 +348,19 @@ SSA make_rc_ssa(const Function& function, const RegUsageInfo& rui, const Functio
}
}
if (block_id == 0) {
SSA::Ins ins(-1);
for (int i = 0; i < arg_count(function); i++) {
auto dest_reg = Register::get_arg_reg(i);
auto it = current_regs.find(dest_reg);
if (it == current_regs.end()) {
current_regs.insert(std::make_pair(dest_reg, ssa.get_phi_dest(block_id, dest_reg)));
}
ins.src.push_back(current_regs.at(dest_reg));
}
ssa.blocks.at(block_id).ins.push_back(ins);
}
// loop over ops, creating and reading from variables as needed.
for (int op_id = start_op; op_id < end_op; op_id++) {
const auto& op = ops.ops.at(op_id);
@@ -546,7 +571,7 @@ void SSA::merge_all_phis() {
* Remaps all SSA variable ids to final variable IDs.
* This forces you to have all positive, consecutive IDs, with 0 being the entry value.
*/
void SSA::remap() {
void SSA::remap(int) {
// this keeps the order of variable assignments in the instruction order, not var_id order.
struct VarIdRecord {
std::unordered_set<int> set;
@@ -676,6 +701,9 @@ void SSA::make_vars(const Function& function, const DecompilerTypeSystem& dts) {
const TypeState* init_types = &function.ir2.env.get_types_at_block_entry(block_id);
for (auto& instr : block.ins) {
auto op_id = instr.op_id;
if (op_id < 0) {
continue;
}
const TypeState* end_types = &function.ir2.env.get_types_after_op(op_id);
@@ -762,6 +790,9 @@ VariableNames SSA::get_vars() const {
const auto& block = blocks.at(block_id);
for (auto& instr : block.ins) {
auto op_id = instr.op_id;
if (op_id < 0) {
continue;
}
if (instr.dst.has_value()) {
auto& ids = result.write_opid_to_varid[instr.dst->reg()];
if (int(ids.size()) <= op_id) {
@@ -776,6 +807,9 @@ VariableNames SSA::get_vars() const {
const auto& block = blocks.at(block_id);
for (auto& instr : block.ins) {
auto op_id = instr.op_id;
if (op_id < 0) {
continue;
}
for (auto& src : instr.src) {
auto& ids = result.read_opid_to_varid[src.reg()];
if (int(ids.size()) <= op_id) {
@@ -944,7 +978,7 @@ std::optional<VariableNames> run_variable_renaming(const Function& function,
// merge same vars (decided this made things worse)
// do rename
ssa.remap();
ssa.remap(arg_count(function));
if (debug_prints) {
fmt::print("{}", ssa.print());
}
decompiler/analysis/variable_naming.h
+4 -3
View File
@@ -81,9 +81,10 @@ class VarMapSSA {
private:
int get_next_var_id(Register reg);
// var id's are per register.
struct Entry {
int var_id = -1;
int entry_id = -1;
int var_id = -1; // our ID as a program variable (used for output)
int entry_id = -1; // our index in the entry list (used for remapping)
Register reg;
};
@@ -142,7 +143,7 @@ struct SSA {
bool simplify();
void merge_all_phis();
void remap();
void remap(int nargs);
void make_vars(const Function& function, const DecompilerTypeSystem& dts);
std::unordered_map<RegId, UseDefInfo, RegId::hash> get_use_def_info(
const RegAccessMap<int>& ssa_info) const;
decompiler/config/all-types.gc
+14 -14
View File
@@ -6174,7 +6174,7 @@
)
(deftype ocean-mid-masks (basic)
((data uint32 :offset-assert 4)
((data uint32 :offset-assert 4)
)
:pack-me
:method-count-assert 9
@@ -6242,10 +6242,10 @@
(far-color vector :inline :offset-assert 32)
(ocean-spheres ocean-spheres :offset-assert 48)
(ocean-colors ocean-colors :offset-assert 52)
(ocean-mid-indices basic :offset-assert 56)
(ocean-trans-indices basic :offset-assert 60)
(ocean-near-indices basic :offset-assert 64)
(ocean-mid-masks basic :offset-assert 68)
(ocean-mid-indices basic :offset-assert 56)
(ocean-trans-indices basic :offset-assert 60)
(ocean-near-indices basic :offset-assert 64)
(ocean-mid-masks basic :offset-assert 68)
)
:method-count-assert 9
:size-assert #x48
@@ -20414,7 +20414,7 @@
;; - Unknowns
;;(define-extern *ocean-map* object) ;; unknown type
(define-extern *ocean-map* ocean-map) ;; unknown type
;;(define-extern *swamp-low-ocean-marker* object) ;; unknown type
@@ -20464,15 +20464,15 @@
(define-extern ocean-mid-add-constants function)
(define-extern ocean-mid-add-call function)
(define-extern ocean-mid-add-upload function)
(define-extern ocean-mid-add-call-flush function)
(define-extern draw-ocean-transition function)
(define-extern ocean-mid-add-call-flush (function dma-buffer uint none))
(define-extern draw-ocean-transition (function dma-buffer none))
(define-extern draw-ocean-mid-seams function)
(define-extern ocean-seams-add-constants function)
(define-extern ocean-mid-add-upload-top function)
(define-extern ocean-mid-add-upload-bottom function)
(define-extern ocean-mid-add-upload-middle function)
(define-extern ocean-mid-camera-masks-bit? function)
(define-extern ocean-mid-mask-ptrs-bit? function)
(define-extern ocean-mid-camera-masks-bit? (function uint uint))
(define-extern ocean-mid-mask-ptrs-bit? (function uint uint))
(define-extern ocean-mid-add-upload-table function)
(define-extern ocean-mid-camera-masks-set! function)
(define-extern ocean-mid-add-matrices function)
@@ -20494,10 +20494,10 @@
;; - Functions
(define-extern ocean-make-trans-camera-masks function)
(define-extern ocean-trans-add-upload-strip function)
(define-extern ocean-trans-add-constants function)
(define-extern draw-ocean-transition-seams function)
(define-extern ocean-make-trans-camera-masks (function uint uint uint uint none))
(define-extern ocean-trans-add-upload-strip (function dma-buffer uint uint uint uint none))
(define-extern ocean-trans-add-constants (function dma-buffer none))
(define-extern draw-ocean-transition-seams (function dma-buffer none))
(define-extern ocean-trans-camera-masks-bit? function)
(define-extern ocean-trans-add-upload function)
(define-extern ocean-trans-mask-ptrs-bit? function)
decompiler/config/jak1_ntsc_black_label/stack_structures.jsonc
+12 -1
View File
@@ -376,5 +376,16 @@
"add-debug-curve": [[16, "vector"], [32, "vector"]],
"add-debug-points": [[16, "vector"]],
"add-debug-light": [[16, "vector"]],
"dma-timeout-cam": [[16, "vector"], [32, "matrix"]]
"dma-timeout-cam": [[16, "vector"], [32, "matrix"]],
"(method 18 tracking-spline)": [
[16, "tracking-spline-sampler"],
[32, "tracking-spline-sampler"]
],
"draw-ocean-transition": [
[16, "sphere"]
]
}
decompiler/util/DecompilerTypeSystem.cpp
+6 -1
View File
@@ -400,10 +400,15 @@ int DecompilerTypeSystem::get_format_arg_count(const std::string& str) const {
continue;
}
// ~3L, ~0L do'nt seem to take arguments either.
// ~3L, ~0L don't seem to take arguments either.
if (i + 1 < str.length() && (str.at(i) == '0' || str.at(i) == '3') && str.at(i + 1) == 'L') {
continue;
}
// ~1K
if (i + 1 < str.length() && (str.at(i) == '1') && str.at(i + 1) == 'K') {
continue;
}
arg_count++;
}
}
test/decompiler/reference/engine/gfx/ripple_REF.gc
+4 -4
View File
@@ -207,16 +207,16 @@
(local-vars (sv-16 float) (sv-32 float))
(let* ((f30-0 (-> arg0 root trans y))
(v1-1 (-> arg0 draw))
(a1-4 (-> v1-1 lod-set lod (-> v1-1 cur-lod) geo effect))
(a1-5 (-> v1-1 lod-set lod (-> v1-1 cur-lod) geo effect))
)
(if (or (zero? (logand (-> a1-4 0 effect-bits) 4)) (not (-> v1-1 ripple)))
(if (or (zero? (logand (-> a1-5 0 effect-bits) 4)) (not (-> v1-1 ripple)))
(return f30-0)
)
(let* ((a1-5 (-> a1-4 0 extra-info))
(let* ((a1-6 (-> a1-5 0 extra-info))
(s4-0
(the-as
mei-ripple
(+ (the-as uint a1-5) (the-as uint (* (-> a1-5 ripple-offset) 16)))
(+ (the-as uint a1-6) (the-as uint (* (-> a1-6 ripple-offset) 16)))
)
)
(gp-0 (-> v1-1 ripple))