add asan build (#179)

CMakeLists.txt

@@ -43,6 +43,11 @@ IF (WIN32)
     set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)
 ENDIF ()
 
+IF (ASAN_BUILD)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address")
+    message(STATUS "Doing ASAN build")
+ENDIF ()
+
 option(CODE_COVERAGE "Enable Code Coverage Compiler Flags" OFF)
 set(CMAKE_MODULE_PATH ${PROJECT_SOURCE_DIR}/third-party/cmake/modules/)
 

README.md

@@ -177,4 +177,17 @@ Check out these files for more documentation. Some of it is still in progress
 - `doc/porting_to_x86.md`: Summary of changes we're making to port to x86-64
 - `doc/goos.md`: GOOS macro language
 
-## Tests
+## ASan Build
+The project supports building with Address Sanitizer (https://github.com/google/sanitizers/wiki/AddressSanitizer) in Linux.
+```
+export CXX=clang++
+cmake .. -DASAN_BUILD=TRUE
+```
+You will have to delete the build folder when changing compilers.  When running `cmake`, you should see something like:
+```
+-- The CXX compiler identification is Clang 10.0.0
+...
+-- Doing ASAN build
+```
+
+Then you can run the tests, runtime, and compiler as normal and they will abort if ASan finds an error.

decompiler/IR/IR_ExpressionStack.cpp

@@ -184,6 +184,7 @@ bool IR_Compare::update_from_stack(const std::unordered_set<Register, Register::
 }
 
 bool IR_ShortCircuit::expression_stack(ExpressionStack& stack, LinkedObjectFile& file) {
+  (void)file;
   // this one is weird. All forms but the last implicitly set final_destination.
   // the last form should somewhere set final_destination, but due to tricky coloring we
   // can't identify this 100% of the time.

decompiler/ObjectFile/ObjectFileDB.cpp

@@ -467,11 +467,11 @@ std::string ObjectFileDB::generate_obj_listing() {
       unique_count++;
       all_unique_names.insert(x.to_unique_name());
     }
+    // this check is extremely important. It makes sure we don't have any repeat names. This could
+    // be caused by two files with the same name, in the same DGOs, but different data.
+    assert(int(all_unique_names.size()) == unique_count);
   }
 
-  // this check is extremely important. It makes sure we don't have any repeat names. This could
-  // be caused by two files with the same name, in the same DGOs, but different data.
-  assert(int(all_unique_names.size()) == unique_count);
   result.pop_back();  // kill last new line
   result.pop_back();  // kill last comma
   return result + "]";

decompiler/util/DecompilerTypeSystem.cpp

@@ -104,9 +104,9 @@ std::string DecompilerTypeSystem::dump_symbol_types() {
 void DecompilerTypeSystem::add_type_flags(const std::string& name, u64 flags) {
   auto kv = type_flags.find(name);
   if (kv != type_flags.end()) {
-    spdlog::warn("duplicated type flags for {}, was 0x{:x}, now 0x{:x}", name.c_str(), kv->second,
-                 flags);
     if (kv->second != flags) {
+      spdlog::warn("duplicated type flags for {}, was 0x{:x}, now 0x{:x}", name.c_str(), kv->second,
+                   flags);
       spdlog::warn("duplicated type flags that are inconsistent!");
     }
   }
@@ -116,9 +116,9 @@ void DecompilerTypeSystem::add_type_flags(const std::string& name, u64 flags) {
 void DecompilerTypeSystem::add_type_parent(const std::string& child, const std::string& parent) {
   auto kv = type_parents.find(child);
   if (kv != type_parents.end()) {
-    spdlog::warn("duplicated type parents for {} was {} now {}", child.c_str(), kv->second.c_str(),
-                 parent.c_str());
     if (kv->second != parent) {
+      spdlog::warn("duplicated type parents for {} was {} now {}", child.c_str(),
+                   kv->second.c_str(), parent.c_str());
       throw std::runtime_error("duplicated type parents that are inconsistent!");
     }
   }

game/kernel/klink.cpp

@@ -883,7 +883,7 @@ void* ultimate_memcpy(void* dst, void* src, uint32_t size) {
       // GOAL function is unknown, lets see if its loaded:
       auto sym = find_symbol_from_c("ultimate-memcpy");
       if (sym->value == 0) {
-        return memcpy(dst, src, size);
+        return memmove(dst, src, size);
       }
       gfunc_774.offset = sym->value;
     }
@@ -892,7 +892,7 @@ void* ultimate_memcpy(void* dst, void* src, uint32_t size) {
                              s7.offset, g_ee_main_mem))
         .c();
   } else {
-    return memcpy(dst, src, size);
+    return memmove(dst, src, size);
   }
 }
 

game/kernel/kmachine.h

@@ -12,7 +12,10 @@
 #include "Ptr.h"
 
 //! How much space to leave for the stack when creating the debug heap
-constexpr u32 DEBUG_HEAP_SPACE_FOR_STACK = 0x4000;
+// In the game, it's 16 kB, but we increase it to 64 kB.
+// ASAN builds + fmt / spdlog stuff uses a _ton_ of stack when no optimizations are on and we
+// need more.
+constexpr u32 DEBUG_HEAP_SPACE_FOR_STACK = 0x10000;
 
 //! First free address for the GOAL heap
 constexpr u32 HEAP_START = 0x13fd20;

game/kernel/kscheme.cpp

@@ -324,16 +324,26 @@ Ptr<Function> make_function_from_c_linux(void* func) {
   auto f = (uint64_t)func;
   auto fp = (u8*)&f;
 
+  int i = 0;
   // we will put the function address in RAX with a movabs rax, imm8
-  mem.c()[0] = 0x48;
-  mem.c()[1] = 0xb8;
-  for (int i = 0; i < 8; i++) {
-    mem.c()[2 + i] = fp[i];
+  mem.c()[i++] = 0x48;
+  mem.c()[i++] = 0xb8;
+  for (int j = 0; j < 8; j++) {
+    mem.c()[i++] = fp[j];
   }
 
-  // jmp rax
-  mem.c()[10] = 0xff;
-  mem.c()[11] = 0xe0;
+  // push r10
+  // push r11
+  // sub rsp, 8
+  // call rax
+  // add rsp, 8
+  // pop r11
+  // pop r10
+  // ret
+  for (auto x : {0x41, 0x52, 0x41, 0x53, 0x48, 0x83, 0xEC, 0x08, 0xFF, 0xD0, 0x48, 0x83, 0xC4, 0x08,
+                 0x41, 0x5B, 0x41, 0x5A, 0xC3}) {
+    mem.c()[i++] = x;
+  }
 
   // the C function's ret will return to the caller of this trampoline.
 

goalc/listener/Listener.cpp

@@ -295,7 +295,7 @@ void Listener::receive_func() {
           handle_output_message(str_buff);
         }
         rcv_mtx.unlock();
-
+        delete[] str_buff;
       } break;
 
       default: