Add Address Sanitizer support to MSVC and CMake (#182)

* msvc: Add Address Sanitizer

https://devblogs.microsoft.com/cppblog/addresssanitizer-asan-for-windows-with-msvc/

* cmake: Add Address Sanitizer

* docs: Update ReadME

* see if we can replicate the test failure

* windows fixes maybe

* cmake: Compile with gcc and clang (only coverage on

* hacky try again

* clean up

* fix clang issues

* linting

Co-authored-by: water <awaterford111445@gmail.com>

.github/workflows/linux-workflow.yaml

@@ -18,9 +18,10 @@ jobs:
       matrix:
         os: [ubuntu-20.04]
         config: [Debug] # TODO - Eventually we need to make a Release Config
+        compiler: [clang, gcc]
         experimental: [false]
 
-    name: ${{ matrix.config }}
+    name: ${{ matrix.config }}-${{ matrix.compiler }}
     runs-on: ${{ matrix.os }}
     continue-on-error: ${{ matrix.experimental }}
     # Set some sort of timeout in the event of run-away builds.  We are limited on concurrent jobs so, get rid of them.
@@ -53,6 +54,7 @@ jobs:
         run: git submodule update --init --recursive --jobs 2
 
       - name: Prepare Artifact Git Info
+        id: git-vars
         shell: bash
         run: |
           echo "##[set-output name=branch;]${GITHUB_REF#refs/heads/}"
@@ -66,10 +68,9 @@ jobs:
             echo "##[set-output name=short-sha;]$(git rev-parse --short "$GITHUB_SHA")"
           fi
           echo "##[set-output name=artifact-metadata;]${ARTIFACT_NAME}"
-        id: git-vars
 
       - name: Get Package Dependencies
-        run: sudo apt install build-essential cmake ccache gcc g++ lcov make nasm
+        run: sudo apt install build-essential cmake ccache clang gcc g++ lcov make nasm
 
       # # -- SETUP CCACHE - https://cristianadam.eu/20200113/speeding-up-c-plus-plus-github-actions-using-ccache/
       # - name: Prepare ccache timestamp
@@ -87,9 +88,18 @@ jobs:
       #     restore-keys: |
       #       ${{ matrix.config }}-ccache-
 
-      - name: CMake Generation
+      - name:  CMake Generation
         # run: cmake -D CMAKE_C_COMPILER_LAUNCHER=ccache -D CMAKE_CXX_COMPILER_LAUNCHER=ccache -B build -DCODE_COVERAGE=ON
-        run: cmake -B build -DCODE_COVERAGE=ON
+        run: |
+          if [ "${{ matrix.compiler }}" == 'clang' ]; then
+            export CC=clang
+            export CXX=clang++
+            cmake -B build -DCODE_COVERAGE=ON -DASAN_BUILD=ON
+          else
+            export CC=gcc
+            export CXX=g++
+            cmake -B build -DCODE_COVERAGE=ON
+          fi
 
       - name: Build Project
         working-directory: ./build
@@ -106,9 +116,15 @@ jobs:
         run: make -j4
 
       - name: Run Tests
-        run: ./test_code_coverage.sh
+        run: |
+          if [ "${{ matrix.compiler }}" == 'clang' ]; then
+            ./test.sh
+          else
+            ./test_code_coverage.sh
+          fi
 
       - name: Coveralls
+        if: ${{ matrix.compiler }} != 'clang'
         uses: coverallsapp/github-action@master
         continue-on-error: true # Sometimes Coveralls has intermittent problems, and codecoverage isn't critical to our success
         with:

.github/workflows/windows-workflow.yaml

@@ -17,7 +17,7 @@ jobs:
       fail-fast: false
       matrix:
         os: [windows-2019]
-        config: [Debug] # TODO - Eventually we need to make a Release Config
+        config: [Release]
         experimental: [false]
 
     name: ${{ matrix.config }}
@@ -76,7 +76,7 @@ jobs:
         run: |
           call "C:/Program Files (x86)/Microsoft Visual Studio/2019/Enterprise/VC/Auxiliary/Build/vcvars64.bat"
           cmake --version
-          cmake -B build -DCMAKE_BUILD_TYPE=Debug -G "NMake Makefiles" .
+          cmake -B build -DCMAKE_BUILD_TYPE=Release -G "NMake Makefiles" .
 
       - name: Build Project
         working-directory: ./build

CMakeLists.txt

@@ -44,7 +44,7 @@ IF (WIN32)
 ENDIF ()
 
 IF (ASAN_BUILD)
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -O1")
     message(STATUS "Doing ASAN build")
 ENDIF ()
 

CMakeSettings.json

@@ -1,22 +1,42 @@
 {
-  "configurations": [
+	"configurations": [
-    {
+		{
-      "name": "Debug",
+			"name": "Debug",
-      "generator": "Ninja",
+			"generator": "Ninja",
-      "configurationType": "Debug",
+			"configurationType": "Debug",
-      "inheritEnvironments": [ "msvc_x64_x64" ],
+			"inheritEnvironments": [ "msvc_x64_x64" ],
-      "buildRoot": "${projectDir}\\out\\build\\${name}",
+			"buildRoot": "${projectDir}\\out\\build\\${name}",
-      "installRoot": "${projectDir}\\out\\install\\${name}",
+			"installRoot": "${projectDir}\\out\\install\\${name}",
-      "cmakeCommandArgs": "",
+			"cmakeCommandArgs": "",
-      "buildCommandArgs": "",
+			"buildCommandArgs": "",
-      "ctestCommandArgs": "",
+			"addressSanitizerEnabled": true,
-      "variables": [
+			"ctestCommandArgs": "",
-        {
+			"variables": [
-          "name": "INSTALL_GTEST",
+				{
-          "value": "True",
+					"name": "INSTALL_GTEST",
-          "type": "BOOL"
+					"value": "True",
-        }
+					"type": "BOOL"
-      ]
+				}
-    }
+			]
-  ]
+		},
+		{
+			"name": "Release",
+			"generator": "Ninja",
+			"configurationType": "RelWithDebInfo",
+			"buildRoot": "${projectDir}\\out\\build\\${name}",
+			"installRoot": "${projectDir}\\out\\install\\${name}",
+			"cmakeCommandArgs": "",
+			"buildCommandArgs": "",
+			"addressSanitizerEnabled": true,
+			"ctestCommandArgs": "",
+			"inheritEnvironments": [ "msvc_x64_x64" ],
+			"variables": [
+				{
+					"name": "INSTALL_GTEST",
+					"value": "True",
+					"type": "BOOL"
+				}
+			]
+		}
+	]
 }

README.md

@@ -3,16 +3,20 @@
 ![Linux](https://github.com/water111/jak-project/workflows/Linux/badge.svg)
 ![Windows](https://github.com/water111/jak-project/workflows/Windows/badge.svg)
 [![Coverage Status](https://coveralls.io/repos/github/water111/jak-project/badge.svg?branch=master)](https://coveralls.io/github/water111/jak-project?branch=master)
+[![Codacy Badge](https://app.codacy.com/project/badge/Grade/7c3cdc07523f43aca3433484ebc62ff9)](https://www.codacy.com/gh/water111/jak-project/dashboard?utm_source=github.com&utm_medium=referral&utm_content=xTVaser/jak-project&utm_campaign=Badge_Grade)
 
 ## Table of Contents
 
 <!-- toc -->
 
-- [Project Description](#project-description)
 - [Table of Contents](#table-of-contents)
+- [Project Description](#project-description)
 - [Getting Started - Linux (Ubuntu)](#getting-started---linux-ubuntu)
 - [Getting Started - Windows](#getting-started---windows)
 - [Project Layout](#project-layout)
+- [Directory Layout](#directory-layout)
+- [More Documentation](#more-documentation)
+- [ASan Build](#asan-build)
 <!-- tocstop -->
 ## Project Description
 

common/cross_os_debug/xdbg.cpp

@@ -92,32 +92,35 @@ bool attach_and_break(const ThreadID& tid) {
  */
 bool check_stopped(const ThreadID& tid, SignalInfo* out) {
   int status;
-  if (waitpid(tid.id, &status, WNOHANG) < 0) {
+  int rv = waitpid(tid.id, &status, WNOHANG);
+  if (rv < 0) {
     printf("[Debugger] Failed to waitpid: %s.\n", strerror(errno));
     //    assert(false);  // todo, temp because I think we should never hit this.
     return false;
   }
 
-  if (WIFSTOPPED(status)) {
+  if (rv > 0) {
-    auto sig = WSTOPSIG(status);
+    // status has actually changed
-    if (out) {
+    if (WIFSTOPPED(status)) {
-      switch (sig) {
+      auto sig = WSTOPSIG(status);
-        case SIGSEGV:
+      if (out) {
-          out->kind = SignalInfo::SEGFAULT;
+        switch (sig) {
-          break;
+          case SIGSEGV:
-        case SIGFPE:
+            out->kind = SignalInfo::SEGFAULT;
-          out->kind = SignalInfo::MATH_EXCEPTION;
+            break;
-          break;
+          case SIGFPE:
-        case SIGTRAP:
+            out->kind = SignalInfo::MATH_EXCEPTION;
-          out->kind = SignalInfo::BREAK;
+            break;
-          break;
+          case SIGTRAP:
+            out->kind = SignalInfo::BREAK;
+            break;
 
-        default:
+          default:
-          out->kind = SignalInfo::UNKNOWN;
+            out->kind = SignalInfo::UNKNOWN;
+        }
       }
+      return true;
     }
-
-    return true;
   }
 
   return false;

game/kernel/asm_funcs.asm

@@ -188,15 +188,25 @@ _call_goal_asm_win32:
   push r14    ; 96
   push r15    ; 104
 
+  sub rsp, 16
+  movups [rsp], xmm6
+  sub rsp, 16
+  movups [rsp], xmm7
+  
   mov rdi, rcx ;; rdi is GOAL first argument, rcx is windows first argument
   mov rsi, rdx ;; rsi is GOAL second argument, rdx is windows second argument
   mov rdx, r8  ;; rdx is GOAL third argument, r8 is windows third argument
   mov r13, r9  ;; r13 is GOAL fp, r9 is windows fourth argument
-  mov r15, [rsp + 152] ;; symbol table
+  mov r15, [rsp + 184] ;; symbol table
-  mov r14, [rsp + 144] ;; offset
+  mov r14, [rsp + 176] ;; offset
   
   call r13
 
+  movups xmm7, [rsp]
+  add rsp, 16
+  movups xmm6, [rsp]
+  add rsp, 16
+  
   pop r15
   pop r14
   pop r13
@@ -234,6 +244,11 @@ _call_goal_on_stack_asm_win32:
   push r14    ; 96
   push r15    ; 104
 
+  sub rsp, 16
+  movups [rsp], xmm6
+  sub rsp, 16
+  movups [rsp], xmm7
+
   ;; stack swap
   mov rsi, rsp
   mov rsp, rcx
@@ -249,6 +264,11 @@ _call_goal_on_stack_asm_win32:
   pop rsi
   mov rsp, rsi
 
+  movups xmm7, [rsp]
+  add rsp, 16
+  movups xmm6, [rsp]
+  add rsp, 16
+
   pop r15
   pop r14
   pop r13

game/kernel/kboot.cpp

@@ -6,6 +6,8 @@
 
 #include <cstring>
 #include <chrono>
+#include <stdio.h>
+#include <stdlib.h>
 #include <thread>
 
 #include "common/common_types.h"

game/kernel/kprint.cpp

@@ -349,7 +349,7 @@ s32 cvt_float(float x, s32 precision, s32* lead_char, char* buff_start, char* bu
       value = (char)rounder;
     } else if (!(ru32 >> 31)) {  // sign bit
       value = 0;
-      assert(false);  // not sure what happens here.
+      // assert(false);  // not sure what happens here.
     } else {
       value = -1;  // happens on NaN's
     }
@@ -396,7 +396,7 @@ s32 cvt_float(float x, s32 precision, s32* lead_char, char* buff_start, char* bu
           value = (char)next_int;
         } else if (!(ru32 >> 0x1f)) {
           value = 0;
-          assert(false);  // not sure what happens here.
+          // assert(false);  // not sure what happens here.
         } else {
           value = -1;  // happens on NaN's
         }

test-cov.sh

@@ -1,9 +0,0 @@
-#!/bin/bash
-
-# Directory of this script
-DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null 2>&1 && pwd )"
-
-cd $DIR/build/test
-make init
-make gcov
-make lcov

test/CMakeLists.txt

@@ -32,7 +32,7 @@ ELSE()
   target_link_libraries(goalc-test cross_sockets goos common_util runtime compiler type_system gtest Zydis)
 ENDIF()
 
-if(UNIX AND CODE_COVERAGE)
+if(UNIX AND CMAKE_COMPILER_IS_GNUCXX AND CODE_COVERAGE)
   include(CodeCoverage)
   append_coverage_compiler_flags()
   setup_target_for_coverage_lcov(NAME goalc-test_coverage

test/test_emitter.cpp

@@ -3109,6 +3109,7 @@ TEST(EmitterLoadsAndStores, static_addr) {
   }
 }
 
+#ifdef __linux__
 TEST(EmitterXmm32, load32_xmm32_gpr64_plus_gpr64) {
   CodeTester tester;
   tester.init_code_buffer(512);
@@ -3154,10 +3155,10 @@ TEST(EmitterXmm32, load32_xmm32_gpr64_plus_gpr64) {
         float memory[8] = {0, 0, 1.23f, 3.45f, 5.67f, 0, 0, 0};
 
         // run!
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 3 * sizeof(float), 0, 0), 3.45f);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 3 * sizeof(float), 0, 0), 3.45f);
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 2 * sizeof(float), 0, 0), 1.23f);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 2 * sizeof(float), 0, 0), 1.23f);
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 4 * sizeof(float), 0, 0), 5.67f);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 4 * sizeof(float), 0, 0), 5.67f);
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 5 * sizeof(float), 0, 0), 0);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 5 * sizeof(float), 0, 0), 0);
 
         iter++;
       }
@@ -3215,10 +3216,10 @@ TEST(EmitterXmm32, load32_xmm32_gpr64_plus_gpr64_plus_s8) {
         float memory[8] = {0, 0, 1.23f, 3.45f, 5.67f, 0, 0, 0};
 
         // run!
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 3 * sizeof(float) + 3, 0, 0), 3.45f);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 3 * sizeof(float) + 3, 0, 0), 3.45f);
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 2 * sizeof(float) + 3, 0, 0), 1.23f);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 2 * sizeof(float) + 3, 0, 0), 1.23f);
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 4 * sizeof(float) + 3, 0, 0), 5.67f);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 4 * sizeof(float) + 3, 0, 0), 5.67f);
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 5 * sizeof(float) + 3, 0, 0), 0);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 5 * sizeof(float) + 3, 0, 0), 0);
 
         iter++;
       }
@@ -3278,10 +3279,14 @@ TEST(EmitterXmm32, load32_xmm32_gpr64_plus_gpr64_plus_s32) {
         float memory[8] = {0, 0, 1.23f, 3.45f, 5.67f, 0, 0, 0};
 
         // run!
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 3 * sizeof(float) - offset, 0, 0), 3.45f);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 3 * sizeof(float) - offset, 0, 0),
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 2 * sizeof(float) - offset, 0, 0), 1.23f);
+                        3.45f);
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 4 * sizeof(float) - offset, 0, 0), 5.67f);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 2 * sizeof(float) - offset, 0, 0),
-        EXPECT_EQ(tester.execute_ret<float>((u64)memory, 5 * sizeof(float) - offset, 0, 0), 0);
+                        1.23f);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 4 * sizeof(float) - offset, 0, 0),
+                        5.67f);
+        EXPECT_FLOAT_EQ(tester.execute_ret<float>((u64)memory, 5 * sizeof(float) - offset, 0, 0),
+                        0);
         iter++;
       }
     }
@@ -3352,9 +3357,9 @@ TEST(EmitterXmm32, store32_xmm32_gpr64_plus_gpr64) {
 
         // run!
         tester.execute((u64)memory, 12, as_u32(1.234f), 0);
-        EXPECT_EQ(memory[2], 1.23f);
+        EXPECT_FLOAT_EQ(memory[2], 1.23f);
-        EXPECT_EQ(memory[3], 1.234f);
+        EXPECT_FLOAT_EQ(memory[3], 1.234f);
-        EXPECT_EQ(memory[4], 5.67f);
+        EXPECT_FLOAT_EQ(memory[4], 5.67f);
 
         iter++;
       }
@@ -3418,9 +3423,9 @@ TEST(EmitterXmm32, store32_xmm32_gpr64_plus_gpr64_plus_s8) {
 
         // run!
         tester.execute((u64)memory, 12 - offset, as_u32(1.234f), 0);
-        EXPECT_EQ(memory[2], 1.23f);
+        EXPECT_FLOAT_EQ(memory[2], 1.23f);
-        EXPECT_EQ(memory[3], 1.234f);
+        EXPECT_FLOAT_EQ(memory[3], 1.234f);
-        EXPECT_EQ(memory[4], 5.67f);
+        EXPECT_FLOAT_EQ(memory[4], 5.67f);
 
         iter++;
       }
@@ -3484,9 +3489,9 @@ TEST(EmitterXmm32, store32_xmm32_gpr64_plus_gpr64_plus_s32) {
 
         // run!
         tester.execute((u64)memory, 12 - offset, as_u32(1.234f), 0);
-        EXPECT_EQ(memory[2], 1.23f);
+        EXPECT_FLOAT_EQ(memory[2], 1.23f);
-        EXPECT_EQ(memory[3], 1.234f);
+        EXPECT_FLOAT_EQ(memory[3], 1.234f);
-        EXPECT_EQ(memory[4], 5.67f);
+        EXPECT_FLOAT_EQ(memory[4], 5.67f);
 
         iter++;
       }
@@ -3517,7 +3522,7 @@ TEST(EmitterXmm32, static_load_xmm32) {
                       loc_of_load + load_instr.offset_of_disp());
 
     auto result = tester.execute_ret<float>(0, 0, 0, 0);
-    EXPECT_EQ(result, 1.2345f);
+    EXPECT_FLOAT_EQ(result, 1.2345f);
   }
 }
 
@@ -3542,7 +3547,7 @@ TEST(EmitterXmm32, static_store_xmm32) {
     tester.write<s32>(loc_of_float - loc_of_store - store_instr.length(),
                       loc_of_store + store_instr.offset_of_disp());
     tester.execute(as_u32(-44.567f), 0, 0, 0);
-    EXPECT_EQ(-44.567f, tester.read<float>(loc_of_float));
+    EXPECT_FLOAT_EQ(-44.567f, tester.read<float>(loc_of_float));
   }
 }
 
@@ -3583,7 +3588,7 @@ TEST(EmitterXmm32, mul) {
           tester.emit_pop_all_xmms();
           tester.emit_return();
           auto result = tester.execute_ret<float>(0, 0, 0, 0);
-          EXPECT_EQ(result, expected);
+          EXPECT_FLOAT_EQ(result, expected);
         }
       }
     }
@@ -3620,7 +3625,7 @@ TEST(EmitterXmm32, div) {
           tester.emit_pop_all_xmms();
           tester.emit_return();
           auto result = tester.execute_ret<float>(0, 0, 0, 0);
-          EXPECT_EQ(result, expected);
+          EXPECT_FLOAT_EQ(result, expected);
         }
       }
     }
@@ -3656,7 +3661,7 @@ TEST(EmitterXmm32, add) {
           tester.emit_pop_all_xmms();
           tester.emit_return();
           auto result = tester.execute_ret<float>(0, 0, 0, 0);
-          EXPECT_EQ(result, expected);
+          EXPECT_FLOAT_EQ(result, expected);
         }
       }
     }
@@ -3693,7 +3698,7 @@ TEST(EmitterXmm32, sub) {
           tester.emit_pop_all_xmms();
           tester.emit_return();
           auto result = tester.execute_ret<float>(0, 0, 0, 0);
-          EXPECT_EQ(result, expected);
+          EXPECT_FLOAT_EQ(result, expected);
         }
       }
     }
@@ -3801,7 +3806,9 @@ TEST(EmitterSlow, xmm32_move) {
       }
     }
   }
+  // todo - finish this test
 }
+#endif
 
 TEST(Emitter, LEA) {
   CodeTester tester;