Decompiler fixes + decompiling (#276)

* decomp pad

* more decompilation

* update

* fix test name

common/type_system/Type.cpp

@@ -280,8 +280,12 @@ int NullType::get_in_memory_alignment() const {
   throw std::runtime_error("get_in_memory_alignment called on NullType");
 }
 
-int NullType::get_inline_array_alignment() const {
-  throw std::runtime_error("get_inline_array_alignment called on NullType");
+int NullType::get_inline_array_start_alignment() const {
+  throw std::runtime_error("get_inline_array_start_alignment called on NullType");
+}
+
+int NullType::get_inline_array_stride_alignment() const {
+  throw std::runtime_error("get_inline_array_stride_alignment called on NullType");
 }
 
 std::string NullType::print() const {
@@ -372,7 +376,11 @@ int ValueType::get_in_memory_alignment() const {
   return m_size;
 }
 
-int ValueType::get_inline_array_alignment() const {
+int ValueType::get_inline_array_stride_alignment() const {
+  return m_size;
+}
+
+int ValueType::get_inline_array_start_alignment() const {
   return m_size;
 }
 
@@ -528,7 +536,13 @@ int StructureType::get_in_memory_alignment() const {
   return STRUCTURE_ALIGNMENT;
 }
 
-int StructureType::get_inline_array_alignment() const {
+// So the GOAL compiler was weird here.
+// It seems like there were two states:
+// - don't care about alignment of both the first element and the later
+// - don't care about the alignment, but pad the stride.
+// so you end up with a misaligned array of padded structures which seems very stupid.
+
+int StructureType::get_inline_array_stride_alignment() const {
   if (m_pack) {
     // make elements of inline array the minimum allowable alignment.
     int alignment = 1;
@@ -544,6 +558,22 @@ int StructureType::get_inline_array_alignment() const {
   }
 }
 
+int StructureType::get_inline_array_start_alignment() const {
+  if (m_pack || m_allow_misalign) {
+    // make elements of inline array the minimum allowable alignment.
+    int alignment = 1;
+    // TODO - I don't know if GOAL actually did this check, maybe packed inline arrays could
+    // violate these?
+    for (const auto& field : m_fields) {
+      alignment = std::max(alignment, field.alignment());
+    }
+    return alignment;
+  } else {
+    // make elements of inline array properly aligned structures
+    return STRUCTURE_ALIGNMENT;
+  }
+}
+
 bool StructureType::lookup_field(const std::string& name, Field* out) {
   for (auto& x : m_fields) {
     if (x.name() == name) {
@@ -580,6 +610,22 @@ int BasicType::get_offset() const {
   return BASIC_OFFSET;
 }
 
+int BasicType::get_inline_array_start_alignment() const {
+  if (m_pack) {
+    // make elements of inline array the minimum allowable alignment.
+    int alignment = 8;
+    // TODO - I don't know if GOAL actually did this check, maybe packed inline arrays could
+    // violate these?
+    for (const auto& field : m_fields) {
+      alignment = std::max(alignment, field.alignment());
+    }
+    return alignment;
+  } else {
+    // make elements of inline array properly aligned structures
+    return STRUCTURE_ALIGNMENT;
+  }
+}
+
 /////////////////
 // Bitfield
 /////////////////

common/type_system/Type.h

@@ -52,7 +52,8 @@ class Type {
   // get the alignment for the "size in memory" data.
   virtual int get_in_memory_alignment() const = 0;
 
-  virtual int get_inline_array_alignment() const = 0;
+  virtual int get_inline_array_stride_alignment() const = 0;
+  virtual int get_inline_array_start_alignment() const = 0;
 
   virtual bool operator==(const Type& other) const = 0;
 
@@ -115,7 +116,8 @@ class NullType : public Type {
   int get_load_size() const override;
   bool get_load_signed() const override;
   int get_size_in_memory() const override;
-  int get_inline_array_alignment() const override;
+  int get_inline_array_stride_alignment() const override;
+  int get_inline_array_start_alignment() const override;
   RegClass get_preferred_reg_class() const override;
   int get_offset() const override;
   int get_in_memory_alignment() const override;
@@ -143,7 +145,8 @@ class ValueType : public Type {
   RegClass get_preferred_reg_class() const override;
   int get_offset() const override;
   int get_in_memory_alignment() const override;
-  int get_inline_array_alignment() const override;
+  int get_inline_array_stride_alignment() const override;
+  int get_inline_array_start_alignment() const override;
   std::string print() const override;
   bool operator==(const Type& other) const override;
   ~ValueType() = default;
@@ -231,11 +234,13 @@ class StructureType : public ReferenceType {
   int get_size_in_memory() const override;
   int get_offset() const override;
   int get_in_memory_alignment() const override;
-  int get_inline_array_alignment() const override;
+  int get_inline_array_stride_alignment() const override;
+  int get_inline_array_start_alignment() const override;
   bool lookup_field(const std::string& name, Field* out);
   bool is_dynamic() const { return m_dynamic; }
   ~StructureType() = default;
   void set_pack(bool pack) { m_pack = pack; }
+  void set_allow_misalign(bool misalign) { m_allow_misalign = misalign; }
 
  protected:
   friend class TypeSystem;
@@ -254,6 +259,7 @@ class StructureType : public ReferenceType {
   bool m_dynamic = false;
   int m_size_in_mem = 0;
   bool m_pack = false;
+  bool m_allow_misalign = false;
   int m_offset = 0;
   size_t m_idx_of_first_unique_field = 0;
 };
@@ -262,6 +268,7 @@ class BasicType : public StructureType {
  public:
   BasicType(std::string parent, std::string name, bool dynamic = false);
   int get_offset() const override;
+  int get_inline_array_start_alignment() const override;
   std::string print() const override;
   ~BasicType() = default;
 };

common/type_system/TypeSystem.cpp

@@ -147,8 +147,8 @@ DerefInfo TypeSystem::get_deref_info(const TypeSpec& ts) const {
     info.sign_extend = false;                // not applicable anyway
 
     if (result_type->is_reference()) {
-      info.stride =
-          align(result_type->get_size_in_memory(), result_type->get_inline_array_alignment());
+      info.stride = align(result_type->get_size_in_memory(),
+                          result_type->get_inline_array_stride_alignment());
     } else {
       // can't have an inline array of value types!
       assert(false);
@@ -886,11 +886,11 @@ int TypeSystem::get_alignment_in_type(const Field& field) {
     if (field.is_array()) {
       // TODO - is this actually correct? or do we use in_memory for the first element and
       // inline_array for the ones that follow?
-      return field_type->get_inline_array_alignment();
+      return field_type->get_inline_array_start_alignment();
     } else {
       // it is an inlined field, so return the alignment in memory
       // TODO - for inline, but not inline array, do we use structure alignment always?
-      return field_type->get_inline_array_alignment();
+      return field_type->get_inline_array_start_alignment();
     }
   }
 
@@ -929,8 +929,8 @@ int TypeSystem::get_size_in_type(const Field& field) const {
         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(), field_type->get_inline_array_alignment());
+      return field.array_size() * align(field_type->get_size_in_memory(),
+                                        field_type->get_inline_array_stride_alignment());
     } else {
       if (field_type->is_reference()) {
         return field.array_size() * POINTER_SIZE;

common/type_system/deftype.cpp

@@ -225,6 +225,7 @@ struct StructureDefResult {
   TypeFlags flags;
   bool generate_runtime_type = true;
   bool pack_me = false;
+  bool allow_misaligned = false;
 };
 
 StructureDefResult parse_structure_def(StructureType* type,
@@ -286,9 +287,9 @@ StructureDefResult parse_structure_def(StructureType* type,
         u16 hb = get_int(car(rest));
         rest = cdr(rest);
         flags.heap_base = hb;
-      }
-
-      else {
+      } else if (opt_name == ":allow-misaligned") {
+        result.allow_misaligned = true;
+      } else {
         throw std::runtime_error("Invalid option in field specification: " + opt_name);
       }
     }
@@ -459,8 +460,13 @@ DeftypeResult parse_deftype(const goos::Object& deftype, TypeSystem* ts) {
     result.flags = sr.flags;
     result.create_runtime_type = sr.generate_runtime_type;
     if (sr.pack_me) {
-      fmt::print("[TypeSystem] :pack-me was set on {}, which is a basic and cannot be packed.",
-                 name);
+      new_type->set_pack(true);
+    }
+    if (sr.allow_misaligned) {
+      fmt::print(
+          "[TypeSystem] :allow-misaligned was set on {}, which is a basic and cannot "
+          "be misaligned\n",
+          name);
       throw std::runtime_error("invalid pack option on basic");
     }
     ts->add_type(name, std::move(new_type));
@@ -476,6 +482,9 @@ DeftypeResult parse_deftype(const goos::Object& deftype, TypeSystem* ts) {
     if (sr.pack_me) {
       new_type->set_pack(true);
     }
+    if (sr.allow_misaligned) {
+      new_type->set_allow_misalign(true);
+    }
     ts->add_type(name, std::move(new_type));
   } else if (is_type("integer", parent_type, ts)) {
     auto pto = ts->lookup_type(parent_type);