Merge "Fix windows 64-bit builds"
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 35e5945..58ccd0a 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -509,14 +509,60 @@
return true;
}
-// Read from a pipe (that we take ownership of) and write what is returned to
-// GetStdHandle(nStdHandle). Return on error or when the pipe is closed.
+// Read from a pipe (that we take ownership of) and write the result to stdout/stderr. Return on
+// error or when the pipe is closed. Internally makes inheritable handles, so this should not be
+// called if subprocesses may be started concurrently.
static unsigned _redirect_pipe_thread(HANDLE h, DWORD nStdHandle) {
// Take ownership of the HANDLE and close when we're done.
unique_handle read_pipe(h);
- const HANDLE write_handle = GetStdHandle(nStdHandle);
- const char* output_name = nStdHandle == STD_OUTPUT_HANDLE ?
- "stdout" : "stderr";
+ const char* output_name = nStdHandle == STD_OUTPUT_HANDLE ? "stdout" : "stderr";
+ const int original_fd = fileno(nStdHandle == STD_OUTPUT_HANDLE ? stdout : stderr);
+ std::unique_ptr<FILE, decltype(&fclose)> stream(nullptr, fclose);
+
+ if (original_fd == -1) {
+ fprintf(stderr, "Failed to get file descriptor for %s: %s\n", output_name, strerror(errno));
+ return EXIT_FAILURE;
+ }
+
+ // If fileno() is -2, stdout/stderr is not associated with an output stream, so we should read,
+ // but don't write. Otherwise, make a FILE* identical to stdout/stderr except that it is in
+ // binary mode with no CR/LR translation since we're reading raw.
+ if (original_fd >= 0) {
+ // This internally makes a duplicate file handle that is inheritable, so callers should not
+ // call this function if subprocesses may be started concurrently.
+ const int fd = dup(original_fd);
+ if (fd == -1) {
+ fprintf(stderr, "Failed to duplicate file descriptor for %s: %s\n", output_name,
+ strerror(errno));
+ return EXIT_FAILURE;
+ }
+
+ // Note that although we call fdopen() below with a binary flag, it may not adhere to that
+ // flag, so we have to set the mode manually.
+ if (_setmode(fd, _O_BINARY) == -1) {
+ fprintf(stderr, "Failed to set binary mode for duplicate of %s: %s\n", output_name,
+ strerror(errno));
+ unix_close(fd);
+ return EXIT_FAILURE;
+ }
+
+ stream.reset(fdopen(fd, "wb"));
+ if (stream.get() == nullptr) {
+ fprintf(stderr, "Failed to open duplicate stream for %s: %s\n", output_name,
+ strerror(errno));
+ unix_close(fd);
+ return EXIT_FAILURE;
+ }
+
+ // Unbuffer the stream because it will be buffered by default and we want subprocess output
+ // to be shown immediately.
+ if (setvbuf(stream.get(), NULL, _IONBF, 0) == -1) {
+ fprintf(stderr, "Failed to unbuffer %s: %s\n", output_name, strerror(errno));
+ return EXIT_FAILURE;
+ }
+
+ // fd will be closed when stream is closed.
+ }
while (true) {
char buf[64 * 1024];
@@ -534,20 +580,13 @@
}
}
- // Don't try to write if our stdout/stderr was not setup by the
- // parent process.
- if (write_handle != NULL && write_handle != INVALID_HANDLE_VALUE) {
- DWORD bytes_written = 0;
- if (!WriteFile(write_handle, buf, bytes_read, &bytes_written,
- NULL)) {
- fprintf(stderr, "Failed to write to %s: %s\n", output_name,
- android::base::SystemErrorCodeToString(GetLastError()).c_str());
- return EXIT_FAILURE;
- }
-
+ // Don't try to write if our stdout/stderr was not setup by the parent process.
+ if (stream) {
+ // fwrite() actually calls adb_fwrite() which can write UTF-8 to the console.
+ const size_t bytes_written = fwrite(buf, 1, bytes_read, stream.get());
if (bytes_written != bytes_read) {
- fprintf(stderr, "Only wrote %lu of %lu bytes to %s\n",
- bytes_written, bytes_read, output_name);
+ fprintf(stderr, "Only wrote %zu of %lu bytes to %s\n", bytes_written, bytes_read,
+ output_name);
return EXIT_FAILURE;
}
}
@@ -596,7 +635,10 @@
return -1;
}
- // create pipes with non-inheritable read handle, inheritable write handle
+ // Create pipes with non-inheritable read handle, inheritable write handle. We need to connect
+ // the subprocess to pipes instead of just letting the subprocess inherit our existing
+ // stdout/stderr handles because a DETACHED_PROCESS cannot write to a console that it is not
+ // attached to.
unique_handle ack_read, ack_write;
if (!_create_anonymous_pipe(&ack_read, &ack_write, &sa)) {
return -1;
@@ -704,8 +746,9 @@
stdout_write.reset();
stderr_write.reset();
- // Start threads to read from subprocess stdout/stderr and write to ours
- // to make subprocess errors easier to diagnose.
+ // Start threads to read from subprocess stdout/stderr and write to ours to make subprocess
+ // errors easier to diagnose. Note that the threads internally create inheritable handles, but
+ // that is ok because we've already spawned the subprocess.
// In the past, reading from a pipe before the child process's C Runtime
// started up and called GetFileType() caused a hang: http://blogs.msdn.com/b/oldnewthing/archive/2011/12/02/10243553.aspx#10244216
diff --git a/adb/file_sync_client.cpp b/adb/file_sync_client.cpp
index 0fa5917..51fc143 100644
--- a/adb/file_sync_client.cpp
+++ b/adb/file_sync_client.cpp
@@ -753,8 +753,11 @@
if (dst_isdir) {
// If we're copying a local file to a remote directory,
// we really want to copy to remote_dir + "/" + local_filename.
- path_holder = android::base::StringPrintf(
- "%s/%s", dst_path, adb_basename(src_path).c_str());
+ path_holder = dst_path;
+ if (path_holder.back() != '/') {
+ path_holder.push_back('/');
+ }
+ path_holder += adb_basename(src_path);
dst_path = path_holder.c_str();
}
sc.SetExpectedTotalBytes(st.st_size);
diff --git a/adb/file_sync_service.cpp b/adb/file_sync_service.cpp
index ef0418e..29c6629 100644
--- a/adb/file_sync_service.cpp
+++ b/adb/file_sync_service.cpp
@@ -130,6 +130,9 @@
return WriteFdExactly(s, &msg.dent, sizeof(msg.dent));
}
+// Make sure that SendFail from adb_io.cpp isn't accidentally used in this file.
+#pragma GCC poison SendFail
+
static bool SendSyncFail(int fd, const std::string& reason) {
D("sync: failure: %s", reason.c_str());
@@ -265,7 +268,7 @@
msg.status.msglen = 0;
if (!WriteFdExactly(s, &msg.status, sizeof(msg.status))) return false;
} else {
- SendFail(s, "invalid data message: expected ID_DONE");
+ SendSyncFail(s, "invalid data message: expected ID_DONE");
return false;
}
@@ -277,7 +280,7 @@
// 'spec' is of the form "/some/path,0755". Break it up.
size_t comma = spec.find_last_of(',');
if (comma == std::string::npos) {
- SendFail(s, "missing , in ID_SEND");
+ SendSyncFail(s, "missing , in ID_SEND");
return false;
}
@@ -286,7 +289,7 @@
errno = 0;
mode_t mode = strtoul(spec.substr(comma + 1).c_str(), nullptr, 0);
if (errno != 0) {
- SendFail(s, "bad mode");
+ SendSyncFail(s, "bad mode");
return false;
}
diff --git a/adb/shell_service.cpp b/adb/shell_service.cpp
index 491bb68..d080e09 100644
--- a/adb/shell_service.cpp
+++ b/adb/shell_service.cpp
@@ -192,7 +192,7 @@
// Sets up FDs, forks a subprocess, starts the subprocess manager thread,
// and exec's the child. Returns false on failure.
- bool ForkAndExec();
+ bool ForkAndExec(std::string* _Nonnull error);
private:
// Opens the file at |pts_name|.
@@ -250,7 +250,7 @@
WaitForExit();
}
-bool Subprocess::ForkAndExec() {
+bool Subprocess::ForkAndExec(std::string* error) {
ScopedFd child_stdinout_sfd, child_stderr_sfd;
ScopedFd parent_error_sfd, child_error_sfd;
char pts_name[PATH_MAX];
@@ -265,7 +265,9 @@
// use threads, logging directly from the child might deadlock due to locks held in another
// thread during the fork.
if (!CreateSocketpair(&parent_error_sfd, &child_error_sfd)) {
- LOG(ERROR) << "failed to create pipe for subprocess error reporting";
+ *error = android::base::StringPrintf(
+ "failed to create pipe for subprocess error reporting: %s", strerror(errno));
+ return false;
}
// Construct the environment for the child before we fork.
@@ -316,18 +318,22 @@
stdinout_sfd_.Reset(fd);
} else {
if (!CreateSocketpair(&stdinout_sfd_, &child_stdinout_sfd)) {
+ *error = android::base::StringPrintf("failed to create socketpair for stdin/out: %s",
+ strerror(errno));
return false;
}
// Raw subprocess + shell protocol allows for splitting stderr.
if (protocol_ == SubprocessProtocol::kShell &&
!CreateSocketpair(&stderr_sfd_, &child_stderr_sfd)) {
+ *error = android::base::StringPrintf("failed to create socketpair for stderr: %s",
+ strerror(errno));
return false;
}
pid_ = fork();
}
if (pid_ == -1) {
- PLOG(ERROR) << "fork failed";
+ *error = android::base::StringPrintf("fork failed: %s", strerror(errno));
return false;
}
@@ -357,7 +363,8 @@
} else {
execle(_PATH_BSHELL, _PATH_BSHELL, "-c", command_.c_str(), nullptr, cenv.data());
}
- WriteFdExactly(child_error_sfd.fd(), "exec '" _PATH_BSHELL "' failed");
+ WriteFdExactly(child_error_sfd.fd(), "exec '" _PATH_BSHELL "' failed: ");
+ WriteFdExactly(child_error_sfd.fd(), strerror(errno));
child_error_sfd.Reset();
_Exit(1);
}
@@ -370,7 +377,7 @@
child_error_sfd.Reset();
std::string error_message = ReadAll(parent_error_sfd.fd());
if (!error_message.empty()) {
- LOG(ERROR) << error_message;
+ *error = error_message;
return false;
}
@@ -382,6 +389,9 @@
} else {
// Shell protocol: create another socketpair to intercept data.
if (!CreateSocketpair(&protocol_sfd_, &local_socket_sfd_)) {
+ *error = android::base::StringPrintf(
+ "failed to create socketpair to intercept data: %s", strerror(errno));
+ kill(pid_, SIGKILL);
return false;
}
D("protocol FD = %d", protocol_sfd_.fd());
@@ -389,7 +399,8 @@
input_.reset(new ShellProtocol(protocol_sfd_.fd()));
output_.reset(new ShellProtocol(protocol_sfd_.fd()));
if (!input_ || !output_) {
- LOG(ERROR) << "failed to allocate shell protocol objects";
+ *error = "failed to allocate shell protocol objects";
+ kill(pid_, SIGKILL);
return false;
}
@@ -400,7 +411,9 @@
for (int fd : {stdinout_sfd_.fd(), stderr_sfd_.fd()}) {
if (fd >= 0) {
if (!set_file_block_mode(fd, false)) {
- LOG(ERROR) << "failed to set non-blocking mode for fd " << fd;
+ *error = android::base::StringPrintf(
+ "failed to set non-blocking mode for fd %d", fd);
+ kill(pid_, SIGKILL);
return false;
}
}
@@ -408,7 +421,9 @@
}
if (!adb_thread_create(ThreadHandler, this)) {
- PLOG(ERROR) << "failed to create subprocess thread";
+ *error =
+ android::base::StringPrintf("failed to create subprocess thread: %s", strerror(errno));
+ kill(pid_, SIGKILL);
return false;
}
@@ -710,6 +725,37 @@
} // namespace
+// Create a pipe containing the error.
+static int ReportError(SubprocessProtocol protocol, const std::string& message) {
+ int pipefd[2];
+ if (pipe(pipefd) != 0) {
+ LOG(ERROR) << "failed to create pipe to report error";
+ return -1;
+ }
+
+ std::string buf = android::base::StringPrintf("error: %s\n", message.c_str());
+ if (protocol == SubprocessProtocol::kShell) {
+ ShellProtocol::Id id = ShellProtocol::kIdStderr;
+ uint32_t length = buf.length();
+ WriteFdExactly(pipefd[1], &id, sizeof(id));
+ WriteFdExactly(pipefd[1], &length, sizeof(length));
+ }
+
+ WriteFdExactly(pipefd[1], buf.data(), buf.length());
+
+ if (protocol == SubprocessProtocol::kShell) {
+ ShellProtocol::Id id = ShellProtocol::kIdExit;
+ uint32_t length = 1;
+ char exit_code = 126;
+ WriteFdExactly(pipefd[1], &id, sizeof(id));
+ WriteFdExactly(pipefd[1], &length, sizeof(length));
+ WriteFdExactly(pipefd[1], &exit_code, sizeof(exit_code));
+ }
+
+ adb_close(pipefd[1]);
+ return pipefd[0];
+}
+
int StartSubprocess(const char* name, const char* terminal_type,
SubprocessType type, SubprocessProtocol protocol) {
D("starting %s subprocess (protocol=%s, TERM=%s): '%s'",
@@ -720,13 +766,14 @@
Subprocess* subprocess = new Subprocess(name, terminal_type, type, protocol);
if (!subprocess) {
LOG(ERROR) << "failed to allocate new subprocess";
- return -1;
+ return ReportError(protocol, "failed to allocate new subprocess");
}
- if (!subprocess->ForkAndExec()) {
- LOG(ERROR) << "failed to start subprocess";
+ std::string error;
+ if (!subprocess->ForkAndExec(&error)) {
+ LOG(ERROR) << "failed to start subprocess: " << error;
delete subprocess;
- return -1;
+ return ReportError(protocol, error);
}
D("subprocess creation successful: local_socket_fd=%d, pid=%d",
diff --git a/base/Android.mk b/base/Android.mk
index d20a81f..1693e74 100644
--- a/base/Android.mk
+++ b/base/Android.mk
@@ -67,7 +67,7 @@
LOCAL_C_INCLUDES := $(LOCAL_PATH)/include
LOCAL_CPPFLAGS := $(libbase_cppflags) $(libbase_linux_cppflags)
LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include
-LOCAL_STATIC_LIBRARIES := libcutils
+LOCAL_STATIC_LIBRARIES := liblog
LOCAL_MULTILIB := both
include $(BUILD_STATIC_LIBRARY)
@@ -77,7 +77,6 @@
LOCAL_WHOLE_STATIC_LIBRARIES := libbase
LOCAL_SHARED_LIBRARIES := liblog
LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include
-LOCAL_SHARED_LIBRARIES := libcutils
LOCAL_MULTILIB := both
include $(BUILD_SHARED_LIBRARY)
@@ -94,7 +93,7 @@
LOCAL_CPPFLAGS_darwin := $(libbase_darwin_cppflags)
LOCAL_CPPFLAGS_linux := $(libbase_linux_cppflags)
LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include
-LOCAL_STATIC_LIBRARIES := libcutils
+LOCAL_STATIC_LIBRARIES := liblog
LOCAL_MULTILIB := both
LOCAL_MODULE_HOST_OS := darwin linux windows
include $(BUILD_HOST_STATIC_LIBRARY)
@@ -104,7 +103,6 @@
LOCAL_WHOLE_STATIC_LIBRARIES := libbase
LOCAL_SHARED_LIBRARIES := liblog
LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH)/include
-LOCAL_STATIC_LIBRARIES := libcutils
LOCAL_MULTILIB := both
LOCAL_MODULE_HOST_OS := darwin linux windows
include $(BUILD_HOST_SHARED_LIBRARY)
diff --git a/crash_reporter/Android.mk b/crash_reporter/Android.mk
index fa564b2..ce9dc73 100644
--- a/crash_reporter/Android.mk
+++ b/crash_reporter/Android.mk
@@ -136,7 +136,7 @@
LOCAL_MODULE := crash_reporter_tests
LOCAL_CPP_EXTENSION := $(crash_reporter_cpp_extension)
ifdef BRILLO
-LOCAL_MODULE_TAGS := debug
+LOCAL_MODULE_TAGS := eng
endif
LOCAL_SHARED_LIBRARIES := libchrome \
libbrillo \
diff --git a/debuggerd/tombstone.cpp b/debuggerd/tombstone.cpp
index dda6677..7cf2ffc 100644
--- a/debuggerd/tombstone.cpp
+++ b/debuggerd/tombstone.cpp
@@ -134,8 +134,15 @@
switch (code) {
case SEGV_MAPERR: return "SEGV_MAPERR";
case SEGV_ACCERR: return "SEGV_ACCERR";
+#if defined(SEGV_BNDERR)
+ case SEGV_BNDERR: return "SEGV_BNDERR";
+#endif
}
+#if defined(SEGV_BNDERR)
+ static_assert(NSIGSEGV == SEGV_BNDERR, "missing SEGV_* si_code");
+#else
static_assert(NSIGSEGV == SEGV_ACCERR, "missing SEGV_* si_code");
+#endif
break;
case SIGTRAP:
switch (code) {
diff --git a/fastboot/socket.cpp b/fastboot/socket.cpp
index d49f47f..14ecd93 100644
--- a/fastboot/socket.cpp
+++ b/fastboot/socket.cpp
@@ -48,18 +48,6 @@
return ret;
}
-bool Socket::SetReceiveTimeout(int timeout_ms) {
- if (timeout_ms != receive_timeout_ms_) {
- if (socket_set_receive_timeout(sock_, timeout_ms) == 0) {
- receive_timeout_ms_ = timeout_ms;
- return true;
- }
- return false;
- }
-
- return true;
-}
-
ssize_t Socket::ReceiveAll(void* data, size_t length, int timeout_ms) {
size_t total = 0;
@@ -82,6 +70,40 @@
return socket_get_local_port(sock_);
}
+// According to Windows setsockopt() documentation, if a Windows socket times out during send() or
+// recv() the state is indeterminate and should not be used. Our UDP protocol relies on being able
+// to re-send after a timeout, so we must use select() rather than SO_RCVTIMEO.
+// See https://msdn.microsoft.com/en-us/library/windows/desktop/ms740476(v=vs.85).aspx.
+bool Socket::WaitForRecv(int timeout_ms) {
+ receive_timed_out_ = false;
+
+ // In our usage |timeout_ms| <= 0 means block forever, so just return true immediately and let
+ // the subsequent recv() do the blocking.
+ if (timeout_ms <= 0) {
+ return true;
+ }
+
+ // select() doesn't always check this case and will block for |timeout_ms| if we let it.
+ if (sock_ == INVALID_SOCKET) {
+ return false;
+ }
+
+ fd_set read_set;
+ FD_ZERO(&read_set);
+ FD_SET(sock_, &read_set);
+
+ timeval timeout;
+ timeout.tv_sec = timeout_ms / 1000;
+ timeout.tv_usec = (timeout_ms % 1000) * 1000;
+
+ int result = TEMP_FAILURE_RETRY(select(sock_ + 1, &read_set, nullptr, nullptr, &timeout));
+
+ if (result == 0) {
+ receive_timed_out_ = true;
+ }
+ return result == 1;
+}
+
// Implements the Socket interface for UDP.
class UdpSocket : public Socket {
public:
@@ -127,7 +149,7 @@
}
ssize_t UdpSocket::Receive(void* data, size_t length, int timeout_ms) {
- if (!SetReceiveTimeout(timeout_ms)) {
+ if (!WaitForRecv(timeout_ms)) {
return -1;
}
@@ -206,7 +228,7 @@
}
ssize_t TcpSocket::Receive(void* data, size_t length, int timeout_ms) {
- if (!SetReceiveTimeout(timeout_ms)) {
+ if (!WaitForRecv(timeout_ms)) {
return -1;
}
diff --git a/fastboot/socket.h b/fastboot/socket.h
index c0bd7c9..de543db 100644
--- a/fastboot/socket.h
+++ b/fastboot/socket.h
@@ -81,13 +81,17 @@
virtual bool Send(std::vector<cutils_socket_buffer_t> buffers) = 0;
// Waits up to |timeout_ms| to receive up to |length| bytes of data. |timout_ms| of 0 will
- // block forever. Returns the number of bytes received or -1 on error/timeout. On timeout
- // errno will be set to EAGAIN or EWOULDBLOCK.
+ // block forever. Returns the number of bytes received or -1 on error/timeout; see
+ // ReceiveTimedOut() to distinguish between the two.
virtual ssize_t Receive(void* data, size_t length, int timeout_ms) = 0;
// Calls Receive() until exactly |length| bytes have been received or an error occurs.
virtual ssize_t ReceiveAll(void* data, size_t length, int timeout_ms);
+ // Returns true if the last Receive() call timed out normally and can be retried; fatal errors
+ // or successful reads will return false.
+ bool ReceiveTimedOut() { return receive_timed_out_; }
+
// Closes the socket. Returns 0 on success, -1 on error.
virtual int Close();
@@ -102,10 +106,13 @@
// Protected constructor to force factory function use.
Socket(cutils_socket_t sock);
- // Update the socket receive timeout if necessary.
- bool SetReceiveTimeout(int timeout_ms);
+ // Blocks up to |timeout_ms| until a read is possible on |sock_|, and sets |receive_timed_out_|
+ // as appropriate to help distinguish between normal timeouts and fatal errors. Returns true if
+ // a subsequent recv() on |sock_| will complete without blocking or if |timeout_ms| <= 0.
+ bool WaitForRecv(int timeout_ms);
cutils_socket_t sock_ = INVALID_SOCKET;
+ bool receive_timed_out_ = false;
// Non-class functions we want to override during tests to verify functionality. Implementation
// should call this rather than using socket_send_buffers() directly.
@@ -113,8 +120,6 @@
socket_send_buffers_function_ = &socket_send_buffers;
private:
- int receive_timeout_ms_ = 0;
-
FRIEND_TEST(SocketTest, TestTcpSendBuffers);
FRIEND_TEST(SocketTest, TestUdpSendBuffers);
diff --git a/fastboot/socket_mock.cpp b/fastboot/socket_mock.cpp
index c962f30..2531b53 100644
--- a/fastboot/socket_mock.cpp
+++ b/fastboot/socket_mock.cpp
@@ -55,7 +55,7 @@
return false;
}
- bool return_value = events_.front().return_value;
+ bool return_value = events_.front().status;
events_.pop();
return return_value;
}
@@ -76,21 +76,28 @@
return -1;
}
- if (events_.front().type != EventType::kReceive) {
+ const Event& event = events_.front();
+ if (event.type != EventType::kReceive) {
ADD_FAILURE() << "Receive() was called out-of-order";
return -1;
}
- if (events_.front().return_value > static_cast<ssize_t>(length)) {
- ADD_FAILURE() << "Receive(): not enough bytes (" << length << ") for "
- << events_.front().message;
+ const std::string& message = event.message;
+ if (message.length() > length) {
+ ADD_FAILURE() << "Receive(): not enough bytes (" << length << ") for " << message;
return -1;
}
- ssize_t return_value = events_.front().return_value;
- if (return_value > 0) {
- memcpy(data, events_.front().message.data(), return_value);
+ receive_timed_out_ = event.status;
+ ssize_t return_value = message.length();
+
+ // Empty message indicates failure.
+ if (message.empty()) {
+ return_value = -1;
+ } else {
+ memcpy(data, message.data(), message.length());
}
+
events_.pop();
return return_value;
}
@@ -124,18 +131,21 @@
}
void SocketMock::AddReceive(std::string message) {
- ssize_t return_value = message.length();
- events_.push(Event(EventType::kReceive, std::move(message), return_value, nullptr));
+ events_.push(Event(EventType::kReceive, std::move(message), false, nullptr));
+}
+
+void SocketMock::AddReceiveTimeout() {
+ events_.push(Event(EventType::kReceive, "", true, nullptr));
}
void SocketMock::AddReceiveFailure() {
- events_.push(Event(EventType::kReceive, "", -1, nullptr));
+ events_.push(Event(EventType::kReceive, "", false, nullptr));
}
void SocketMock::AddAccept(std::unique_ptr<Socket> sock) {
- events_.push(Event(EventType::kAccept, "", 0, std::move(sock)));
+ events_.push(Event(EventType::kAccept, "", false, std::move(sock)));
}
-SocketMock::Event::Event(EventType _type, std::string _message, ssize_t _return_value,
+SocketMock::Event::Event(EventType _type, std::string _message, ssize_t _status,
std::unique_ptr<Socket> _sock)
- : type(_type), message(_message), return_value(_return_value), sock(std::move(_sock)) {}
+ : type(_type), message(_message), status(_status), sock(std::move(_sock)) {}
diff --git a/fastboot/socket_mock.h b/fastboot/socket_mock.h
index 41fe06d..eacd6bb 100644
--- a/fastboot/socket_mock.h
+++ b/fastboot/socket_mock.h
@@ -71,7 +71,10 @@
// Adds data to provide for Receive().
void AddReceive(std::string message);
- // Adds a Receive() failure.
+ // Adds a Receive() timeout after which ReceiveTimedOut() will return true.
+ void AddReceiveTimeout();
+
+ // Adds a Receive() failure after which ReceiveTimedOut() will return false.
void AddReceiveFailure();
// Adds a Socket to return from Accept().
@@ -81,12 +84,12 @@
enum class EventType { kSend, kReceive, kAccept };
struct Event {
- Event(EventType _type, std::string _message, ssize_t _return_value,
+ Event(EventType _type, std::string _message, ssize_t _status,
std::unique_ptr<Socket> _sock);
EventType type;
std::string message;
- ssize_t return_value;
+ bool status; // Return value for Send() or timeout status for Receive().
std::unique_ptr<Socket> sock;
};
diff --git a/fastboot/socket_test.cpp b/fastboot/socket_test.cpp
index cc71075..affbdfd 100644
--- a/fastboot/socket_test.cpp
+++ b/fastboot/socket_test.cpp
@@ -28,7 +28,8 @@
#include <gtest/gtest-spi.h>
#include <gtest/gtest.h>
-enum { kTestTimeoutMs = 3000 };
+static constexpr int kShortTimeoutMs = 10;
+static constexpr int kTestTimeoutMs = 3000;
// Creates connected sockets |server| and |client|. Returns true on success.
bool MakeConnectedSockets(Socket::Protocol protocol, std::unique_ptr<Socket>* server,
@@ -87,6 +88,50 @@
}
}
+TEST(SocketTest, TestReceiveTimeout) {
+ std::unique_ptr<Socket> server, client;
+ char buffer[16];
+
+ for (Socket::Protocol protocol : {Socket::Protocol::kUdp, Socket::Protocol::kTcp}) {
+ ASSERT_TRUE(MakeConnectedSockets(protocol, &server, &client));
+
+ EXPECT_EQ(-1, server->Receive(buffer, sizeof(buffer), kShortTimeoutMs));
+ EXPECT_TRUE(server->ReceiveTimedOut());
+
+ EXPECT_EQ(-1, client->Receive(buffer, sizeof(buffer), kShortTimeoutMs));
+ EXPECT_TRUE(client->ReceiveTimedOut());
+ }
+
+ // UDP will wait for timeout if the other side closes.
+ ASSERT_TRUE(MakeConnectedSockets(Socket::Protocol::kUdp, &server, &client));
+ EXPECT_EQ(0, server->Close());
+ EXPECT_EQ(-1, client->Receive(buffer, sizeof(buffer), kShortTimeoutMs));
+ EXPECT_TRUE(client->ReceiveTimedOut());
+}
+
+TEST(SocketTest, TestReceiveFailure) {
+ std::unique_ptr<Socket> server, client;
+ char buffer[16];
+
+ for (Socket::Protocol protocol : {Socket::Protocol::kUdp, Socket::Protocol::kTcp}) {
+ ASSERT_TRUE(MakeConnectedSockets(protocol, &server, &client));
+
+ EXPECT_EQ(0, server->Close());
+ EXPECT_EQ(-1, server->Receive(buffer, sizeof(buffer), kTestTimeoutMs));
+ EXPECT_FALSE(server->ReceiveTimedOut());
+
+ EXPECT_EQ(0, client->Close());
+ EXPECT_EQ(-1, client->Receive(buffer, sizeof(buffer), kTestTimeoutMs));
+ EXPECT_FALSE(client->ReceiveTimedOut());
+ }
+
+ // TCP knows right away when the other side closes and returns 0 to indicate EOF.
+ ASSERT_TRUE(MakeConnectedSockets(Socket::Protocol::kTcp, &server, &client));
+ EXPECT_EQ(0, server->Close());
+ EXPECT_EQ(0, client->Receive(buffer, sizeof(buffer), kTestTimeoutMs));
+ EXPECT_FALSE(client->ReceiveTimedOut());
+}
+
// Tests sending and receiving large packets.
TEST(SocketTest, TestLargePackets) {
std::string message(1024, '\0');
@@ -290,6 +335,11 @@
mock->AddReceiveFailure();
EXPECT_FALSE(ReceiveString(mock, "foo"));
+ EXPECT_FALSE(mock->ReceiveTimedOut());
+
+ mock->AddReceiveTimeout();
+ EXPECT_FALSE(ReceiveString(mock, "foo"));
+ EXPECT_TRUE(mock->ReceiveTimedOut());
mock->AddReceive("foo");
mock->AddReceiveFailure();
diff --git a/healthd/BatteryMonitor.cpp b/healthd/BatteryMonitor.cpp
index cdfe9c5..8f98f17 100644
--- a/healthd/BatteryMonitor.cpp
+++ b/healthd/BatteryMonitor.cpp
@@ -134,6 +134,7 @@
{ "Mains", ANDROID_POWER_SUPPLY_TYPE_AC },
{ "USB", ANDROID_POWER_SUPPLY_TYPE_USB },
{ "USB_DCP", ANDROID_POWER_SUPPLY_TYPE_AC },
+ { "USB_HVDCP", ANDROID_POWER_SUPPLY_TYPE_AC },
{ "USB_CDP", ANDROID_POWER_SUPPLY_TYPE_AC },
{ "USB_ACA", ANDROID_POWER_SUPPLY_TYPE_AC },
{ "USB_C", ANDROID_POWER_SUPPLY_TYPE_AC },
@@ -147,8 +148,10 @@
return ANDROID_POWER_SUPPLY_TYPE_UNKNOWN;
ret = (BatteryMonitor::PowerSupplyType)mapSysfsString(buf, supplyTypeMap);
- if (ret < 0)
+ if (ret < 0) {
+ KLOG_WARNING(LOG_TAG, "Unknown power supply type '%s'\n", buf);
ret = ANDROID_POWER_SUPPLY_TYPE_UNKNOWN;
+ }
return ret;
}
diff --git a/init/service.cpp b/init/service.cpp
index 0ddc484..bdecc32 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -416,7 +416,7 @@
}
}
- std::string pid_str = StringPrintf("%d", pid);
+ std::string pid_str = StringPrintf("%d", getpid());
for (const auto& file : writepid_files_) {
if (!WriteStringToFile(pid_str, file)) {
ERROR("couldn't write %s to %s: %s\n",
diff --git a/libbacktrace/UnwindMap.cpp b/libbacktrace/UnwindMap.cpp
index 879fea5..34d79f9 100644
--- a/libbacktrace/UnwindMap.cpp
+++ b/libbacktrace/UnwindMap.cpp
@@ -33,14 +33,18 @@
// of maps using the same map cursor.
//-------------------------------------------------------------------------
UnwindMap::UnwindMap(pid_t pid) : BacktraceMap(pid) {
+ unw_map_cursor_clear(&map_cursor_);
}
-UnwindMap::~UnwindMap() {
+UnwindMapRemote::UnwindMapRemote(pid_t pid) : UnwindMap(pid) {
+}
+
+UnwindMapRemote::~UnwindMapRemote() {
unw_map_cursor_destroy(&map_cursor_);
unw_map_cursor_clear(&map_cursor_);
}
-bool UnwindMap::GenerateMap() {
+bool UnwindMapRemote::GenerateMap() {
// Use the map_cursor information to construct the BacktraceMap data
// rather than reparsing /proc/self/maps.
unw_map_cursor_reset(&map_cursor_);
@@ -63,7 +67,7 @@
return true;
}
-bool UnwindMap::Build() {
+bool UnwindMapRemote::Build() {
return (unw_map_cursor_create(&map_cursor_, pid_) == 0) && GenerateMap();
}
@@ -84,6 +88,7 @@
for (int i = 0; i < 3; i++) {
maps_.clear();
+ // Save the map data retrieved so we can tell if it changes.
unw_map_local_cursor_get(&map_cursor_);
unw_map_t unw_map;
@@ -142,7 +147,7 @@
} else if (pid == getpid()) {
map = new UnwindMapLocal();
} else {
- map = new UnwindMap(pid);
+ map = new UnwindMapRemote(pid);
}
if (!map->Build()) {
delete map;
diff --git a/libbacktrace/UnwindMap.h b/libbacktrace/UnwindMap.h
index e292016..111401f 100644
--- a/libbacktrace/UnwindMap.h
+++ b/libbacktrace/UnwindMap.h
@@ -29,29 +29,35 @@
class UnwindMap : public BacktraceMap {
public:
UnwindMap(pid_t pid);
- virtual ~UnwindMap();
-
- virtual bool Build();
unw_map_cursor_t* GetMapCursor() { return &map_cursor_; }
protected:
- virtual bool GenerateMap();
-
unw_map_cursor_t map_cursor_;
};
+class UnwindMapRemote : public UnwindMap {
+public:
+ UnwindMapRemote(pid_t pid);
+ virtual ~UnwindMapRemote();
+
+ bool Build() override;
+
+private:
+ bool GenerateMap();
+};
+
class UnwindMapLocal : public UnwindMap {
public:
UnwindMapLocal();
virtual ~UnwindMapLocal();
- virtual bool Build();
+ bool Build() override;
- virtual void FillIn(uintptr_t addr, backtrace_map_t* map);
+ void FillIn(uintptr_t addr, backtrace_map_t* map) override;
-protected:
- virtual bool GenerateMap();
+private:
+ bool GenerateMap();
bool map_created_;
};
diff --git a/libcutils/ashmem-dev.c b/libcutils/ashmem-dev.c
index d6a48c9..4a07d66 100644
--- a/libcutils/ashmem-dev.c
+++ b/libcutils/ashmem-dev.c
@@ -118,15 +118,16 @@
}
if (rdev) {
- ALOGE("illegal fd=%d mode=0%o rdev=%d:%d expected 0%o %d:%d",
+ LOG_ALWAYS_FATAL("illegal fd=%d mode=0%o rdev=%d:%d expected 0%o %d:%d",
fd, st.st_mode, major(st.st_rdev), minor(st.st_rdev),
S_IFCHR | S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IRGRP,
major(rdev), minor(rdev));
} else {
- ALOGE("illegal fd=%d mode=0%o rdev=%d:%d expected 0%o",
+ LOG_ALWAYS_FATAL("illegal fd=%d mode=0%o rdev=%d:%d expected 0%o",
fd, st.st_mode, major(st.st_rdev), minor(st.st_rdev),
S_IFCHR | S_IRUSR | S_IWUSR | S_IRGRP | S_IWGRP | S_IROTH | S_IRGRP);
}
+ /* NOTREACHED */
errno = ENOTTY;
return -1;
diff --git a/libcutils/str_parms.c b/libcutils/str_parms.c
index 4f23d09..8dafded 100644
--- a/libcutils/str_parms.c
+++ b/libcutils/str_parms.c
@@ -31,6 +31,20 @@
#define UNUSED __attribute__((unused))
+/* When an object is allocated but not freed in a function,
+ * because its ownership is released to other object like a hashmap,
+ * call RELEASE_OWNERSHIP to tell the clang analyzer and avoid
+ * false warnings about potential memory leak.
+ * For now, a "temporary" assignment to global variables
+ * is enough to confuse the clang static analyzer.
+ */
+#ifdef __clang_analyzer__
+static void *released_pointer;
+#define RELEASE_OWNERSHIP(x) { released_pointer = x; released_pointer = 0; }
+#else
+#define RELEASE_OWNERSHIP(x)
+#endif
+
struct str_parms {
Hashmap *map;
};
@@ -170,9 +184,12 @@
/* if we replaced a value, free it */
old_val = hashmapPut(str_parms->map, key, value);
+ RELEASE_OWNERSHIP(value);
if (old_val) {
free(old_val);
free(key);
+ } else {
+ RELEASE_OWNERSHIP(key);
}
items++;
@@ -222,10 +239,13 @@
goto clean_up;
}
// For new keys, hashmap takes ownership of tmp_key and tmp_val.
+ RELEASE_OWNERSHIP(tmp_key);
+ RELEASE_OWNERSHIP(tmp_val);
tmp_key = tmp_val = NULL;
} else {
// For existing keys, hashmap takes ownership of tmp_val.
// (It also gives up ownership of old_val.)
+ RELEASE_OWNERSHIP(tmp_val);
tmp_val = NULL;
}
diff --git a/logcat/Android.mk b/logcat/Android.mk
index c4a9550..b828a9f 100644
--- a/logcat/Android.mk
+++ b/logcat/Android.mk
@@ -11,8 +11,6 @@
LOCAL_CFLAGS := -Werror
-LOCAL_INIT_RC := logcatd.rc
-
include $(BUILD_EXECUTABLE)
include $(CLEAR_VARS)
@@ -20,6 +18,7 @@
LOCAL_MODULE := logpersist.start
LOCAL_MODULE_TAGS := debug
LOCAL_MODULE_CLASS := EXECUTABLES
+LOCAL_INIT_RC := logcatd.rc
LOCAL_MODULE_PATH := $(bin_dir)
LOCAL_SRC_FILES := logpersist
ALL_TOOLS := logpersist.start logpersist.stop logpersist.cat
diff --git a/metricsd/Android.mk b/metricsd/Android.mk
index e140c62..bb262b4 100644
--- a/metricsd/Android.mk
+++ b/metricsd/Android.mk
@@ -200,7 +200,7 @@
LOCAL_SRC_FILES := $(metricsd_tests_sources) $(metricsd_common)
LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock metricsd_protos metricsd_binder_proxy
ifdef BRILLO
-LOCAL_MODULE_TAGS := debug
+LOCAL_MODULE_TAGS := eng
endif
include $(BUILD_NATIVE_TEST)
@@ -218,7 +218,7 @@
LOCAL_STATIC_LIBRARIES := libBionicGtestMain libgmock metricsd_binder_proxy \
$(metrics_collector_static_libraries)
ifdef BRILLO
-LOCAL_MODULE_TAGS := debug
+LOCAL_MODULE_TAGS := eng
endif
include $(BUILD_NATIVE_TEST)
diff --git a/metricsd/uploader/metricsd_service_runner.cc b/metricsd/uploader/metricsd_service_runner.cc
index 5a759d3..4361cac 100644
--- a/metricsd/uploader/metricsd_service_runner.cc
+++ b/metricsd/uploader/metricsd_service_runner.cc
@@ -20,6 +20,7 @@
#include <binder/IServiceManager.h>
#include <brillo/binder_watcher.h>
+#include <brillo/message_loops/base_message_loop.h>
#include <utils/Errors.h>
#include "uploader/bn_metricsd_impl.h"
@@ -40,15 +41,17 @@
CHECK(status == android::OK) << "Metricsd service registration failed";
message_loop_for_io_.reset(new base::MessageLoopForIO);
+ message_loop_.reset(new brillo::BaseMessageLoop(message_loop_for_io_.get()));
- brillo::BinderWatcher watcher;
+ brillo::BinderWatcher watcher(message_loop_.get());
CHECK(watcher.Init()) << "failed to initialize the binder file descriptor "
<< "watcher";
- message_loop_for_io_->Run();
+ message_loop_->Run();
// Delete the message loop here as it needs to be deconstructed in the thread
// it is attached to.
+ message_loop_.reset();
message_loop_for_io_.reset();
}
diff --git a/metricsd/uploader/metricsd_service_runner.h b/metricsd/uploader/metricsd_service_runner.h
index 1715de0..f5dad21 100644
--- a/metricsd/uploader/metricsd_service_runner.h
+++ b/metricsd/uploader/metricsd_service_runner.h
@@ -21,6 +21,7 @@
#include <thread>
#include <base/message_loop/message_loop.h>
+#include <brillo/message_loops/message_loop.h>
#include "uploader/crash_counters.h"
@@ -39,6 +40,7 @@
void Run();
std::unique_ptr<base::MessageLoopForIO> message_loop_for_io_;
+ std::unique_ptr<brillo::MessageLoop> message_loop_;
std::unique_ptr<std::thread> thread_;
std::shared_ptr<CrashCounters> counters_;
diff --git a/rootdir/Android.mk b/rootdir/Android.mk
index 895a25d..d90f988 100644
--- a/rootdir/Android.mk
+++ b/rootdir/Android.mk
@@ -12,6 +12,18 @@
include $(BUILD_PREBUILT)
#######################################
+# init-debug.rc
+include $(CLEAR_VARS)
+
+LOCAL_MODULE := init-debug.rc
+LOCAL_SRC_FILES := $(LOCAL_MODULE)
+LOCAL_MODULE_CLASS := ETC
+LOCAL_MODULE_TAGS := debug
+LOCAL_MODULE_PATH := $(TARGET_OUT_ETC)/init
+
+include $(BUILD_PREBUILT)
+
+#######################################
# asan.options
ifneq ($(filter address,$(SANITIZE_TARGET)),)
include $(CLEAR_VARS)
diff --git a/rootdir/init-debug.rc b/rootdir/init-debug.rc
new file mode 100644
index 0000000..435d4cb
--- /dev/null
+++ b/rootdir/init-debug.rc
@@ -0,0 +1,8 @@
+on property:persist.mmc.max_read_speed=*
+ write /sys/block/mmcblk0/max_read_speed ${persist.mmc.max_read_speed}
+
+on property:persist.mmc.max_write_speed=*
+ write /sys/block/mmcblk0/max_write_speed ${persist.mmc.max_write_speed}
+
+on property:persist.mmc.cache_size=*
+ write /sys/block/mmcblk0/cache_size ${persist.mmc.cache_size}