[runtime] Optionally disable inlining (#8798)

The native runtime by default builds with function inlining enabled,
making heavy use of the feature in order to generate faster code.
However, when debugging a native crash inlining causes stack traces to
point to unlikely locations, reporting the outer function as the crash
location instead of the inlined function where crash actually happened.

In order to make such debugging easier, do one of the following:

  * Export the `XA_NO_INLINE` environment variable to "true-ish"

        export XA_NO_INLINE=1

  * Set the `$(DoNotInlineMonodroid)` MSBuild property to true,
    either by adding to `Configuration.Override.props`:

        <DoNotInlineMonodroid>true</DoNotInlineMonodroid>>

    or by specifying on the command-line:

        ./dotnet-local.sh build Xamarin.Android.sln -p:DoNotInlineMonodroid=true …

This will force all normally inlined functions to be strictly
preserved and kept separate.  The generated code will be slower, but
crash stack traces should be much more precise.

Additionally, `strip`ing of the native shared runtime can be disabled,
making it easier to get full debug symbols.  This can similarly be
enabled by doing one of the following:

  * Export the `XA_NO_STRIP` environment variable to "true-ish"

        export XA_NO_STRIP=1

  * Set the `$(DoNotStripMonodroid)` MSBuild property to true,
    either by adding to `Configuration.Override.props`:

        <DoNotStripMonodroid>true</DoNotStripMonodroid>>

    or by specifying on the command-line:

        ./dotnet-local.sh build Xamarin.Android.sln -p:DoNotStripMonodroid=true …

In a Debug configuration build of xamarin-android,
`libmono-android*.so` will *not* be `strip`d by `src/monodroid`.

Author: grendello

Documentation/building/configuration.md

@@ -1,3 +1,12 @@
+<!--toc:start-->
+- [Build Configuration](#build-configuration)
+  - [Options suitable to use in builds for public use](#options-suitable-to-use-in-builds-for-public-use)
+  - [Options suitable for local development](#options-suitable-for-local-development)
+    - [Native runtime (`src/monodroid`)](#native-runtime-srcmonodroid)
+      - [Disable function inlining](#disable-function-inlining)
+      - [Don't strip the runtime shared libraries](#dont-strip-the-runtime-shared-libraries)
+<!--toc:end-->
+
 # Build Configuration
 
 The Xamarin.Android build is heavily dependent on MSBuild, with the *intention*
@@ -10,6 +19,8 @@ However, some properties may need to be altered in order to suit your
 requirements, such as the location of a cache directory to store
 the Android SDK and NDK.
 
+## Options suitable to use in builds for public use
+
 To modify the build process, copy
 [`Configuration.Override.props.in`](../../Configuration.Override.props.in)
 to `Configuration.Override.props`, and edit the file as appropriate.
@@ -93,7 +104,7 @@ Overridable MSBuild properties include:
 
   * `$(IgnoreMaxMonoVersion)`: Skip the enforcement of the `$(MonoRequiredMaximumVersion)`
     property. This is so that developers can run against the latest
-    and greatest. But the build system can enforce the min and max 
+    and greatest. But the build system can enforce the min and max
     versions. The default is `true`, however on CI we use:
 
          /p:IgnoreMaxMonoVersion=False
@@ -129,6 +140,43 @@ Overridable MSBuild properties include:
       * `4`: Mono 4.6 support.
       * `5`: Mono 4.8 and above support. This is the default.
 
-  * `$(AndroidEnableAssemblyCompression)`: Defaults to `True`. When enabled, all the 
+  * `$(AndroidEnableAssemblyCompression)`: Defaults to `True`. When enabled, all the
      assemblies placed in the APK will be compressed in `Release` builds. `Debug`
      builds are not affected.
+
+## Options suitable for local development
+
+### Native runtime (`src/monodroid`)
+
+Note that in order for the native build settings to have full effect, one needs to make sure that
+the entire native runtime is rebuilt **and** that all `cmake` files are regenerated.  This is true
+on the very first build, but rebuilds may require forcing the entire runtime to be rebuilt.
+
+The simplest way to do it is to remove `src/monodroid/obj` and run the usual build from the
+repository's root directory.
+
+#### Disable function inlining
+
+The native runtime by default builds with function inlining enabled, making heavy use of
+the feature in order to generate faster code.  However, when debugging a native crash inlining
+causes stack traces to point to unlikely locations, reporting the outer function as the crash
+location instead of the inlined function where crash actually happened.  There are two ways to
+enable this mode of operation:
+
+  1. Export the `XA_NO_INLINE` environment variable before building either the entire repository
+     or just `src/monodroid/`
+  2. Set the MSBuild property `DoNotInlineMonodroid` to `true`, when building `src/monodroid/monodroid.csproj`
+
+Doing either will force all normally inlined functions to be strictly preserved and kept
+separate.  The generated code will be slower, but crash stack traces should be much more precise.
+
+#### Don't strip the runtime shared libraries
+
+Similar to the previous section, this option makes crash stack traces more informative.  In normal
+builds, all the debugging information is stripped from the runtime shared libraries, thus making
+stack traces rarely point to anything more than the surrounding function name (which may sometimes
+be misleading, too).  Just as for inlining, the no-strip mode can be enabled with one of two ways:
+
+  1. Export the `XA_NO_STRIP` environment variable before building either the entire repository
+     or just `src/monodroid/`
+  2. Set the MSBuild property `DoNotStripMonodroid` to `true`, when building `src/monodroid/monodroid.csproj`

build-tools/cmake/xa_macros.cmake

@@ -202,7 +202,6 @@ function(xa_common_prepare)
     -fno-strict-aliasing
     -ffunction-sections
     -funswitch-loops
-    -finline-limit=300
     -Wa,-noexecstack
     -fPIC
     -g

src/monodroid/CMakeLists.txt

@@ -29,21 +29,43 @@ set(CMAKE_C_STANDARD 11)
 set(CMAKE_C_STANDARD_REQUIRED ON)
 set(CMAKE_C_EXTENSIONS OFF)
 
+if(CMAKE_BUILD_TYPE STREQUAL Debug)
+  set(DEBUG_BUILD True)
+else()
+  set(DEBUG_BUILD False)
+endif()
+
+set(XA_NO_INLINE "$ENV{XA_NO_INLINE}")
+if(XA_NO_INLINE)
+  set(DONT_INLINE_DEFAULT ON)
+else()
+  set(DONT_INLINE_DEFAULT OFF)
+endif()
+
+set(XA_NO_STRIP "$ENV{XA_NO_STRIP}")
+if(XA_NO_STRIP OR DEBUG_BUILD)
+  set(STRIP_DEBUG_DEFAULT OFF)
+endif()
+
 option(ENABLE_CLANG_ASAN "Enable the clang AddressSanitizer support" OFF)
 option(ENABLE_CLANG_UBSAN "Enable the clang UndefinedBehaviorSanitizer support" OFF)
 
 if(ENABLE_CLANG_ASAN OR ENABLE_CLANG_UBSAN)
+  # ASAN and UBSAN always require the debug symbols to be left in the binary
   set(STRIP_DEBUG_DEFAULT OFF)
   set(ANALYZERS_ENABLED ON)
 else()
-  set(STRIP_DEBUG_DEFAULT ON)
+  if(NOT XA_NO_STRIP)
+    set(STRIP_DEBUG_DEFAULT ON)
+  endif()
   set(ANALYZERS_ENABLED OFF)
 endif()
 
 option(ENABLE_TIMING "Build with timing support" OFF)
 option(STRIP_DEBUG "Strip debugging information when linking" ${STRIP_DEBUG_DEFAULT})
 option(DISABLE_DEBUG "Disable the built-in debugging code" OFF)
 option(USE_CCACHE "Use ccache, if found, to speed up recompilation" ON)
+option(DONT_INLINE "Do not inline any functions which are usually inlined, to get better stack traces" ${DONT_INLINE_DEFAULT})
 
 if(USE_CCACHE)
   if(CMAKE_CXX_COMPILER MATCHES "/ccache/")
@@ -58,11 +80,7 @@ if(USE_CCACHE)
   endif()
 endif()
 
-if(CMAKE_BUILD_TYPE STREQUAL Debug)
-  set(DEBUG_BUILD True)
-else()
-  set(DEBUG_BUILD False)
-endif()
+
 
 if(ANDROID_STL STREQUAL none)
   set(USES_LIBSTDCPP False)
@@ -179,6 +197,10 @@ add_compile_definitions(MONO_DLL_EXPORT)
 add_compile_definitions(NET)
 add_compile_definitions(JI_NO_VISIBILITY)
 
+if(DONT_INLINE)
+  add_compile_definitions(NO_INLINE)
+endif()
+
 if(DEBUG_BUILD AND NOT DISABLE_DEBUG)
   add_compile_definitions(DEBUG)
 endif()
@@ -293,6 +315,13 @@ endif()
 
 if(STRIP_DEBUG)
   list(APPEND LOCAL_COMMON_LINKER_ARGS LINKER:-S)
+else()
+  # When not stripping symbols, we likely want to have precise stack traces, so
+  # we won't omit frame pointers
+  list(APPEND LOCAL_COMMON_COMPILER_ARGS
+    -fno-omit-frame-pointer
+    -fno-limit-debug-info
+  )
 endif()
 
 # Parameters to both functions are (all required):

src/monodroid/jni/cpp-util.hh

@@ -33,6 +33,11 @@ do_abort_unless (const char* fmt, ...)
 #define abort_if_invalid_pointer_argument(_ptr_) abort_unless ((_ptr_) != nullptr, "Parameter '%s' must be a valid pointer", #_ptr_)
 #define abort_if_negative_integer_argument(_arg_) abort_unless ((_arg_) > 0, "Parameter '%s' must be larger than 0", #_arg_)
 
+// Helpers to use in "printf debugging". Normally not used in code anywhere. No code should be shipped with any
+// of the macros present.
+#define PD_LOG_LOCATION() log_info_nocheck (LOG_DEFAULT, "loc: %s:%d (%s)", __FILE__, __LINE__, __FUNCTION__)
+#define PD_LOG_FUNCTION() log_info_nocheck (LOG_DEFAULT, "%s [%s:%d]", __PRETTY_FUNCTION__, __FILE__, __LINE__)
+
 namespace xamarin::android
 {
 	template <typename T>

src/monodroid/jni/embedded-assemblies.hh

@@ -282,7 +282,7 @@ namespace xamarin::android::internal {
 				&& ((application_config.have_runtime_config_blob && runtime_config_blob_found) || !application_config.have_runtime_config_blob);
 		}
 
-		static force_inline c_unique_ptr<char> to_utf8 (const MonoString *s) noexcept
+		force_inline static c_unique_ptr<char> to_utf8 (const MonoString *s) noexcept
 		{
 			return c_unique_ptr<char> (mono_string_to_utf8 (const_cast<MonoString*>(s)));
 		}

src/monodroid/jni/helpers.hh

@@ -16,7 +16,7 @@ namespace xamarin::android
 	{
 	public:
 		template<typename Ret, typename P1, typename P2>
-		static force_inline Ret add_with_overflow_check (const char *file, uint32_t line, P1 a, P2 b) noexcept
+		force_inline static Ret add_with_overflow_check (const char *file, uint32_t line, P1 a, P2 b) noexcept
 		{
 			Ret ret;
 
@@ -40,7 +40,7 @@ namespace xamarin::android
 		// fail
 		//
 		template<typename Ret>
-		static force_inline Ret multiply_with_overflow_check (const char *file, uint32_t line, size_t a, size_t b) noexcept
+		force_inline static Ret multiply_with_overflow_check (const char *file, uint32_t line, size_t a, size_t b) noexcept
 		{
 			Ret ret;
 

src/monodroid/jni/platform-compat.hh

@@ -5,7 +5,12 @@
 
 static inline constexpr int DEFAULT_DIRECTORY_MODE = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
 
-#define force_inline inline __attribute__((always_inline))
-#define never_inline __attribute__((noinline))
+#if defined(NO_INLINE)
+#define force_inline [[gnu::noinline]]
+#define inline_calls [[gnu::flatten]]
+#else
+#define force_inline [[gnu::always_inline]]
+#define inline_calls
+#endif
 
 #endif // __PLATFORM_COMPAT_HH

src/monodroid/monodroid.targets

@@ -75,7 +75,9 @@
       Inputs="@(_ConfigureRuntimesInputs)"
       Outputs="@(_ConfigureRuntimesOutputs)">
     <PropertyGroup>
-      <_CmakeAndroidFlags>--debug-output -GNinja -DCMAKE_MAKE_PROGRAM="$(NinjaPath)" -DXA_BUILD_CONFIGURATION=$(Configuration) -DXA_LIB_TOP_DIR=$(MicrosoftAndroidSdkOutDir) -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DMONO_PATH="$(MonoSourceFullPath)" -DANDROID_STL="none" -DANDROID_CPP_FEATURES="no-rtti no-exceptions" -DANDROID_TOOLCHAIN=clang -DCMAKE_TOOLCHAIN_FILE="$(AndroidNdkDirectory)/build/cmake/android.toolchain.cmake" -DANDROID_NDK=$(AndroidNdkDirectory)</_CmakeAndroidFlags>
+      <_NoInline Condition=" '$(DoNotInlineMonodroid)' == 'true' ">-DDONT_INLINE=ON</_NoInline>
+      <_NoStrip Condition=" '$(DoNotStripMonodroid)' == 'true' ">-DSTRIP_DEBUG=OFF</_NoStrip>
+      <_CmakeAndroidFlags>$(_NoInline) $(_NoStrip) --debug-output -GNinja -DCMAKE_MAKE_PROGRAM="$(NinjaPath)" -DXA_BUILD_CONFIGURATION=$(Configuration) -DXA_LIB_TOP_DIR=$(MicrosoftAndroidSdkOutDir) -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DMONO_PATH="$(MonoSourceFullPath)" -DANDROID_STL="none" -DANDROID_CPP_FEATURES="no-rtti no-exceptions" -DANDROID_TOOLCHAIN=clang -DCMAKE_TOOLCHAIN_FILE="$(AndroidNdkDirectory)/build/cmake/android.toolchain.cmake" -DANDROID_NDK=$(AndroidNdkDirectory)</_CmakeAndroidFlags>
     </PropertyGroup>
     <MakeDir Directories="@(_OutputDirsToCreate)"/>
     <ItemGroup>