From 433993074adcef2ca81db5cb051128ef9190ba6b Mon Sep 17 00:00:00 2001
From: water111 <48171810+water111@users.noreply.github.com>
Date: Tue, 11 May 2021 16:43:13 -0400
Subject: [PATCH] Clean up some errors/crashes in decompiler (#452)

* fix offline tests, clean up some warnings

* clean up warnings during decomp

* fix remaining crash issues
---
 common/type_system/TypeSystem.cpp             | 139 +++++++++---------
 common/type_system/TypeSystem.h               |   1 +
 decompiler/Function/CfgVtx.cpp                |   2 -
 decompiler/IR2/Env.cpp                        |   4 +-
 decompiler/IR2/Env.h                          |   4 +-
 decompiler/IR2/FormExpressionAnalysis.cpp     |  32 ++--
 decompiler/IR2/IR2_common.h                   |  11 +-
 decompiler/ObjectFile/ObjectFileDB_IR2.cpp    |   6 +-
 decompiler/analysis/cfg_builder.cpp           |  18 +--
 decompiler/util/DecompilerTypeSystem.cpp      |   2 +-
 decompiler/util/data_decompile.cpp            |   6 +-
 goalc/compiler/Compiler.cpp                   |  11 +-
 goalc/compiler/Compiler.h                     |   1 +
 test/decompiler/FormRegressionTest.cpp        |   1 +
 .../reference/all_forward_declarations.gc     |  58 +++++++-
 .../reference/engine/load/file-io_REF.gc      |   1 +
 .../reference/engine/target/target-h_REF.gc   |   6 +-
 test/offline/offline_test_main.cpp            |   4 +-
 18 files changed, 194 insertions(+), 113 deletions(-)

diff --git a/common/type_system/TypeSystem.cpp b/common/type_system/TypeSystem.cpp
index a18bdbca3d..fc1e80c157 100644
--- a/common/type_system/TypeSystem.cpp
+++ b/common/type_system/TypeSystem.cpp
@@ -10,6 +10,21 @@
 #include <third-party/fmt/core.h>
 #include "TypeSystem.h"
 #include "common/util/math_util.h"
+#include "third-party/fmt/color.h"
+
+namespace {
+template <typename... Args>
+[[noreturn]] void throw_typesystem_error(const std::string& str, Args&&... args) {
+  fmt::print(fg(fmt::color::crimson) | fmt::emphasis::bold, "-- Type Error! --\n");
+  if (!str.empty() && str.back() == '\n') {
+    fmt::print(fg(fmt::color::yellow), str, std::forward<Args>(args)...);
+  } else {
+    fmt::print(fg(fmt::color::yellow), str + '\n', std::forward<Args>(args)...);
+  }
+
+  throw std::runtime_error("Type Error");
+}
+}  // namespace
 
 TypeSystem::TypeSystem() {
   // the "none" and "_type_" types are included by default.
@@ -29,10 +44,10 @@ Type* TypeSystem::add_type(const std::string& name, std::unique_ptr<Type> type)
 
     if (*kv->second != *type) {
       // exists, and we are trying to change it!
-      fmt::print("[TypeSystem] Type {} was originally\n{}\nand is redefined as\n{}\n",
-                 kv->second->get_name(), kv->second->print(), type->print());
 
       if (m_allow_redefinition) {
+        fmt::print("[TypeSystem] Type {} was originally\n{}\nand is redefined as\n{}\n",
+                   kv->second->get_name(), kv->second->print(), type->print());
         // extra dangerous, we have allowed type redefinition!
 
         // keep the unique_ptr around, just in case somebody references this old type pointer.
@@ -41,7 +56,9 @@ Type* TypeSystem::add_type(const std::string& name, std::unique_ptr<Type> type)
         // update the type
         m_types[name] = std::move(type);
       } else {
-        throw std::runtime_error("Type was redefined with throw_on_redefine set.");
+        throw_typesystem_error(
+            "Inconsistent type definition. Type {} was originally\n{}\nand is redefined as\n{}\n",
+            kv->second->get_name(), kv->second->print(), type->print());
       }
     }
   } else {
@@ -50,15 +67,14 @@ Type* TypeSystem::add_type(const std::string& name, std::unique_ptr<Type> type)
     // none/object get to skip these checks because they are roots.
     if (name != "object" && name != "none" && name != "_type_" && name != "_varargs_") {
       if (m_forward_declared_types.find(type->get_parent()) != m_forward_declared_types.end()) {
-        fmt::print("[TypeSystem] Type {} has incompletely defined parent {}\n", type->get_name(),
-                   type->get_parent());
-        throw std::runtime_error("add_type failed");
+        throw_typesystem_error(
+            "Cannot create new type {}. The parent type {} is not fully defined.\n",
+            type->get_name(), type->get_parent());
       }
 
       if (m_types.find(type->get_parent()) == m_types.end()) {
-        fmt::print("[TypeSystem] Type {} has undefined parent {}\n", type->get_name(),
-                   type->get_parent());
-        throw std::runtime_error("add_type failed");
+        throw_typesystem_error("Cannot create new type {}. The parent type {} is not defined.\n",
+                               type->get_name(), type->get_parent());
       }
     }
 
@@ -185,8 +201,7 @@ TypeSpec TypeSystem::make_typespec(const std::string& name) const {
       m_forward_declared_types.find(name) != m_forward_declared_types.end()) {
     return TypeSpec(name);
   } else {
-    fmt::print("[TypeSystem] The type {} is unknown.\n", name);
-    throw std::runtime_error("make_typespec failed");
+    throw_typesystem_error("Type {} is unknown\n", name);
   }
 }
 
@@ -194,6 +209,10 @@ bool TypeSystem::fully_defined_type_exists(const std::string& name) const {
   return m_types.find(name) != m_types.end();
 }
 
+bool TypeSystem::fully_defined_type_exists(const TypeSpec& type) const {
+  return fully_defined_type_exists(type.base_type());
+}
+
 bool TypeSystem::partially_defined_type_exists(const std::string& name) const {
   return m_forward_declared_types.find(name) != m_forward_declared_types.end();
 }
@@ -257,9 +276,9 @@ Type* TypeSystem::lookup_type(const std::string& name) const {
   }
 
   if (m_forward_declared_types.find(name) != m_forward_declared_types.end()) {
-    fmt::print("[TypeSystem] The type {} is not fully defined.\n", name);
+    throw_typesystem_error("Type {} is not fully defined.\n", name);
   } else {
-    fmt::print("[TypeSystem] The type {} is not defined.\n", name);
+    throw_typesystem_error("Type {} is not defined.\n", name);
   }
   throw std::runtime_error("lookup_type failed");
 }
@@ -300,17 +319,13 @@ Type* TypeSystem::lookup_type_allow_partial_def(const std::string& name) const {
     } else if (fwd_dec->second == BASIC) {
       return lookup_type("basic");
     } else {
-      fmt::print(
-          "[TypeSystem] The type {} is known to be a type, but has not been defined with deftype "
+      throw_typesystem_error(
+          "The type {} is known to be a type, but has not been defined with deftype "
           "or properly forward declared with declare-type\n",
           name);
     }
-
-  } else {
-    fmt::print("[TypeSystem] The type {} is not defined.\n", name);
   }
-
-  throw std::runtime_error("lookup_type_allow_partial_def failed");
+  throw_typesystem_error("The type {} is unknown.\n", name);
 }
 
 MethodInfo TypeSystem::add_method(const std::string& type_name,
@@ -359,21 +374,17 @@ MethodInfo TypeSystem::add_method(Type* type,
   if (got_existing) {
     // make sure we aren't changing anything.
     if (!existing_info.type.is_compatible_child_method(ts, type->get_name())) {
-      fmt::print(
-          "[TypeSystem] The method {} of type {} was originally defined as {}, but has been "
+      throw_typesystem_error(
+          "The method {} of type {} was originally defined as {}, but has been "
           "redefined as {}\n",
           method_name, type->get_name(), existing_info.type.print(), ts.print());
-      // unlike type re-definition, method re-definition is almost certain to go wrong.
-      // probably better to give up.
-      throw std::runtime_error("method redefinition");
     }
 
     return existing_info;
   } else {
     if (!allow_new_method) {
-      fmt::print("[TypeSystem] Attempted to add method {} to type {} but it was not declared.\n",
-                 method_name, type->get_name());
-      throw std::runtime_error("illegal method definition");
+      throw_typesystem_error("Cannot add method {} to type {} because it was not declared.\n",
+                             method_name, type->get_name());
     }
     // add a new method!
     return type->add_method({get_next_method_id(type), method_name, ts, type->get_name()});
@@ -390,11 +401,10 @@ MethodInfo TypeSystem::add_new_method(Type* type, const TypeSpec& ts) {
   if (type->get_my_new_method(&existing)) {
     // it exists!
     if (!existing.type.is_compatible_child_method(ts, type->get_name())) {
-      fmt::print(
-          "[TypeSystem] The new method of {} was originally defined as {}, but has been redefined "
-          "as {}\n",
+      throw_typesystem_error(
+          "Cannot add new method. Type does not match declaration. The new method of {} was "
+          "originally defined as {}, but has been redefined as {}\n",
           type->get_name(), existing.type.print(), ts.print());
-      throw std::runtime_error("add_new_method failed");
     }
 
     return existing;
@@ -433,8 +443,7 @@ MethodInfo TypeSystem::lookup_method(const std::string& type_name,
     }
   }
 
-  fmt::print("[TypeSystem] The method {} of type {} could not be found.\n", method_name, type_name);
-  throw std::runtime_error("lookup_method failed");
+  throw_typesystem_error("The method {} of type {} could not be found.\n", method_name, type_name);
 }
 
 bool TypeSystem::try_lookup_method(const std::string& type_name,
@@ -531,9 +540,8 @@ MethodInfo TypeSystem::lookup_method(const std::string& type_name, int method_id
     }
   }
 
-  fmt::print("[TypeSystem] The method with id {} of type {} could not be found.\n", method_id,
-             type_name);
-  throw std::runtime_error("lookup_method failed");
+  throw_typesystem_error("The method with id {} of type {} could not be found.\n", method_id,
+                         type_name);
 }
 
 /*!
@@ -560,8 +568,7 @@ MethodInfo TypeSystem::lookup_new_method(const std::string& type_name) const {
     }
   }
 
-  fmt::print("[TypeSystem] The new method of type {} could not be found.\n", type_name);
-  throw std::runtime_error("lookup_new_method failed");
+  throw_typesystem_error("The new method of type {} could not be found.\n", type_name);
 }
 
 /*!
@@ -572,9 +579,9 @@ void TypeSystem::assert_method_id(const std::string& type_name,
                                   int id) {
   auto info = lookup_method(type_name, method_name);
   if (info.id != id) {
-    fmt::print(
-        "[TypeSystem] Method ID assertion failed: type {}, method {} id was {}, expected {}\n",
-        type_name, method_name, info.id, id);
+    throw_typesystem_error(
+        "Method ID assertion failed: type {}, method {} id was {}, expected {}\n", type_name,
+        method_name, info.id, id);
   }
 }
 
@@ -634,8 +641,8 @@ void TypeSystem::assert_field_offset(const std::string& type_name,
                                      int offset) {
   Field field = lookup_field(type_name, field_name);
   if (field.offset() != offset) {
-    fmt::print("[TypeSystem] assert_field_offset({}, {}, {}) failed - got {}\n", type_name,
-               field_name, offset);
+    throw_typesystem_error("assert_field_offset({}, {}, {}) failed - got {}\n", type_name,
+                           field_name, offset);
     throw std::runtime_error("assert_field_offset failed");
   }
 }
@@ -652,8 +659,7 @@ int TypeSystem::add_field_to_type(StructureType* type,
                                   int offset_override,
                                   bool skip_in_static_decomp) {
   if (type->lookup_field(field_name, nullptr)) {
-    fmt::print("[TypeSystem] Type {} already has a field named {}\n", type->get_name(), field_name);
-    throw std::runtime_error("add_field_to_type duplicate field names");
+    throw_typesystem_error("Type {} already has a field named {}\n", type->get_name(), field_name);
   }
 
   // first, construct the field
@@ -681,11 +687,8 @@ int TypeSystem::add_field_to_type(StructureType* type,
     int aligned_offset = align(offset, field_alignment);
     field.mark_as_user_placed();
     if (offset != aligned_offset) {
-      fmt::print(
-          "[TypeSystem] Tried to overwrite offset of field to be {}, but it is not aligned "
-          "correctly\n",
-          offset);
-      throw std::runtime_error("add_field_to_type bad offset_override");
+      throw_typesystem_error("Tried to place field {} at {}, but it is not aligned correctly\n",
+                             field_name, offset);
     }
   }
 
@@ -909,8 +912,7 @@ Field TypeSystem::lookup_field(const std::string& type_name, const std::string&
   auto type = get_type_of_type<StructureType>(type_name);
   Field field;
   if (!type->lookup_field(field_name, &field)) {
-    fmt::print("[TypeSystem] Type {} has no field named {}\n", type_name, field_name);
-    throw std::runtime_error("lookup_field failed");
+    throw_typesystem_error("Type {} has no field named {}\n", type_name, field_name);
   }
   return field;
 }
@@ -961,11 +963,14 @@ int TypeSystem::get_size_in_type(const Field& field) const {
 
   if (field.is_array()) {
     if (field.is_inline()) {
+      if (!fully_defined_type_exists(field.type())) {
+        throw_typesystem_error("Cannot use the forward-declared type {} in an inline array.\n",
+                               field.type().print());
+      }
       if (!allow_inline(field_type)) {
-        fmt::print(
-            "[Type System] Attempted to use `{}` inline, this probably isn't what you wanted.\n",
+        throw_typesystem_error(
+            "Attempted to use `{}` inline, this probably isn't what you wanted.\n",
             field_type->get_name());
-        throw std::runtime_error("bad get size in type");
       }
       assert(field_type->is_reference());
       return field.array_size() * align(field_type->get_size_in_memory(),
@@ -981,12 +986,15 @@ int TypeSystem::get_size_in_type(const Field& field) const {
   } else {
     // not an array
     if (field.is_inline()) {
+      if (!fully_defined_type_exists(field.type())) {
+        throw_typesystem_error("Cannot use the forward-declared type {} inline.\n",
+                               field.type().print());
+      }
       if (!allow_inline(field_type)) {
-        fmt::print(
-            "[Type System] Attempted to use `{}` inline, this probably isn't what you wanted. Type "
+        throw_typesystem_error(
+            "Attempted to use `{}` inline, this probably isn't what you wanted. Type "
             "may not be defined fully.\n",
             field_type->get_name());
-        throw std::runtime_error("bad get size in type");
       }
       assert(field_type->is_reference());
       // return align(field_type->get_size_in_memory(), field_type->get_in_memory_alignment());
@@ -1275,8 +1283,7 @@ BitfieldLookupInfo TypeSystem::lookup_bitfield_info(const std::string& type_name
   auto type = get_type_of_type<BitFieldType>(type_name);
   BitField f;
   if (!type->lookup_field(field_name, &f)) {
-    fmt::print("[TypeSystem] Type {} has no bitfield named {}\n", type_name, field_name);
-    throw std::runtime_error("lookup_bitfield failed");
+    throw_typesystem_error("Type {} has no bitfield named {}\n", type_name, field_name);
   }
 
   BitfieldLookupInfo result;
@@ -1303,19 +1310,17 @@ void TypeSystem::add_field_to_bitfield(BitFieldType* type,
   }
 
   if (field_size > load_size) {
-    fmt::print(
-        "[TypeSystem] Type {}'s bitfield {}'s set size is {}, which is larger than the actual "
+    throw_typesystem_error(
+        "Type {}'s bitfield {}'s set size is {}, which is larger than the actual "
         "type: {}\n",
         type->get_name(), field_name, field_size, load_size);
-    throw std::runtime_error("Failed to add bitfield to type");
   }
 
   if (field_size + offset > type->get_load_size() * 8) {
-    fmt::print(
-        "[TypeSystem] Type {}'s bitfield {} will run off the end of the type (ends at {} bits, "
+    throw_typesystem_error(
+        "Type {}'s bitfield {} will run off the end of the type (ends at {} bits, "
         "type is {} bits)\n",
         type->get_name(), field_name, field_size + offset, type->get_load_size() * 8);
-    throw std::runtime_error("Failed to add bitfield to type");
   }
   BitField field(field_type, field_name, offset, field_size);
   type->m_fields.push_back(field);
diff --git a/common/type_system/TypeSystem.h b/common/type_system/TypeSystem.h
index cc558c7f85..f88df24f92 100644
--- a/common/type_system/TypeSystem.h
+++ b/common/type_system/TypeSystem.h
@@ -100,6 +100,7 @@ class TypeSystem {
   FieldReverseLookupOutput reverse_field_lookup(const FieldReverseLookupInput& input) const;
 
   bool fully_defined_type_exists(const std::string& name) const;
+  bool fully_defined_type_exists(const TypeSpec& type) const;
   bool partially_defined_type_exists(const std::string& name) const;
   TypeSpec make_typespec(const std::string& name) const;
   TypeSpec make_array_typespec(const TypeSpec& element_type) const;
diff --git a/decompiler/Function/CfgVtx.cpp b/decompiler/Function/CfgVtx.cpp
index 812ba132c0..1737d0cbdd 100644
--- a/decompiler/Function/CfgVtx.cpp
+++ b/decompiler/Function/CfgVtx.cpp
@@ -1386,8 +1386,6 @@ bool ControlFlowGraph::find_cond_w_empty_else() {
       new_cwe->prev->next = new_cwe;
     }
 
-    lg::error("There is a very strange control flow here, please check it manually.");
-
     // link new_cwe <-> end
     std::vector<CfgVtx*> to_replace;
     // to_replace.push_back(else_block);
diff --git a/decompiler/IR2/Env.cpp b/decompiler/IR2/Env.cpp
index 22758c5e41..0443ad6800 100644
--- a/decompiler/IR2/Env.cpp
+++ b/decompiler/IR2/Env.cpp
@@ -483,9 +483,9 @@ const UseDefInfo& Env::get_use_def_info(const RegisterAccess& ra) const {
   return m_var_names.use_def_info.at(var_id);
 }
 
-void Env::disable_def(const RegisterAccess& access) {
+void Env::disable_def(const RegisterAccess& access, DecompWarnings& warnings) {
   if (has_local_vars()) {
-    m_var_names.disable_def(access);
+    m_var_names.disable_def(access, warnings);
   }
 }
 
diff --git a/decompiler/IR2/Env.h b/decompiler/IR2/Env.h
index 832d3c331a..bacb7d0e92 100644
--- a/decompiler/IR2/Env.h
+++ b/decompiler/IR2/Env.h
@@ -168,7 +168,7 @@ class Env {
     }
   }
 
-  void disable_def(const RegisterAccess& access);
+  void disable_def(const RegisterAccess& access, DecompWarnings& warnings);
 
   void set_defined_in_let(const std::string& var) { m_vars_defined_in_let.insert(var); }
 
@@ -180,6 +180,8 @@ class Env {
   // todo - remove these hacks at some point.
   LinkedObjectFile* file = nullptr;
   DecompilerTypeSystem* dts = nullptr;
+  Function* func = nullptr;
+
   std::unordered_map<int, StackSpillEntry> stack_slot_entries;
 
   std::string get_spill_slot_var_name(int offset) const {
diff --git a/decompiler/IR2/FormExpressionAnalysis.cpp b/decompiler/IR2/FormExpressionAnalysis.cpp
index a4a59a58fd..337ea05bea 100644
--- a/decompiler/IR2/FormExpressionAnalysis.cpp
+++ b/decompiler/IR2/FormExpressionAnalysis.cpp
@@ -52,6 +52,10 @@
 
 namespace decompiler {
 
+namespace {
+bool debug_method_calls = false;
+}
+
 bool Form::has_side_effects() {
   bool has_side_effect = false;
   apply([&](FormElement* elt) {
@@ -1740,16 +1744,20 @@ void FunctionCallElement::update_from_stack(const Env& env,
         if (unsafe->try_as_single_element()->to_form(env) ==
             mr.maps.forms.at(0)->try_as_single_element()->to_form(env)) {
           resolved = true;
-          lg::warn(
-              fmt::format("Rare method call (type 1). {} vs {}. Not an error, but check carefully",
-                          unsafe->to_string(env), mr.maps.forms.at(0)->to_string(env)));
+          if (debug_method_calls) {
+            lg::warn(fmt::format(
+                "Rare method call (type 1). {} vs {}. Not an error, but check carefully",
+                unsafe->to_string(env), mr.maps.forms.at(0)->to_string(env)));
+          }
         }
       }
 
       if (!resolved) {
-        lg::warn(
-            fmt::format("Rare method call (type 2). {} vs {}. Not an error, but check carefully",
-                        unsafe->to_string(env), mr.maps.forms.at(0)->to_string(env)));
+        if (debug_method_calls) {
+          lg::warn(
+              fmt::format("Rare method call (type 2). {} vs {}. Not an error, but check carefully",
+                          unsafe->to_string(env), mr.maps.forms.at(0)->to_string(env)));
+        }
 
         auto unsafe_as_se = dynamic_cast<SimpleExpressionElement*>(unsafe->try_as_single_element());
         if (unsafe_as_se && unsafe_as_se->expr().is_identity() &&
@@ -1763,7 +1771,9 @@ void FunctionCallElement::update_from_stack(const Env& env,
             } else {
               if (unsafe_2->try_as_single_element()->to_form(env) ==
                   mr.maps.forms.at(0)->try_as_single_element()->to_form(env)) {
-                lg::warn("Check even more carefully");
+                if (debug_method_calls) {
+                  lg::warn("Check even more carefully");
+                }
                 resolved = true;
                 unsafe = unsafe_2;
               }
@@ -1879,12 +1889,8 @@ void FunctionCallElement::update_from_stack(const Env& env,
             result->push_back(new_op);
             return;
           }
-        } else {
-          lg::warn("Got a suspicious new method. This may be fine, but should be uncommon: {}",
-                   temp_form->to_string(env));
-          //          throw std::runtime_error("Failed to match new method: " +
-          //          temp_form->to_string(env));
         }
+        // possible else case here to catch fixed-type new's in a nicer way
       }
     }
   }
@@ -2165,7 +2171,7 @@ void CondWithElseElement::push_to_stack(const Env& env, FormPool& pool, FormStac
     for (size_t i = 0; i < dest_sets.size() - 1; i++) {
       auto var = dest_sets.at(i)->dst();
       auto* env2 = const_cast<Env*>(&env);
-      env2->disable_def(var);
+      env2->disable_def(var, env2->func->warnings);
     }
   }
 
diff --git a/decompiler/IR2/IR2_common.h b/decompiler/IR2/IR2_common.h
index bb008a1c21..353a4a721b 100644
--- a/decompiler/IR2/IR2_common.h
+++ b/decompiler/IR2/IR2_common.h
@@ -5,6 +5,7 @@
 #include "decompiler/Disasm/Register.h"
 #include "decompiler/util/TP_Type.h"
 #include "third-party/fmt/core.h"
+#include "decompiler/Function/Warnings.h"
 
 namespace decompiler {
 enum class AccessMode : u8 {
@@ -172,13 +173,13 @@ struct UseDefInfo {
     throw std::runtime_error("Invalid disable use");
   }
 
-  void disable_def(int op_id) {
+  void disable_def(int op_id, DecompWarnings& warnings) {
     for (auto& x : defs) {
       if (x.op_id == op_id) {
         if (x.disabled) {
-          lg::warn(
+          warnings.general_warning(
               "disable def twice: {}. This may happen when a cond (no else) is nested inside of "
-              "another conditional, but it should be rare.\n",
+              "another conditional, but it should be rare.",
               x.op_id);
         }
         x.disabled = true;
@@ -240,10 +241,10 @@ struct VariableNames {
     use_def_info.at(RegId(access.reg(), var_id)).disable_use(access.idx());
   }
 
-  void disable_def(const RegisterAccess& access) {
+  void disable_def(const RegisterAccess& access, DecompWarnings& warnings) {
     assert(access.mode() == AccessMode::WRITE);
     auto var_id = write_opid_to_varid.at(access.reg()).at(access.idx());
-    use_def_info.at(RegId(access.reg(), var_id)).disable_def(access.idx());
+    use_def_info.at(RegId(access.reg(), var_id)).disable_def(access.idx(), warnings);
   }
 
   const VarInfo& lookup(Register reg, int op_id, AccessMode mode) const {
diff --git a/decompiler/ObjectFile/ObjectFileDB_IR2.cpp b/decompiler/ObjectFile/ObjectFileDB_IR2.cpp
index c11a48c198..e5976a92d9 100644
--- a/decompiler/ObjectFile/ObjectFileDB_IR2.cpp
+++ b/decompiler/ObjectFile/ObjectFileDB_IR2.cpp
@@ -183,6 +183,7 @@ void ObjectFileDB::ir2_basic_block_pass() {
     total_functions++;
     func.ir2.env.file = &data.linked_data;
     func.ir2.env.dts = &dts;
+    func.ir2.env.func = &func;
 
     // first, find basic blocks.
     auto blocks = find_blocks_in_function(data.linked_data, segment_id, func);
@@ -206,14 +207,12 @@ void ObjectFileDB::ir2_basic_block_pass() {
       // run analysis
 
       // build a control flow graph, just looking at branch instructions.
-      //      if (func.guessed_name.to_string() == "abs") {
       func.cfg = build_cfg(data.linked_data, segment_id, func);
       if (!func.cfg->is_fully_resolved()) {
         lg::warn("Function {} from {} failed to build control flow graph!",
                  func.guessed_name.to_string(), data.to_unique_name());
         failed_to_build_cfg++;
       }
-      //      }
 
       // if we got an inspect method, inspect it.
       if (func.is_inspect_method) {
@@ -317,7 +316,8 @@ void ObjectFileDB::ir2_type_analysis_pass() {
     if (!func.suspected_asm) {
       non_asm_functions++;
       TypeSpec ts;
-      if (lookup_function_type(func.guessed_name, data.to_unique_name(), &ts)) {
+      if (lookup_function_type(func.guessed_name, data.to_unique_name(), &ts) &&
+          func.ir2.atomic_ops_succeeded) {
         func.type = ts;
         attempted_functions++;
         // try type analysis here.
diff --git a/decompiler/analysis/cfg_builder.cpp b/decompiler/analysis/cfg_builder.cpp
index 83f9227815..f2cd360492 100644
--- a/decompiler/analysis/cfg_builder.cpp
+++ b/decompiler/analysis/cfg_builder.cpp
@@ -341,11 +341,11 @@ bool try_clean_up_sc_as_and(FormPool& pool, Function& func, ShortCircuitElement*
           } else {
             // lg::warn("Disabling def of {} in final or delay slot",
             // as_set->to_string(func.ir2.env));
-            func.ir2.env.disable_def(as_set->dst());
+            func.ir2.env.disable_def(as_set->dst(), func.warnings);
           }
         } else {
           // lg::warn("Disabling def of {} in or delay slot", as_set->to_string(func.ir2.env));
-          func.ir2.env.disable_def(as_set->dst());
+          func.ir2.env.disable_def(as_set->dst(), func.warnings);
         }
       }
 
@@ -464,12 +464,12 @@ bool try_clean_up_sc_as_or(FormPool& pool, Function& func, ShortCircuitElement*
           } else {
             // lg::warn("Disabling def of {} in final or delay slot",
             // as_set->to_string(func.ir2.env));
-            func.ir2.env.disable_def(as_set->dst());
+            func.ir2.env.disable_def(as_set->dst(), func.warnings);
           }
 
         } else {
           // lg::warn("Disabling def of {} in or delay slot", as_set->to_string(func.ir2.env));
-          func.ir2.env.disable_def(as_set->dst());
+          func.ir2.env.disable_def(as_set->dst(), func.warnings);
         }
       }
 
@@ -612,7 +612,7 @@ void convert_cond_no_else_to_compare(FormPool& pool,
   assert(cne->entries.size() == 1);
 
   // safe to do this here because we never give up on this.
-  f.ir2.env.disable_def(condition.first->op()->branch_delay().var(0));
+  f.ir2.env.disable_def(condition.first->op()->branch_delay().var(0), f.warnings);
 
   auto condition_as_single =
       dynamic_cast<BranchElement*>(cne->entries.front().condition->try_as_single_element());
@@ -692,7 +692,7 @@ void clean_up_cond_no_else_final(Function& func, CondNoElseElement* cne) {
       if (func.ir2.env.has_reg_use()) {
         auto reg = cne->entries.at(i).false_destination;
         // lg::warn("Disable def of {} at {}\n", reg->to_string(func.ir2.env), reg->idx());
-        func.ir2.env.disable_def(*reg);
+        func.ir2.env.disable_def(*reg, func.warnings);
       }
     }
   }
@@ -1132,7 +1132,7 @@ Form* try_sc_as_ash(FormPool& pool, Function& f, const ShortCircuit* vtx) {
   f.ir2.env.disable_use(dsubu_var);
 
   // and the def too
-  f.ir2.env.disable_def(dsrav_set->dst());
+  f.ir2.env.disable_def(dsrav_set->dst(), f.warnings);
 
   return b0_c_ptr;
 }
@@ -1283,8 +1283,8 @@ Form* try_sc_as_type_of(FormPool& pool, Function& f, const ShortCircuit* vtx) {
   b0_ptr->push_back(op);
 
   // fix register info
-  f.ir2.env.disable_def(b0_delay_op.dst());
-  f.ir2.env.disable_def(b1_delay_op.dst());
+  f.ir2.env.disable_def(b0_delay_op.dst(), f.warnings);
+  f.ir2.env.disable_def(b1_delay_op.dst(), f.warnings);
   f.ir2.env.disable_use(shift->expr().get_arg(0).var());
 
   return b0_ptr;
diff --git a/decompiler/util/DecompilerTypeSystem.cpp b/decompiler/util/DecompilerTypeSystem.cpp
index d5aa9acf1e..592c62a407 100644
--- a/decompiler/util/DecompilerTypeSystem.cpp
+++ b/decompiler/util/DecompilerTypeSystem.cpp
@@ -89,7 +89,7 @@ void DecompilerTypeSystem::parse_type_defs(const std::vector<std::string>& file_
       }
     } catch (std::exception& e) {
       auto info = m_reader.db.get_info_for(o);
-      lg::error("Error {} when parsing decompiler type file:\n{}", e.what(), info);
+      lg::error("{} when parsing decompiler type file:\n{}", e.what(), info);
       throw e;
     }
   });
diff --git a/decompiler/util/data_decompile.cpp b/decompiler/util/data_decompile.cpp
index 9db4e70698..c18ef210d0 100644
--- a/decompiler/util/data_decompile.cpp
+++ b/decompiler/util/data_decompile.cpp
@@ -324,8 +324,6 @@ goos::Object decompile_structure(const TypeSpec& type,
         // we can specify a more specific type.
         auto got_type = TypeSpec(word.symbol_name);
         if (ts.tc(actual_type, got_type)) {
-          lg::info("For type {}, got more specific type {}\n", actual_type.print(),
-                   got_type.print());
           actual_type = got_type;
           if (actual_type == TypeSpec("string")) {
             return decompile_string_at_label(label, words);
@@ -601,7 +599,7 @@ goos::Object decompile_value(const TypeSpec& type,
     assert(bytes.size() == 4);
     s32 value;
     memcpy(&value, bytes.data(), 4);
-    if (value > 100 && value <= INT32_MAX) {
+    if (value > 100) {
       return pretty_print::to_symbol(fmt::format("#x{:x}", value));
     } else {
       return pretty_print::to_symbol(fmt::format("{}", value));
@@ -615,7 +613,7 @@ goos::Object decompile_value(const TypeSpec& type,
     assert(bytes.size() == 2);
     s16 value;
     memcpy(&value, bytes.data(), 2);
-    if (value > 100 && value <= INT16_MAX) {
+    if (value > 100) {
       return pretty_print::to_symbol(fmt::format("#x{:x}", value));
     } else {
       return pretty_print::to_symbol(fmt::format("{}", value));
diff --git a/goalc/compiler/Compiler.cpp b/goalc/compiler/Compiler.cpp
index d97e017d42..37778ce5ee 100644
--- a/goalc/compiler/Compiler.cpp
+++ b/goalc/compiler/Compiler.cpp
@@ -329,13 +329,22 @@ bool Compiler::connect_to_target() {
 
 /*!
  * Just run the front end on a string. Will not do register allocation or code generation.
- * Useful for typechecking or running strings that invoke the compiler again.
+ * Useful for typechecking, defining types,  or running strings that invoke the compiler again.
  */
 void Compiler::run_front_end_on_string(const std::string& src) {
   auto code = m_goos.reader.read_from_string({src});
   compile_object_file("run-on-string", code, true);
 }
 
+/*!
+ * Just run the front end on a file. Will not do register allocation or code generation.
+ * Useful for typechecking, defining types,  or running strings that invoke the compiler again.
+ */
+void Compiler::run_front_end_on_file(const std::vector<std::string>& path) {
+  auto code = m_goos.reader.read_from_file(path);
+  compile_object_file("run-on-file", code, true);
+}
+
 /*!
  * Run the entire compilation process on the input source code. Will generate an object file, but
  * won't save it anywhere.
diff --git a/goalc/compiler/Compiler.h b/goalc/compiler/Compiler.h
index 366a2c46b4..792782b1f2 100644
--- a/goalc/compiler/Compiler.h
+++ b/goalc/compiler/Compiler.h
@@ -38,6 +38,7 @@ class Compiler {
   std::vector<std::string> run_test_no_load(const std::string& source_code);
   void compile_and_send_from_string(const std::string& source_code);
   void run_front_end_on_string(const std::string& src);
+  void run_front_end_on_file(const std::vector<std::string>& path);
   void run_full_compiler_on_string_no_save(const std::string& src);
   void shutdown_target();
   void enable_throw_on_redefines() { m_throw_on_define_extern_redefinition = true; }
diff --git a/test/decompiler/FormRegressionTest.cpp b/test/decompiler/FormRegressionTest.cpp
index f03e934e62..6d0f0d756d 100644
--- a/test/decompiler/FormRegressionTest.cpp
+++ b/test/decompiler/FormRegressionTest.cpp
@@ -133,6 +133,7 @@ std::unique_ptr<FormRegressionTest::TestData> FormRegressionTest::make_function(
   // Set up the environment
   test->func.ir2.env.file = &test->file;
   test->func.ir2.env.dts = dts.get();
+  test->func.ir2.env.func = &test->func;
   // Set up the function
   test->func.instructions = program.instructions;
   test->func.guessed_name.set_as_global("test-function");
diff --git a/test/decompiler/reference/all_forward_declarations.gc b/test/decompiler/reference/all_forward_declarations.gc
index f2ac324fa5..703109df15 100644
--- a/test/decompiler/reference/all_forward_declarations.gc
+++ b/test/decompiler/reference/all_forward_declarations.gc
@@ -472,6 +472,19 @@
   )
 
 ;; game-h - TODO
+(deftype vector (structure)
+  ((data float 4 :offset-assert 0)
+   (x float :offset 0)
+   (y float :offset 4)
+   (z float :offset 8)
+   (w float :offset 12)
+   (quad uint128 :offset 0)
+   )
+  :method-count-assert 9
+  :size-assert         #x10
+  :flag-assert         #x900000010
+  )
+
 (deftype attack-info (structure)
   ((trans        vector :inline :offset-assert 0)
    (vector       vector :inline :offset-assert 16)
@@ -496,4 +509,47 @@
   (:methods
     (dummy-9 () none 9)
     )
-  )
\ No newline at end of file
+  )
+
+;; definition of type quaternion
+(deftype quaternion (structure)
+  ((x    float                       :offset-assert 0)
+   (y    float                       :offset-assert 4)
+   (z    float                       :offset-assert 8)
+   (w    float                       :offset-assert 12)
+   (data float   4 :offset   0)
+   (vec  vector  :inline :offset   0)
+   (quad uint128 :offset   0)
+   )
+  :method-count-assert 9
+  :size-assert         #x10
+  :flag-assert         #x900000010
+  )
+
+(deftype transform (structure)
+  ((trans vector :inline :offset-assert 0)
+   (rot   vector :inline :offset-assert 16)
+   (scale vector :inline :offset-assert 32)
+   )
+  :method-count-assert 9
+  :size-assert         #x30
+  :flag-assert         #x900000030
+  )
+
+;; transformq
+(deftype transformq (transform)
+  ;; this overlays the rot field of transform.
+  ((quat  quaternion :inline :offset 16)
+   )
+  :method-count-assert 9
+  :size-assert         #x30
+  :flag-assert         #x900000030
+  )
+
+
+(declare-type target basic)
+(define-extern *target* target)
+
+(declare-type sidekick basic)
+(define-extern *sidekick* basic)
+
diff --git a/test/decompiler/reference/engine/load/file-io_REF.gc b/test/decompiler/reference/engine/load/file-io_REF.gc
index 4a85650886..e1e07671d2 100644
--- a/test/decompiler/reference/engine/load/file-io_REF.gc
+++ b/test/decompiler/reference/engine/load/file-io_REF.gc
@@ -149,6 +149,7 @@
   )
 
 ;; 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/reference/engine/target/target-h_REF.gc b/test/decompiler/reference/engine/target/target-h_REF.gc
index d5182ae6e2..6a024dabf4 100644
--- a/test/decompiler/reference/engine/target/target-h_REF.gc
+++ b/test/decompiler/reference/engine/target/target-h_REF.gc
@@ -28,9 +28,10 @@
    (no-load-wait        uint64                  :offset-assert 568)
    (no-look-around-wait uint64                  :offset-assert 576)
    )
+  :heap-base #x1e0
   :method-count-assert 21
   :size-assert         #x248
-  :flag-assert         #x1500000248
+  :flag-assert         #x1501e00248
   (:methods
     (dummy-20 () none 20)
     )
@@ -79,9 +80,10 @@
    (anim-seed        uint64 :offset 192)
    (shadow-in-movie? basic              :offset-assert 200)
    )
+  :heap-base #x60
   :method-count-assert 20
   :size-assert         #xcc
-  :flag-assert         #x14000000cc
+  :flag-assert         #x14006000cc
   )
 
 ;; definition for method 3 of type sidekick
diff --git a/test/offline/offline_test_main.cpp b/test/offline/offline_test_main.cpp
index 4232eafe19..727563f878 100644
--- a/test/offline/offline_test_main.cpp
+++ b/test/offline/offline_test_main.cpp
@@ -489,8 +489,8 @@ int line_count(const std::string& str) {
 TEST_F(OfflineDecompilation, Compile) {
   Compiler compiler;
 
-  compiler.run_front_end_on_string(file_util::read_text_file(file_util::get_file_path(
-      {"test", "decompiler", "reference", "all_forward_declarations.gc"})));
+  compiler.run_front_end_on_file(
+      {"test", "decompiler", "reference", "all_forward_declarations.gc"});
 
   Timer timer;
   int total_lines = 0;