Skip to content

Commit aaa37c3

Browse files
authored
[monodroid] Add native code static checkers and sanitizers (#5094)
Context: https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1139593 Begin running Clang-based code analysis tools against our C++ codebase: * [`clang-tidy`][0] static analysis * [`ASAN`][1] and [`UBSAN`][2] runtime sanitizers, which can find some issues that cannot be detected by static analysis. All of the required tools are shipped with the Android NDK, which currently ships the Clang 9 toolchain. All of the tools require separate builds of the native code. `clang-tidy` builds the entire codebase to generate an AST and then analyze it, reporting possible errors, but it does not produce any binaries. The ASAN and UBSAN runtime sanitizers instrument the generated code in numerous ways, which make runtime code much slower to execute, up to 3x longer to run various tasks. We now build our native code 4 additional times, once for each sanitizer in both `Debug` and `Release` configs. The output binaries have the `-checked+SANITIZER_NAME` infix added to their name, e.g. `libmono-android-checked+asan.debug.so`. The instrumented binaries will not be currently distributed to the end users since they are intended for internal use. Also added are two [`wrap.sh`][3] scripts, one for `ASAN` and the other for `UBSAN`; both can be found in the `build-tools/wrap.sh/` directory. The instrumented binaries can then be placed in the `.apk` instead of the standard binaries by adding the following to the project file: <PropertyGroup> <AndroidIncludeWrapSh Condition=" '$(UseASAN)' != '' Or '$(UseUBSAN)' != '' ">true</AndroidIncludeWrapSh> <_AndroidCheckedBuild Condition=" '$(UseASAN)' != '' ">asan</_AndroidCheckedBuild> <_AndroidCheckedBuild Condition=" '$(UseUBSAN)' != '' ">ubsan</_AndroidCheckedBuild> <_ASANScript>../path/to/build-scripts/wrap.sh/asan.sh</_ASANScript> <_UBSANScript>../path/to/build-scripts/wrap.sh/ubsan.sh</_UBSANScript> </PropertyGroup> <ItemGroup> <AndroidNativeLibrary Include="$(_ASANScript)" Condition=" '$(UseASAN)' != '' "> <Link>lib\arm64-v8a\wrap.sh</Link> </AndroidNativeLibrary> <AndroidNativeLibrary Include="$(_ASANScript)" Condition=" '$(UseASAN)' != '' "> <Link>lib\armeabi-v7a\wrap.sh</Link> </AndroidNativeLibrary> <AndroidNativeLibrary Include="$(_ASANScript)" Condition=" '$(UseASAN)' != '' "> <Link>lib\x86\wrap.sh</Link> </AndroidNativeLibrary> <AndroidNativeLibrary Include="$(_ASANScript)" Condition=" '$(UseASAN)' != '' "> <Link>lib\x86_64\wrap.sh</Link> </AndroidNativeLibrary> <AndroidNativeLibrary Include="$(_UBSANScript)" Condition=" '$(UseUBSAN)' != '' "> <Link>lib\arm64-v8a\wrap.sh</Link> </AndroidNativeLibrary> <AndroidNativeLibrary Include="$(_UBSANScript)" Condition=" '$(UseUBSAN)' != '' "> <Link>lib\armeabi-v7a\wrap.sh</Link> </AndroidNativeLibrary> <AndroidNativeLibrary Include="$(_UBSANScript)" Condition=" '$(UseUBSAN)' != '' "> <Link>lib\x86\wrap.sh</Link> </AndroidNativeLibrary> <AndroidNativeLibrary Include="$(_UBSANScript)" Condition=" '$(UseUBSAN)' != '' "> <Link>lib\x86_64\wrap.sh</Link> </AndroidNativeLibrary> </ItemGroup> The `%(AndroidNativeLibrary.Link)` metadata value **must** be a valid path to the native library directory inside the `.apk` since the `wrap.sh` scripts are architecture-specific and are expected to be found in specific directories by the Android loader. The checked build is enabled by setting the `$(_AndroidCheckedBuild)` MSBuild property to either `asan` or `ubsan`. TODO: While "checked" versions of the native libraries are built, they need to be *used* to be of full usefulness. We still need to update CI so that `Mono.Android-Tests` will be executed with `asan` or `ubsan` enabled, to assert that future issues are not introduced. [0]: https://releases.llvm.org/9.0.0/tools/clang/tools/extra/docs/clang-tidy/ [1]: https://releases.llvm.org/9.0.0/tools/clang/docs/AddressSanitizer.html [2]: https://releases.llvm.org/9.0.0/tools/clang/docs/UndefinedBehaviorSanitizer.html [3]: https://developer.android.com/ndk/guides/wrap-script
1 parent 71dc718 commit aaa37c3

File tree

26 files changed

+565
-58
lines changed

26 files changed

+565
-58
lines changed

.editorconfig

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,12 @@ tab_width = 8
4141
indent_size = 8
4242
max_line_length = 180
4343

44+
# CMake files
45+
[CMakeLists.txt]
46+
indent_style = space
47+
indent_size = 2
48+
insert_final_newline = true
49+
4450
###############################
4551
# .NET Coding Conventions #
4652
###############################

Documentation/guides/building-apps/build-properties.md

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -555,6 +555,50 @@ The most common values for this property are:
555555
556556
Added in Xamarin.Android 6.1.
557557

558+
## AndroidIncludeWrapSh
559+
560+
A boolean value which indicates whether the Android wrapper script
561+
([`wrap.sh`](https://developer.android.com/ndk/guides/wrap-script))
562+
should be packaged into the APK. The property defaults to `false`
563+
since the wrapper script may significantly influence the way the
564+
application starts up and works and the script should be included only
565+
when necessary e.g. for debugging or otherwise changing the
566+
application startup/runtime behavior.
567+
568+
The script is added to the project using the
569+
[`@(AndroidNativeLibrary)`](~/android/deploy-test/building-apps/build-items.md#androidnativelibrary)
570+
build action, because it is placed in the same directory where
571+
architecture-specific native libraries, and must be named `wrap.sh`.
572+
573+
The easiest way to specify path to the `wrap.sh` script is to put it
574+
in a directory named after the target architecture. This approach will
575+
work if you have just one `wrap.sh` per architecture:
576+
577+
```xml
578+
<AndroidNativeLibrary Include="path/to/arm64-v8a/wrap.sh" />
579+
```
580+
581+
However, if your project needs more than one `wrap.sh` per
582+
architecture, for different purposes, this approach won't work.
583+
Instead, in such cases the name can be specified using the `Link`
584+
metadata of the `AndroidNativeLibrary`:
585+
586+
```xml
587+
<AndroidNativeLibrary Include="/path/to/my/arm64-wrap.sh">
588+
<Link>lib\arm64-v8a\wrap.sh</Link>
589+
</AndroidNativeLibrary>
590+
```
591+
592+
If the `Link` metadata is used, the path specified in its value must
593+
be a valid native architecture-specific library path, relative to the
594+
APK root directory. The format of the path is `lib\ARCH\wrap.sh` where
595+
`ARCH` can be one of:
596+
597+
+ `arm64-v8a`
598+
+ `armeabi-v7a`
599+
+ `x86_64`
600+
+ `x86`
601+
558602
## AndroidKeyStore
559603

560604
A boolean value which indicates whether

build-tools/Xamarin.Android.Tools.BootstrapTasks/result-packaging.targets

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
<_BuildStatusFiles Include="$(XamarinAndroidSourcePath)**\CMakeFiles\*.log" />
3030
<_BuildStatusFiles Include="$(XamarinAndroidSourcePath)**\.ninja_log" />
3131
<_BuildStatusFiles Include="$(XamarinAndroidSourcePath)**\android-*.config.cache" />
32+
<_BuildStatusFiles Include="$(XamarinAndroidSourcePath)\bin\Build$(Configuration)\clang-tidy*.log" />
3233
</ItemGroup>
3334
<ItemGroup>
3435
<_TestResultFiles Include="$(XamarinAndroidSourcePath)TestResult-*.xml" />

build-tools/cmake/xa_macros.cmake

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@ macro(xa_common_prepare)
5050
Wa,-noexecstack
5151
fPIC
5252
g
53-
fomit-frame-pointer
5453
O2
5554
)
5655

build-tools/installers/create-installers.targets

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,8 +160,14 @@
160160
<_MSBuildFiles Include="$(MSBuildSrcDir)\jnimarshalmethod-gen.pdb" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
161161
<_MSBuildFiles Include="$(MSBuildSrcDir)\LayoutBinding.cs" />
162162
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libmono-android.debug.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
163+
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libmono-android-checked+asan.debug.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
164+
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libmono-android-checked+ubsan.debug.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
163165
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libmono-android.release.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
166+
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libmono-android-checked+asan.release.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
167+
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libmono-android-checked+ubsan.release.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
164168
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libxa-internal-api.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
169+
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libxa-internal-api-checked+asan.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
170+
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libxa-internal-api-checked+ubsan.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
165171
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libmono-btls-shared.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
166172
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libmono-btls-shared.d.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
167173
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libmono-profiler-aot.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
@@ -174,6 +180,8 @@
174180
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libmonosgen-2.0.d.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
175181
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libsqlite3_xamarin.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
176182
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libxamarin-debug-app-helper.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
183+
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libxamarin-debug-app-helper-checked+asan.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
184+
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\%(Identity)\libxamarin-debug-app-helper-checked+ubsan.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
177185
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\interpreter-%(Identity)\libmono-btls-shared.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
178186
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\interpreter-%(Identity)\libmono-profiler-aot.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />
179187
<_MSBuildFiles Include="@(AndroidSupportedTargetJitAbi->'$(MSBuildSrcDir)\lib\interpreter-%(Identity)\libmono-profiler-log.so')" Condition=" '$(PackageId)' != 'Microsoft.Android.Sdk' " />

build-tools/wrap.sh/asan.sh

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#!/system/bin/sh
2+
HERE="$(cd "$(dirname "$0")" && pwd)"
3+
export ASAN_OPTIONS="log_to_syslog=false,allow_user_segv_handler=1,check_initialization_order=true,print_stats=true,detect_invalid_pointer_pairs=2"
4+
ASAN_LIB=$(ls $HERE/libclang_rt.asan-*-android.so)
5+
if [ -f "$HERE/libc++_shared.so" ]; then
6+
# Workaround for https://github.com/android-ndk/ndk/issues/988.
7+
export LD_PRELOAD="$ASAN_LIB $HERE/libc++_shared.so"
8+
else
9+
export LD_PRELOAD="$ASAN_LIB"
10+
fi
11+
"$@"

build-tools/wrap.sh/ubsan.sh

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
#!/system/bin/sh
2+
HERE="$(cd "$(dirname "$0")" && pwd)"
3+
UBSAN_LIB=$(ls $HERE/libclang_rt.ubsan_standalone-*-android.so)
4+
if [ -f "$HERE/libc++_shared.so" ]; then
5+
# Workaround for https://github.com/android-ndk/ndk/issues/988.
6+
export LD_PRELOAD="$UBSAN_LIB $HERE/libc++_shared.so"
7+
else
8+
export LD_PRELOAD="$UBSAN_LIB"
9+
fi
10+
"$@"

build-tools/xaprepare/xaprepare/Resources/Configuration.OperatingSystem.props.in

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,6 @@
2020
<JarPath Condition=" '$(JarPath)' == '' ">@jar@</JarPath>
2121
<JavaPath Condition=" '$(JavaPath)' == '' ">@java@</JavaPath>
2222
<HostHomebrewPrefix Condition=" '$(HostHomebrewPrefix)' == '' ">@HOST_HOMEBREW_PREFIX@</HostHomebrewPrefix>
23+
<NdkLlvmTag Condition=" '$(NdkLlvmTag)' == '' ">@NDK_LLVM_TAG@</NdkLlvmTag>
2324
</PropertyGroup>
2425
</Project>

build-tools/xaprepare/xaprepare/Steps/Step_GenerateFiles.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ GeneratedFile Get_Configuration_OperatingSystem_props (Context context)
101101
{ "@javac@", context.OS.JavaCPath },
102102
{ "@java@", context.OS.JavaPath },
103103
{ "@jar@", context.OS.JarPath },
104+
{ "@NDK_LLVM_TAG@", $"{context.OS.Type.ToLowerInvariant ()}-x86_64" },
104105
};
105106

106107
return new GeneratedPlaceholdersFile (

src/Mono.Android/Test/Mono.Android-Tests.csproj

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@
2424
<TargetFrameworkVersion>v11.0</TargetFrameworkVersion>
2525
<AndroidDexTool Condition=" '$(AndroidDexTool)' == '' ">d8</AndroidDexTool>
2626
<_SkipJniAddNativeMethodRegistrationAttributeScan>True</_SkipJniAddNativeMethodRegistrationAttributeScan>
27+
<AndroidIncludeWrapSh Condition=" '$(UseASAN)' != '' Or '$(UseUBSAN)' != '' ">true</AndroidIncludeWrapSh>
28+
<_AndroidCheckedBuild Condition=" '$(UseASAN)' != '' ">asan</_AndroidCheckedBuild>
29+
<_AndroidCheckedBuild Condition=" '$(UseUBSAN)' != '' ">ubsan</_AndroidCheckedBuild>
30+
<_ASANScript>..\..\..\build-tools\wrap.sh\asan.sh</_ASANScript>
31+
<_UBSANScript>..\..\..\build-tools\wrap.sh\ubsan.sh</_UBSANScript>
2732
</PropertyGroup>
2833
<Import Project="Mono.Android-Test.Shared.projitems" Label="Shared" Condition="Exists('Mono.Android-Test.Shared.projitems')" />
2934
<Import Project="..\..\..\Configuration.props" />
@@ -124,4 +129,31 @@
124129
<ItemGroup>
125130
<Folder Include="Localization\" />
126131
</ItemGroup>
132+
<ItemGroup>
133+
<AndroidNativeLibrary Include="$(_ASANScript)" Condition=" '$(UseASAN)' != '' ">
134+
<Link>lib\arm64-v8a\wrap.sh</Link>
135+
</AndroidNativeLibrary>
136+
<AndroidNativeLibrary Include="$(_ASANScript)" Condition=" '$(UseASAN)' != '' ">
137+
<Link>lib\armeabi-v7a\wrap.sh</Link>
138+
</AndroidNativeLibrary>
139+
<AndroidNativeLibrary Include="$(_ASANScript)" Condition=" '$(UseASAN)' != '' ">
140+
<Link>lib\x86\wrap.sh</Link>
141+
</AndroidNativeLibrary>
142+
<AndroidNativeLibrary Include="$(_ASANScript)" Condition=" '$(UseASAN)' != '' ">
143+
<Link>lib\x86_64\wrap.sh</Link>
144+
</AndroidNativeLibrary>
145+
146+
<AndroidNativeLibrary Include="$(_UBSANScript)" Condition=" '$(UseUBSAN)' != '' ">
147+
<Link>lib\arm64-v8a\wrap.sh</Link>
148+
</AndroidNativeLibrary>
149+
<AndroidNativeLibrary Include="$(_UBSANScript)" Condition=" '$(UseUBSAN)' != '' ">
150+
<Link>lib\armeabi-v7a\wrap.sh</Link>
151+
</AndroidNativeLibrary>
152+
<AndroidNativeLibrary Include="$(_UBSANScript)" Condition=" '$(UseUBSAN)' != '' ">
153+
<Link>lib\x86\wrap.sh</Link>
154+
</AndroidNativeLibrary>
155+
<AndroidNativeLibrary Include="$(_UBSANScript)" Condition=" '$(UseUBSAN)' != '' ">
156+
<Link>lib\x86_64\wrap.sh</Link>
157+
</AndroidNativeLibrary>
158+
</ItemGroup>
127159
</Project>

0 commit comments

Comments
 (0)