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
This commit is contained in:
Christopher Ferris 2015-06-17 18:35:59 -07:00
parent 8c0478309e
commit b36b592338
14 changed files with 100 additions and 55 deletions

View file

@ -77,12 +77,12 @@ include $(BUILD_EXECUTABLE)
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 := \

View file

@ -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, &regs)) {
_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_memory_and_code(log_t* log, Backtrace* backtrace) {
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 @@ void dump_registers(log_t* log, pid_t tid) {
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;
}

View file

@ -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 @@ void dump_memory_and_code(log_t* log, Backtrace* backtrace) {
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 @@ void dump_registers(log_t* log, pid_t tid) {
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 @@ void dump_registers(log_t* log, pid_t tid) {
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;
}

View file

@ -105,7 +105,7 @@ static void dump_thread(
}
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;
}
}

View file

@ -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_memory_and_code(log_t* log, Backtrace* backtrace) {
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;
}

View file

@ -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_memory_and_code(log_t* log, Backtrace* backtrace) {
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;
}

View file

@ -288,9 +288,9 @@ TEST_F(DumpMemoryTest, memory_partially_unreadable_unaligned_return) {
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 @@ TEST_F(DumpMemoryTest, memory_partially_unreadable_two_unaligned_reads) {
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

View file

@ -14,6 +14,7 @@
* limitations under the License.
*/
#include <errno.h>
#include <stdarg.h>
#include <string>
@ -44,14 +45,16 @@ std::string getFakeLogPrint() {
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" log_id_t android_name_to_log_id(const char*) {
}
extern "C" struct logger_list* android_logger_list_open(log_id_t, int, unsigned int, pid_t) {
errno = EACCES;
return nullptr;
}

View file

@ -45,7 +45,7 @@ void dump_memory_and_code(log_t*, Backtrace*) {
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 @@ class DumpMapsTest : public ::testing::Test {
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 @@ TEST_F(DumpMapsTest, single_map) {
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 @@ TEST_F(DumpMapsTest, single_map_elf_build_id) {
// 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 @@ TEST_F(DumpMapsTest, single_map_no_build_id) {
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 @@ TEST_F(DumpMapsTest, multiple_maps) {
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 @@ TEST_F(DumpMapsTest, multiple_maps_fault_address_before) {
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 @@ TEST_F(DumpMapsTest, multiple_maps_fault_address_between) {
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 @@ TEST_F(DumpMapsTest, multiple_maps_fault_address_in_map) {
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 @@ TEST_F(DumpMapsTest, multiple_maps_fault_address_after) {
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 @@ TEST_F(DumpMapsTest, multiple_maps_getsiginfo_fail) {
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 @@ TEST_F(DumpMapsTest, multiple_maps_getsiginfo_fail) {
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 @@ TEST_F(DumpMapsTest, multiple_maps_check_signal_has_si_addr) {
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());
}

View file

@ -180,7 +180,7 @@ static void dump_signal_info(log_t* log, pid_t tid, int signal, int si_code) {
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 @@ static void dump_all_maps(Backtrace* backtrace, BacktraceMap* map, log_t* log, p
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 @@ static bool dump_sibling_thread_report(
// 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 @@ static bool dump_sibling_thread_report(
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 @@ static void dump_log_file(
// 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 @@ char* engrave_tombstone(pid_t pid, pid_t tid, int signal, int original_si_code,
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 @@ char* engrave_tombstone(pid_t pid, pid_t tid, int signal, int original_si_code,
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;
}

View file

@ -35,8 +35,7 @@ const int MAX_TOTAL_SLEEP_USEC = 10000000; // 10 seconds
// 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;

View file

@ -59,7 +59,6 @@ struct log_t{
// List of types of logs to simplify the logging decision in _LOG
enum logtype {
ERROR,
HEADER,
THREAD,
REGISTERS,

View file

@ -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_memory_and_code(log_t* log, Backtrace* backtrace) {
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;
}

View file

@ -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_memory_and_code(log_t* log, Backtrace* backtrace) {
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;
}