Skip to content

Address sanitizer/ndk28c - #118

Merged
CedricGuillemet merged 39 commits into
BabylonJS:mainfrom
CedricGuillemet:ASAN
Nov 13, 2025
Merged

Address sanitizer/ndk28c#118
CedricGuillemet merged 39 commits into
BabylonJS:mainfrom
CedricGuillemet:ASAN

Conversation

@CedricGuillemet

@CedricGuillemetCedricGuillemet commented Oct 29, 2025•
edited
Loading

Copy link
Copy Markdown
Collaborator
  • 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)

@CedricGuillemet
CedricGuillemet marked this pull request as ready for review November 4, 2025 10:11
@CedricGuillemetCedricGuillemet changed the title Address sanitizerAddress sanitizer/ndk28cNov 4, 2025
bghgary
bghgary reviewed Nov 4, 2025
View reviewed changes
Comment threadCore/AppRuntime/V8Inspector/Source/V8InspectorAgent.cpp Outdated
Comment threadCore/Node-API/Source/js_native_api_javascriptcore.cc Outdated
Comment thread.github/jobs/linux.yml Outdated
Comment thread.github/jobs/macos.yml Outdated
Comment thread.github/jobs/linux.yml Outdated
Comment threadTests/UnitTests/CMakeLists.txt Outdated
@CedricGuillemetCedricGuillemet mentioned this pull request Nov 4, 2025
Closed
matthargett
matthargett reviewed Nov 4, 2025
View reviewed changes

@matthargettmatthargett left a comment •
edited
Loading

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor questions, thanks for peeling this out of the other PR!

Comment threadTests/UnitTests/CMakeLists.txt Outdated
Comment thread.github/azure-pipelines.yml
Comment threadTests/UnitTests/Android/app/src/main/cpp/CMakeLists.txt Outdated
bghgary
bghgary reviewed Nov 6, 2025
View reviewed changes
Comment threadCore/Node-API/Source/js_native_api_javascriptcore.cc Outdated
matthargett
matthargett reviewed Nov 8, 2025
View reviewed changes

@matthargettmatthargett left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one minor question, otherwise looks beautiful! (I'd approve, but I don't have that permission on this repo)

Comment threadCMakeLists.txt
@CedricGuillemetCedricGuillemet mentioned this pull request Nov 13, 2025
Merged
bghgary
bghgary approved these changes Nov 13, 2025
View reviewed changes
Comment threadCore/Node-API/Include/Shared/napi/napi-inl.h
Comment threadPolyfills/Blob/Source/Blob.cpp
@CedricGuillemet
CedricGuillemet merged commit 70ba1e5 into BabylonJS:mainNov 13, 2025
19 checks passed
This was referenced Jul 10, 2026
Closed
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bghgarybghgarybghgary approved these changes
+1 more reviewer
@matthargettmatthargettmatthargett left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@CedricGuillemet@matthargett@bghgary