From 73330302bafef0036103f693abeacb7a267dbdf9 Mon Sep 17 00:00:00 2001 From: water111 <48171810+water111@users.noreply.github.com> Date: Mon, 12 Jul 2021 19:29:56 -0400 Subject: [PATCH] fix bug in field lookup (#700) --- common/type_system/TypeFieldLookup.cpp | 16 +++---- goal_src/engine/gfx/generic/generic.gc | 42 ++++--------------- .../engine/gfx/generic/generic_REF.gc | 40 ++++-------------- test/decompiler/test_DataParser.cpp | 12 ++++++ 4 files changed, 37 insertions(+), 73 deletions(-) diff --git a/common/type_system/TypeFieldLookup.cpp b/common/type_system/TypeFieldLookup.cpp index 3985b8a662..83c5abb068 100644 --- a/common/type_system/TypeFieldLookup.cpp +++ b/common/type_system/TypeFieldLookup.cpp @@ -231,6 +231,8 @@ void try_reverse_lookup_array_like(const FieldReverseLookupInput& input, * - get something inside an object (variable idx) * - get a constant idx reference object (we pick this over just getting the array for idx = 0) * - get something inside a constant idx reference object + * + * Note: for an inline array of basics, the offset should include the basic offset. */ void try_reverse_lookup_inline_array(const FieldReverseLookupInput& input, const TypeSystem& ts, @@ -264,7 +266,7 @@ void try_reverse_lookup_inline_array(const FieldReverseLookupInput& input, FieldReverseLookupInput next_input; next_input.deref = input.deref; next_input.stride = 0; - next_input.offset = input.offset; + next_input.offset = input.offset; // includes the offset. next_input.base_type = di.result_type; try_reverse_lookup(next_input, ts, &var_idx_node, output, max_count); return; @@ -272,18 +274,17 @@ void try_reverse_lookup_inline_array(const FieldReverseLookupInput& input, // constant lookup, or accessing within the first one // which element we are in - int elt_idx = input.offset / di.stride; - // how many bytes into the element we look + int elt_idx = (ts.lookup_type(di.result_type)->get_offset() + input.offset) / di.stride; + // how many bytes into the element we look (including offset) int offset_into_elt = input.offset - (elt_idx * di.stride); // the expected number of bytes into the element we would look to grab a ref to the elt. - int expected_offset_into_elt = ts.lookup_type(di.result_type)->get_offset(); ReverseLookupNode const_idx_node; const_idx_node.prev = parent; const_idx_node.token.kind = FieldReverseLookupOutput::Token::Kind::CONSTANT_IDX; const_idx_node.token.idx = elt_idx; - if (offset_into_elt == expected_offset_into_elt && !input.deref.has_value()) { + if (offset_into_elt == 0 && !input.deref.has_value()) { // just get an element (possibly zero, and we want to include the 0 if so) // for the degenerate inline-array case, it seems more likely that we get the zeroth object // rather than the array, so this goes before that case. @@ -295,7 +296,7 @@ void try_reverse_lookup_inline_array(const FieldReverseLookupInput& input, } // can we just return the array? - if (expected_offset_into_elt == offset_into_elt && !input.deref.has_value() && elt_idx == 0) { + if (offset_into_elt == 0 && !input.deref.has_value() && elt_idx == 0) { auto parent_vec = parent_to_vector(parent); if (!parent_vec.empty()) { output->results.emplace_back(false, input.base_type, parent_to_vector(parent)); @@ -311,8 +312,7 @@ void try_reverse_lookup_inline_array(const FieldReverseLookupInput& input, FieldReverseLookupInput next_input; next_input.deref = input.deref; next_input.stride = input.stride; - // try_reverse_lookup expects "offset_into_field - boxed_offset" - next_input.offset = offset_into_elt - expected_offset_into_elt; + next_input.offset = offset_into_elt; next_input.base_type = di.result_type; try_reverse_lookup(next_input, ts, &const_idx_node, output, max_count); } diff --git a/goal_src/engine/gfx/generic/generic.gc b/goal_src/engine/gfx/generic/generic.gc index e73f4b4f82..febf5984d6 100644 --- a/goal_src/engine/gfx/generic/generic.gc +++ b/goal_src/engine/gfx/generic/generic.gc @@ -23,55 +23,37 @@ ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 0) - (the-as - generic-dma-foreground-sink - (-> *level* level0 foreground-sink-group 0 merc-sink) - ) + (-> *level* level0 foreground-sink-group 0 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 1) - (the-as - generic-dma-foreground-sink - (-> *level* level0 foreground-sink-group 1 merc-sink) - ) + (-> *level* level0 foreground-sink-group 1 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 2) - (the-as - generic-dma-foreground-sink - (-> *level* level1 foreground-sink-group 0 merc-sink) - ) + (-> *level* level1 foreground-sink-group 0 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 3) - (the-as - generic-dma-foreground-sink - (-> *level* level1 foreground-sink-group 1 merc-sink) - ) + (-> *level* level1 foreground-sink-group 1 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 4) - (the-as - generic-dma-foreground-sink - (-> *level* level-default foreground-sink-group 0 merc-sink) - ) + (-> *level* level-default foreground-sink-group 0 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 5) - (the-as - generic-dma-foreground-sink - (-> *level* level-default foreground-sink-group 1 merc-sink) - ) + (-> *level* level-default foreground-sink-group 1 generic-sink) ) ;; failed to figure out what this is: @@ -83,19 +65,13 @@ ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 7) - (the-as - generic-dma-foreground-sink - (-> *level* level0 foreground-sink-group 2 merc-sink) - ) + (-> *level* level0 foreground-sink-group 2 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 8) - (the-as - generic-dma-foreground-sink - (-> *level* level1 foreground-sink-group 2 merc-sink) - ) + (-> *level* level1 foreground-sink-group 2 generic-sink) ) ;; definition for function generic-dma-foreground-sink-init @@ -135,7 +111,7 @@ ) (dma-bucket-insert-tag (-> *display* frames (-> *display* on-screen) frame bucket-group) - (the bucket-id s3-0) ;; TODO, correct types here + (the-as bucket-id s3-0) s2-0 (the-as (pointer dma-tag) a3-0) ) diff --git a/test/decompiler/reference/engine/gfx/generic/generic_REF.gc b/test/decompiler/reference/engine/gfx/generic/generic_REF.gc index bd3048af6e..94ef55b64b 100644 --- a/test/decompiler/reference/engine/gfx/generic/generic_REF.gc +++ b/test/decompiler/reference/engine/gfx/generic/generic_REF.gc @@ -16,55 +16,37 @@ ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 0) - (the-as - generic-dma-foreground-sink - (-> *level* level0 foreground-sink-group 0 merc-sink) - ) + (-> *level* level0 foreground-sink-group 0 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 1) - (the-as - generic-dma-foreground-sink - (-> *level* level0 foreground-sink-group 1 merc-sink) - ) + (-> *level* level0 foreground-sink-group 1 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 2) - (the-as - generic-dma-foreground-sink - (-> *level* level1 foreground-sink-group 0 merc-sink) - ) + (-> *level* level1 foreground-sink-group 0 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 3) - (the-as - generic-dma-foreground-sink - (-> *level* level1 foreground-sink-group 1 merc-sink) - ) + (-> *level* level1 foreground-sink-group 1 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 4) - (the-as - generic-dma-foreground-sink - (-> *level* level-default foreground-sink-group 0 merc-sink) - ) + (-> *level* level-default foreground-sink-group 0 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 5) - (the-as - generic-dma-foreground-sink - (-> *level* level-default foreground-sink-group 1 merc-sink) - ) + (-> *level* level-default foreground-sink-group 1 generic-sink) ) ;; failed to figure out what this is: @@ -76,19 +58,13 @@ ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 7) - (the-as - generic-dma-foreground-sink - (-> *level* level0 foreground-sink-group 2 merc-sink) - ) + (-> *level* level0 foreground-sink-group 2 generic-sink) ) ;; failed to figure out what this is: (set! (-> *generic-foreground-sinks* 8) - (the-as - generic-dma-foreground-sink - (-> *level* level1 foreground-sink-group 2 merc-sink) - ) + (-> *level* level1 foreground-sink-group 2 generic-sink) ) ;; definition for function generic-dma-foreground-sink-init diff --git a/test/decompiler/test_DataParser.cpp b/test/decompiler/test_DataParser.cpp index 5341649643..b369ecb11a 100644 --- a/test/decompiler/test_DataParser.cpp +++ b/test/decompiler/test_DataParser.cpp @@ -390,3 +390,15 @@ TEST_F(DataDecompTest, KernelContext) { " :relocating-process #f\n" " :low-memory-message #t)\n"); } + +TEST_F(DataDecompTest, ReverseArtExt) { + FieldReverseLookupInput input; + input.base_type = TypeSpec("external-art-control"); + input.offset = 124; + auto result = dts->ts.reverse_field_multi_lookup(input); + EXPECT_EQ(result.results.at(0).tokens.at(2).print(), "name"); + + input.offset = 108; + result = dts->ts.reverse_field_multi_lookup(input); + EXPECT_EQ(result.results.at(0).tokens.at(2).print(), "type"); +} \ No newline at end of file