Replace all _LOG error calls with ALOGE.
The debuggerd code sometimes calls _LOG(..., logtype::ERROR, ...)
and sometimes ALOGE(). Standardize on ALOGE since the _LOG message
will wind up in the tombstone in weird places, but using ALOGE
will wind up in the logcat portion of the tombstone.
Bug: 21467089
Change-Id: Ie893f5e91d45b48ef3f5864c3a714e60ac848fb3
diff --git a/debuggerd/Android.mk b/debuggerd/Android.mk
index 6cfb541..3fca709 100644
--- a/debuggerd/Android.mk
+++ b/debuggerd/Android.mk
@@ -77,12 +77,12 @@
debuggerd_test_src_files := \
utility.cpp \
- test/dump_maps_test.cpp \
test/dump_memory_test.cpp \
test/elf_fake.cpp \
test/log_fake.cpp \
test/property_fake.cpp \
test/ptrace_fake.cpp \
+ test/tombstone_test.cpp \
test/selinux_fake.cpp \
debuggerd_shared_libraries := \
diff --git a/debuggerd/arm/machine.cpp b/debuggerd/arm/machine.cpp
index b7d6997..78c2306 100644
--- a/debuggerd/arm/machine.cpp
+++ b/debuggerd/arm/machine.cpp
@@ -15,12 +15,15 @@
* limitations under the License.
*/
+#define LOG_TAG "DEBUG"
+
#include <errno.h>
#include <stdint.h>
#include <string.h>
#include <sys/ptrace.h>
#include <backtrace/Backtrace.h>
+#include <log/log.h>
#include "machine.h"
#include "utility.h"
@@ -28,7 +31,7 @@
void dump_memory_and_code(log_t* log, Backtrace* backtrace) {
pt_regs regs;
if (ptrace(PTRACE_GETREGS, backtrace->Tid(), 0, ®s)) {
- _LOG(log, logtype::ERROR, "cannot get registers: %s\n", strerror(errno));
+ ALOGE("cannot get registers: %s\n", strerror(errno));
return;
}
@@ -48,7 +51,7 @@
void dump_registers(log_t* log, pid_t tid) {
pt_regs r;
if (ptrace(PTRACE_GETREGS, tid, 0, &r)) {
- _LOG(log, logtype::ERROR, "cannot get registers: %s\n", strerror(errno));
+ ALOGE("cannot get registers: %s\n", strerror(errno));
return;
}
@@ -68,7 +71,7 @@
user_vfp vfp_regs;
if (ptrace(PTRACE_GETVFPREGS, tid, 0, &vfp_regs)) {
- _LOG(log, logtype::ERROR, "cannot get FP registers: %s\n", strerror(errno));
+ ALOGE("cannot get FP registers: %s\n", strerror(errno));
return;
}
diff --git a/debuggerd/arm64/machine.cpp b/debuggerd/arm64/machine.cpp
index 2e097da..e7bf79a 100644
--- a/debuggerd/arm64/machine.cpp
+++ b/debuggerd/arm64/machine.cpp
@@ -15,6 +15,8 @@
* limitations under the License.
*/
+#define LOG_TAG "DEBUG"
+
#include <elf.h>
#include <errno.h>
#include <stdint.h>
@@ -23,6 +25,7 @@
#include <sys/uio.h>
#include <backtrace/Backtrace.h>
+#include <log/log.h>
#include "machine.h"
#include "utility.h"
@@ -34,8 +37,7 @@
io.iov_len = sizeof(regs);
if (ptrace(PTRACE_GETREGSET, backtrace->Tid(), reinterpret_cast<void*>(NT_PRSTATUS), &io) == -1) {
- _LOG(log, logtype::ERROR, "%s: ptrace failed to get registers: %s",
- __func__, strerror(errno));
+ ALOGE("ptrace failed to get registers: %s", strerror(errno));
return;
}
@@ -57,7 +59,7 @@
io.iov_len = sizeof(r);
if (ptrace(PTRACE_GETREGSET, tid, (void*) NT_PRSTATUS, (void*) &io) == -1) {
- _LOG(log, logtype::ERROR, "ptrace error: %s\n", strerror(errno));
+ ALOGE("ptrace error: %s\n", strerror(errno));
return;
}
@@ -81,7 +83,7 @@
io.iov_len = sizeof(f);
if (ptrace(PTRACE_GETREGSET, tid, (void*) NT_PRFPREG, (void*) &io) == -1) {
- _LOG(log, logtype::ERROR, "ptrace error: %s\n", strerror(errno));
+ ALOGE("ptrace error: %s\n", strerror(errno));
return;
}
diff --git a/debuggerd/backtrace.cpp b/debuggerd/backtrace.cpp
index b8084c5..b46f8f4 100644
--- a/debuggerd/backtrace.cpp
+++ b/debuggerd/backtrace.cpp
@@ -105,7 +105,7 @@
}
if (!attached && ptrace(PTRACE_DETACH, tid, 0, 0) != 0) {
- _LOG(log, logtype::ERROR, "ptrace detach from %d failed: %s\n", tid, strerror(errno));
+ ALOGE("ptrace detach from %d failed: %s\n", tid, strerror(errno));
*detach_failed = true;
}
}
diff --git a/debuggerd/mips/machine.cpp b/debuggerd/mips/machine.cpp
index f7b8a86..cbf272a 100644
--- a/debuggerd/mips/machine.cpp
+++ b/debuggerd/mips/machine.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define LOG_TAG "DEBUG"
+
#include <errno.h>
#include <inttypes.h>
#include <stdint.h>
@@ -21,6 +23,7 @@
#include <sys/ptrace.h>
#include <backtrace/Backtrace.h>
+#include <log/log.h>
#include "machine.h"
#include "utility.h"
@@ -32,7 +35,7 @@
void dump_memory_and_code(log_t* log, Backtrace* backtrace) {
pt_regs r;
if (ptrace(PTRACE_GETREGS, backtrace->Tid(), 0, &r)) {
- _LOG(log, logtype::ERROR, "cannot get registers: %s\n", strerror(errno));
+ ALOGE("cannot get registers: %s\n", strerror(errno));
return;
}
@@ -61,7 +64,7 @@
void dump_registers(log_t* log, pid_t tid) {
pt_regs r;
if(ptrace(PTRACE_GETREGS, tid, 0, &r)) {
- _LOG(log, logtype::ERROR, "cannot get registers: %s\n", strerror(errno));
+ ALOGE("cannot get registers: %s\n", strerror(errno));
return;
}
diff --git a/debuggerd/mips64/machine.cpp b/debuggerd/mips64/machine.cpp
index 293dcf6..0a8d532 100644
--- a/debuggerd/mips64/machine.cpp
+++ b/debuggerd/mips64/machine.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#define LOG_TAG "DEBUG"
+
#include <errno.h>
#include <inttypes.h>
#include <stdint.h>
@@ -21,6 +23,7 @@
#include <sys/ptrace.h>
#include <backtrace/Backtrace.h>
+#include <log/log.h>
#include "machine.h"
#include "utility.h"
@@ -32,7 +35,7 @@
void dump_memory_and_code(log_t* log, Backtrace* backtrace) {
pt_regs r;
if (ptrace(PTRACE_GETREGS, backtrace->Tid(), 0, &r)) {
- _LOG(log, logtype::ERROR, "cannot get registers: %s\n", strerror(errno));
+ ALOGE("cannot get registers: %s\n", strerror(errno));
return;
}
@@ -61,7 +64,7 @@
void dump_registers(log_t* log, pid_t tid) {
pt_regs r;
if(ptrace(PTRACE_GETREGS, tid, 0, &r)) {
- _LOG(log, logtype::ERROR, "cannot get registers: %s\n", strerror(errno));
+ ALOGE("cannot get registers: %s\n", strerror(errno));
return;
}
diff --git a/debuggerd/test/dump_memory_test.cpp b/debuggerd/test/dump_memory_test.cpp
index fcb0108..537dfc2 100644
--- a/debuggerd/test/dump_memory_test.cpp
+++ b/debuggerd/test/dump_memory_test.cpp
@@ -288,9 +288,9 @@
ASSERT_STREQ(g_expected_partial_dump, tombstone_contents.c_str());
#if defined(__LP64__)
- ASSERT_STREQ("DEBUG Bytes read 102, is not a multiple of 8\n", getFakeLogPrint().c_str());
+ ASSERT_STREQ("6 DEBUG Bytes read 102, is not a multiple of 8\n", getFakeLogPrint().c_str());
#else
- ASSERT_STREQ("DEBUG Bytes read 102, is not a multiple of 4\n", getFakeLogPrint().c_str());
+ ASSERT_STREQ("6 DEBUG Bytes read 102, is not a multiple of 4\n", getFakeLogPrint().c_str());
#endif
// Verify that the log buf is empty, and no error messages.
@@ -313,12 +313,12 @@
ASSERT_STREQ(g_expected_partial_dump, tombstone_contents.c_str());
#if defined(__LP64__)
- ASSERT_STREQ("DEBUG Bytes read 45, is not a multiple of 8\n"
- "DEBUG Bytes after second read 106, is not a multiple of 8\n",
+ ASSERT_STREQ("6 DEBUG Bytes read 45, is not a multiple of 8\n"
+ "6 DEBUG Bytes after second read 106, is not a multiple of 8\n",
getFakeLogPrint().c_str());
#else
- ASSERT_STREQ("DEBUG Bytes read 45, is not a multiple of 4\n"
- "DEBUG Bytes after second read 106, is not a multiple of 4\n",
+ ASSERT_STREQ("6 DEBUG Bytes read 45, is not a multiple of 4\n"
+ "6 DEBUG Bytes after second read 106, is not a multiple of 4\n",
getFakeLogPrint().c_str());
#endif
diff --git a/debuggerd/test/log_fake.cpp b/debuggerd/test/log_fake.cpp
index 26523ad..d584a5e 100644
--- a/debuggerd/test/log_fake.cpp
+++ b/debuggerd/test/log_fake.cpp
@@ -14,6 +14,7 @@
* limitations under the License.
*/
+#include <errno.h>
#include <stdarg.h>
#include <string>
@@ -44,14 +45,16 @@
return g_fake_log_print;
}
-extern "C" int __android_log_buf_write(int, int, const char* tag, const char* msg) {
+extern "C" int __android_log_buf_write(int bufId, int prio, const char* tag, const char* msg) {
+ g_fake_log_buf += std::to_string(bufId) + ' ' + std::to_string(prio) + ' ';
g_fake_log_buf += tag;
g_fake_log_buf += ' ';
g_fake_log_buf += msg;
return 1;
}
-extern "C" int __android_log_print(int, const char* tag, const char* fmt, ...) {
+extern "C" int __android_log_print(int prio, const char* tag, const char* fmt, ...) {
+ g_fake_log_print += std::to_string(prio) + ' ';
g_fake_log_print += tag;
g_fake_log_print += ' ';
@@ -70,6 +73,7 @@
}
extern "C" struct logger_list* android_logger_list_open(log_id_t, int, unsigned int, pid_t) {
+ errno = EACCES;
return nullptr;
}
diff --git a/debuggerd/test/dump_maps_test.cpp b/debuggerd/test/tombstone_test.cpp
similarity index 90%
rename from debuggerd/test/dump_maps_test.cpp
rename to debuggerd/test/tombstone_test.cpp
index 230f4f5..a771a39 100644
--- a/debuggerd/test/dump_maps_test.cpp
+++ b/debuggerd/test/tombstone_test.cpp
@@ -45,7 +45,7 @@
void dump_backtrace_to_log(Backtrace*, log_t*, char const*) {
}
-class DumpMapsTest : public ::testing::Test {
+class TombstoneTest : public ::testing::Test {
protected:
virtual void SetUp() {
map_mock_.reset(new BacktraceMapMock());
@@ -92,7 +92,7 @@
log_t log_;
};
-TEST_F(DumpMapsTest, single_map) {
+TEST_F(TombstoneTest, single_map) {
backtrace_map_t map;
#if defined(__LP64__)
map.start = 0x123456789abcd000UL;
@@ -122,7 +122,7 @@
ASSERT_STREQ("", getFakeLogPrint().c_str());
}
-TEST_F(DumpMapsTest, single_map_elf_build_id) {
+TEST_F(TombstoneTest, single_map_elf_build_id) {
backtrace_map_t map;
#if defined(__LP64__)
map.start = 0x123456789abcd000UL;
@@ -157,7 +157,7 @@
// Even though build id is present, it should not be printed in either of
// these cases.
-TEST_F(DumpMapsTest, single_map_no_build_id) {
+TEST_F(TombstoneTest, single_map_no_build_id) {
backtrace_map_t map;
#if defined(__LP64__)
map.start = 0x123456789abcd000UL;
@@ -194,7 +194,7 @@
ASSERT_STREQ("", getFakeLogPrint().c_str());
}
-TEST_F(DumpMapsTest, multiple_maps) {
+TEST_F(TombstoneTest, multiple_maps) {
backtrace_map_t map;
map.start = 0xa234000;
@@ -256,7 +256,7 @@
ASSERT_STREQ("", getFakeLogPrint().c_str());
}
-TEST_F(DumpMapsTest, multiple_maps_fault_address_before) {
+TEST_F(TombstoneTest, multiple_maps_fault_address_before) {
backtrace_map_t map;
map.start = 0xa434000;
@@ -310,7 +310,7 @@
ASSERT_STREQ("", getFakeLogPrint().c_str());
}
-TEST_F(DumpMapsTest, multiple_maps_fault_address_between) {
+TEST_F(TombstoneTest, multiple_maps_fault_address_between) {
backtrace_map_t map;
map.start = 0xa434000;
@@ -364,7 +364,7 @@
ASSERT_STREQ("", getFakeLogPrint().c_str());
}
-TEST_F(DumpMapsTest, multiple_maps_fault_address_in_map) {
+TEST_F(TombstoneTest, multiple_maps_fault_address_in_map) {
backtrace_map_t map;
map.start = 0xa434000;
@@ -416,7 +416,7 @@
ASSERT_STREQ("", getFakeLogPrint().c_str());
}
-TEST_F(DumpMapsTest, multiple_maps_fault_address_after) {
+TEST_F(TombstoneTest, multiple_maps_fault_address_after) {
backtrace_map_t map;
map.start = 0xa434000;
@@ -474,7 +474,7 @@
ASSERT_STREQ("", getFakeLogPrint().c_str());
}
-TEST_F(DumpMapsTest, multiple_maps_getsiginfo_fail) {
+TEST_F(TombstoneTest, multiple_maps_getsiginfo_fail) {
backtrace_map_t map;
map.start = 0xa434000;
@@ -493,7 +493,6 @@
ASSERT_TRUE(lseek(log_.tfd, 0, SEEK_SET) == 0);
ASSERT_TRUE(android::base::ReadFdToString(log_.tfd, &tombstone_contents));
const char* expected_dump = \
-"Cannot get siginfo for 100: Bad address\n"
"\nmemory map:\n"
#if defined(__LP64__)
" 00000000'0a434000-00000000'0a434fff -w- 1000 1000 (load base 0xd000)\n";
@@ -503,12 +502,11 @@
ASSERT_STREQ(expected_dump, tombstone_contents.c_str());
// Verify that the log buf is empty, and no error messages.
- ASSERT_STREQ("DEBUG Cannot get siginfo for 100: Bad address\n",
- getFakeLogBuf().c_str());
- ASSERT_STREQ("", getFakeLogPrint().c_str());
+ ASSERT_STREQ("", getFakeLogBuf().c_str());
+ ASSERT_STREQ("6 DEBUG Cannot get siginfo for 100: Bad address\n\n", getFakeLogPrint().c_str());
}
-TEST_F(DumpMapsTest, multiple_maps_check_signal_has_si_addr) {
+TEST_F(TombstoneTest, multiple_maps_check_signal_has_si_addr) {
backtrace_map_t map;
map.start = 0xa434000;
@@ -569,3 +567,33 @@
ASSERT_STREQ("", getFakeLogPrint().c_str());
}
}
+
+TEST_F(TombstoneTest, dump_signal_info_error) {
+ siginfo_t si;
+ si.si_signo = 0;
+ ptrace_set_fake_getsiginfo(si);
+
+ dump_signal_info(&log_, 123, SIGSEGV, 10);
+
+ std::string tombstone_contents;
+ ASSERT_TRUE(lseek(log_.tfd, 0, SEEK_SET) == 0);
+ ASSERT_TRUE(android::base::ReadFdToString(log_.tfd, &tombstone_contents));
+ ASSERT_STREQ("", tombstone_contents.c_str());
+
+ ASSERT_STREQ("", getFakeLogBuf().c_str());
+ ASSERT_STREQ("6 DEBUG cannot get siginfo: Bad address\n\n", getFakeLogPrint().c_str());
+}
+
+TEST_F(TombstoneTest, dump_log_file_error) {
+ log_.should_retrieve_logcat = true;
+ dump_log_file(&log_, 123, "/fake/filename", 10);
+
+ std::string tombstone_contents;
+ ASSERT_TRUE(lseek(log_.tfd, 0, SEEK_SET) == 0);
+ ASSERT_TRUE(android::base::ReadFdToString(log_.tfd, &tombstone_contents));
+ ASSERT_STREQ("", tombstone_contents.c_str());
+
+ ASSERT_STREQ("", getFakeLogBuf().c_str());
+ ASSERT_STREQ("6 DEBUG Unable to open /fake/filename: Permission denied\n\n",
+ getFakeLogPrint().c_str());
+}
diff --git a/debuggerd/tombstone.cpp b/debuggerd/tombstone.cpp
index b0ad274..114c7e4 100644
--- a/debuggerd/tombstone.cpp
+++ b/debuggerd/tombstone.cpp
@@ -180,7 +180,7 @@
siginfo_t si;
memset(&si, 0, sizeof(si));
if (ptrace(PTRACE_GETSIGINFO, tid, 0, &si) == -1) {
- _LOG(log, logtype::HEADER, "cannot get siginfo: %s\n", strerror(errno));
+ ALOGE("cannot get siginfo: %s\n", strerror(errno));
return;
}
@@ -338,7 +338,7 @@
print_fault_address_marker = signal_has_si_addr(si.si_signo);
addr = reinterpret_cast<uintptr_t>(si.si_addr);
} else {
- _LOG(log, logtype::ERROR, "Cannot get siginfo for %d: %s\n", tid, strerror(errno));
+ ALOGE("Cannot get siginfo for %d: %s\n", tid, strerror(errno));
}
_LOG(log, logtype::MAPS, "\n");
@@ -448,7 +448,7 @@
// Skip this thread if cannot ptrace it
if (ptrace(PTRACE_ATTACH, new_tid, 0, 0) < 0) {
- _LOG(log, logtype::ERROR, "ptrace attach to %d failed: %s\n", new_tid, strerror(errno));
+ ALOGE("ptrace attach to %d failed: %s\n", new_tid, strerror(errno));
continue;
}
@@ -471,7 +471,7 @@
log->current_tid = log->crashed_tid;
if (ptrace(PTRACE_DETACH, new_tid, 0, 0) != 0) {
- _LOG(log, logtype::ERROR, "ptrace detach from %d failed: %s\n", new_tid, strerror(errno));
+ ALOGE("ptrace detach from %d failed: %s\n", new_tid, strerror(errno));
detach_failed = true;
}
}
@@ -517,13 +517,11 @@
// non-blocking EOF; we're done
break;
} else {
- _LOG(log, logtype::ERROR, "Error while reading log: %s\n",
- strerror(-actual));
+ ALOGE("Error while reading log: %s\n", strerror(-actual));
break;
}
} else if (actual == 0) {
- _LOG(log, logtype::ERROR, "Got zero bytes while reading log: %s\n",
- strerror(errno));
+ ALOGE("Got zero bytes while reading log: %s\n", strerror(errno));
break;
}
@@ -789,11 +787,11 @@
log.crashed_tid = tid;
if ((mkdir(TOMBSTONE_DIR, 0755) == -1) && (errno != EEXIST)) {
- _LOG(&log, logtype::ERROR, "failed to create %s: %s\n", TOMBSTONE_DIR, strerror(errno));
+ ALOGE("failed to create %s: %s\n", TOMBSTONE_DIR, strerror(errno));
}
if (chown(TOMBSTONE_DIR, AID_SYSTEM, AID_SYSTEM) == -1) {
- _LOG(&log, logtype::ERROR, "failed to change ownership of %s: %s\n", TOMBSTONE_DIR, strerror(errno));
+ ALOGE("failed to change ownership of %s: %s\n", TOMBSTONE_DIR, strerror(errno));
}
int fd = -1;
@@ -801,11 +799,11 @@
if (selinux_android_restorecon(TOMBSTONE_DIR, 0) == 0) {
path = find_and_open_tombstone(&fd);
} else {
- _LOG(&log, logtype::ERROR, "Failed to restore security context, not writing tombstone.\n");
+ ALOGE("Failed to restore security context, not writing tombstone.\n");
}
if (fd < 0) {
- _LOG(&log, logtype::ERROR, "Skipping tombstone write, nothing to do.\n");
+ ALOGE("Skipping tombstone write, nothing to do.\n");
*detach_failed = false;
return NULL;
}
diff --git a/debuggerd/utility.cpp b/debuggerd/utility.cpp
index 9f340a8..545a572 100644
--- a/debuggerd/utility.cpp
+++ b/debuggerd/utility.cpp
@@ -35,8 +35,7 @@
// Whitelist output desired in the logcat output.
bool is_allowed_in_logcat(enum logtype ltype) {
- if ((ltype == ERROR)
- || (ltype == HEADER)
+ if ((ltype == HEADER)
|| (ltype == REGISTERS)
|| (ltype == BACKTRACE)) {
return true;
diff --git a/debuggerd/utility.h b/debuggerd/utility.h
index 263374d..8bef192 100644
--- a/debuggerd/utility.h
+++ b/debuggerd/utility.h
@@ -59,7 +59,6 @@
// List of types of logs to simplify the logging decision in _LOG
enum logtype {
- ERROR,
HEADER,
THREAD,
REGISTERS,
diff --git a/debuggerd/x86/machine.cpp b/debuggerd/x86/machine.cpp
index c5f9259..af10817 100644
--- a/debuggerd/x86/machine.cpp
+++ b/debuggerd/x86/machine.cpp
@@ -14,12 +14,15 @@
* limitations under the License.
*/
+#define LOG_TAG "DEBUG"
+
#include <errno.h>
#include <stdint.h>
#include <string.h>
#include <sys/ptrace.h>
#include <backtrace/Backtrace.h>
+#include <log/log.h>
#include "machine.h"
#include "utility.h"
@@ -27,7 +30,7 @@
void dump_memory_and_code(log_t* log, Backtrace* backtrace) {
struct pt_regs r;
if (ptrace(PTRACE_GETREGS, backtrace->Tid(), 0, &r) == -1) {
- _LOG(log, logtype::ERROR, "cannot get registers: %s\n", strerror(errno));
+ ALOGE("cannot get registers: %s\n", strerror(errno));
return;
}
@@ -44,7 +47,7 @@
void dump_registers(log_t* log, pid_t tid) {
struct pt_regs r;
if (ptrace(PTRACE_GETREGS, tid, 0, &r) == -1) {
- _LOG(log, logtype::ERROR, "cannot get registers: %s\n", strerror(errno));
+ ALOGE("cannot get registers: %s\n", strerror(errno));
return;
}
diff --git a/debuggerd/x86_64/machine.cpp b/debuggerd/x86_64/machine.cpp
index 4f09a5d..fc86bc2 100644
--- a/debuggerd/x86_64/machine.cpp
+++ b/debuggerd/x86_64/machine.cpp
@@ -14,6 +14,8 @@
** limitations under the License.
*/
+#define LOG_TAG "DEBUG"
+
#include <errno.h>
#include <stdint.h>
#include <sys/ptrace.h>
@@ -21,6 +23,7 @@
#include <sys/user.h>
#include <backtrace/Backtrace.h>
+#include <log/log.h>
#include "machine.h"
#include "utility.h"
@@ -28,7 +31,7 @@
void dump_memory_and_code(log_t* log, Backtrace* backtrace) {
struct user_regs_struct r;
if (ptrace(PTRACE_GETREGS, backtrace->Tid(), 0, &r) == -1) {
- _LOG(log, logtype::ERROR, "cannot get registers: %s\n", strerror(errno));
+ ALOGE("cannot get registers: %s\n", strerror(errno));
return;
}
@@ -45,7 +48,7 @@
void dump_registers(log_t* log, pid_t tid) {
struct user_regs_struct r;
if (ptrace(PTRACE_GETREGS, tid, 0, &r) == -1) {
- _LOG(log, logtype::ERROR, "cannot get registers: %s\n", strerror(errno));
+ ALOGE("cannot get registers: %s\n", strerror(errno));
return;
}