X-Git-Url: https://review.openocd.org/gitweb?a=blobdiff_plain;f=src%2Fserver%2Fgdb_server.c;h=f5736196eddfb4717d389688d631c1e76f31bceb;hb=4e5dbecd9b1ac780181e04f8b51a4cd133c4cdbe;hp=a9c2a64062f3d4971cf80b8d8f45ea71371a6f15;hpb=a077715b71dbe2e02561c3e33d15a0f8bc9be19a;p=openocd.git diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c index a9c2a64062..f5736196ed 100644 --- a/src/server/gdb_server.c +++ b/src/server/gdb_server.c @@ -43,6 +43,7 @@ #include #include #include +#include #include "server.h" #include #include "gdb_server.h" @@ -60,6 +61,13 @@ * found in most modern embedded processors. */ +enum gdb_output_flag { + /* GDB doesn't accept 'O' packets */ + GDB_OUTPUT_NO, + /* GDB accepts 'O' packets */ + GDB_OUTPUT_ALL, +}; + struct target_desc_format { char *tdesc; uint32_t tdesc_length; @@ -67,10 +75,10 @@ struct target_desc_format { /* private connection data for GDB */ struct gdb_connection { - char buffer[GDB_BUFFER_SIZE]; + char buffer[GDB_BUFFER_SIZE + 1]; /* Extra byte for null-termination */ char *buf_p; int buf_cnt; - int ctrl_c; + bool ctrl_c; enum target_state frontend_state; struct image *vflash_image; bool closed; @@ -90,10 +98,14 @@ struct gdb_connection { * normally we reply with a S reply via gdb_last_signal_packet. * as a side note this behaviour only effects gdb > 6.8 */ bool attached; + /* set when extended protocol is used */ + bool extended_protocol; /* temporarily used for target description support */ struct target_desc_format target_desc; /* temporarily used for thread list support */ char *thread_list; + /* flag to mask the output from gdb_log_callback() */ + enum gdb_output_flag output_flag; }; #if 0 @@ -153,6 +165,8 @@ static int gdb_last_signal(struct target *target) return 0x05; /* SIGTRAP */ case DBG_REASON_SINGLESTEP: return 0x05; /* SIGTRAP */ + case DBG_REASON_EXC_CATCH: + return 0x05; case DBG_REASON_NOTHALTED: return 0x0; /* no signal... shouldn't happen */ default: @@ -172,7 +186,7 @@ static int check_pending(struct connection *connection, fd_set read_fds; struct gdb_connection *gdb_con = connection->priv; int t; - if (got_data == NULL) + if (!got_data) got_data = &t; *got_data = 0; @@ -222,6 +236,7 @@ static int gdb_get_char_inner(struct connection *connection, int *next_char) if (gdb_con->buf_cnt > 0) break; if (gdb_con->buf_cnt == 0) { + LOG_DEBUG("GDB connection closed by the remote client"); gdb_con->closed = true; return ERROR_SERVER_REMOTE_CLOSED; } @@ -272,9 +287,9 @@ static int gdb_get_char_inner(struct connection *connection, int *next_char) gdb_con->buf_cnt--; *next_char = *(gdb_con->buf_p++); if (gdb_con->buf_cnt > 0) - connection->input_pending = 1; + connection->input_pending = true; else - connection->input_pending = 0; + connection->input_pending = false; #ifdef _DEBUG_GDB_IO_ LOG_DEBUG("returned char '%c' (0x%2.2x)", *next_char, *next_char); #endif @@ -297,9 +312,9 @@ static inline int gdb_get_char_fast(struct connection *connection, *next_char = **buf_p; (*buf_p)++; if (*buf_cnt > 0) - connection->input_pending = 1; + connection->input_pending = true; else - connection->input_pending = 0; + connection->input_pending = false; #ifdef _DEBUG_GDB_IO_ LOG_DEBUG("returned char '%c' (0x%2.2x)", *next_char, *next_char); @@ -343,23 +358,63 @@ static int gdb_putback_char(struct connection *connection, int last_char) static int gdb_write(struct connection *connection, void *data, int len) { struct gdb_connection *gdb_con = connection->priv; - if (gdb_con->closed) + if (gdb_con->closed) { + LOG_DEBUG("GDB socket marked as closed, cannot write to it."); return ERROR_SERVER_REMOTE_CLOSED; + } if (connection_write(connection, data, len) == len) return ERROR_OK; + + LOG_WARNING("Error writing to GDB socket. Dropping the connection."); gdb_con->closed = true; return ERROR_SERVER_REMOTE_CLOSED; } +static void gdb_log_incoming_packet(char *packet) +{ + if (!LOG_LEVEL_IS(LOG_LVL_DEBUG)) + return; + + /* Avoid dumping non-printable characters to the terminal */ + const unsigned packet_len = strlen(packet); + const char *nonprint = find_nonprint_char(packet, packet_len); + if (nonprint) { + /* Does packet at least have a prefix that is printable? + * Look within the first 50 chars of the packet. */ + const char *colon = memchr(packet, ':', MIN(50, packet_len)); + const bool packet_has_prefix = (colon); + const bool packet_prefix_printable = (packet_has_prefix && nonprint > colon); + + if (packet_prefix_printable) { + const unsigned int prefix_len = colon - packet + 1; /* + 1 to include the ':' */ + const unsigned int payload_len = packet_len - prefix_len; + LOG_DEBUG("received packet: %.*s", prefix_len, packet, payload_len); + } else { + LOG_DEBUG("received packet: ", packet_len); + } + } else { + /* All chars printable, dump the packet as is */ + LOG_DEBUG("received packet: %s", packet); + } +} + +static void gdb_log_outgoing_packet(char *packet_buf, unsigned int packet_len, unsigned char checksum) +{ + if (!LOG_LEVEL_IS(LOG_LVL_DEBUG)) + return; + + if (find_nonprint_char(packet_buf, packet_len)) + LOG_DEBUG("sending packet: $#%2.2x", packet_len, checksum); + else + LOG_DEBUG("sending packet: $%.*s#%2.2x'", packet_len, packet_buf, checksum); +} + static int gdb_put_packet_inner(struct connection *connection, char *buffer, int len) { int i; unsigned char my_checksum = 0; -#ifdef _DEBUG_GDB_IO_ - char *debug_buffer; -#endif int reply; int retval; struct gdb_connection *gdb_con = connection->priv; @@ -395,11 +450,7 @@ static int gdb_put_packet_inner(struct connection *connection, #endif while (1) { -#ifdef _DEBUG_GDB_IO_ - debug_buffer = strndup(buffer, len); - LOG_DEBUG("sending packet '$%s#%2.2x'", debug_buffer, my_checksum); - free(debug_buffer); -#endif + gdb_log_outgoing_packet(buffer, len, my_checksum); char local_buffer[1024]; local_buffer[0] = '$'; @@ -436,10 +487,10 @@ static int gdb_put_packet_inner(struct connection *connection, break; else if (reply == '-') { /* Stop sending output packets for now */ - log_remove_callback(gdb_log_callback, connection); + gdb_con->output_flag = GDB_OUTPUT_NO; LOG_WARNING("negative reply, retrying"); } else if (reply == 0x3) { - gdb_con->ctrl_c = 1; + gdb_con->ctrl_c = true; retval = gdb_get_char(connection, &reply); if (retval != ERROR_OK) return retval; @@ -447,7 +498,7 @@ static int gdb_put_packet_inner(struct connection *connection, break; else if (reply == '-') { /* Stop sending output packets for now */ - log_remove_callback(gdb_log_callback, connection); + gdb_con->output_flag = GDB_OUTPUT_NO; LOG_WARNING("negative reply, retrying"); } else if (reply == '$') { LOG_ERROR("GDB missing ack(1) - assumed good"); @@ -644,7 +695,7 @@ static int gdb_get_packet_inner(struct connection *connection, LOG_WARNING("negative acknowledgment, but no packet pending"); break; case 0x3: - gdb_con->ctrl_c = 1; + gdb_con->ctrl_c = true; *len = 0; return ERROR_OK; default: @@ -700,7 +751,7 @@ static int gdb_output_con(struct connection *connection, const char *line) bin_size = strlen(line); hex_buffer = malloc(bin_size * 2 + 2); - if (hex_buffer == NULL) + if (!hex_buffer) return ERROR_GDB_BUFFER_TOO_SMALL; hex_buffer[0] = 'O'; @@ -722,8 +773,8 @@ static int gdb_output(struct command_context *context, const char *line) static void gdb_signal_reply(struct target *target, struct connection *connection) { struct gdb_connection *gdb_connection = connection->priv; - char sig_reply[45]; - char stop_reason[20]; + char sig_reply[65]; + char stop_reason[32]; char current_thread[25]; int sig_reply_len; int signal_var; @@ -733,17 +784,25 @@ static void gdb_signal_reply(struct target *target, struct connection *connectio if (target->debug_reason == DBG_REASON_EXIT) { sig_reply_len = snprintf(sig_reply, sizeof(sig_reply), "W00"); } else { + struct target *ct; + if (target->rtos) { + target->rtos->current_threadid = target->rtos->current_thread; + target->rtos->gdb_target_for_threadid(connection, target->rtos->current_threadid, &ct); + } else { + ct = target; + } + if (gdb_connection->ctrl_c) { signal_var = 0x2; } else - signal_var = gdb_last_signal(target); + signal_var = gdb_last_signal(ct); stop_reason[0] = '\0'; - if (target->debug_reason == DBG_REASON_WATCHPOINT) { + if (ct->debug_reason == DBG_REASON_WATCHPOINT) { enum watchpoint_rw hit_wp_type; target_addr_t hit_wp_address; - if (watchpoint_hit(target, &hit_wp_type, &hit_wp_address) == ERROR_OK) { + if (watchpoint_hit(ct, &hit_wp_type, &hit_wp_address) == ERROR_OK) { switch (hit_wp_type) { case WPT_WRITE: @@ -765,20 +824,14 @@ static void gdb_signal_reply(struct target *target, struct connection *connectio } current_thread[0] = '\0'; - if (target->rtos != NULL) { - struct target *ct; - snprintf(current_thread, sizeof(current_thread), "thread:%016" PRIx64 ";", + if (target->rtos) + snprintf(current_thread, sizeof(current_thread), "thread:%" PRIx64 ";", target->rtos->current_thread); - target->rtos->current_threadid = target->rtos->current_thread; - target->rtos->gdb_target_for_threadid(connection, target->rtos->current_threadid, &ct); - if (!gdb_connection->ctrl_c) - signal_var = gdb_last_signal(ct); - } sig_reply_len = snprintf(sig_reply, sizeof(sig_reply), "T%2.2x%s%s", signal_var, stop_reason, current_thread); - gdb_connection->ctrl_c = 0; + gdb_connection->ctrl_c = false; } gdb_put_packet(connection, sig_reply, sig_reply_len); @@ -795,7 +848,7 @@ static void gdb_fileio_reply(struct target *target, struct connection *connectio if (strcmp(target->fileio_info->identifier, "open") == 0) sprintf(fileio_command, "F%s,%" PRIx64 "/%" PRIx64 ",%" PRIx64 ",%" PRIx64, target->fileio_info->identifier, target->fileio_info->param_1, - target->fileio_info->param_2, + target->fileio_info->param_2 + 1, /* len + trailing zero */ target->fileio_info->param_3, target->fileio_info->param_4); else if (strcmp(target->fileio_info->identifier, "close") == 0) @@ -819,13 +872,13 @@ static void gdb_fileio_reply(struct target *target, struct connection *connectio else if (strcmp(target->fileio_info->identifier, "rename") == 0) sprintf(fileio_command, "F%s,%" PRIx64 "/%" PRIx64 ",%" PRIx64 "/%" PRIx64, target->fileio_info->identifier, target->fileio_info->param_1, - target->fileio_info->param_2, + target->fileio_info->param_2 + 1, /* len + trailing zero */ target->fileio_info->param_3, - target->fileio_info->param_4); + target->fileio_info->param_4 + 1); /* len + trailing zero */ else if (strcmp(target->fileio_info->identifier, "unlink") == 0) sprintf(fileio_command, "F%s,%" PRIx64 "/%" PRIx64, target->fileio_info->identifier, target->fileio_info->param_1, - target->fileio_info->param_2); + target->fileio_info->param_2 + 1); /* len + trailing zero */ else if (strcmp(target->fileio_info->identifier, "stat") == 0) sprintf(fileio_command, "F%s,%" PRIx64 "/%" PRIx64 ",%" PRIx64, target->fileio_info->identifier, target->fileio_info->param_1, @@ -845,7 +898,7 @@ static void gdb_fileio_reply(struct target *target, struct connection *connectio else if (strcmp(target->fileio_info->identifier, "system") == 0) sprintf(fileio_command, "F%s,%" PRIx64 "/%" PRIx64, target->fileio_info->identifier, target->fileio_info->param_1, - target->fileio_info->param_2); + target->fileio_info->param_2 + 1); /* len + trailing zero */ else if (strcmp(target->fileio_info->identifier, "exit") == 0) { /* If target hits exit syscall, report to GDB the program is terminated. * In addition, let target run its own exit syscall handler. */ @@ -888,7 +941,7 @@ static void gdb_frontend_halted(struct target *target, struct connection *connec */ if (gdb_connection->frontend_state == TARGET_RUNNING) { /* stop forwarding log packets! */ - log_remove_callback(gdb_log_callback, connection); + gdb_connection->output_flag = GDB_OUTPUT_NO; /* check fileio first */ if (target_get_gdb_fileio_info(target, target->fileio_info) == ERROR_OK) @@ -901,7 +954,6 @@ static void gdb_frontend_halted(struct target *target, struct connection *connec static int gdb_target_callback_event_handler(struct target *target, enum target_event event, void *priv) { - int retval; struct connection *connection = priv; struct gdb_service *gdb_service = connection->service->priv; @@ -915,11 +967,6 @@ static int gdb_target_callback_event_handler(struct target *target, case TARGET_EVENT_HALTED: target_call_event_callbacks(target, TARGET_EVENT_GDB_END); break; - case TARGET_EVENT_GDB_FLASH_ERASE_START: - retval = jtag_execute_queue(); - if (retval != ERROR_OK) - return retval; - break; default: break; } @@ -936,11 +983,12 @@ static int gdb_new_connection(struct connection *connection) target = get_target_from_connection(connection); connection->priv = gdb_connection; + connection->cmd_ctx->current_target = target; /* initialize gdb connection information */ gdb_connection->buf_p = gdb_connection->buffer; gdb_connection->buf_cnt = 0; - gdb_connection->ctrl_c = 0; + gdb_connection->ctrl_c = false; gdb_connection->frontend_state = TARGET_HALTED; gdb_connection->vflash_image = NULL; gdb_connection->closed = false; @@ -949,9 +997,11 @@ static int gdb_new_connection(struct connection *connection) gdb_connection->sync = false; gdb_connection->mem_write_error = false; gdb_connection->attached = true; + gdb_connection->extended_protocol = false; gdb_connection->target_desc.tdesc = NULL; gdb_connection->target_desc.tdesc_length = 0; gdb_connection->thread_list = NULL; + gdb_connection->output_flag = GDB_OUTPUT_NO; /* send ACK to GDB for debug request */ gdb_write(connection, "+", 1); @@ -966,6 +1016,21 @@ static int gdb_new_connection(struct connection *connection) breakpoint_clear_target(target); watchpoint_clear_target(target); + /* Since version 3.95 (gdb-19990504), with the exclusion of 6.5~6.8, GDB + * sends an ACK at connection with the following comment in its source code: + * "Ack any packet which the remote side has already sent." + * LLDB does the same since the first gdb-remote implementation. + * Remove the initial ACK from the incoming buffer. + */ + retval = gdb_get_char(connection, &initial_ack); + if (retval != ERROR_OK) + return retval; + + if (initial_ack != '+') + gdb_putback_char(connection, initial_ack); + + target_call_event_callbacks(target, TARGET_EVENT_GDB_ATTACH); + if (target->rtos) { /* clean previous rtos session if supported*/ if (target->rtos->type->clean) @@ -975,33 +1040,20 @@ static int gdb_new_connection(struct connection *connection) rtos_update_threads(target); } - /* remove the initial ACK from the incoming buffer */ - retval = gdb_get_char(connection, &initial_ack); - if (retval != ERROR_OK) - return retval; - - /* FIX!!!??? would we actually ever receive a + here??? - * Not observed. - */ - if (initial_ack != '+') - gdb_putback_char(connection, initial_ack); - target_call_event_callbacks(target, TARGET_EVENT_GDB_ATTACH); - if (gdb_use_memory_map) { /* Connect must fail if the memory map can't be set up correctly. * * This will cause an auto_probe to be invoked, which is either * a no-op or it will fail when the target isn't ready(e.g. not halted). */ - int i; - for (i = 0; i < flash_get_bank_count(); i++) { + for (unsigned int i = 0; i < flash_get_bank_count(); i++) { struct flash_bank *p; p = get_flash_bank_by_num_noprobe(i); if (p->target != target) continue; retval = get_flash_bank_by_num(i, &p); if (retval != ERROR_OK) { - LOG_ERROR("Connect failed. Consider setting up a gdb-attach event for the target " \ + LOG_ERROR("Connect failed. Consider setting up a gdb-attach event for the target " "to prepare target for GDB connect, or use 'gdb_memory_map disable'."); return retval; } @@ -1009,13 +1061,24 @@ static int gdb_new_connection(struct connection *connection) } gdb_actual_connections++; - log_printf_lf(all_targets->next != NULL ? LOG_LVL_INFO : LOG_LVL_DEBUG, + log_printf_lf(all_targets->next ? LOG_LVL_INFO : LOG_LVL_DEBUG, __FILE__, __LINE__, __func__, "New GDB Connection: %d, Target %s, state: %s", gdb_actual_connections, target_name(target), target_state_name(target)); + if (!target_was_examined(target)) { + LOG_ERROR("Target %s not examined yet, refuse gdb connection %d!", + target_name(target), gdb_actual_connections); + gdb_actual_connections--; + return ERROR_TARGET_NOT_EXAMINED; + } + + if (target->state != TARGET_HALTED) + LOG_WARNING("GDB connection %d on target %s not halted", + gdb_actual_connections, target_name(target)); + /* DANGER! If we fail subsequently, we must remove this handler, * otherwise we occasionally see crashes as the timer can invoke the * callback fn. @@ -1023,6 +1086,8 @@ static int gdb_new_connection(struct connection *connection) * register callback to be informed about target events */ target_register_event_callback(gdb_target_callback_event_handler, connection); + log_add_callback(gdb_log_callback, connection); + return ERROR_OK; } @@ -1054,11 +1119,8 @@ static int gdb_connection_closed(struct connection *connection) /* if this connection registered a debug-message receiver delete it */ delete_debug_msg_receiver(connection->cmd_ctx, target); - if (connection->priv) { - free(connection->priv); - connection->priv = NULL; - } else - LOG_ERROR("BUG: connection->priv == NULL"); + free(connection->priv); + connection->priv = NULL; target_unregister_event_callback(gdb_target_callback_event_handler, connection); @@ -1170,7 +1232,7 @@ static int gdb_get_registers_packet(struct connection *connection, LOG_DEBUG("-"); #endif - if ((target->rtos != NULL) && (ERROR_OK == rtos_get_gdb_reg_list(connection))) + if ((target->rtos) && (rtos_get_gdb_reg_list(connection) == ERROR_OK)) return ERROR_OK; retval = target_get_gdb_reg_list(target, ®_list, ®_list_size, @@ -1178,18 +1240,23 @@ static int gdb_get_registers_packet(struct connection *connection, if (retval != ERROR_OK) return gdb_error(connection, retval); - for (i = 0; i < reg_list_size; i++) + for (i = 0; i < reg_list_size; i++) { + if (!reg_list[i] || reg_list[i]->exist == false || reg_list[i]->hidden) + continue; reg_packet_size += DIV_ROUND_UP(reg_list[i]->size, 8) * 2; + } assert(reg_packet_size > 0); reg_packet = malloc(reg_packet_size + 1); /* plus one for string termination null */ - if (reg_packet == NULL) + if (!reg_packet) return ERROR_FAIL; reg_packet_p = reg_packet; for (i = 0; i < reg_list_size; i++) { + if (!reg_list[i] || reg_list[i]->exist == false || reg_list[i]->hidden) + continue; if (!reg_list[i]->valid) { retval = reg_list[i]->type->get(reg_list[i]); if (retval != ERROR_OK && gdb_report_register_access_error) { @@ -1295,13 +1362,16 @@ static int gdb_get_register_packet(struct connection *connection, LOG_DEBUG("-"); #endif - retval = target_get_gdb_reg_list(target, ®_list, ®_list_size, + if ((target->rtos) && (rtos_get_gdb_reg(connection, reg_num) == ERROR_OK)) + return ERROR_OK; + + retval = target_get_gdb_reg_list_noread(target, ®_list, ®_list_size, REG_CLASS_ALL); if (retval != ERROR_OK) return gdb_error(connection, retval); if (reg_list_size <= reg_num) { - LOG_ERROR("gdb requested a non-existing register"); + LOG_ERROR("gdb requested a non-existing register (reg_num=%d)", reg_num); return ERROR_SERVER_REMOTE_CLOSED; } @@ -1314,7 +1384,7 @@ static int gdb_get_register_packet(struct connection *connection, } } - reg_packet = malloc(DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2 + 1); /* plus one for string termination null */ + reg_packet = calloc(DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2 + 1, 1); /* plus one for string termination null */ gdb_str_to_target(target, reg_packet, reg_list[reg_num]); @@ -1331,36 +1401,49 @@ static int gdb_set_register_packet(struct connection *connection, { struct target *target = get_target_from_connection(connection); char *separator; - uint8_t *bin_buf; int reg_num = strtoul(packet + 1, &separator, 16); struct reg **reg_list; int reg_list_size; int retval; +#ifdef _DEBUG_GDB_IO_ LOG_DEBUG("-"); +#endif - retval = target_get_gdb_reg_list(target, ®_list, ®_list_size, + if (*separator != '=') { + LOG_ERROR("GDB 'set register packet', but no '=' following the register number"); + return ERROR_SERVER_REMOTE_CLOSED; + } + size_t chars = strlen(separator + 1); + uint8_t *bin_buf = malloc(chars / 2); + gdb_target_to_reg(target, separator + 1, chars, bin_buf); + + if ((target->rtos) && + (rtos_set_reg(connection, reg_num, bin_buf) == ERROR_OK)) { + free(bin_buf); + gdb_put_packet(connection, "OK", 2); + return ERROR_OK; + } + + retval = target_get_gdb_reg_list_noread(target, ®_list, ®_list_size, REG_CLASS_ALL); - if (retval != ERROR_OK) + if (retval != ERROR_OK) { + free(bin_buf); return gdb_error(connection, retval); - - if (reg_list_size <= reg_num) { - LOG_ERROR("gdb requested a non-existing register"); - return ERROR_SERVER_REMOTE_CLOSED; } - if (*separator != '=') { - LOG_ERROR("GDB 'set register packet', but no '=' following the register number"); + if (reg_list_size <= reg_num) { + LOG_ERROR("gdb requested a non-existing register (reg_num=%d)", reg_num); + free(bin_buf); + free(reg_list); return ERROR_SERVER_REMOTE_CLOSED; } - /* convert from GDB-string (target-endian) to hex-string (big-endian) */ - bin_buf = malloc(DIV_ROUND_UP(reg_list[reg_num]->size, 8)); - int chars = (DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2); - - if ((unsigned int)chars != strlen(separator + 1)) { - LOG_ERROR("gdb sent a packet with wrong register size"); + if (chars != (DIV_ROUND_UP(reg_list[reg_num]->size, 8) * 2)) { + LOG_ERROR("gdb sent %zu bits for a %" PRIu32 "-bit register (%s)", + chars * 4, reg_list[reg_num]->size, reg_list[reg_num]->name); free(bin_buf); + free(reg_list); return ERROR_SERVER_REMOTE_CLOSED; } @@ -1395,8 +1478,6 @@ static int gdb_error(struct connection *connection, int retval) /* We don't have to worry about the default 2 second timeout for GDB packets, * because GDB breaks up large memory reads into smaller reads. - * - * 8191 bytes by the looks of it. Why 8191 bytes instead of 8192????? */ static int gdb_read_memory_packet(struct connection *connection, char const *packet, int packet_size) @@ -1425,7 +1506,7 @@ static int gdb_read_memory_packet(struct connection *connection, if (!len) { LOG_WARNING("invalid read memory packet received (len == 0)"); - gdb_put_packet(connection, NULL, 0); + gdb_put_packet(connection, "", 0); return ERROR_OK; } @@ -1433,7 +1514,11 @@ static int gdb_read_memory_packet(struct connection *connection, LOG_DEBUG("addr: 0x%16.16" PRIx64 ", len: 0x%8.8" PRIx32 "", addr, len); - retval = target_read_buffer(target, addr, len, buffer); + retval = ERROR_NOT_IMPLEMENTED; + if (target->rtos) + retval = rtos_read_buffer(target, addr, len, buffer); + if (retval == ERROR_NOT_IMPLEMENTED) + retval = target_read_buffer(target, addr, len, buffer); if ((retval != ERROR_OK) && !gdb_report_data_abort) { /* TODO : Here we have to lie and send back all zero's lest stack traces won't work. @@ -1504,7 +1589,11 @@ static int gdb_write_memory_packet(struct connection *connection, if (unhexify(buffer, separator, len) != len) LOG_ERROR("unable to decode memory packet"); - retval = target_write_buffer(target, addr, len, buffer); + retval = ERROR_NOT_IMPLEMENTED; + if (target->rtos) + retval = rtos_write_buffer(target, addr, len, buffer); + if (retval == ERROR_NOT_IMPLEMENTED) + retval = target_write_buffer(target, addr, len, buffer); if (retval == ERROR_OK) gdb_put_packet(connection, "OK", 2); @@ -1573,7 +1662,12 @@ static int gdb_write_memory_binary_packet(struct connection *connection, if (len) { LOG_DEBUG("addr: 0x%" PRIx64 ", len: 0x%8.8" PRIx32 "", addr, len); - retval = target_write_buffer(target, addr, len, (uint8_t *)separator); + retval = ERROR_NOT_IMPLEMENTED; + if (target->rtos) + retval = rtos_write_buffer(target, addr, len, (uint8_t *)separator); + if (retval == ERROR_NOT_IMPLEMENTED) + retval = target_write_buffer(target, addr, len, (uint8_t *)separator); + if (retval != ERROR_OK) gdb_connection->mem_write_error = true; } @@ -1630,7 +1724,7 @@ static int gdb_breakpoint_watchpoint_packet(struct connection *connection, char *separator; int retval; - LOG_DEBUG("-"); + LOG_DEBUG("[%s]", target_name(target)); type = strtoul(packet + 1, &separator, 16); @@ -1710,24 +1804,23 @@ static int gdb_breakpoint_watchpoint_packet(struct connection *connection, /* print out a string and allocate more space as needed, * mainly used for XML at this point */ -static void xml_printf(int *retval, char **xml, int *pos, int *size, - const char *fmt, ...) +static __attribute__ ((format (PRINTF_ATTRIBUTE_FORMAT, 5, 6))) void xml_printf(int *retval, + char **xml, int *pos, int *size, const char *fmt, ...) { if (*retval != ERROR_OK) return; int first = 1; for (;; ) { - if ((*xml == NULL) || (!first)) { + if ((!*xml) || (!first)) { /* start by 0 to exercise all the code paths. * Need minimum 2 bytes to fit 1 char and 0 terminator. */ *size = *size * 2 + 2; char *t = *xml; *xml = realloc(*xml, *size); - if (*xml == NULL) { - if (t) - free(t); + if (!*xml) { + free(t); *retval = ERROR_SERVER_REMOTE_CLOSED; return; } @@ -1751,7 +1844,7 @@ static int decode_xfer_read(char const *buf, char **annex, int *ofs, unsigned in { /* Locate the annex. */ const char *annex_end = strchr(buf, ':'); - if (annex_end == NULL) + if (!annex_end) return ERROR_FAIL; /* After the read marker and annex, qXfer looks like a @@ -1765,9 +1858,9 @@ static int decode_xfer_read(char const *buf, char **annex, int *ofs, unsigned in *len = strtoul(separator + 1, NULL, 16); /* Extract the annex if needed */ - if (annex != NULL) { + if (annex) { *annex = strndup(buf, annex_end - buf); - if (*annex == NULL) + if (!*annex) return ERROR_FAIL; } @@ -1809,8 +1902,7 @@ static int gdb_memory_map(struct connection *connection, int length; char *separator; target_addr_t ram_start = 0; - int i; - int target_flash_banks = 0; + unsigned int target_flash_banks = 0; /* skip command character */ packet += 23; @@ -1826,7 +1918,7 @@ static int gdb_memory_map(struct connection *connection, */ banks = malloc(sizeof(struct flash_bank *)*flash_get_bank_count()); - for (i = 0; i < flash_get_bank_count(); i++) { + for (unsigned int i = 0; i < flash_get_bank_count(); i++) { p = get_flash_bank_by_num_noprobe(i); if (p->target != target) continue; @@ -1842,8 +1934,7 @@ static int gdb_memory_map(struct connection *connection, qsort(banks, target_flash_banks, sizeof(struct flash_bank *), compare_bank); - for (i = 0; i < target_flash_banks; i++) { - int j; + for (unsigned int i = 0; i < target_flash_banks; i++) { unsigned sector_size = 0; unsigned group_len = 0; @@ -1852,7 +1943,7 @@ static int gdb_memory_map(struct connection *connection, if (ram_start < p->base) xml_printf(&retval, &xml, &pos, &size, "\n", + "length=\"" TARGET_ADDR_FMT "\"/>\n", ram_start, p->base - ram_start); /* Report adjacent groups of same-size sectors. So for @@ -1861,7 +1952,7 @@ static int gdb_memory_map(struct connection *connection, * smaller ones at the end (maybe 32KB). STR7 will have * regions with 8KB, 32KB, and 64KB sectors; etc. */ - for (j = 0; j < p->num_sectors; j++) { + for (unsigned int j = 0; j < p->num_sectors; j++) { /* Maybe start a new group of sectors. */ if (sector_size == 0) { @@ -1909,11 +2000,10 @@ static int gdb_memory_map(struct connection *connection, if (ram_start != 0) xml_printf(&retval, &xml, &pos, &size, "\n", - ram_start, 0-ram_start); - /* ELSE a flash chip could be at the very end of the 32 bit address - * space, in which case ram_start will be precisely 0 - */ + "length=\"" TARGET_ADDR_FMT "\"/>\n", + ram_start, target_address_max(target) - ram_start + 1); + /* ELSE a flash chip could be at the very end of the address space, in + * which case ram_start will be precisely 0 */ free(banks); @@ -1989,7 +2079,7 @@ static int lookup_add_arch_defined_types(char const **arch_defined_types_list[], { int tbl_sz = *num_arch_defined_types; - if (type_id != NULL && (strcmp(type_id, ""))) { + if (type_id && (strcmp(type_id, ""))) { for (int j = 0; j < (tbl_sz + 1); j++) { if (!((*arch_defined_types_list)[j])) { (*arch_defined_types_list)[tbl_sz++] = type_id; @@ -2010,7 +2100,7 @@ static int lookup_add_arch_defined_types(char const **arch_defined_types_list[], static int gdb_generate_reg_type_description(struct target *target, char **tdesc, int *pos, int *size, struct reg_data_type *type, - char const **arch_defined_types_list[], int * num_arch_defined_types) + char const **arch_defined_types_list[], int *num_arch_defined_types) { int retval = ERROR_OK; @@ -2025,14 +2115,14 @@ static int gdb_generate_reg_type_description(struct target *target, } /* */ xml_printf(&retval, tdesc, pos, size, - "\n", + "\n", type->id, type->reg_type_vector->type->id, type->reg_type_vector->count); } else if (type->type_class == REG_TYPE_CLASS_UNION) { struct reg_data_type_union_field *field; field = type->reg_type_union->fields; - while (field != NULL) { + while (field) { struct reg_data_type *data_type = field->type; if (data_type->type == REG_TYPE_ARCH_DEFINED) { if (lookup_add_arch_defined_types(arch_defined_types_list, data_type->id, @@ -2052,7 +2142,7 @@ static int gdb_generate_reg_type_description(struct target *target, type->id); field = type->reg_type_union->fields; - while (field != NULL) { + while (field) { xml_printf(&retval, tdesc, pos, size, "\n", field->name, field->type->id); @@ -2072,18 +2162,18 @@ static int gdb_generate_reg_type_description(struct target *target, * ... * */ xml_printf(&retval, tdesc, pos, size, - "\n", + "\n", type->id, type->reg_type_struct->size); - while (field != NULL) { + while (field) { xml_printf(&retval, tdesc, pos, size, - "\n", + "\n", field->name, field->bitfield->start, field->bitfield->end, gdb_get_reg_type_name(field->bitfield->type)); field = field->next; } } else { - while (field != NULL) { + while (field) { struct reg_data_type *data_type = field->type; if (data_type->type == REG_TYPE_ARCH_DEFINED) { if (lookup_add_arch_defined_types(arch_defined_types_list, data_type->id, @@ -2100,7 +2190,7 @@ static int gdb_generate_reg_type_description(struct target *target, xml_printf(&retval, tdesc, pos, size, "\n", type->id); - while (field != NULL) { + while (field) { xml_printf(&retval, tdesc, pos, size, "\n", field->name, field->type->id); @@ -2117,14 +2207,14 @@ static int gdb_generate_reg_type_description(struct target *target, * ... * */ xml_printf(&retval, tdesc, pos, size, - "\n", + "\n", type->id, type->reg_type_flags->size); struct reg_data_type_flags_field *field; field = type->reg_type_flags->fields; - while (field != NULL) { + while (field) { xml_printf(&retval, tdesc, pos, size, - "\n", + "\n", field->name, field->bitfield->start, field->bitfield->end, gdb_get_reg_type_name(field->bitfield->type)); @@ -2151,11 +2241,11 @@ static int get_reg_features_list(struct target *target, char const **feature_lis *feature_list = calloc(1, sizeof(char *)); for (int i = 0; i < reg_list_size; i++) { - if (reg_list[i]->exist == false) + if (reg_list[i]->exist == false || reg_list[i]->hidden) continue; - if (reg_list[i]->feature != NULL - && reg_list[i]->feature->name != NULL + if (reg_list[i]->feature + && reg_list[i]->feature->name && (strcmp(reg_list[i]->feature->name, ""))) { /* We found a feature, check if the feature is already in the * table. If not, allocate a new entry for the table and @@ -2181,23 +2271,137 @@ static int get_reg_features_list(struct target *target, char const **feature_lis return ERROR_OK; } +/* Create a register list that's the union of all the registers of the SMP + * group this target is in. If the target is not part of an SMP group, this + * returns the same as target_get_gdb_reg_list_noread(). + */ +static int smp_reg_list_noread(struct target *target, + struct reg **combined_list[], int *combined_list_size, + enum target_register_class reg_class) +{ + if (!target->smp) + return target_get_gdb_reg_list_noread(target, combined_list, + combined_list_size, REG_CLASS_ALL); + + unsigned int combined_allocated = 256; + struct reg **local_list = malloc(combined_allocated * sizeof(struct reg *)); + if (!local_list) { + LOG_ERROR("malloc(%zu) failed", combined_allocated * sizeof(struct reg *)); + return ERROR_FAIL; + } + unsigned int local_list_size = 0; + + struct target_list *head; + foreach_smp_target(head, target->smp_targets) { + if (!target_was_examined(head->target)) + continue; + + struct reg **reg_list = NULL; + int reg_list_size; + int result = target_get_gdb_reg_list_noread(head->target, ®_list, + ®_list_size, reg_class); + if (result != ERROR_OK) { + free(local_list); + return result; + } + for (int i = 0; i < reg_list_size; i++) { + bool found = false; + struct reg *a = reg_list[i]; + if (a->exist) { + /* Nested loop makes this O(n^2), but this entire function with + * 5 RISC-V targets takes just 2ms on my computer. Fast enough + * for me. */ + for (unsigned int j = 0; j < local_list_size; j++) { + struct reg *b = local_list[j]; + if (!strcmp(a->name, b->name)) { + found = true; + if (a->size != b->size) { + LOG_ERROR("SMP register %s is %d bits on one " + "target, but %d bits on another target.", + a->name, a->size, b->size); + free(reg_list); + free(local_list); + return ERROR_FAIL; + } + break; + } + } + if (!found) { + LOG_DEBUG("[%s] %s not found in combined list", target_name(target), a->name); + if (local_list_size >= combined_allocated) { + combined_allocated *= 2; + local_list = realloc(local_list, combined_allocated * sizeof(struct reg *)); + if (!local_list) { + LOG_ERROR("realloc(%zu) failed", combined_allocated * sizeof(struct reg *)); + return ERROR_FAIL; + } + } + local_list[local_list_size] = a; + local_list_size++; + } + } + } + free(reg_list); + } + + if (local_list_size == 0) { + LOG_ERROR("Unable to get register list"); + free(local_list); + return ERROR_FAIL; + } + + /* Now warn the user about any registers that weren't found in every target. */ + foreach_smp_target(head, target->smp_targets) { + if (!target_was_examined(head->target)) + continue; + + struct reg **reg_list = NULL; + int reg_list_size; + int result = target_get_gdb_reg_list_noread(head->target, ®_list, + ®_list_size, reg_class); + if (result != ERROR_OK) { + free(local_list); + return result; + } + for (unsigned int i = 0; i < local_list_size; i++) { + bool found = false; + struct reg *a = local_list[i]; + for (int j = 0; j < reg_list_size; j++) { + struct reg *b = reg_list[j]; + if (b->exist && !strcmp(a->name, b->name)) { + found = true; + break; + } + } + if (!found) { + LOG_WARNING("Register %s does not exist in %s, which is part of an SMP group where " + "this register does exist.", + a->name, target_name(head->target)); + } + } + free(reg_list); + } + + *combined_list = local_list; + *combined_list_size = local_list_size; + return ERROR_OK; +} + static int gdb_generate_target_description(struct target *target, char **tdesc_out) { int retval = ERROR_OK; struct reg **reg_list = NULL; int reg_list_size; + char const *architecture; char const **features = NULL; - char const **arch_defined_types = NULL; int feature_list_size = 0; - int num_arch_defined_types = 0; char *tdesc = NULL; int pos = 0; int size = 0; - arch_defined_types = calloc(1, sizeof(char *)); - retval = target_get_gdb_reg_list(target, ®_list, - ®_list_size, REG_CLASS_ALL); + retval = smp_reg_list_noread(target, ®_list, ®_list_size, + REG_CLASS_ALL); if (retval != ERROR_OK) { LOG_ERROR("get register list failed"); @@ -2227,10 +2431,19 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o "\n" "\n"); + /* generate architecture element if supported by target */ + architecture = target_get_gdb_arch(target); + if (architecture) + xml_printf(&retval, &tdesc, &pos, &size, + "%s\n", architecture); + /* generate target description according to register list */ - if (features != NULL) { + if (features) { while (features[current_feature]) { + char const **arch_defined_types = NULL; + int num_arch_defined_types = 0; + arch_defined_types = calloc(1, sizeof(char *)); xml_printf(&retval, &tdesc, &pos, &size, "\n", features[current_feature]); @@ -2238,14 +2451,14 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o int i; for (i = 0; i < reg_list_size; i++) { - if (reg_list[i]->exist == false) + if (reg_list[i]->exist == false || reg_list[i]->hidden) continue; if (strcmp(reg_list[i]->feature->name, features[current_feature])) continue; const char *type_str; - if (reg_list[i]->reg_data_type != NULL) { + if (reg_list[i]->reg_data_type) { if (reg_list[i]->reg_data_type->type == REG_TYPE_ARCH_DEFINED) { /* generate name); xml_printf(&retval, &tdesc, &pos, &size, - " bitsize=\"%d\"", reg_list[i]->size); + " bitsize=\"%" PRIu32 "\"", reg_list[i]->size); xml_printf(&retval, &tdesc, &pos, &size, - " regnum=\"%d\"", reg_list[i]->number); + " regnum=\"%" PRIu32 "\"", reg_list[i]->number); if (reg_list[i]->caller_save) xml_printf(&retval, &tdesc, &pos, &size, " save-restore=\"yes\""); @@ -2283,7 +2496,7 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o xml_printf(&retval, &tdesc, &pos, &size, " type=\"%s\"", type_str); - if (reg_list[i]->group != NULL) + if (reg_list[i]->group) xml_printf(&retval, &tdesc, &pos, &size, " group=\"%s\"", reg_list[i]->group); @@ -2295,6 +2508,7 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o "\n"); current_feature++; + free(arch_defined_types); } } @@ -2304,7 +2518,6 @@ static int gdb_generate_target_description(struct target *target, char **tdesc_o error: free(features); free(reg_list); - free(arch_defined_types); if (retval == ERROR_OK) *tdesc_out = tdesc; @@ -2317,7 +2530,7 @@ error: static int gdb_get_target_description_chunk(struct target *target, struct target_desc_format *target_desc, char **chunk, int32_t offset, uint32_t length) { - if (target_desc == NULL) { + if (!target_desc) { LOG_ERROR("Unable to Generate Target Description"); return ERROR_FAIL; } @@ -2325,7 +2538,7 @@ static int gdb_get_target_description_chunk(struct target *target, struct target char *tdesc = target_desc->tdesc; uint32_t tdesc_length = target_desc->tdesc_length; - if (tdesc == NULL) { + if (!tdesc) { int retval = gdb_generate_target_description(target, &tdesc); if (retval != ERROR_OK) { LOG_ERROR("Unable to Generate Target Description"); @@ -2343,7 +2556,7 @@ static int gdb_get_target_description_chunk(struct target *target, struct target transfer_type = 'l'; *chunk = malloc(length + 2); - if (*chunk == NULL) { + if (!*chunk) { LOG_ERROR("Unable to allocate memory"); return ERROR_FAIL; } @@ -2376,7 +2589,9 @@ static int gdb_target_description_supported(struct target *target, int *supporte char const **features = NULL; int feature_list_size = 0; - retval = target_get_gdb_reg_list(target, ®_list, + char const *architecture = target_get_gdb_arch(target); + + retval = target_get_gdb_reg_list_noread(target, ®_list, ®_list_size, REG_CLASS_ALL); if (retval != ERROR_OK) { LOG_ERROR("get register list failed"); @@ -2397,7 +2612,7 @@ static int gdb_target_description_supported(struct target *target, int *supporte } if (supported) { - if (feature_list_size) + if (architecture || feature_list_size) *supported = 1; else *supported = 0; @@ -2423,26 +2638,32 @@ static int gdb_generate_thread_list(struct target *target, char **thread_list_ou "\n" "\n"); - if (rtos != NULL) { + if (rtos) { for (int i = 0; i < rtos->thread_count; i++) { struct thread_detail *thread_detail = &rtos->thread_details[i]; if (!thread_detail->exists) continue; - xml_printf(&retval, &thread_list, &pos, &size, - "", thread_detail->threadid); + if (thread_detail->thread_name_str) + xml_printf(&retval, &thread_list, &pos, &size, + "", + thread_detail->threadid, + thread_detail->thread_name_str); + else + xml_printf(&retval, &thread_list, &pos, &size, + "", thread_detail->threadid); - if (thread_detail->thread_name_str != NULL) + if (thread_detail->thread_name_str) xml_printf(&retval, &thread_list, &pos, &size, "Name: %s", thread_detail->thread_name_str); - if (thread_detail->extra_info_str != NULL) { - if (thread_detail->thread_name_str != NULL) + if (thread_detail->extra_info_str) { + if (thread_detail->thread_name_str) xml_printf(&retval, &thread_list, &pos, &size, ", "); xml_printf(&retval, &thread_list, &pos, &size, - thread_detail->extra_info_str); + "%s", thread_detail->extra_info_str); } xml_printf(&retval, &thread_list, &pos, &size, @@ -2464,7 +2685,7 @@ static int gdb_generate_thread_list(struct target *target, char **thread_list_ou static int gdb_get_thread_list_chunk(struct target *target, char **thread_list, char **chunk, int32_t offset, uint32_t length) { - if (*thread_list == NULL) { + if (!*thread_list) { int retval = gdb_generate_thread_list(target, thread_list); if (retval != ERROR_OK) { LOG_ERROR("Unable to Generate Thread List"); @@ -2482,11 +2703,11 @@ static int gdb_get_thread_list_chunk(struct target *target, char **thread_list, transfer_type = 'l'; *chunk = malloc(length + 2 + 3); - /* Allocating extra 3 bytes prevents false positive valgrind report + /* Allocating extra 3 bytes prevents false positive valgrind report * of strlen(chunk) word access: * Invalid read of size 4 * Address 0x4479934 is 44 bytes inside a block of size 45 alloc'd */ - if (*chunk == NULL) { + if (!*chunk) { LOG_ERROR("Unable to allocate memory"); return ERROR_FAIL; } @@ -2519,7 +2740,7 @@ static int gdb_query_packet(struct connection *connection, cmd[len] = 0; /* We want to print all debug output to GDB connection */ - log_add_callback(gdb_log_callback, connection); + gdb_connection->output_flag = GDB_OUTPUT_ALL; target_call_timer_callbacks_now(); /* some commands need to know the GDB connection, make note of current * GDB connection. */ @@ -2527,7 +2748,7 @@ static int gdb_query_packet(struct connection *connection, command_run_line(cmd_ctx, cmd); current_gdb_connection = NULL; target_call_timer_callbacks_now(); - log_remove_callback(gdb_log_callback, connection); + gdb_connection->output_flag = GDB_OUTPUT_NO; free(cmd); } gdb_put_packet(connection, "OK", 2); @@ -2594,7 +2815,7 @@ static int gdb_query_packet(struct connection *connection, &pos, &size, "PacketSize=%x;qXfer:memory-map:read%c;qXfer:features:read%c;qXfer:threads:read+;QStartNoAckMode+;vContSupported+", - (GDB_BUFFER_SIZE - 1), + GDB_BUFFER_SIZE, ((gdb_use_memory_map == 1) && (flash_get_bank_count() > 0)) ? '+' : '-', (gdb_target_desc_supported == 1) ? '+' : '-'); @@ -2691,7 +2912,7 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p /* query for vCont supported */ if (parse[0] == '?') { - if (target->type->step != NULL) { + if (target->type->step) { /* gdb doesn't accept c without C and s without S */ gdb_put_packet(connection, "vCont;c;C;s;S", 13); return true; @@ -2708,7 +2929,7 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p if (parse[0] == 'c') { gdb_running_type = 'c'; LOG_DEBUG("target %s continue", target_name(target)); - log_add_callback(gdb_log_callback, connection); + gdb_connection->output_flag = GDB_OUTPUT_ALL; retval = target_resume(target, 1, 0, 0, 0); if (retval == ERROR_TARGET_NOT_HALTED) LOG_INFO("target %s was not halted when resume was requested", target_name(target)); @@ -2746,12 +2967,12 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p packet_size -= 2; thread_id = strtoll(parse, &endp, 16); - if (endp != NULL) { + if (endp) { packet_size -= endp - parse; parse = endp; } - if (target->rtos != NULL) { + if (target->rtos) { /* FIXME: why is this necessary? rtos state should be up-to-date here already! */ rtos_update_threads(target); @@ -2771,13 +2992,11 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p if (parse[0] == 'c') { parse += 1; - packet_size -= 1; /* check if thread-id follows */ if (parse[0] == ':') { int64_t tid; parse += 1; - packet_size -= 1; tid = strtoll(parse, &endp, 16); if (tid == thread_id) { @@ -2799,7 +3018,7 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p } LOG_DEBUG("target %s single-step thread %"PRIx64, target_name(ct), thread_id); - log_add_callback(gdb_log_callback, connection); + gdb_connection->output_flag = GDB_OUTPUT_ALL; target_call_event_callbacks(ct, TARGET_EVENT_GDB_START); /* @@ -2820,7 +3039,7 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p "T05thread:%016"PRIx64";", thread_id); gdb_put_packet(connection, sig_reply, sig_reply_len); - log_remove_callback(gdb_log_callback, connection); + gdb_connection->output_flag = GDB_OUTPUT_NO; return true; } @@ -2829,10 +3048,10 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p if (gdb_connection->sync) { gdb_connection->sync = false; if (ct->state == TARGET_HALTED) { - LOG_WARNING("stepi ignored. GDB will now fetch the register state " \ + LOG_DEBUG("stepi ignored. GDB will now fetch the register state " "from the target."); gdb_sig_halted(connection); - log_remove_callback(gdb_log_callback, connection); + gdb_connection->output_flag = GDB_OUTPUT_NO; } else gdb_connection->frontend_state = TARGET_RUNNING; return true; @@ -2850,7 +3069,7 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p /* send back signal information */ gdb_signal_reply(ct, connection); /* stop forwarding log packets! */ - log_remove_callback(gdb_log_callback, connection); + gdb_connection->output_flag = GDB_OUTPUT_NO; } else gdb_connection->frontend_state = TARGET_RUNNING; } else { @@ -2863,14 +3082,105 @@ static bool gdb_handle_vcont_packet(struct connection *connection, const char *p return false; } +static char *next_hex_encoded_field(const char **str, char sep) +{ + size_t hexlen; + const char *hex = *str; + if (hex[0] == '\0') + return NULL; + + const char *end = strchr(hex, sep); + if (!end) + hexlen = strlen(hex); + else + hexlen = end - hex; + *str = hex + hexlen + 1; + + if (hexlen % 2 != 0) { + /* Malformed hex data */ + return NULL; + } + + size_t count = hexlen / 2; + char *decoded = malloc(count + 1); + if (!decoded) + return NULL; + + size_t converted = unhexify((void *)decoded, hex, count); + if (converted != count) { + free(decoded); + return NULL; + } + + decoded[count] = '\0'; + return decoded; +} + +/* handle extended restart packet */ +static void gdb_restart_inferior(struct connection *connection, const char *packet, int packet_size) +{ + struct gdb_connection *gdb_con = connection->priv; + struct target *target = get_target_from_connection(connection); + + breakpoint_clear_target(target); + watchpoint_clear_target(target); + command_run_linef(connection->cmd_ctx, "ocd_gdb_restart %s", + target_name(target)); + /* set connection as attached after reset */ + gdb_con->attached = true; + /* info rtos parts */ + gdb_thread_packet(connection, packet, packet_size); +} + +static bool gdb_handle_vrun_packet(struct connection *connection, const char *packet, int packet_size) +{ + struct target *target = get_target_from_connection(connection); + const char *parse = packet; + + /* Skip "vRun" */ + parse += 4; + + if (parse[0] != ';') + return false; + parse++; + + /* Skip first field "filename"; don't know what to do with it. */ + free(next_hex_encoded_field(&parse, ';')); + + char *cmdline = next_hex_encoded_field(&parse, ';'); + while (cmdline) { + char *arg = next_hex_encoded_field(&parse, ';'); + if (!arg) + break; + char *new_cmdline = alloc_printf("%s %s", cmdline, arg); + free(cmdline); + free(arg); + cmdline = new_cmdline; + } + + if (cmdline) { + if (target->semihosting) { + LOG_INFO("GDB set inferior command line to '%s'", cmdline); + free(target->semihosting->cmdline); + target->semihosting->cmdline = cmdline; + } else { + LOG_INFO("GDB set inferior command line to '%s' but semihosting is unavailable", cmdline); + free(cmdline); + } + } + + gdb_restart_inferior(connection, packet, packet_size); + gdb_put_packet(connection, "S00", 3); + return true; +} + static int gdb_v_packet(struct connection *connection, char const *packet, int packet_size) { struct gdb_connection *gdb_connection = connection->priv; - struct target *target; int result; - target = get_target_from_connection(connection); + struct target *target = get_target_from_connection(connection); if (strncmp(packet, "vCont", 5) == 0) { bool handled; @@ -2885,6 +3195,16 @@ static int gdb_v_packet(struct connection *connection, return ERROR_OK; } + if (strncmp(packet, "vRun", 4) == 0) { + bool handled; + + handled = gdb_handle_vrun_packet(connection, packet, packet_size); + if (!handled) + gdb_put_packet(connection, "", 0); + + return ERROR_OK; + } + /* if flash programming disabled - send a empty reply */ if (gdb_flash_program == 0) { @@ -2966,7 +3286,7 @@ static int gdb_v_packet(struct connection *connection, length = packet_size - (parse - packet); /* create a new image if there isn't already one */ - if (gdb_connection->vflash_image == NULL) { + if (!gdb_connection->vflash_image) { gdb_connection->vflash_image = malloc(sizeof(struct image)); image_open(gdb_connection->vflash_image, "", "build"); } @@ -2990,7 +3310,7 @@ static int gdb_v_packet(struct connection *connection, target_call_event_callbacks(target, TARGET_EVENT_GDB_FLASH_WRITE_START); result = flash_write(target, gdb_connection->vflash_image, - &written, 0); + &written, false); target_call_event_callbacks(target, TARGET_EVENT_GDB_FLASH_WRITE_END); if (result != ERROR_OK) { @@ -3016,9 +3336,12 @@ static int gdb_v_packet(struct connection *connection, static int gdb_detach(struct connection *connection) { - target_call_event_callbacks(get_target_from_connection(connection), - TARGET_EVENT_GDB_DETACH); - + /* + * Only reply "OK" to GDB + * it will close the connection and this will trigger a call to + * gdb_connection_closed() that will in turn trigger the event + * TARGET_EVENT_GDB_DETACH + */ return gdb_put_packet(connection, "OK", 2); } @@ -3076,6 +3399,10 @@ static void gdb_log_callback(void *priv, const char *file, unsigned line, struct connection *connection = priv; struct gdb_connection *gdb_con = connection->priv; + if (gdb_con->output_flag == GDB_OUTPUT_NO) + /* No out allowed */ + return; + if (gdb_con->busy) { /* do not reply this using the O packet */ return; @@ -3094,14 +3421,14 @@ static void gdb_sig_halted(struct connection *connection) static int gdb_input_inner(struct connection *connection) { /* Do not allocate this on the stack */ - static char gdb_packet_buffer[GDB_BUFFER_SIZE]; + static char gdb_packet_buffer[GDB_BUFFER_SIZE + 1]; /* Extra byte for null-termination */ struct target *target; char const *packet = gdb_packet_buffer; int packet_size; int retval; struct gdb_connection *gdb_con = connection->priv; - static int extended_protocol; + static bool warn_use_ext; target = get_target_from_connection(connection); @@ -3117,7 +3444,7 @@ static int gdb_input_inner(struct connection *connection) * drain the rest of the buffer. */ do { - packet_size = GDB_BUFFER_SIZE-1; + packet_size = GDB_BUFFER_SIZE; retval = gdb_get_packet(connection, gdb_packet_buffer, &packet_size); if (retval != ERROR_OK) return retval; @@ -3125,18 +3452,7 @@ static int gdb_input_inner(struct connection *connection) /* terminate with zero */ gdb_packet_buffer[packet_size] = '\0'; - if (LOG_LEVEL_IS(LOG_LVL_DEBUG)) { - if (packet[0] == 'X') { - /* binary packets spew junk into the debug log stream */ - char buf[50]; - int x; - for (x = 0; (x < 49) && (packet[x] != ':'); x++) - buf[x] = packet[x]; - buf[x] = 0; - LOG_DEBUG("received packet: '%s:'", buf); - } else - LOG_DEBUG("received packet: '%s'", packet); - } + gdb_log_incoming_packet(gdb_packet_buffer); if (packet_size > 0) { retval = ERROR_OK; @@ -3178,12 +3494,18 @@ static int gdb_input_inner(struct connection *connection) break; case '?': gdb_last_signal_packet(connection, packet, packet_size); + /* '?' is sent after the eventual '!' */ + if (!warn_use_ext && !gdb_con->extended_protocol) { + warn_use_ext = true; + LOG_WARNING("Prefer GDB command \"target extended-remote :%s\" instead of \"target remote :%s\"", + connection->service->port, connection->service->port); + } break; case 'c': case 's': { gdb_thread_packet(connection, packet, packet_size); - log_add_callback(gdb_log_callback, connection); + gdb_con->output_flag = GDB_OUTPUT_ALL; if (gdb_con->mem_write_error) { LOG_ERROR("Memory write failure!"); @@ -3201,7 +3523,7 @@ static int gdb_input_inner(struct connection *connection) "Waiting for target to halt."); already_running = true; } else if (target->state != TARGET_HALTED) { - LOG_WARNING("The target is not in the halted nor running stated, " \ + LOG_WARNING("The target is not in the halted nor running stated, " "stepi/continue ignored."); nostep = true; } else if ((packet[0] == 's') && gdb_con->sync) { @@ -3210,7 +3532,7 @@ static int gdb_input_inner(struct connection *connection) * make only the single stepping have the sync feature... */ nostep = true; - LOG_WARNING("stepi ignored. GDB will now fetch the register state " \ + LOG_DEBUG("stepi ignored. GDB will now fetch the register state " "from the target."); } gdb_con->sync = false; @@ -3226,7 +3548,7 @@ static int gdb_input_inner(struct connection *connection) gdb_sig_halted(connection); /* stop forwarding log packets! */ - log_remove_callback(gdb_log_callback, connection); + gdb_con->output_flag = GDB_OUTPUT_NO; } else { /* We're running/stepping, in which case we can * forward log output until the target is halted @@ -3253,7 +3575,6 @@ static int gdb_input_inner(struct connection *connection) break; case 'D': retval = gdb_detach(connection); - extended_protocol = 0; break; case 'X': retval = gdb_write_memory_binary_packet(connection, packet, packet_size); @@ -3261,7 +3582,7 @@ static int gdb_input_inner(struct connection *connection) return retval; break; case 'k': - if (extended_protocol != 0) { + if (gdb_con->extended_protocol) { gdb_con->attached = false; break; } @@ -3269,19 +3590,12 @@ static int gdb_input_inner(struct connection *connection) return ERROR_SERVER_REMOTE_CLOSED; case '!': /* handle extended remote protocol */ - extended_protocol = 1; + gdb_con->extended_protocol = true; gdb_put_packet(connection, "OK", 2); break; case 'R': /* handle extended restart packet */ - breakpoint_clear_target(target); - watchpoint_clear_target(target); - command_run_linef(connection->cmd_ctx, "ocd_gdb_restart %s", - target_name(target)); - /* set connection as attached after reset */ - gdb_con->attached = true; - /* info rtos parts */ - gdb_thread_packet(connection, packet, packet_size); + gdb_restart_inferior(connection, packet, packet_size); break; case 'j': @@ -3306,14 +3620,14 @@ static int gdb_input_inner(struct connection *connection) * Fretcode,errno,Ctrl-C flag;call-specific attachment */ gdb_con->frontend_state = TARGET_RUNNING; - log_add_callback(gdb_log_callback, connection); + gdb_con->output_flag = GDB_OUTPUT_ALL; gdb_fileio_response_packet(connection, packet, packet_size); break; default: /* ignore unknown packets */ LOG_DEBUG("ignoring 0x%2.2x packet", packet[0]); - gdb_put_packet(connection, NULL, 0); + gdb_put_packet(connection, "", 0); break; } @@ -3332,7 +3646,7 @@ static int gdb_input_inner(struct connection *connection) retval = target_poll(t); if (retval != ERROR_OK) target_call_event_callbacks(target, TARGET_EVENT_GDB_HALT); - gdb_con->ctrl_c = 0; + gdb_con->ctrl_c = false; } else { LOG_INFO("The target is not running when halt was requested, stopping GDB."); target_call_event_callbacks(target, TARGET_EVENT_GDB_HALT); @@ -3359,33 +3673,61 @@ static int gdb_input(struct connection *connection) return ERROR_OK; } +static void gdb_keep_client_alive(struct connection *connection) +{ + struct gdb_connection *gdb_con = connection->priv; + + if (gdb_con->busy) { + /* do not send packets, retry asap */ + return; + } + + switch (gdb_con->output_flag) { + case GDB_OUTPUT_NO: + /* no need for keep-alive */ + break; + case GDB_OUTPUT_ALL: + /* send an empty O packet */ + gdb_output_con(connection, ""); + break; + default: + break; + } +} + +static const struct service_driver gdb_service_driver = { + .name = "gdb", + .new_connection_during_keep_alive_handler = NULL, + .new_connection_handler = gdb_new_connection, + .input_handler = gdb_input, + .connection_closed_handler = gdb_connection_closed, + .keep_client_alive_handler = gdb_keep_client_alive, +}; + static int gdb_target_start(struct target *target, const char *port) { struct gdb_service *gdb_service; int ret; gdb_service = malloc(sizeof(struct gdb_service)); - if (NULL == gdb_service) + if (!gdb_service) return -ENOMEM; + LOG_INFO("starting gdb server for %s on %s", target_name(target), port); + gdb_service->target = target; gdb_service->core[0] = -1; gdb_service->core[1] = -1; target->gdb_service = gdb_service; - ret = add_service("gdb", - port, 1, &gdb_new_connection, &gdb_input, - &gdb_connection_closed, gdb_service); - /* initialialize all targets gdb service with the same pointer */ + ret = add_service(&gdb_service_driver, port, target->gdb_max_connections, gdb_service); + /* initialize all targets gdb service with the same pointer */ { struct target_list *head; - struct target *curr; - head = target->head; - while (head != (struct target_list *)NULL) { - curr = head->target; + foreach_smp_target(head, target->smp_targets) { + struct target *curr = head->target; if (curr != target) curr->gdb_service = gdb_service; - head = head->next; } } return ret; @@ -3393,16 +3735,36 @@ static int gdb_target_start(struct target *target, const char *port) static int gdb_target_add_one(struct target *target) { + /* one gdb instance per smp list */ + if ((target->smp) && (target->gdb_service)) + return ERROR_OK; + + /* skip targets that cannot handle a gdb connections (e.g. mem_ap) */ + if (!target_supports_gdb_connection(target)) { + LOG_DEBUG("skip gdb server for target %s", target_name(target)); + return ERROR_OK; + } + + if (target->gdb_port_override) { + if (strcmp(target->gdb_port_override, "disabled") == 0) { + LOG_INFO("gdb port disabled"); + return ERROR_OK; + } + return gdb_target_start(target, target->gdb_port_override); + } + if (strcmp(gdb_port, "disabled") == 0) { LOG_INFO("gdb port disabled"); return ERROR_OK; } - /* one gdb instance per smp list */ - if ((target->smp) && (target->gdb_service)) - return ERROR_OK; int retval = gdb_target_start(target, gdb_port_next); if (retval == ERROR_OK) { + /* save the port number so can be queried with + * $target_name cget -gdb-port + */ + target->gdb_port_override = strdup(gdb_port_next); + long portnumber; /* If we can parse the port number * then we increment the port number for the next target. @@ -3413,7 +3775,7 @@ static int gdb_target_add_one(struct target *target) if (parse_long(gdb_port_next, &portnumber) == ERROR_OK) { free(gdb_port_next); if (portnumber) { - gdb_port_next = alloc_printf("%d", portnumber+1); + gdb_port_next = alloc_printf("%ld", portnumber+1); } else { /* Don't increment if gdb_port is 0, since we're just * trying to allocate an unused port. */ @@ -3427,19 +3789,14 @@ static int gdb_target_add_one(struct target *target) int gdb_target_add_all(struct target *target) { - if (strcmp(gdb_port, "disabled") == 0) { - LOG_INFO("gdb server disabled"); - return ERROR_OK; - } - - if (NULL == target) { + if (!target) { LOG_WARNING("gdb services need one or more targets defined"); return ERROR_OK; } - while (NULL != target) { + while (target) { int retval = gdb_target_add_one(target); - if (ERROR_OK != retval) + if (retval != ERROR_OK) return retval; target = target->next; @@ -3453,8 +3810,8 @@ COMMAND_HANDLER(handle_gdb_sync_command) if (CMD_ARGC != 0) return ERROR_COMMAND_SYNTAX_ERROR; - if (current_gdb_connection == NULL) { - command_print(CMD_CTX, + if (!current_gdb_connection) { + command_print(CMD, "gdb_sync command can only be run from within gdb using \"monitor gdb_sync\""); return ERROR_FAIL; } @@ -3468,7 +3825,7 @@ COMMAND_HANDLER(handle_gdb_sync_command) COMMAND_HANDLER(handle_gdb_port_command) { int retval = CALL_COMMAND_HANDLER(server_pipe_command, &gdb_port); - if (ERROR_OK == retval) { + if (retval == ERROR_OK) { free(gdb_port_next); gdb_port_next = strdup(gdb_port); } @@ -3562,7 +3919,7 @@ COMMAND_HANDLER(handle_gdb_save_tdesc_command) size_t size_written; char *tdesc_filename = alloc_printf("%s.xml", target_type_name(target)); - if (tdesc_filename == NULL) { + if (!tdesc_filename) { retval = ERROR_FAIL; goto out; } @@ -3601,7 +3958,7 @@ static const struct command_registration gdb_command_handlers[] = { { .name = "gdb_port", .handler = handle_gdb_port_command, - .mode = COMMAND_ANY, + .mode = COMMAND_CONFIG, .help = "Normally gdb listens to a TCP/IP port. Each subsequent GDB " "server listens for the next port number after the " "base port number specified. " @@ -3659,6 +4016,7 @@ static const struct command_registration gdb_command_handlers[] = { .handler = handle_gdb_save_tdesc_command, .mode = COMMAND_EXEC, .help = "Save the target description file", + .usage = "", }, COMMAND_REGISTRATION_DONE };