[Decompiler] Test framework for decompiler regression tests and gcomm…

…on tests (#200) * test framework for pre-expression compact stuff * check ordering * more tests * final tests gcommon
open-goal · Jan 18, 2021 · 40d328f · 40d328f
1 parent e8ad91a
commit 40d328f
Show file tree

Hide file tree

Showing 13 changed files with 1,116 additions and 16 deletions.
diff --git a/common/goos/Reader.cpp b/common/goos/Reader.cpp
@@ -159,13 +159,13 @@ Object Reader::read_from_stdin(const std::string& prompt_name) {
 /*!
  * Read a string.
  */
-Object Reader::read_from_string(const std::string& str) {
+Object Reader::read_from_string(const std::string& str, bool add_top_level) {
   // create text fragment and add to the DB
   auto textFrag = std::make_shared<ProgramString>(str);
   db.insert(textFrag);
 
   // perform read
-  auto result = internal_read(textFrag);
+  auto result = internal_read(textFrag, add_top_level);
   db.link(result, textFrag, 0);
   return result;
 }
@@ -185,7 +185,7 @@ Object Reader::read_from_file(const std::vector<std::string>& file_path) {
 /*!
  * Common read for a SourceText
  */
-Object Reader::internal_read(std::shared_ptr<SourceText> text) {
+Object Reader::internal_read(std::shared_ptr<SourceText> text, bool add_top_level) {
   // first create stream
   TextStream ts(text);
 
@@ -194,7 +194,11 @@ Object Reader::internal_read(std::shared_ptr<SourceText> text) {
 
   // read list!
   auto objs = read_list(ts, false);
-  return PairObject::make_new(SymbolObject::make_new(symbolTable, "top-level"), objs);
+  if (add_top_level) {
+    return PairObject::make_new(SymbolObject::make_new(symbolTable, "top-level"), objs);
+  } else {
+    return objs;
+  }
 }
 
 /*!

diff --git a/common/goos/Reader.h b/common/goos/Reader.h
@@ -67,7 +67,7 @@ struct Token {
 class Reader {
  public:
   Reader();
-  Object read_from_string(const std::string& str);
+  Object read_from_string(const std::string& str, bool add_top_level = true);
   Object read_from_stdin(const std::string& prompt_name);
   Object read_from_file(const std::vector<std::string>& file_path);
 
@@ -77,7 +77,7 @@ class Reader {
   TextDb db;
 
  private:
-  Object internal_read(std::shared_ptr<SourceText> text);
+  Object internal_read(std::shared_ptr<SourceText> text, bool add_top_level = true);
   Object read_list(TextStream& stream, bool expect_close_paren = true);
   bool read_object(Token& tok, TextStream& ts, Object& obj);
   bool read_array(TextStream& stream, Object& o);

diff --git a/decompiler/Disasm/Register.h b/decompiler/Disasm/Register.h
@@ -144,6 +144,7 @@ class Register {
 
   bool operator==(const Register& other) const;
   bool operator!=(const Register& other) const;
+  bool operator<(const Register& other) { return id < other.id; }
 
   struct hash {
     auto operator()(const Register& x) const { return std::hash<uint16_t>()(x.id); }

diff --git a/decompiler/Function/TypeAnalysis.cpp b/decompiler/Function/TypeAnalysis.cpp
@@ -142,10 +142,12 @@ bool Function::run_type_analysis_ir2(const TypeSpec& my_type,
   (void)file;
   // STEP 0 - set decompiler type system settings for this function. In config we can manually
   // specify some settings for type propagation to reduce the strictness of type propagation.
-  dts.type_prop_settings.reset();
-  if (get_config().pair_functions_by_name.find(guessed_name.to_string()) !=
-      get_config().pair_functions_by_name.end()) {
-    dts.type_prop_settings.allow_pair = true;
+  if (!dts.type_prop_settings.locked) {
+    dts.type_prop_settings.reset();
+    if (get_config().pair_functions_by_name.find(guessed_name.to_string()) !=
+        get_config().pair_functions_by_name.end()) {
+      dts.type_prop_settings.allow_pair = true;
+    }
   }
 
   if (guessed_name.kind == FunctionName::FunctionKind::METHOD) {

diff --git a/decompiler/IR2/AtomicOp.h b/decompiler/IR2/AtomicOp.h
@@ -199,7 +199,7 @@ class SimpleAtom {
  private:
   Kind m_kind = Kind::INVALID;
   std::string m_string;  // for symbol ptr and symbol val
-  s64 m_int = 0;         // for integer constant and static address label id
+  s64 m_int = -1;        // for integer constant and static address label id
   Variable m_variable;
 };
 

diff --git a/decompiler/IR2/Form.cpp b/decompiler/IR2/Form.cpp
@@ -383,7 +383,7 @@ void CondWithElseElement::apply_form(const std::function<void(Form*)>& f) {
 // EmptyElement
 /////////////////////////////
 
-goos::Object EmptyElement::to_form(const Env& env) const {
+goos::Object EmptyElement::to_form(const Env&) const {
   return pretty_print::build_list("empty");
 }
 

diff --git a/decompiler/IR2/variable_naming.cpp b/decompiler/IR2/variable_naming.cpp
@@ -60,19 +60,49 @@ int VarMapSSA::get_next_var_id(Register reg) {
  */
 void VarMapSSA::merge(const VarSSA& var_a, const VarSSA& var_b) {
   auto& a = m_entries.at(var_a.m_entry_id);
-  auto& b = m_entries.at(var_b.m_entry_id);
+  auto b = m_entries.at(var_b.m_entry_id);
   assert(a.reg == b.reg);
   if (b.var_id == 0) {
+    //    fmt::print("Merge {} <- {}\n", to_string(var_b), to_string(var_a));
+
+    for (auto& entry : m_entries) {
+      if (entry.var_id == a.var_id && entry.reg == a.reg) {
+        entry.var_id = b.var_id;
+      }
+    }
     a.var_id = b.var_id;
   } else {
+    //    fmt::print("Merge {} <- {}\n", to_string(var_a), to_string(var_b));
+
+    for (auto& entry : m_entries) {
+      if (entry.var_id == b.var_id && entry.reg == b.reg) {
+        entry.var_id = a.var_id;
+      }
+    }
     b.var_id = a.var_id;
   }
 }
 
+/*!
+ * Make all Bs A.
+ */
 void VarMapSSA::merge_to_first(const VarSSA& var_a, const VarSSA& var_b) {
   auto& a = m_entries.at(var_a.m_entry_id);
-  auto& b = m_entries.at(var_b.m_entry_id);
+  auto b = m_entries.at(var_b.m_entry_id);
+
+  //  fmt::print("Merge-to-first {} <- {}\n", to_string(var_a), to_string(var_b));
   assert(a.reg == b.reg);
+
+  //  for (auto& entry : m_entries) {
+  for (size_t i = 0; i < m_entries.size(); i++) {
+    auto& entry = m_entries.at(i);
+    if (entry.var_id == b.var_id && entry.reg == b.reg) {
+      //      fmt::print("remap extra {} var_id from {} to {}\n", i, entry.var_id, a.var_id);
+      entry.var_id = a.var_id;
+    } else {
+      //      fmt::print("no remap at {} (prev is {} {})\n", i, entry.reg.to_charp(), entry.var_id);
+    }
+  }
   b.var_id = a.var_id;
 }
 
@@ -117,6 +147,12 @@ void VarMapSSA::remap_reg(Register reg, const std::unordered_map<int, int>& rema
   }
 }
 
+void VarMapSSA::debug_print_map() const {
+  for (auto& entry : m_entries) {
+    fmt::print("[{:02d}] {} {}\n", entry.entry_id, entry.reg.to_charp(), entry.var_id);
+  }
+}
+
 std::string SSA::Phi::print(const VarMapSSA& var_map) const {
   std::string result = var_map.to_string(dest);
   result += " <- phi(";
@@ -177,6 +213,7 @@ SSA::Phi& SSA::get_phi(int block, Register dest_reg) {
   auto& phi_map = blocks.at(block).phis;
   auto kv = phi_map.find(dest_reg);
   if (kv == phi_map.end()) {
+    //    printf("Allocate new get_phi for %s B%d\n", dest_reg.to_charp(), block);
     auto dest_var = map.allocate_init_phi(dest_reg, block);
     phi_map.insert(std::make_pair(dest_reg, dest_var));
   }
@@ -385,7 +422,18 @@ void SSA::merge_all_phis() {
 }
 
 void SSA::remap() {
-  std::unordered_map<Register, std::set<int>, Register::hash> used_vars;
+  // this keeps the order of variable assignments in the instruction order, not var_id order.
+  struct VarIdRecord {
+    std::unordered_set<int> set;
+    std::vector<int> order;
+    void insert(int x) {
+      if (set.find(x) == set.end()) {
+        set.insert(x);
+        order.push_back(x);
+      }
+    }
+  };
+  std::unordered_map<Register, VarIdRecord, Register::hash> used_vars;
   for (auto& block : blocks) {
     assert(block.phis.empty());
     for (auto& instr : block.ins) {
@@ -401,7 +449,7 @@ void SSA::remap() {
   for (auto& reg_vars : used_vars) {
     std::unordered_map<int, int> var_remap;
     int i = 0;
-    for (auto var_id : reg_vars.second) {
+    for (auto var_id : reg_vars.second.order) {
       var_remap[var_id] = i++;
     }
     map.remap_reg(reg_vars.first, var_remap);
@@ -547,10 +595,17 @@ std::optional<VariableNames> run_variable_renaming(const Function& function,
   }
 
   // Merge phis to return to executable code.
+  if (debug_prints) {
+    ssa.map.debug_print_map();
+  }
+
   ssa.merge_all_phis();
   if (debug_prints) {
     fmt::print("{}", ssa.print());
   }
+  if (debug_prints) {
+    ssa.map.debug_print_map();
+  }
 
   // merge same vars (decided this made things worse)
 

diff --git a/decompiler/IR2/variable_naming.h b/decompiler/IR2/variable_naming.h
@@ -74,6 +74,7 @@ class VarMapSSA {
   bool same(const VarSSA& var_a, const VarSSA& var_b) const;
   int var_id(const VarSSA& var);
   void remap_reg(Register reg, const std::unordered_map<int, int>& remap);
+  void debug_print_map() const;
 
  private:
   int get_next_var_id(Register reg);

diff --git a/decompiler/ObjectFile/LinkedObjectFile.cpp b/decompiler/ObjectFile/LinkedObjectFile.cpp
@@ -1065,4 +1065,13 @@ std::string LinkedObjectFile::get_goal_string_by_label(const DecompilerLabel& la
   assert(0 == (label.offset % 4));
   return get_goal_string(label.target_segment, (label.offset / 4) - 1, false);
 }
+
+const DecompilerLabel& LinkedObjectFile::get_label_by_name(const std::string& name) const {
+  for (auto& label : labels) {
+    if (label.name == name) {
+      return label;
+    }
+  }
+  throw std::runtime_error("Can't find label " + name);
+}
 }  // namespace decompiler
diff --git a/decompiler/ObjectFile/LinkedObjectFile.h b/decompiler/ObjectFile/LinkedObjectFile.h
@@ -62,6 +62,7 @@ class LinkedObjectFile {
   std::string print_asm_function_disassembly(const std::string& my_name);
 
   u32 read_data_word(const DecompilerLabel& label);
+  const DecompilerLabel& get_label_by_name(const std::string& name) const;
   std::string get_goal_string_by_label(const DecompilerLabel& label) const;
   std::string get_goal_string(int seg, int word_idx, bool with_quotes = true) const;
   bool is_string(int seg, int byte_idx) const;

diff --git a/decompiler/util/DecompilerTypeSystem.h b/decompiler/util/DecompilerTypeSystem.h
@@ -43,6 +43,7 @@ class DecompilerTypeSystem {
   int get_format_arg_count(const std::string& str) const;
   int get_format_arg_count(const TP_Type& type) const;
   struct {
+    bool locked = false;
     bool allow_pair;
     std::string current_method_type;
     void reset() {

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
@@ -19,6 +19,7 @@ add_executable(goalc-test
         test_zydis.cpp
         goalc/test_goal_kernel.cpp
         decompiler/test_AtomicOpBuilder.cpp
+        decompiler/test_FormRegression.cpp
         decompiler/test_InstructionParser.cpp
         ${GOALC_TEST_FRAMEWORK_SOURCES}
         ${GOALC_TEST_CASES})