ART: Fix or disable some tidy warnings.

Add a strlcpy shim for the host, so we can use strlcpy instead of
strcpy everywhere.

Fixed warnings include unused-decls, (some) unreachable code, use
after std::move, string char append, leaks, (some) excessive padding.

Disable some warnings we cannot or do not want to avoid.

Bug: 32619234
Test: m
Test: m test-art-host
Change-Id: Ie191985eebb160d94b988b41735d4f0a1fa1b54e

17 files changed, 77 insertions, 26 deletions

diff --git a/build/Android.bp b/build/Android.bp
index c54f436b35..836497c569 100644
--- a/build/Android.bp
+++ b/build/Android.bp
@@ -145,6 +145,15 @@ art_global_defaults {
 
     tidy_checks: [
         "-google-default-arguments",
+        // We have local stores that are only used for debug checks.
+        "-clang-analyzer-deadcode.DeadStores",
+        // We are OK with some static globals and that they can, in theory, throw.
+        "-cert-err58-cpp",
+        // We have lots of C-style variadic functions, and are OK with them. JNI ensures
+        // that working around this warning would be extra-painful.
+        "-cert-dcl50-cpp",
+        // No exceptions.
+        "-misc-noexcept-move-constructor",
     ],
 }
 
diff --git a/compiler/dex/verification_results.cc b/compiler/dex/verification_results.cc
index b87cb61ed6..04ceca0513 100644
--- a/compiler/dex/verification_results.cc
+++ b/compiler/dex/verification_results.cc
@@ -110,12 +110,12 @@ void VerificationResults::CreateVerifiedMethodFor(MethodReference ref) {
   // This method should only be called for classes verified at compile time,
   // which have no verifier error, nor has methods that we know will throw
   // at runtime.
-  atomic_verified_methods_.Insert(
-      ref,
-      /*expected*/ nullptr,
-      new VerifiedMethod(/* encountered_error_types */ 0, /* has_runtime_throw */ false));
-  // We don't check the result of `Insert` as we could insert twice for the same
-  // MethodReference in the presence of duplicate methods.
+  std::unique_ptr<VerifiedMethod> verified_method = std::make_unique<VerifiedMethod>(
+      /* encountered_error_types */ 0, /* has_runtime_throw */ false);
+  if (atomic_verified_methods_.Insert(ref, /*expected*/ nullptr, verified_method.get()) ==
+          AtomicMap::InsertResult::kInsertResultSuccess) {
+    verified_method.release();
+  }
 }
 
 void VerificationResults::AddRejectedClass(ClassReference ref) {
diff --git a/compiler/optimizing/induction_var_analysis.cc b/compiler/optimizing/induction_var_analysis.cc
index 88473f02e5..84b20f65e3 100644
--- a/compiler/optimizing/induction_var_analysis.cc
+++ b/compiler/optimizing/induction_var_analysis.cc
@@ -695,8 +695,8 @@ HInductionVarAnalysis::InductionInfo* HInductionVarAnalysis::SolveOp(HLoopInform
                                  /*fetch*/ nullptr,
                                  type_);
         default:
-          CHECK(false) << op;
-          break;
+          LOG(FATAL) << op;
+          UNREACHABLE();
       }
     }
   }
diff --git a/compiler/optimizing/loop_optimization.cc b/compiler/optimizing/loop_optimization.cc
index c3aa976d49..68ee272d25 100644
--- a/compiler/optimizing/loop_optimization.cc
+++ b/compiler/optimizing/loop_optimization.cc
@@ -499,6 +499,7 @@ void HLoopOptimization::OptimizeInnerLoop(LoopNode* node) {
       body = it.Current();
     }
   }
+  CHECK(body != nullptr);
   // Ensure there is only a single exit point.
   if (header->GetSuccessors().size() != 2) {
     return;
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 5efcb9d97b..be6f09bad3 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -524,13 +524,14 @@ class WatchDog {
     CHECK_WATCH_DOG_PTHREAD_CALL(pthread_mutex_unlock, (&mutex_), reason);
   }
 
-  const int64_t timeout_in_milliseconds_;
-  bool shutting_down_;
   // TODO: Switch to Mutex when we can guarantee it won't prevent shutdown in error cases.
   pthread_mutex_t mutex_;
   pthread_cond_t cond_;
   pthread_attr_t attr_;
   pthread_t pthread_;
+
+  const int64_t timeout_in_milliseconds_;
+  bool shutting_down_;
 };
 
 class Dex2Oat FINAL {
diff --git a/dexdump/dexdump.cc b/dexdump/dexdump.cc
index 5656ddd59c..1541d7b39e 100644
--- a/dexdump/dexdump.cc
+++ b/dexdump/dexdump.cc
@@ -1747,9 +1747,8 @@ static void dumpCallSite(const DexFile* pDexFile, u4 idx) {
       case EncodedArrayValueIterator::ValueType::kArray:
       case EncodedArrayValueIterator::ValueType::kAnnotation:
         // Unreachable based on current EncodedArrayValueIterator::Next().
-        UNIMPLEMENTED(FATAL) << " type " << type;
+        UNIMPLEMENTED(FATAL) << " type " << it.GetValueType();
         UNREACHABLE();
-        break;
       case EncodedArrayValueIterator::ValueType::kNull:
         type = "Null";
         value = "null";
diff --git a/runtime/base/strlcpy.h b/runtime/base/strlcpy.h
new file mode 100644
index 0000000000..3d0ec35f9f
--- /dev/null
+++ b/runtime/base/strlcpy.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2017 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#ifndef ART_RUNTIME_BASE_STRLCPY_H_
+#define ART_RUNTIME_BASE_STRLCPY_H_
+
+#include <cstdio>
+#include <cstring>
+
+// Expose a simple implementation of strlcpy when we're not compiling against bionic. This is to
+// make static analyzers happy not using strcpy.
+//
+// Bionic exposes this function, but the host glibc does not. Remove this shim when we compile
+// against bionic on the host, also.
+
+#ifndef __BIONIC__
+
+static inline size_t strlcpy(char* dst, const char* src, size_t size) {
+  // Extra-lazy implementation: this is only a host shim, and we don't have to call this often.
+  return snprintf(dst, size, "%s", src);
+}
+
+#endif
+
+#endif  // ART_RUNTIME_BASE_STRLCPY_H_
diff --git a/runtime/exec_utils.cc b/runtime/exec_utils.cc
index 9efb1a353c..db1baa76f9 100644
--- a/runtime/exec_utils.cc
+++ b/runtime/exec_utils.cc
@@ -28,7 +28,6 @@
 
 namespace art {
 
-using android::base::StringAppendF;
 using android::base::StringPrintf;
 
 int ExecAndReturnCode(std::vector<std::string>& arg_vector, std::string* error_msg) {
diff --git a/runtime/gc/heap.cc b/runtime/gc/heap.cc
index 1af3b57830..d944ce4904 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -90,8 +90,6 @@ namespace art {
 
 namespace gc {
 
-using android::base::StringPrintf;
-
 static constexpr size_t kCollectorTransitionStressIterations = 0;
 static constexpr size_t kCollectorTransitionStressWait = 10 * 1000;  // Microseconds
 // Minimum amount of remaining bytes before a concurrent GC is triggered.
diff --git a/runtime/jdwp/jdwp_event.cc b/runtime/jdwp/jdwp_event.cc
index 96249f9b58..4ab3d69e35 100644
--- a/runtime/jdwp/jdwp_event.cc
+++ b/runtime/jdwp/jdwp_event.cc
@@ -500,8 +500,8 @@ static bool ModsMatch(JdwpEvent* pEvent, const ModBasket& basket)
       }
       break;
     case MK_CONDITIONAL:
-      CHECK(false);  // should not be getting these
-      break;
+      LOG(FATAL) << "Unexpected MK_CONDITIONAL";  // should not be getting these
+      UNREACHABLE();
     case MK_THREAD_ONLY:
       if (!Dbg::MatchThread(pMod->threadOnly.threadId, basket.thread)) {
         return false;
diff --git a/runtime/jdwp/jdwp_expand_buf.cc b/runtime/jdwp/jdwp_expand_buf.cc
index 961dd369c8..f0b8c918dc 100644
--- a/runtime/jdwp/jdwp_expand_buf.cc
+++ b/runtime/jdwp/jdwp_expand_buf.cc
@@ -152,7 +152,9 @@ void expandBufAdd8BE(ExpandBuf* pBuf, uint64_t val) {
 
 static void SetUtf8String(uint8_t* buf, const char* str, size_t strLen) {
   Set4BE(buf, strLen);
-  memcpy(buf + sizeof(uint32_t), str, strLen);
+  if (str != nullptr) {
+    memcpy(buf + sizeof(uint32_t), str, strLen);
+  }
 }
 
 /*
diff --git a/runtime/mem_map.cc b/runtime/mem_map.cc
index 12793e433e..654922a4c5 100644
--- a/runtime/mem_map.cc
+++ b/runtime/mem_map.cc
@@ -187,7 +187,7 @@ static bool CheckNonOverlapping(uintptr_t begin,
     *error_msg = StringPrintf("Failed to build process map");
     return false;
   }
-  ScopedBacktraceMapIteratorLock(map.get());
+  ScopedBacktraceMapIteratorLock lock(map.get());
   for (BacktraceMap::const_iterator it = map->begin(); it != map->end(); ++it) {
     if ((begin >= it->start && begin < it->end)      // start of new within old
         || (end > it->start && end < it->end)        // end of new within old
diff --git a/runtime/oat.cc b/runtime/oat.cc
index 267a975425..21e20e9b74 100644
--- a/runtime/oat.cc
+++ b/runtime/oat.cc
@@ -23,6 +23,7 @@
 
 #include "arch/instruction_set_features.h"
 #include "base/bit_utils.h"
+#include "base/strlcpy.h"
 
 namespace art {
 
@@ -520,9 +521,9 @@ void OatHeader::Flatten(const SafeMap<std::string, std::string>* key_value_store
     SafeMap<std::string, std::string>::const_iterator it = key_value_store->begin();
     SafeMap<std::string, std::string>::const_iterator end = key_value_store->end();
     for ( ; it != end; ++it) {
-      strcpy(data_ptr, it->first.c_str());
+      strlcpy(data_ptr, it->first.c_str(), it->first.length() + 1);
       data_ptr += it->first.length() + 1;
-      strcpy(data_ptr, it->second.c_str());
+      strlcpy(data_ptr, it->second.c_str(), it->second.length() + 1);
       data_ptr += it->second.length() + 1;
     }
   }
diff --git a/runtime/openjdkjvmti/art_jvmti.h b/runtime/openjdkjvmti/art_jvmti.h
index 2a2aa4c199..4b83b7c873 100644
--- a/runtime/openjdkjvmti/art_jvmti.h
+++ b/runtime/openjdkjvmti/art_jvmti.h
@@ -41,6 +41,7 @@
 #include "base/casts.h"
 #include "base/logging.h"
 #include "base/macros.h"
+#include "base/strlcpy.h"
 #include "events.h"
 #include "java_vm_ext.h"
 #include "jni_env_ext.h"
@@ -187,7 +188,7 @@ static inline JvmtiUniquePtr<char[]> CopyString(jvmtiEnv* env, const char* src,
   size_t len = strlen(src) + 1;
   JvmtiUniquePtr<char[]> ret = AllocJvmtiUniquePtr<char[]>(env, len, error);
   if (ret != nullptr) {
-    strcpy(ret.get(), src);
+    strlcpy(ret.get(), src, len);
   }
   return ret;
 }
diff --git a/runtime/openjdkjvmti/ti_class.cc b/runtime/openjdkjvmti/ti_class.cc
index 0aa93dfb57..ed54cd13c3 100644
--- a/runtime/openjdkjvmti/ti_class.cc
+++ b/runtime/openjdkjvmti/ti_class.cc
@@ -103,7 +103,8 @@ static std::unique_ptr<const art::DexFile> MakeSingleDexFile(art::Thread* self,
     return nullptr;
   }
   uint32_t checksum = reinterpret_cast<const art::DexFile::Header*>(map->Begin())->checksum_;
-  std::unique_ptr<const art::DexFile> dex_file(art::DexFile::Open(map->GetName(),
+  std::string map_name = map->GetName();
+  std::unique_ptr<const art::DexFile> dex_file(art::DexFile::Open(map_name,
                                                                   checksum,
                                                                   std::move(map),
                                                                   /*verify*/true,
diff --git a/runtime/ti/agent.cc b/runtime/ti/agent.cc
index 86f5282664..82b9af33d5 100644
--- a/runtime/ti/agent.cc
+++ b/runtime/ti/agent.cc
@@ -18,6 +18,7 @@
 
 #include "android-base/stringprintf.h"
 
+#include "base/strlcpy.h"
 #include "java_vm_ext.h"
 #include "runtime.h"
 
@@ -57,7 +58,7 @@ Agent::LoadError Agent::DoLoadHelper(bool attaching,
   }
   // Need to let the function fiddle with the array.
   std::unique_ptr<char[]> copied_args(new char[args_.size() + 1]);
-  strcpy(copied_args.get(), args_.c_str());
+  strlcpy(copied_args.get(), args_.c_str(), args_.size() + 1);
   // TODO Need to do some checks that we are at a good spot etc.
   *call_res = callback(Runtime::Current()->GetJavaVM(),
                        copied_args.get(),
diff --git a/runtime/utils.cc b/runtime/utils.cc
index 20a53b7c69..c4b044110c 100644
--- a/runtime/utils.cc
+++ b/runtime/utils.cc
@@ -303,7 +303,7 @@ std::string PrintableChar(uint16_t ch) {
   if (NeedsEscaping(ch)) {
     StringAppendF(&result, "\\u%04x", ch);
   } else {
-    result += ch;
+    result += static_cast<std::string::value_type>(ch);
   }
   result += '\'';
   return result;
@@ -330,7 +330,7 @@ std::string PrintableString(const char* utf) {
       if (NeedsEscaping(leading)) {
         StringAppendF(&result, "\\u%04x", leading);
       } else {
-        result += leading;
+        result += static_cast<std::string::value_type>(leading);
       }
 
       const uint32_t trailing = GetTrailingUtf16Char(ch);