[runtime] Optionally disable 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.

In order to make such debugging easier, the `XA_NO_INLINE` environment
property may be exported before building the repository.  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.

Also add support for the `XA_NO_STRIP` variable which makes it easy
to produce runtime shared libraries with full debug symbols.

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).  The `XA_NO_STRIP` environment variable can be
exported before building the native runtime in order to leave the
symbols in the resulting shared library.

Author: grendello

Documentation/building/configuration.md

@@ -1,3 +1,10 @@
+<!--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)
+<!--toc:end-->
+
 # Build Configuration
 
 The Xamarin.Android build is heavily dependent on MSBuild, with the *intention*
@@ -10,6 +17,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 +102,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 +138,35 @@ 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 (`XA_NO_INLINE`)
+
+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, the `XA_NO_INLINE` environment property may be exported before building the
+repository.  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.
+
+#### Don't strip the runtime shared libraries (`XA_NO_STRIP`)
+
+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).  The `XA_NO_STRIP` environment variable can be exported before building the
+native runtime in order to leave the symbols in the resulting shared library.

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

@@ -35,21 +35,37 @@ else()
   set(CCACHE_OPTION_DEFAULT ON)
 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)
+  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" ${CCACHE_OPTION_DEFAULT})
+option(DONT_INLINE "Do not inline any functions which are usually inlined, to get better stack traces" ${DONT_INLINE_DEFAULT})
 
 if((MINGW OR NOT WIN32) AND USE_CCACHE)
   if(CMAKE_CXX_COMPILER MATCHES "/ccache/")
@@ -271,6 +287,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()
@@ -422,6 +442,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

@@ -248,7 +248,7 @@ namespace xamarin::android::internal {
 				;
 		}
 
-		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