Address sanitizer/ndk28c (#118)

CedricGuillemet · web-flow · commit 70ba1e53d386 · 2025-11-13T18:26:39.000+01:00
- up Android to ndk28c - add sanitizer builds to macOS and Linux - up AndroidExtensions commit id - fix uint16_t / char16_ casting - up macOS13 (soon deprecated) to macOS14 - use clang for sanitizer on Linux ( https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80578 ) - disable vptr check for `ObjectWrap<T>::ObjectWrap` (false positive)
diff --git a/.github/azure-pipelines.yml b/.github/azure-pipelines.yml
@@ -14,7 +14,7 @@ schedules:
 
 variables:
 - name: ndkVersion
-  value: 25.2.9519653
+  value: 28.2.13676358
 
 jobs:
   # WIN32
@@ -85,6 +85,13 @@ jobs:
       vmImage: 'macOS-latest'
       xCodeVersion: 16.4
 
+  - template: jobs/macos.yml
+    parameters:
+      name: 'macOS_Xcode164_Sanitizers'
+      vmImage: 'macOS-latest'
+      xCodeVersion: 16.4
+      enableSanitizers: true
+
   # iOS
   - template: jobs/ios.yml
     parameters:
@@ -96,9 +103,24 @@ jobs:
   - template: jobs/ios.yml
     parameters:
       name: 'iOS_Xcode152'
-      vmImage: 'macOS-13'
+      vmImage: 'macOS-14'
       xCodeVersion: 15.2
-      simulator: 'iPhone 14'
+      simulator: 'iPhone 15'
 
   # Linux
-  - template: jobs/linux.yml
+  - template: jobs/linux.yml
+    parameters:
+      name: Ubuntu_gcc
+
+  - template: jobs/linux.yml
+    parameters:
+      name: Ubuntu_clang
+      CC: clang
+      CXX: clang++
+
+  - template: jobs/linux.yml
+    parameters:
+      name: Ubuntu_Sanitizers_clang
+      enableSanitizers: true
+      CC: clang
+      CXX: clang++
diff --git a/.github/jobs/android.yml b/.github/jobs/android.yml
@@ -1,15 +1,17 @@
 parameters:
 - name: name
   type: string
+  default: ''
 - name: jsEngine
   type: string
+  default: ''
 
 jobs:
 - job: ${{parameters.name}}
   timeoutInMinutes: 30
 
   pool:
-    vmImage: macos-13
+    vmImage: macos-14
 
   steps:
   - script: |
diff --git a/.github/jobs/linux.yml b/.github/jobs/linux.yml
@@ -1,17 +1,29 @@
+parameters:
+  name: ''
+  enableSanitizers: false
+  CC: gcc
+  CXX: g++
+
 jobs:
-- job: ubuntu
+- job: ${{parameters.name}}
   timeoutInMinutes: 15
 
   pool:
     vmImage: ubuntu-latest
 
+  variables:
+    SANITIZER_FLAG: ${{ coalesce(replace(format('{0}', parameters.enableSanitizers), 'True', 'ON'), 'OFF') }}
+
   steps:
   - script: |
       sudo apt-get update
-      sudo apt-get install libjavascriptcoregtk-4.1-dev libcurl4-openssl-dev ninja-build
+      sudo apt-get install libjavascriptcoregtk-4.1-dev libcurl4-openssl-dev ninja-build clang
     displayName: 'Install packages'
 
-  - script: cmake -B Build/ubuntu -GNinja -D CMAKE_BUILD_TYPE=RelWithDebInfo
+  - script: |
+      export CC=${{parameters.CC}}
+      export CXX=${{parameters.CXX}}
+      cmake -B Build/ubuntu -G Ninja -D CMAKE_BUILD_TYPE=RelWithDebInfo -D ENABLE_SANITIZERS=$(SANITIZER_FLAG) -D CMAKE_C_COMPILER=${{parameters.CC}} -D CMAKE_CXX_COMPILER=${{parameters.CXX}}
     displayName: 'Configure CMake'
 
   - script: |
diff --git a/.github/jobs/macos.yml b/.github/jobs/macos.yml
@@ -2,6 +2,7 @@ parameters:
   name: ''
   vmImage: ''
   xCodeVersion: ''
+  enableSanitizers: false
 
 jobs:
 - job: ${{parameters.name}}
@@ -10,13 +11,16 @@ jobs:
   pool:
     vmImage: ${{parameters.vmImage}}
 
+  variables:
+    SANITIZER_FLAG: ${{ coalesce(replace(format('{0}', parameters.enableSanitizers), 'True', 'ON'), 'OFF') }}
+
   steps:
   - script: |
       sudo xcode-select --switch /Applications/Xcode_${{parameters.xCodeVersion}}.app/Contents/Developer
     displayName: 'Select Xcode ${{parameters.xCodeVersion}}'
 
   - script: |
-      cmake -B Build/macOS -GXcode
+      cmake -B Build/macOS -G Xcode -D ENABLE_SANITIZERS=$(SANITIZER_FLAG)
     displayName: 'Configure CMake'
 
   - task: Xcode@5
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -14,7 +14,7 @@ FetchContent_Declare(arcana.cpp
     GIT_TAG c726dbe58713eda65bfb139c257093c43479b894)
 FetchContent_Declare(AndroidExtensions
     GIT_REPOSITORY https://github.com/BabylonJS/AndroidExtensions.git
-    GIT_TAG        7d88a601fda9892791e7b4e994e375e049615688)
+    GIT_TAG        f7ed149b5360cc8a4908fece66607c5ce1e6095b)
 FetchContent_Declare(asio
     GIT_REPOSITORY https://github.com/chriskohlhoff/asio.git
     GIT_TAG f693a3eb7fe72a5f19b975289afc4f437d373d9c)
@@ -73,6 +73,31 @@ option(JSRUNTIMEHOST_POLYFILL_ABORT_CONTROLLER "Include JsRuntimeHost Polyfills
 option(JSRUNTIMEHOST_POLYFILL_WEBSOCKET "Include JsRuntimeHost Polyfill WebSocket." ON)
 option(JSRUNTIMEHOST_POLYFILL_BLOB "Include JsRuntimeHost Polyfill Blob." ON)
 
+# Sanitizers
+option(ENABLE_SANITIZERS "Enable AddressSanitizer and UBSan" OFF)
+
+if(ENABLE_SANITIZERS)
+    set(ENABLE_RTTI ON CACHE BOOL "" FORCE)
+    if(CMAKE_CXX_COMPILER_ID MATCHES "Clang|GNU")
+        set(SANITIZERS "address,undefined")
+        # Check for Clang since vptr and fdsan are Clang-specific
+        if (CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+            list(APPEND SANITIZERS "vptr")
+            # FDSan only works on Android builds with Clang
+            if (ANDROID)
+                list(APPEND SANITIZERS "fdsan")
+            endif()
+        endif()
+
+        string(JOIN "," SANITIZER_FLAGS ${SANITIZERS})
+
+        add_compile_options(-fsanitize=${SANITIZER_FLAGS} -fno-omit-frame-pointer)
+        add_link_options(-fsanitize=${SANITIZER_FLAGS})
+    else()
+        message(WARNING "Sanitizers not supported on this compiler.")
+    endif()
+endif()
+
 # --------------------------------------------------
 
 FetchContent_MakeAvailable_With_Message(arcana.cpp)
diff --git a/Core/AppRuntime/V8Inspector/Source/V8InspectorAgent.cpp b/Core/AppRuntime/V8Inspector/Source/V8InspectorAgent.cpp
@@ -426,10 +426,9 @@ namespace Babylon
         }
         v8::Local<v8::String> string_value = v8::Local<v8::String>::Cast(value);
         int len = string_value->Length();
-        std::basic_string<uint16_t> buffer(len, '\0');
-        string_value->Write(v8::Isolate::GetCurrent(), &buffer[0], 0, len);
-        return v8_inspector::StringBuffer::create(
-            v8_inspector::StringView(buffer.data(), len));
+        std::basic_string<char16_t> buffer(len, '\0');
+        string_value->Write(v8::Isolate::GetCurrent(), reinterpret_cast<uint16_t*>(&buffer[0]), 0, len); // Write expects uint16_t* but the template parameter is char16_t
+        return v8_inspector::StringBuffer::create(v8_inspector::StringView(reinterpret_cast<uint16_t*>(buffer.data()), len));
     }
 
     bool AgentImpl::AppendMessage(
diff --git a/Core/Node-API/Include/Shared/napi/napi-inl.h b/Core/Node-API/Include/Shared/napi/napi-inl.h
@@ -4494,8 +4494,11 @@ inline napi_value InstanceWrap<T>::WrappedMethod(
 ////////////////////////////////////////////////////////////////////////////////
 // ObjectWrap<T> class
 ////////////////////////////////////////////////////////////////////////////////
-
 template <typename T>
+// [BABYLON-NATIVE-ADDITION]
+#ifndef _MSC_VER
+__attribute__((no_sanitize("vptr")))
+#endif
 inline ObjectWrap<T>::ObjectWrap(const Napi::CallbackInfo& callbackInfo) {
   napi_env env = callbackInfo.Env();
   napi_value wrapper = callbackInfo.This();
diff --git a/Core/Node-API/Source/js_native_api_javascriptcore.cc b/Core/Node-API/Source/js_native_api_javascriptcore.cc
@@ -19,6 +19,12 @@ struct napi_callback_info__ {
 };
 
 namespace {
+  size_t jschar_length(const JSChar* str) {
+    size_t len = 0;
+    while (str[len] != 0) { ++len; }
+    return len;
+  }
+
   class JSString {
    public:
     JSString(const JSString&) = delete;
@@ -33,7 +39,7 @@ namespace {
     }
 
     JSString(const JSChar* string, size_t length = NAPI_AUTO_LENGTH)
-      : _string{JSStringCreateWithCharacters(string, length == NAPI_AUTO_LENGTH ? std::char_traits<JSChar>::length(string) : length)} {
+      : _string{JSStringCreateWithCharacters(string, length == NAPI_AUTO_LENGTH ? jschar_length(string) : length)} {
     }
 
     ~JSString() {
@@ -1658,9 +1664,15 @@ napi_status napi_get_value_int32(napi_env env, napi_value value, int32_t* result
   CHECK_ARG(env, result);
 
   JSValueRef exception{};
-  *result = static_cast<int32_t>(JSValueToNumber(env->context, ToJSValue(value), &exception));
+
+  double num = JSValueToNumber(env->context, ToJSValue(value), &exception);
   CHECK_JSC(env, exception);
 
+  if (std::isfinite(num)) {
+    *result = static_cast<int32_t>(num);
+  } else {
+    *result = 0;
+  }
   return napi_ok;
 }
 
@@ -1670,9 +1682,15 @@ napi_status napi_get_value_uint32(napi_env env, napi_value value, uint32_t* resu
   CHECK_ARG(env, result);
 
   JSValueRef exception{};
-  *result = static_cast<uint32_t>(JSValueToNumber(env->context, ToJSValue(value), &exception));
+
+  double num = JSValueToNumber(env->context, ToJSValue(value), &exception);
   CHECK_JSC(env, exception);
 
+  if (std::isfinite(num)) {
+    *result = static_cast<uint32_t>(num);
+  } else {
+    *result = 0;
+  }
   return napi_ok;
 }
 
diff --git a/Polyfills/Blob/Source/Blob.cpp b/Polyfills/Blob/Source/Blob.cpp
@@ -78,7 +78,10 @@ namespace Babylon::Polyfills::Internal
     Napi::Value Blob::ArrayBuffer(const Napi::CallbackInfo&)
     {
         const auto arrayBuffer = Napi::ArrayBuffer::New(Env(), m_data.size());
-        std::memcpy(arrayBuffer.Data(), m_data.data(), m_data.size());
+        if (m_data.data())
+        {
+            std::memcpy(arrayBuffer.Data(), m_data.data(), m_data.size());
+        }
 
         const auto deferred = Napi::Promise::Deferred::New(Env());
         deferred.Resolve(arrayBuffer);
@@ -88,7 +91,10 @@ namespace Babylon::Polyfills::Internal
     Napi::Value Blob::Bytes(const Napi::CallbackInfo&)
     {
         const auto arrayBuffer = Napi::ArrayBuffer::New(Env(), m_data.size());
-        std::memcpy(arrayBuffer.Data(), m_data.data(), m_data.size());
+        if (m_data.data())
+        {
+            std::memcpy(arrayBuffer.Data(), m_data.data(), m_data.size());
+        }
         const auto uint8Array = Napi::Uint8Array::New(Env(), m_data.size(), arrayBuffer, 0);
 
         const auto deferred = Napi::Promise::Deferred::New(Env());

Original file line number	Diff line number	Diff line change
`@@ -426,10 +426,9 @@ namespace Babylon`
`426`	`426`	`}`
`427`	`427`	`v8::Local<v8::String> string_value = v8::Local<v8::String>::Cast(value);`
`428`	`428`	`int len = string_value->Length();`
`429`		`- std::basic_string<uint16_t> buffer(len, '\0');`
`430`		`- string_value->Write(v8::Isolate::GetCurrent(), &buffer[0], 0, len);`
`431`		`- return v8_inspector::StringBuffer::create(`
`432`		`- v8_inspector::StringView(buffer.data(), len));`
	`429`	`+ std::basic_string<char16_t> buffer(len, '\0');`
	`430`	`+ string_value->Write(v8::Isolate::GetCurrent(), reinterpret_cast<uint16_t>(&buffer[0]), 0, len); // Write expects uint16_t but the template parameter is char16_t`
	`431`	`+ return v8_inspector::StringBuffer::create(v8_inspector::StringView(reinterpret_cast<uint16_t*>(buffer.data()), len));`
`433`	`432`	`}`
`434`	`433`
`435`	`434`	`bool AgentImpl::AppendMessage(`