Fix to make the framework asan compatible.
Free any used memory before exiting.
Replace all android::base functions so that it's not necessary to
include libbase as a whole static library. This fixes odr violationgs
that asan finds.
Replace the PLOG and LOG macros with FATAL_PLOG and FATAL_LOG macros
that don't use android::base.
Remove the definition of GetInitialArgs and change the code to check
if this weak function is defined before calling it.
Free the old listener to avoid leaking it away.
Test: Ran unit tests.
Test: Ran art/test/testrunner/run_build_test_target.py art-gtest-asan
Change-Id: Idb9fd31eaa53798fb5e4f72b64b9c170bd4d7996
diff --git a/Android.bp b/Android.bp
index e339a4f..651797b 100644
--- a/Android.bp
+++ b/Android.bp
@@ -27,11 +27,7 @@
"Test.cpp",
],
- // NOTE: libbase is re-exported by including them below.
- // When Soong supports transitive static dependency includes, this
- // library can be removed.
whole_static_libs: [
- "libbase",
"libgtest",
],
diff --git a/Isolate.cpp b/Isolate.cpp
index 889b183..6ed4cb4 100644
--- a/Isolate.cpp
+++ b/Isolate.cpp
@@ -20,21 +20,21 @@
#include <poll.h>
#include <signal.h>
#include <stdio.h>
+#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <atomic>
+#include <memory>
#include <string>
#include <tuple>
#include <vector>
-#include <android-base/logging.h>
-#include <android-base/strings.h>
-#include <android-base/unique_fd.h>
#include <gtest/gtest.h>
#include "Color.h"
#include "Isolate.h"
+#include "Log.h"
#include "NanoTime.h"
#include "Test.h"
@@ -50,22 +50,22 @@
static void RegisterSignalHandler() {
auto ret = signal(SIGINT, SignalHandler);
if (ret == SIG_ERR) {
- PLOG(FATAL) << "Setting up SIGINT handler failed";
+ FATAL_PLOG("Setting up SIGINT handler failed");
}
ret = signal(SIGQUIT, SignalHandler);
if (ret == SIG_ERR) {
- PLOG(FATAL) << "Setting up SIGQUIT handler failed";
+ FATAL_PLOG("Setting up SIGQUIT handler failed");
}
}
static void UnregisterSignalHandler() {
auto ret = signal(SIGINT, SIG_DFL);
if (ret == SIG_ERR) {
- PLOG(FATAL) << "Disabling SIGINT handler failed";
+ FATAL_PLOG("Disabling SIGINT handler failed");
}
ret = signal(SIGQUIT, SIG_DFL);
if (ret == SIG_ERR) {
- PLOG(FATAL) << "Disabling SIGQUIT handler failed";
+ FATAL_PLOG("Disabling SIGQUIT handler failed");
}
}
@@ -81,6 +81,12 @@
return string;
}
+inline static bool StartsWithDisabled(const std::string& str) {
+ static constexpr char kDisabledStr[] = "DISABLED_";
+ static constexpr size_t kDisabledStrLen = sizeof(kDisabledStr) - 1;
+ return str.compare(0, kDisabledStrLen, kDisabledStr) == 0;
+}
+
void Isolate::EnumerateTests() {
// Only apply --gtest_filter if present. This is the only option that changes
// what tests are listed.
@@ -89,13 +95,14 @@
command += " --gtest_filter=" + options_.filter();
}
command += " --gtest_list_tests";
-#if defined(__APPLE__)
- FILE* fp = popen(command.c_str(), "r");
-#else
+#if defined(__BIONIC__)
+ // Only bionic is guaranteed to support the 'e' option.
FILE* fp = popen(command.c_str(), "re");
+#else
+ FILE* fp = popen(command.c_str(), "r");
#endif
if (fp == nullptr) {
- PLOG(FATAL) << "Unexpected failure from popen";
+ FATAL_PLOG("Unexpected failure from popen");
}
size_t total_shards = options_.total_shards();
@@ -122,7 +129,7 @@
suite_name.resize(suite_name.size() - 1);
}
- if (!options_.allow_disabled_tests() && android::base::StartsWith(suite_name, "DISABLED_")) {
+ if (!options_.allow_disabled_tests() && StartsWithDisabled(suite_name)) {
// This whole set of tests have been disabled, skip them all.
skip_until_next_suite = true;
} else {
@@ -139,7 +146,7 @@
if (test_name.back() == '\n') {
test_name.resize(test_name.size() - 1);
}
- if (options_.allow_disabled_tests() || !android::base::StartsWith(test_name, "DISABLED_")) {
+ if (options_.allow_disabled_tests() || !StartsWithDisabled(test_name)) {
if (!sharded || --test_count == 0) {
tests_.push_back(std::make_tuple(suite_name, test_name));
total_tests_++;
@@ -167,7 +174,7 @@
}
free(buffer);
if (pclose(fp) == -1) {
- PLOG(FATAL) << "Unexpected failure from pclose";
+ FATAL_PLOG("Unexpected failure from pclose");
}
}
@@ -178,7 +185,7 @@
// Add the filter argument.
std::vector<char*> args(child_args_);
std::string filter("--gtest_filter=" + GetTestName(test));
- args.push_back(strdup(filter.c_str()));
+ args.push_back(filter.data());
int argc = args.size();
// Add the null terminator.
@@ -187,22 +194,45 @@
return RUN_ALL_TESTS();
}
+static bool Pipe(int* read_fd, int* write_fd) {
+ int pipefd[2];
+
+#if defined(__linux__)
+ if (pipe2(pipefd, O_CLOEXEC) != 0) {
+ return false;
+ }
+#else // defined(__APPLE__)
+ if (pipe(pipefd) != 0) {
+ return false;
+ }
+ if (fcntl(pipefd[0], F_SETFD, FD_CLOEXEC) != 0 || fcntl(pipefd[1], F_SETFD, FD_CLOEXEC)) {
+ close(pipefd[0]);
+ close(pipefd[1]);
+ return false;
+ }
+#endif
+
+ *read_fd = pipefd[0];
+ *write_fd = pipefd[1];
+ return true;
+}
+
void Isolate::LaunchTests() {
while (!running_indices_.empty() && cur_test_index_ < tests_.size()) {
- android::base::unique_fd read_fd, write_fd;
+ int read_fd, write_fd;
if (!Pipe(&read_fd, &write_fd)) {
- PLOG(FATAL) << "Unexpected failure from pipe";
+ FATAL_PLOG("Unexpected failure from pipe");
}
- if (fcntl(read_fd.get(), F_SETFL, O_NONBLOCK) == -1) {
- PLOG(FATAL) << "Unexpected failure from fcntl";
+ if (fcntl(read_fd, F_SETFL, O_NONBLOCK) == -1) {
+ FATAL_PLOG("Unexpected failure from fcntl");
}
pid_t pid = fork();
if (pid == -1) {
- PLOG(FATAL) << "Unexpected failure from fork";
+ FATAL_PLOG("Unexpected failure from fork");
}
if (pid == 0) {
- read_fd.reset();
+ close(read_fd);
close(STDOUT_FILENO);
close(STDERR_FILENO);
if (dup2(write_fd, STDOUT_FILENO) == -1) {
@@ -211,13 +241,14 @@
if (dup2(write_fd, STDERR_FILENO) == -1) {
exit(1);
}
+ close(write_fd);
UnregisterSignalHandler();
exit(ChildProcessFn(tests_[cur_test_index_]));
}
size_t run_index = running_indices_.back();
running_indices_.pop_back();
- Test* test = new Test(tests_[cur_test_index_], cur_test_index_, run_index, read_fd.release());
+ Test* test = new Test(tests_[cur_test_index_], cur_test_index_, run_index, read_fd);
running_by_pid_.emplace(pid, test);
running_[run_index] = test;
running_by_test_index_[cur_test_index_] = test;
@@ -226,6 +257,7 @@
pollfd->fd = test->fd();
pollfd->events = POLLIN;
cur_test_index_++;
+ close(write_fd);
}
}
@@ -254,9 +286,12 @@
int status;
pid_t pid;
while ((pid = TEMP_FAILURE_RETRY(waitpid(-1, &status, WNOHANG))) > 0) {
+ if (pid == -1) {
+ FATAL_PLOG("Unexpected failure from waitpid");
+ }
auto entry = running_by_pid_.find(pid);
if (entry == running_by_pid_.end()) {
- LOG(FATAL) << "Pid " << pid << " was not spawned by the isolation framework.";
+ FATAL_LOG("Found process not spawned by the isolation framework");
}
std::unique_ptr<Test>& test_ptr = entry->second;
@@ -326,7 +361,7 @@
total_skipped_tests_++;
break;
case TEST_NONE:
- LOG(FATAL) << "Test result is TEST_NONE, this should not be possible.";
+ FATAL_LOG("Test result is TEST_NONE, this should not be possible");
}
finished_tests++;
size_t test_index = test->test_index();
@@ -348,7 +383,7 @@
// The only valid error case is if ECHILD is returned because there are
// no more processes left running.
if (pid == -1 && errno != ECHILD) {
- PLOG(FATAL) << "Unexpected failure from waitpid";
+ FATAL_PLOG("Unexpected failure from waitpid");
}
return finished_tests;
}
@@ -640,7 +675,7 @@
const tm* time_struct = localtime(&start_time);
if (time_struct == nullptr) {
- PLOG(FATAL) << "Unexpected failure from localtime";
+ FATAL_PLOG("Unexpected failure from localtime");
}
char timestamp[40];
snprintf(timestamp, sizeof(timestamp), "%4d-%02d-%02dT%02d:%02d:%02d",
@@ -739,7 +774,7 @@
EnumerateTests();
// Stop default result printer to avoid environment setup/teardown information for each test.
- ::testing::UnitTest::GetInstance()->listeners().Release(
+ delete ::testing::UnitTest::GetInstance()->listeners().Release(
::testing::UnitTest::GetInstance()->listeners().default_result_printer());
::testing::UnitTest::GetInstance()->listeners().Append(new TestResultPrinter);
RegisterSignalHandler();
diff --git a/IsolateMain.cpp b/IsolateMain.cpp
index 30b871b..bffdee9 100644
--- a/IsolateMain.cpp
+++ b/IsolateMain.cpp
@@ -22,7 +22,6 @@
#include <vector>
-#include <android-base/file.h>
#include <gtest/gtest.h>
#include <gtest_extras/IsolateMain.h>
@@ -91,9 +90,7 @@
} // namespace android
// Tests that override this weak function can add default arguments.
-extern "C" bool __attribute__((weak)) GetInitialArgs(const char***, size_t*) {
- return false;
-}
+extern "C" bool __attribute__((weak)) GetInitialArgs(const char***, size_t*);
int IsolateMain(int argc, char** argv, char**) {
std::vector<const char*> args{argv[0]};
@@ -126,7 +123,7 @@
const char** start_args;
size_t num_args;
- if (GetInitialArgs(&start_args, &num_args)) {
+ if (GetInitialArgs != nullptr && GetInitialArgs(&start_args, &num_args)) {
std::vector<const char*> initial_args;
for (size_t i = 0; i < num_args; i++) {
initial_args.push_back(start_args[i]);
@@ -149,5 +146,9 @@
::testing::GTEST_FLAG(print_time) = options.print_time();
android::gtest_extras::Isolate isolate(options, child_args);
- return isolate.Run();
+ int return_val = isolate.Run();
+ for (auto child_arg : child_args) {
+ free(child_arg);
+ }
+ return return_val;
}
diff --git a/Log.h b/Log.h
new file mode 100644
index 0000000..d424c51
--- /dev/null
+++ b/Log.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2020 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.
+ */
+
+#pragma once
+
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include <android/log.h>
+
+#define GTEST_EXTRAS_TAG "gtest_extras"
+
+#define FATAL_PLOG(msg) \
+ __android_log_print(ANDROID_LOG_FATAL, GTEST_EXTRAS_TAG, "%s:%d] " msg ": %s", __FILE__, \
+ __LINE__, strerror(errno)); \
+ abort();
+
+#define FATAL_LOG(msg) \
+ __android_log_print(ANDROID_LOG_FATAL, GTEST_EXTRAS_TAG, "%s:%d] " msg, __FILE__, __LINE__); \
+ abort();
diff --git a/Options.cpp b/Options.cpp
index bcc4b06..96842a1 100644
--- a/Options.cpp
+++ b/Options.cpp
@@ -15,20 +15,22 @@
*/
#include <errno.h>
+#include <fcntl.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
+#include <sys/stat.h>
+#include <sys/types.h>
#include <unistd.h>
#include <algorithm>
#include <cctype>
+#include <charconv>
+#include <regex>
#include <string>
#include <unordered_map>
#include <vector>
-#include <android-base/file.h>
-#include <android-base/parseint.h>
-#include <android-base/strings.h>
#include <gtest/gtest.h>
#include "Options.h"
@@ -90,20 +92,16 @@
}
template <typename IntType>
-static bool GetNumeric(const char* arg, const char* value, IntType* numeric_value, bool from_env) {
- bool result = false;
- if constexpr (std::is_unsigned<IntType>::value) {
- result = android::base::ParseUint<IntType>(value, numeric_value);
- } else {
- result = android::base::ParseInt<IntType>(value, numeric_value);
- }
- if (!result) {
- if (errno == ERANGE) {
- PrintError(arg, std::string("value overflows (") + value + ")", from_env);
- } else {
- PrintError(arg, std::string("value is not formatted as a numeric value (") + value + ")",
- from_env);
- }
+static bool GetNumeric(const std::string& arg, const std::string& value, IntType* numeric_value,
+ bool from_env) {
+ auto result = std::from_chars(value.c_str(), value.c_str() + value.size(), *numeric_value, 10);
+ if (result.ec == std::errc::result_out_of_range) {
+ PrintError(arg, std::string("value overflows (") + value + ")", from_env);
+ return false;
+ } else if (result.ec == std::errc::invalid_argument || result.ptr == nullptr ||
+ *result.ptr != '\0') {
+ PrintError(arg, std::string("value is not formatted as a numeric value (") + value + ")",
+ from_env);
return false;
}
return true;
@@ -118,7 +116,7 @@
bool Options::SetNumeric(const std::string& arg, const std::string& value, bool from_env) {
uint64_t* numeric = &numerics_.find(arg)->second;
- if (!GetNumeric<uint64_t>(arg.c_str(), value.c_str(), numeric, from_env)) {
+ if (!GetNumeric<uint64_t>(arg, value, numeric, from_env)) {
return false;
}
if (*numeric == 0) {
@@ -134,7 +132,7 @@
return false;
}
uint64_t* numeric = &numerics_.find(arg)->second;
- if (!GetNumeric<uint64_t>(arg.c_str(), value.c_str(), numeric, from_env)) {
+ if (!GetNumeric<uint64_t>(arg, value, numeric, from_env)) {
return false;
}
return true;
@@ -146,7 +144,7 @@
}
bool Options::SetIterations(const std::string& arg, const std::string& value, bool from_env) {
- if (!GetNumeric<int>(arg.c_str(), value.c_str(), &num_iterations_, from_env)) {
+ if (!GetNumeric<int>(arg, value, &num_iterations_, from_env)) {
return false;
}
return true;
@@ -214,13 +212,29 @@
return true;
}
+static bool ReadFileToString(const std::string& file, std::string* contents) {
+ int fd = TEMP_FAILURE_RETRY(open(file.c_str(), O_RDONLY | O_CLOEXEC));
+ if (fd == -1) {
+ return false;
+ }
+ char buf[4096];
+ ssize_t bytes_read;
+ while ((bytes_read = TEMP_FAILURE_RETRY(read(fd, &buf, sizeof(buf)))) > 0) {
+ contents->append(buf, bytes_read);
+ }
+ close(fd);
+ return true;
+}
+
bool Options::ProcessFlagfile(const std::string& file, std::vector<char*>* child_args) {
std::string contents;
- if (!android::base::ReadFileToString(file, &contents)) {
+ if (!ReadFileToString(file, &contents)) {
printf("Unable to read data from file %s\n", file.c_str());
return false;
}
+ std::regex flag_regex("^\\s*(\\S.*\\S)\\s*$");
+ std::regex empty_line_regex("^\\s*$");
size_t idx = 0;
while (idx < contents.size()) {
size_t newline_idx = contents.find('\n', idx);
@@ -229,8 +243,10 @@
}
std::string line(&contents[idx], newline_idx - idx);
idx = newline_idx + 1;
- line = android::base::Trim(line);
- if (line.empty()) {
+ std::smatch match;
+ if (std::regex_match(line, match, flag_regex)) {
+ line = match[1];
+ } else if (std::regex_match(line, match, empty_line_regex)) {
// Skip lines with only whitespace.
continue;
}
diff --git a/Test.cpp b/Test.cpp
index 327facd..23d139c 100644
--- a/Test.cpp
+++ b/Test.cpp
@@ -23,10 +23,11 @@
#include <tuple>
#include <vector>
-#include <android-base/logging.h>
+#include <android/log.h>
#include <gtest/gtest.h>
#include "Color.h"
+#include "Log.h"
#include "NanoTime.h"
#include "Test.h"
@@ -49,7 +50,10 @@
}
void Test::CloseFd() {
- fd_.reset();
+ if (fd_ != -1) {
+ close(fd_);
+ fd_ = -1;
+ }
}
void Test::Print() {
@@ -85,7 +89,7 @@
// Reading would block. Since this is not an error keep going.
return true;
}
- PLOG(FATAL) << "Unexpected failure from read";
+ FATAL_PLOG("Unexpected failure from read");
return false;
}
diff --git a/Test.h b/Test.h
index b4a1182..dd54dc6 100644
--- a/Test.h
+++ b/Test.h
@@ -20,8 +20,6 @@
#include <string>
#include <tuple>
-#include <android-base/unique_fd.h>
-
namespace android {
namespace gtest_extras {
@@ -38,6 +36,7 @@
class Test {
public:
Test(std::tuple<std::string, std::string>& test, size_t test_index, size_t run_index, int fd);
+ ~Test() { CloseFd(); }
void Print();
@@ -87,7 +86,7 @@
std::string name_;
size_t test_index_; // Index into test list.
size_t run_index_; // Index into running list.
- android::base::unique_fd fd_;
+ int fd_ = -1;
uint64_t start_ns_;
uint64_t end_ns_ = 0;
