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
diff --git a/build/Android.bp b/build/Android.bp
index c54f436..836497c 100644
--- a/build/Android.bp
+++ b/build/Android.bp
@@ -145,6 +145,15 @@
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 b87cb61..04ceca0 100644
--- a/compiler/dex/verification_results.cc
+++ b/compiler/dex/verification_results.cc
@@ -110,12 +110,12 @@
// 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 88473f02..84b20f6 100644
--- a/compiler/optimizing/induction_var_analysis.cc
+++ b/compiler/optimizing/induction_var_analysis.cc
@@ -695,8 +695,8 @@
/*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 c3aa976..68ee272 100644
--- a/compiler/optimizing/loop_optimization.cc
+++ b/compiler/optimizing/loop_optimization.cc
@@ -499,6 +499,7 @@
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 5efcb9d..be6f09b 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -524,13 +524,14 @@
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 5656ddd..1541d7b 100644
--- a/dexdump/dexdump.cc
+++ b/dexdump/dexdump.cc
@@ -1747,9 +1747,8 @@
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 0000000..3d0ec35
--- /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 9efb1a3..db1baa7 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 1af3b57..d944ce4 100644
--- a/runtime/gc/heap.cc
+++ b/runtime/gc/heap.cc
@@ -90,8 +90,6 @@
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 96249f9..4ab3d69 100644
--- a/runtime/jdwp/jdwp_event.cc
+++ b/runtime/jdwp/jdwp_event.cc
@@ -500,8 +500,8 @@
}
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 961dd36..f0b8c91 100644
--- a/runtime/jdwp/jdwp_expand_buf.cc
+++ b/runtime/jdwp/jdwp_expand_buf.cc
@@ -152,7 +152,9 @@
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 12793e4..654922a 100644
--- a/runtime/mem_map.cc
+++ b/runtime/mem_map.cc
@@ -187,7 +187,7 @@
*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 267a975..21e20e9 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 @@
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 2a2aa4c..4b83b7c 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 @@
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 0aa93df..ed54cd1 100644
--- a/runtime/openjdkjvmti/ti_class.cc
+++ b/runtime/openjdkjvmti/ti_class.cc
@@ -103,7 +103,8 @@
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 86f5282..82b9af3 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 @@
}
// 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 20a53b7..c4b0441 100644
--- a/runtime/utils.cc
+++ b/runtime/utils.cc
@@ -303,7 +303,7 @@
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 @@
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);