-
Notifications
You must be signed in to change notification settings - Fork 152
Enable sanitizers for macOS, Linux, Windows #1559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
351c87b
b78acf4
c2f1138
ed44d18
a388975
f46d415
ba0e5c3
121cd8a
46a5c25
a1c6286
2fa6784
18f0dab
c11fba6
f17242c
8d910e7
8f93ea4
55ee800
816ee5d
31f815f
c80dd5c
88724f3
9345b67
51740d3
a46ad85
654407c
5c1c3af
2bbe45b
fa7fa44
8b77c08
2b399ad
32a2773
a1bcc71
8b0f51c
5db4de6
e9c3faa
47143d7
c220203
50bee86
c3d8bd8
47cede9
68b827d
b4050c3
9fb5fa2
1142393
1a52baa
25d7ce0
097313c
39d2ebd
e327445
b121c08
60ff440
b329df2
e497429
eb9e0d5
260aa88
f5e5002
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| # Suppress leaks from dbus. No direct leak visible with BN. | ||
| # command to dump leaks and suppress annecessary : | ||
| # ASAN_OPTIONS=detect_leaks=1 LSAN_OPTIONS=suppressions=../../../.github/asan_suppress.txt ./UnitTests | ||
| # ASAN_OPTIONS=detect_leaks=1 LSAN_OPTIONS=suppressions=../../../.github/asan_suppress.txt xvfb-run ./Playground app:///Scripts/validation_native.js | ||
| leak:bmalloc_allocate_casual | ||
| leak:reallocate_for_length | ||
| leak:dbus_message_new_empty_header |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ parameters: | |
| CC: '' | ||
| CXX: '' | ||
| JSEngine: '' | ||
| enableSanitizers: false | ||
|
|
||
| jobs: | ||
| - job: ${{ parameters.name }} | ||
|
|
@@ -12,6 +13,7 @@ jobs: | |
| vmImage: ${{ parameters.vmImage }} | ||
|
|
||
| variables: | ||
| SANITIZER_FLAG: ${{ coalesce(replace(format('{0}', parameters.enableSanitizers), 'True', 'ON'), 'OFF') }} | ||
| CC: ${{ parameters.CC }} | ||
| CXX: ${{ parameters.CXX }} | ||
|
|
||
|
|
@@ -22,16 +24,26 @@ jobs: | |
|
|
||
| - script: | | ||
| sudo apt-get update | ||
| sudo apt-get install libjavascriptcoregtk-4.1-dev libgl1-mesa-dev libcurl4-openssl-dev libwayland-dev | ||
| sudo apt-get install libjavascriptcoregtk-4.1-dev libgl1-mesa-dev libcurl4-openssl-dev libwayland-dev clang | ||
| displayName: 'Install packages' | ||
|
|
||
| - script: | | ||
| cmake -G Ninja -B build -D JAVASCRIPTCORE_LIBRARY=/usr/lib/x86_64-linux-gnu/libjavascriptcoregtk-4.1.so -D NAPI_JAVASCRIPT_ENGINE=${{ parameters.JSEngine }} -D CMAKE_BUILD_TYPE=RelWithDebInfo -D BX_CONFIG_DEBUG=ON -D CMAKE_UNITY_BUILD=$(UNITY_BUILD) -D OpenGL_GL_PREFERENCE=GLVND -D BABYLON_DEBUG_TRACE=ON | ||
| cmake -G Ninja -B build -D JAVASCRIPTCORE_LIBRARY=/usr/lib/x86_64-linux-gnu/libjavascriptcoregtk-4.1.so -D NAPI_JAVASCRIPT_ENGINE=${{ parameters.JSEngine }} -D CMAKE_BUILD_TYPE=RelWithDebInfo -D BX_CONFIG_DEBUG=ON -D CMAKE_UNITY_BUILD=$(UNITY_BUILD) -D OpenGL_GL_PREFERENCE=GLVND -D BABYLON_DEBUG_TRACE=ON -D ENABLE_SANITIZERS=$(SANITIZER_FLAG) . | ||
| ninja -C build | ||
| displayName: 'Build X11' | ||
|
|
||
| # Memory leaks on CI is disabled due to memory leaks reported with xvfb and impossible to add to ignore list. | ||
| # See https://github.com/BabylonJS/BabylonNative/issues/1575 | ||
|
|
||
| - script: | | ||
| cd build/Apps/UnitTests | ||
| xvfb-run ./UnitTests | ||
| displayName: 'Unit Tests' | ||
|
|
||
|
Comment on lines
+38
to
+42
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you intend to move this? |
||
| - script: | | ||
| cd build/Apps/Playground | ||
| # Command line to suppress false positive memory leaks : | ||
| # ASAN_OPTIONS=detect_leaks=1 LSAN_OPTIONS=suppressions=../../../.github/asan_suppress.txt xvfb-run ./Playground app:///Scripts/validation_native.js | ||
| xvfb-run ./Playground app:///Scripts/validation_native.js | ||
| displayName: 'Validation Tests' | ||
|
|
||
|
|
@@ -47,8 +59,3 @@ jobs: | |
| pathtoPublish: 'build/Apps/Playground/Errors' | ||
| displayName: 'Publish Tests ${{ parameters.name }} Errors' | ||
| condition: failed() | ||
|
|
||
| - script: | | ||
| cd build/Apps/UnitTests | ||
| xvfb-run ./UnitTests | ||
| displayName: 'Unit Tests' | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,13 +15,17 @@ parameters: | |
| - name: graphics_api | ||
| type: string | ||
| default: D3D11 | ||
| - name: enableSanitizers | ||
| type: boolean | ||
| default: false | ||
|
|
||
| jobs: | ||
| - job: ${{ parameters.name }} | ||
| timeoutInMinutes: 60 | ||
| pool: | ||
| vmImage: ${{ parameters.vmImage }} | ||
| variables: | ||
| SANITIZER_FLAG: ${{ coalesce(replace(format('{0}', parameters.enableSanitizers), 'True', 'ON'), 'OFF') }} | ||
| ${{ if eq(parameters.napiType, 'jsi') }}: | ||
| napiSuffix: '_JSI' | ||
| jsEngineDefine: '-DNAPI_JAVASCRIPT_ENGINE=JSI' | ||
|
|
@@ -40,7 +44,7 @@ jobs: | |
|
|
||
| # BGFX_CONFIG_MAX_FRAME_BUFFERS is set so enough Framebuffers are available before V8 starts disposing unused ones | ||
| - script: | | ||
| cmake -G "Visual Studio 17 2022" -B build${{ variables.solutionName }} -A ${{ parameters.platform }} ${{ variables.jsEngineDefine }} -D BX_CONFIG_DEBUG=ON -D GRAPHICS_API=${{ parameters.graphics_api }} -D CMAKE_UNITY_BUILD=$(UNITY_BUILD) -D BGFX_CONFIG_MAX_FRAME_BUFFERS=256 -D BABYLON_DEBUG_TRACE=ON | ||
| cmake -G "Visual Studio 17 2022" -B build${{ variables.solutionName }} -A ${{ parameters.platform }} ${{ variables.jsEngineDefine }} -D BX_CONFIG_DEBUG=ON -D GRAPHICS_API=${{ parameters.graphics_api }} -D CMAKE_UNITY_BUILD=$(UNITY_BUILD) -D BGFX_CONFIG_MAX_FRAME_BUFFERS=256 -D BABYLON_DEBUG_TRACE=ON -D ENABLE_SANITIZERS=$(SANITIZER_FLAG) | ||
| displayName: 'Generate ${{ variables.solutionName }} solution' | ||
|
|
||
| - task: MSBuild@1 | ||
|
|
@@ -62,6 +66,23 @@ jobs: | |
|
|
||
| displayName: 'Enable Crash Dumps' | ||
|
|
||
| - powershell: | | ||
| $vs = vswhere -latest -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath | ||
| $msvc = Get-ChildItem "$vs\VC\Tools\MSVC" | Sort-Object Name -Descending | Select-Object -First 1 | ||
| $asan = "$($msvc.FullName)\bin\Hostx64\x64" | ||
|
|
||
| $destinations = @( | ||
| "build${{ variables.solutionName }}\Apps\Playground\RelWithDebInfo", | ||
| "build${{ variables.solutionName }}\Apps\UnitTests\RelWithDebInfo" | ||
| ) | ||
|
|
||
| foreach ($dest in $destinations) { | ||
| Get-ChildItem "$asan\clang_rt.asan_*.dll" | | ||
| Copy-Item -Destination $dest | ||
| } | ||
| displayName: "Copy ASAN runtime DLLs" | ||
| condition: ${{ parameters.enableSanitizers }} | ||
|
Comment on lines
+69
to
+84
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine for now. Consider doing this inside CMake since it seems like it's part of the build.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure. These dll are in PATH when debugging/testing with Visual Studio.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, maybe we should do the same and add them to the path? Do you know what adds them to the path? |
||
|
|
||
| - script: | | ||
| cd build${{ variables.solutionName }}\Apps\Playground | ||
| cd RelWithDebInfo | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Maybe this file should go in the source and copied with the app so that you don't have to do the
../../..thing.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
agreed