From 736a235878ab8c40c6cde264259aa5f2e73fbebe Mon Sep 17 00:00:00 2001 From: Marek Habersack Date: Fri, 1 Nov 2019 00:17:14 +0100 Subject: [PATCH] [C++ification] Take care of a few warnings and don't use LTO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Desktop compilers mis-detect memory allocation size in our bundled property code if fortified version of `strncpy` is used (even though the size calculation is correct). Use `memcpy` instead. LTO doesn't appear to be ready for use just yet. It gives warnings similar to these: jni-wrappers.hh(143,41): warning GD616DC71: argument 1 value ‘18446744073709551615’ exceeds maximum object size 9223372036854775807 util.cc(75,8): warning GA5933D32: ‘MEM[(struct timing_point *)&partial_time].ns’ may be used uninitialized in this function [-Wmaybe-uninitialized] Which, even though well-intentioned (and, technically, correct) are irrelevant in our case and the compiler+linker diagnostics fail to understand the intention and usage of the code. Additionally, fix a few instances where type conversion is different for different platforms/targets. --- build-tools/cmake/xa_macros.cmake | 8 -------- src/monodroid/jni/android-system.cc | 6 +++--- src/monodroid/jni/embedded-assemblies-zip.cc | 3 +-- src/monodroid/jni/monodroid-glue.cc | 2 +- src/monodroid/jni/timing.hh | 18 +++++++++++++++--- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/build-tools/cmake/xa_macros.cmake b/build-tools/cmake/xa_macros.cmake index 574db50dc52..59770b38f49 100644 --- a/build-tools/cmake/xa_macros.cmake +++ b/build-tools/cmake/xa_macros.cmake @@ -51,14 +51,6 @@ macro(xa_common_prepare) fPIC ) - # Using flto seems to breaks LLDB debugging as debug symbols are not properly included - # thus disable on desktop builds where we care less about its benefits and would rather - # keep debuggability - if(NOT MINGW AND NOT WIN32 AND NOT APPLE) - # -flto leaves a lot of temporary files with mingw builds, turn the optimization off as we don't really need it there - set(XA_COMPILER_FLAGS ${XA_COMPILER_FLAGS} flto) - endif() - if(CMAKE_BUILD_TYPE STREQUAL Debug) set(XA_COMPILER_FLAGS ${XA_COMPILER_FLAGS} ggdb3 fno-omit-frame-pointer O0) else() diff --git a/src/monodroid/jni/android-system.cc b/src/monodroid/jni/android-system.cc index d2fd27ab675..70ab5a5949d 100644 --- a/src/monodroid/jni/android-system.cc +++ b/src/monodroid/jni/android-system.cc @@ -133,7 +133,7 @@ AndroidSystem::add_system_property (const char *name, const char *value) return; p->name = ((char*) p) + sizeof (struct BundledProperty); - strncpy (p->name, name, name_len); + memcpy (p->name, name, name_len); p->name [name_len] = '\0'; if (value == nullptr) { @@ -192,8 +192,8 @@ AndroidSystem::_monodroid__system_property_get (const char *name, char *sp_value // ../../../jni/android-system.cc(206,10): warning G20816D19: ‘char* strncpy(char*, const char*, size_t)’ specified bound 93 equals destination size [-Wstringop-truncation] [/home/grendel/vc/xamarin/xamarin-android-worktrees/code-quality-improvements/src/monodroid/monodroid.csproj] // strncpy (sp_value, env_value, sp_value_len); // - strncpy (sp_value, env_value, sp_value_len - 1); - sp_value[sp_value_len] = '\0'; + strncpy (sp_value, env_value, sp_value_len - 2); + sp_value[sp_value_len - 1] = '\0'; return static_cast(strlen (sp_value)); } diff --git a/src/monodroid/jni/embedded-assemblies-zip.cc b/src/monodroid/jni/embedded-assemblies-zip.cc index c3bd7fa6067..9e4222ab7a1 100644 --- a/src/monodroid/jni/embedded-assemblies-zip.cc +++ b/src/monodroid/jni/embedded-assemblies-zip.cc @@ -330,8 +330,7 @@ EmbeddedAssemblies::zip_read_field (uint8_t* buf, size_t buf_len, size_t index, return false; } - u = static_cast (buf [index + 1] << 8) | - static_cast (buf [index]); + u = static_cast((buf [index + 1] << 8) | buf [index]); return true; } diff --git a/src/monodroid/jni/monodroid-glue.cc b/src/monodroid/jni/monodroid-glue.cc index ccb638c161e..87552e105f1 100644 --- a/src/monodroid/jni/monodroid-glue.cc +++ b/src/monodroid/jni/monodroid-glue.cc @@ -916,7 +916,7 @@ MonodroidRuntime::init_android_runtime (MonoDomain *domain, JNIEnv *env, jclass init.localRefsAreIndirect = LocalRefsAreIndirect (env, runtimeClass, init.androidSdkVersion); init.isRunningOnDesktop = is_running_on_desktop ? 1 : 0; init.brokenExceptionTransitions = application_config.broken_exception_transitions ? 1 : 0; - init.packageNamingPolicy = application_config.package_naming_policy; + init.packageNamingPolicy = static_cast(application_config.package_naming_policy); // GC threshold is 90% of the max GREF count init.grefGcThreshold = static_cast(androidSystem.get_gref_gc_threshold ()); diff --git a/src/monodroid/jni/timing.hh b/src/monodroid/jni/timing.hh index 69bc871c8d1..925fb7d84a5 100644 --- a/src/monodroid/jni/timing.hh +++ b/src/monodroid/jni/timing.hh @@ -17,10 +17,16 @@ namespace xamarin::android { struct timing_point { - time_t sec; - uint64_t ns; + time_t sec = 0; + uint64_t ns = 0; void mark (); + + void reset () + { + sec = 0; + ns = 0; + } }; struct timing_period @@ -37,6 +43,12 @@ namespace xamarin::android { end.mark (); } + + void reset () + { + start.reset (); + end.reset (); + } }; struct timing_diff @@ -124,7 +136,7 @@ namespace xamarin::android std::lock_guard lock (sequence_lock); if (sequence->dynamic) { - memset (sequence, 0, sizeof(*sequence)); + sequence->period.reset (); delete sequence; return; }