From 71a6177b8a5361e6febf5f06d85b66f38085e514 Mon Sep 17 00:00:00 2001 From: pancake Date: Mon, 25 Sep 2017 10:47:27 +0200 Subject: [PATCH] Fixes for the gdb-avr backend, lower min pkgtsz and fix tid<1 issue --- libr/asm/p/asm_avr.c | 7 +++-- libr/io/p/io_gdb.c | 4 +-- shlr/gdb/include/libgdbr.h | 1 + shlr/gdb/src/gdbclient/core.c | 54 +++++++++++++++++++++-------------- shlr/gdb/src/packet.c | 4 +-- 5 files changed, 41 insertions(+), 29 deletions(-) diff --git a/libr/asm/p/asm_avr.c b/libr/asm/p/asm_avr.c index a568d28b6a..6315d5d49b 100644 --- a/libr/asm/p/asm_avr.c +++ b/libr/asm/p/asm_avr.c @@ -181,7 +181,7 @@ static int parse_registerpair(const char *operand) { return -1; } - second = strtok(NULL, ":"); + second = strtok (NULL, ":"); /* the next code handles two possible different representation of pair by pair rx+1:rx @@ -208,8 +208,9 @@ static int parse_registerpair(const char *operand) { // the pair by even register (first) if (first[0] == 'r') { snum = atoi(first+1); - if (snum >= 0 && snum <= 30) + if (snum >= 0 && snum <= 30) { res = snum / 2; + } } else if (first[0] >= 'x' && first[0] <= 'z') { res = (2 - ('z' - first[0])) + 12; } @@ -429,7 +430,7 @@ RAsmPlugin r_asm_plugin_avr = { }; #ifndef CORELIB -struct r_lib_struct_t radare_plugin = { +RLibStruct radare_plugin = { .type = R_LIB_TYPE_ASM, .data = &r_asm_plugin_avr, .version = R2_VERSION diff --git a/libr/io/p/io_gdb.c b/libr/io/p/io_gdb.c index 53965efc1a..06ac5f73ee 100644 --- a/libr/io/p/io_gdb.c +++ b/libr/io/p/io_gdb.c @@ -117,7 +117,7 @@ static RIODesc *__open(RIO *io, const char *file, int rw, int mode) { if (gdbr_connect (&riog->desc, host, i_port) == 0) { desc = &riog->desc; - if (pid) { // FIXME this is here for now because RDebug's pid and libgdbr's aren't properly synced. + if (pid > 0) { // FIXME this is here for now because RDebug's pid and libgdbr's aren't properly synced. desc->pid = i_pid; if (gdbr_attach (desc, i_pid) < 0) { eprintf ("gdbr: Failed to attach to PID %i\n", i_pid); @@ -248,7 +248,7 @@ static int __system(RIO *io, RIODesc *fd, const char *cmd) { // pktsz = 0 doesn't make sense return false; } - desc->stub_features.pkt_sz = R_MAX (pktsz, 64); // min = 64 + desc->stub_features.pkt_sz = R_MAX (pktsz, 8); // min = 64 return true; } if (!strncmp (cmd, "detach", 6)) { diff --git a/shlr/gdb/include/libgdbr.h b/shlr/gdb/include/libgdbr.h index 07abdd4639..d6c05fb185 100644 --- a/shlr/gdb/include/libgdbr.h +++ b/shlr/gdb/include/libgdbr.h @@ -17,6 +17,7 @@ typedef unsigned int ssize_t; #define GDB_REMOTE_TYPE_GDB 0 #define GDB_REMOTE_TYPE_LLDB 1 +#define GDB_MAX_PKTSZ 4 /*! * Structure that saves a gdb message diff --git a/shlr/gdb/src/gdbclient/core.c b/shlr/gdb/src/gdbclient/core.c index eac3d4b1a1..e7c80b4092 100644 --- a/shlr/gdb/src/gdbclient/core.c +++ b/shlr/gdb/src/gdbclient/core.c @@ -106,9 +106,9 @@ int gdbr_connect(libgdbr_t *g, const char *host, int port) { g->stub_features.pkt_sz = 64; char *env_pktsz_str; ut32 env_pktsz; - if ((env_pktsz_str = getenv ("R2_GDB_PKTSZ"))) { + if ((env_pktsz_str = r_sys_getenv ("R2_GDB_PKTSZ"))) { if ((env_pktsz = (ut32) strtoul (env_pktsz_str, NULL, 10))) { - g->stub_features.pkt_sz = R_MAX (env_pktsz, 64); + g->stub_features.pkt_sz = R_MAX (env_pktsz, GDB_MAX_PKTSZ); } } ret = snprintf (tmp.buf, sizeof (tmp.buf) - 1, "%d", port); @@ -118,7 +118,7 @@ int gdbr_connect(libgdbr_t *g, const char *host, int port) { if (*host == '/') { ret = r_socket_connect_serial (g->sock, host, port, 1); } else { - ret = r_socket_connect_tcp (g->sock, host, tmp.buf, 200); + ret = r_socket_connect_tcp (g->sock, host, tmp.buf, 400); } if (!ret) { return -1; @@ -139,7 +139,7 @@ int gdbr_connect(libgdbr_t *g, const char *host, int port) { return ret; } if (env_pktsz > 0) { - g->stub_features.pkt_sz = R_MAX (R_MIN (env_pktsz, g->stub_features.pkt_sz), 64); + g->stub_features.pkt_sz = R_MAX (R_MIN (env_pktsz, g->stub_features.pkt_sz), GDB_MAX_PKTSZ); } // If no-ack supported, enable no-ack mode (should speed up things) if (g->stub_features.QStartNoAckMode) { @@ -485,12 +485,12 @@ int gdbr_read_registers(libgdbr_t *g) { if (ret < 0) { return ret; } - if (read_packet (g) < 0 || handle_g (g) < 0) { return -1; } if (reg_cache.init) { reg_cache.buflen = g->data_len; + memset (reg_cache.buf, 0, reg_cache.buflen); memcpy (reg_cache.buf, g->data, reg_cache.buflen); reg_cache.valid = true; } @@ -498,20 +498,19 @@ int gdbr_read_registers(libgdbr_t *g) { } static int gdbr_read_memory_page(libgdbr_t *g, ut64 address, ut8 *buf, int len) { - char command[64] = {0}; - int num_pkts, last, data_sz, ret_len; - int pkt; + char command[128] = {0}; + int last, ret_len, pkt; if (!g) { return -1; } - g->stub_features.pkt_sz = R_MAX (g->stub_features.pkt_sz, 64); - data_sz = g->stub_features.pkt_sz / 2; - num_pkts = len / data_sz; + g->stub_features.pkt_sz = R_MAX (g->stub_features.pkt_sz, GDB_MAX_PKTSZ); + int data_sz = g->stub_features.pkt_sz / 2; + int num_pkts = len / data_sz; last = len % data_sz; ret_len = 0; for (pkt = 0; pkt < num_pkts; pkt++) { if (snprintf (command, sizeof (command) - 1, - "%s%016"PFMT64x ",%"PFMT64x, CMD_READMEM, + "%s%"PFMT64x ",%"PFMT64x, CMD_READMEM, (ut64)address + (pkt * data_sz), (ut64)data_sz) < 0) { return -1; @@ -525,17 +524,22 @@ static int gdbr_read_memory_page(libgdbr_t *g, ut64 address, ut8 *buf, int len) if (handle_m (g) < 0) { return -1; } - int delta = num_pkts * data_sz; + int delta = (pkt * data_sz); + + if (delta > len) { + eprintf ("oops\n"); + break; + } int left = R_MIN (g->data_len, len - delta); if (left > 0) { - memcpy (buf + (pkt * data_sz), g->data, left); + memcpy (buf + delta, g->data, left); ret_len += g->data_len; } } if (last) { if (snprintf (command, sizeof (command) - 1, "%s%016"PFMT64x ",%"PFMT64x, CMD_READMEM, - (ut64) (address + (num_pkts * data_sz)), + (ut64)(address + (num_pkts * data_sz)), (ut64)last) < 0) { return -1; } @@ -602,7 +606,7 @@ int gdbr_write_memory(libgdbr_t *g, ut64 address, const uint8_t *data, ut64 len) if (!g || !data) { return -1; } - g->stub_features.pkt_sz = R_MAX (g->stub_features.pkt_sz, 64); + g->stub_features.pkt_sz = R_MAX (g->stub_features.pkt_sz, GDB_MAX_PKTSZ); data_sz = g->stub_features.pkt_sz / 2; if (data_sz < 1) { return -1; @@ -654,9 +658,11 @@ fail: } int gdbr_step(libgdbr_t *g, int tid) { - char thread_id[64]; - if (tid < 0 || write_thread_id (thread_id, sizeof (thread_id) - 1, g->pid, tid, + char thread_id[64] = {0}; + if (tid <= 0 || write_thread_id (thread_id, sizeof (thread_id) - 1, g->pid, tid, g->stub_features.multiprocess) < 0) { + send_vcont (g, "vCont?", NULL); + send_vcont (g, "Hc0", NULL); return send_vcont (g, CMD_C_STEP, NULL); } return send_vcont (g, CMD_C_STEP, thread_id); @@ -670,7 +676,7 @@ int gdbr_continue(libgdbr_t *g, int pid, int tid, int sig) { } else { snprintf (command, sizeof (command) - 1, "%s%02x", CMD_C_CONT_SIG, sig); } - if (write_thread_id (thread_id, sizeof (thread_id) - 1, g->pid, tid, + if (tid <= 0 || write_thread_id (thread_id, sizeof (thread_id) - 1, g->pid, tid, g->stub_features.multiprocess) < 0) { return send_vcont (g, command, NULL); } @@ -706,7 +712,11 @@ int gdbr_write_register(libgdbr_t *g, int index, char *value, int len) { return -1; } reg_cache.valid = false; - ret = snprintf (command, 255, "%s%d=", CMD_WRITEREG, index); + ret = snprintf (command, sizeof (command) - 1, "%s%d=", CMD_WRITEREG, index); + if (len + ret >= sizeof (command)) { + eprintf ("command is too small\n"); + return -1; + } memcpy (command + ret, value, len); pack_hex (value, len, (command + ret)); if (send_msg (g, command) < 0) { @@ -769,7 +779,7 @@ int gdbr_write_registers(libgdbr_t *g, char *registers) { } memcpy (buff, registers, len); reg = strtok (buff, ","); - while (reg != NULL) { + while (reg) { char *name_end = strchr (reg, '='); if (name_end == NULL) { eprintf ("Malformed argument: %s\n", reg); @@ -1080,7 +1090,7 @@ int gdbr_read_file(libgdbr_t *g, ut8 *buf, ut64 max_len) { eprintf ("%s: No remote file opened\n", __func__); return -1; } - g->stub_features.pkt_sz = R_MAX (g->stub_features.pkt_sz, 64); + g->stub_features.pkt_sz = R_MAX (g->stub_features.pkt_sz, GDB_MAX_PKTSZ); data_sz = g->stub_features.pkt_sz / 2; ret = 0; while (ret < max_len) { diff --git a/shlr/gdb/src/packet.c b/shlr/gdb/src/packet.c index 481253f7aa..e17a579beb 100644 --- a/shlr/gdb/src/packet.c +++ b/shlr/gdb/src/packet.c @@ -64,8 +64,8 @@ static int unpack(libgdbr_t *g, struct parse_ctx *ctx, int len) { g->read_buff[g->read_len] = '\0'; return 0; } - eprintf ("%s: Garbage at end of packet: %s\n", - __func__, g->read_buff + i + 1); + eprintf ("%s: Garbage at end of packet: %s (%s)\n", + __func__, g->read_buff + i + 1, g->read_buff + i + 1); } g->read_len = 0; return 0;