gdbserver: gdb cmds returning failure on success
[fw/openocd] / src / server / gdb_server.c
index b2527d21d818e2f12324b0ca576c1a5a11423b41..a84c618aff084177becbf0c85e9918e0a880c383 100644 (file)
@@ -2,7 +2,7 @@
  *   Copyright (C) 2005 by Dominic Rath                                    *
  *   Dominic.Rath@gmx.de                                                   *
  *                                                                         *
- *   Copyright (C) 2007-2009 Øyvind Harboe                                 *
+ *   Copyright (C) 2007-2010 Øyvind Harboe                                 *
  *   oyvind.harboe@zylin.com                                               *
  *                                                                         *
  *   Copyright (C) 2008 by Spencer Oliver                                  *
@@ -61,7 +61,12 @@ struct gdb_connection
        bool sync;      /* set flag to true if you want the next stepi to return immediately.
                       allowing GDB to pick up a fresh set of register values from the target
                       without modifying the target state. */
-
+       /* We delay reporting memory write errors until next step/continue or memory
+        * write. This improves performance of gdb load significantly as the GDB packet
+        * can be replied immediately and a new GDB packet will be ready without delay
+        * (ca. 10% or so...).
+        */
+       bool mem_write_error;
 };
 
 
@@ -166,6 +171,9 @@ static int gdb_get_char_inner(struct connection *connection, int* next_char)
        struct gdb_connection *gdb_con = connection->priv;
        int retval = ERROR_OK;
 
+#ifdef _DEBUG_GDB_IO_
+       char *debug_buffer;
+#endif
        for (;;)
        {
                if (connection->service->type == CONNECTION_PIPE)
@@ -821,6 +829,7 @@ static int gdb_new_connection(struct connection *connection)
        gdb_connection->busy = 0;
        gdb_connection->noack_mode = 0;
        gdb_connection->sync = true;
+       gdb_connection->mem_write_error = false;
 
        /* send ACK to GDB for debug request */
        gdb_write(connection, "+", 1);
@@ -849,6 +858,26 @@ static int gdb_new_connection(struct connection *connection)
                gdb_putback_char(connection, initial_ack);
        target_call_event_callbacks(gdb_service->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++)
+               {
+                       struct flash_bank *p;
+                       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 to prepare target for GDB connect.");
+                               return retval;
+                       }
+               }
+       }
+
        gdb_actual_connections++;
        LOG_DEBUG("New GDB Connection: %d, Target %s, state: %s",
                  gdb_actual_connections,
@@ -1361,7 +1390,7 @@ static int gdb_write_memory_binary_packet(struct connection *connection,
        uint32_t addr = 0;
        uint32_t len = 0;
 
-       int retval;
+       int retval = ERROR_OK;
 
        /* skip command character */
        packet++;
@@ -1382,14 +1411,18 @@ static int gdb_write_memory_binary_packet(struct connection *connection,
                return ERROR_SERVER_REMOTE_CLOSED;
        }
 
-       retval = ERROR_OK;
-       if (len)
-       {
-               LOG_DEBUG("addr: 0x%8.8" PRIx32 ", len: 0x%8.8" PRIx32 "", addr, len);
+       struct gdb_connection *gdb_connection = connection->priv;
 
-               retval = target_write_buffer(target, addr, len, (uint8_t*)separator);
+       if (gdb_connection->mem_write_error)
+       {
+               retval = ERROR_FAIL;
+               /* now that we have reported the memory write error, we can clear the condition */
+               gdb_connection->mem_write_error = false;
        }
 
+       /* By replying the packet *immediately* GDB will send us a new packet
+        * while we write the last one to the target.
+        */
        if (retval == ERROR_OK)
        {
                gdb_put_packet(connection, "OK", 2);
@@ -1400,6 +1433,17 @@ static int gdb_write_memory_binary_packet(struct connection *connection,
                        return retval;
        }
 
+       if (len)
+       {
+               LOG_DEBUG("addr: 0x%8.8" PRIx32 ", len: 0x%8.8" PRIx32 "", addr, len);
+
+               retval = target_write_buffer(target, addr, len, (uint8_t*)separator);
+               if (retval != ERROR_OK)
+               {
+                       gdb_connection->mem_write_error = true;
+               }
+       }
+
        return ERROR_OK;
 }
 
@@ -1613,22 +1657,6 @@ static int decode_xfer_read(char *buf, char **annex, int *ofs, unsigned int *len
        return 0;
 }
 
-static int gdb_calc_blocksize(struct flash_bank *bank)
-{
-       uint32_t i;
-       uint32_t block_size = 0xffffffff;
-
-       /* loop through all sectors and return smallest sector size */
-
-       for (i = 0; i < (uint32_t)bank->num_sectors; i++)
-       {
-               if (bank->sectors[i].size < block_size)
-                       block_size = bank->sectors[i].size;
-       }
-
-       return block_size;
-}
-
 static int compare_bank (const void * a, const void * b)
 {
        struct flash_bank *b1, *b2;
@@ -1647,8 +1675,151 @@ static int compare_bank (const void * a, const void * b)
        }
 }
 
-static int gdb_query_packet(struct connection *connection,
+static int gdb_memory_map(struct connection *connection,
                struct target *target, char *packet, int packet_size)
+{
+       /* We get away with only specifying flash here. Regions that are not
+        * specified are treated as if we provided no memory map(if not we
+        * could detect the holes and mark them as RAM).
+        * Normally we only execute this code once, but no big deal if we
+        * have to regenerate it a couple of times.
+        */
+
+       struct flash_bank *p;
+       char *xml = NULL;
+       int size = 0;
+       int pos = 0;
+       int retval = ERROR_OK;
+       struct flash_bank **banks;
+       int offset;
+       int length;
+       char *separator;
+       uint32_t ram_start = 0;
+       int i;
+
+       /* skip command character */
+       packet += 23;
+
+       offset = strtoul(packet, &separator, 16);
+       length = strtoul(separator + 1, &separator, 16);
+
+       xml_printf(&retval, &xml, &pos, &size, "<memory-map>\n");
+
+       /* Sort banks in ascending order.  We need to report non-flash
+        * memory as ram (or rather read/write) by default for GDB, since
+        * it has no concept of non-cacheable read/write memory (i/o etc).
+        *
+        * FIXME Most non-flash addresses are *NOT* RAM!  Don't lie.
+        * Current versions of GDB assume unlisted addresses are RAM...
+        */
+       banks = malloc(sizeof(struct flash_bank *)*flash_get_bank_count());
+
+       for (i = 0; i < flash_get_bank_count(); i++) {
+               retval = get_flash_bank_by_num(i, &p);
+               if (retval != ERROR_OK)
+               {
+                       free(banks);
+                       gdb_send_error(connection, retval);
+                       return retval;
+               }
+               banks[i] = p;
+       }
+
+       qsort(banks, flash_get_bank_count(), sizeof(struct flash_bank *),
+                       compare_bank);
+
+       for (i = 0; i < flash_get_bank_count(); i++) {
+               int j;
+               unsigned sector_size = 0;
+               uint32_t start, end;
+
+               p = banks[i];
+               start = p->base;
+               end = p->base + p->size;
+
+               if (ram_start < p->base)
+                       xml_printf(&retval, &xml, &pos, &size,
+                               "<memory type=\"ram\" start=\"0x%x\" "
+                                       "length=\"0x%x\"/>\n",
+                               ram_start, p->base - ram_start);
+
+               /* Report adjacent groups of same-size sectors.  So for
+                * example top boot CFI flash will list an initial region
+                * with several large sectors (maybe 128KB) and several
+                * 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++) {
+                       unsigned group_len;
+
+                       /* Maybe start a new group of sectors. */
+                       if (sector_size == 0) {
+                               start = p->base + p->sectors[j].offset;
+                               xml_printf(&retval, &xml, &pos, &size,
+                                       "<memory type=\"flash\" "
+                                               "start=\"0x%x\" ",
+                                       start);
+                               sector_size = p->sectors[j].size;
+                       }
+
+                       /* Does this finish a group of sectors?
+                        * If not, continue an already-started group.
+                        */
+                       if (j == p->num_sectors -1)
+                               group_len = (p->base + p->size) - start;
+                       else if (p->sectors[j + 1].size != sector_size)
+                               group_len = p->base + p->sectors[j + 1].offset
+                                               - start;
+                       else
+                               continue;
+
+                       xml_printf(&retval, &xml, &pos, &size,
+                               "length=\"0x%x\">\n"
+                               "<property name=\"blocksize\">"
+                                       "0x%x</property>\n"
+                               "</memory>\n",
+                               group_len,
+                               sector_size);
+                       sector_size = 0;
+               }
+
+               ram_start = p->base + p->size;
+       }
+
+       if (ram_start != 0)
+               xml_printf(&retval, &xml, &pos, &size,
+                       "<memory type=\"ram\" start=\"0x%x\" "
+                               "length=\"0x%x\"/>\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
+        */
+
+       free(banks);
+       banks = NULL;
+
+       xml_printf(&retval, &xml, &pos, &size, "</memory-map>\n");
+
+       if (retval != ERROR_OK) {
+               gdb_send_error(connection, retval);
+               return retval;
+       }
+
+       if (offset + length > pos)
+               length = pos - offset;
+
+       char *t = malloc(length + 1);
+       t[0] = 'l';
+       memcpy(t + 1, xml + offset, length);
+       gdb_put_packet(connection, t, length + 1);
+
+       free(t);
+       free(xml);
+       return ERROR_OK;
+}
+
+static int gdb_query_packet(struct connection *connection,
+       struct target *target, char *packet, int packet_size)
 {
        struct command_context *cmd_ctx = connection->cmd_ctx;
        struct gdb_connection *gdb_connection = connection->priv;
@@ -1747,112 +1918,9 @@ static int gdb_query_packet(struct connection *connection,
 
                return ERROR_OK;
        }
-       else if (strstr(packet, "qXfer:memory-map:read::") && (flash_get_bank_count() > 0))
-       {
-               /* We get away with only specifying flash here. Regions that are not
-                * specified are treated as if we provided no memory map(if not we
-                * could detect the holes and mark them as RAM).
-                * Normally we only execute this code once, but no big deal if we
-                * have to regenerate it a couple of times. */
-
-               struct flash_bank *p;
-               char *xml = NULL;
-               int size = 0;
-               int pos = 0;
-               int retval = ERROR_OK;
-
-               int offset;
-               int length;
-               char *separator;
-               int blocksize;
-
-               /* skip command character */
-               packet += 23;
-
-               offset = strtoul(packet, &separator, 16);
-               length = strtoul(separator + 1, &separator, 16);
-
-               xml_printf(&retval, &xml, &pos, &size, "<memory-map>\n");
-
-               /*
-               sort banks in ascending order, we need to make non-flash memory be ram(or rather
-               read/write) by default for GDB.
-               GDB does not have a concept of non-cacheable read/write memory.
-                */
-               struct flash_bank **banks = malloc(sizeof(struct flash_bank *)*flash_get_bank_count());
-               int i;
-
-               for (i = 0; i < flash_get_bank_count(); i++)
-               {
-                       p = get_flash_bank_by_num(i);
-                       if (p == NULL)
-                       {
-                               free(banks);
-                               retval = ERROR_FAIL;
-                               gdb_send_error(connection, retval);
-                               return retval;
-                       }
-                       banks[i]=p;
-               }
-
-               qsort(banks, flash_get_bank_count(), sizeof(struct flash_bank *), compare_bank);
-
-               uint32_t ram_start = 0;
-               for (i = 0; i < flash_get_bank_count(); i++)
-               {
-                       p = banks[i];
-
-                       if (ram_start < p->base)
-                       {
-                               xml_printf(&retval, &xml, &pos, &size, "<memory type=\"ram\" start=\"0x%x\" length=\"0x%x\"/>\n",
-                                       ram_start, p->base-ram_start);
-                       }
-
-                       /* if device has uneven sector sizes, eg. str7, lpc
-                        * we pass the smallest sector size to gdb memory map */
-                       blocksize = gdb_calc_blocksize(p);
-
-                       xml_printf(&retval, &xml, &pos, &size, "<memory type=\"flash\" start=\"0x%x\" length=\"0x%x\">\n" \
-                               "<property name=\"blocksize\">0x%x</property>\n" \
-                               "</memory>\n", \
-                               p->base, p->size, blocksize);
-                       ram_start = p->base + p->size;
-               }
-               if (ram_start != 0)
-               {
-                       xml_printf(&retval, &xml, &pos, &size, "<memory type=\"ram\" start=\"0x%x\" length=\"0x%x\"/>\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 */
-               }
-
-               free(banks);
-               banks = NULL;
-
-               xml_printf(&retval, &xml, &pos, &size, "</memory-map>\n");
-
-               if (retval != ERROR_OK)
-               {
-                       gdb_send_error(connection, retval);
-                       return retval;
-               }
-
-               if (offset + length > pos)
-               {
-                       length = pos - offset;
-               }
-
-               char *t = malloc(length + 1);
-               t[0] = 'l';
-               memcpy(t + 1, xml + offset, length);
-               gdb_put_packet(connection, t, length + 1);
-
-               free(t);
-               free(xml);
-               return ERROR_OK;
-       }
+       else if (strstr(packet, "qXfer:memory-map:read::")
+                       && (flash_get_bank_count() > 0))
+               return gdb_memory_map(connection, target, packet, packet_size);
        else if (strstr(packet, "qXfer:features:read:"))
        {
                char *xml = NULL;
@@ -2187,13 +2255,22 @@ static int gdb_input_inner(struct connection *connection)
                                                struct gdb_connection *gdb_con = connection->priv;
                                                log_add_callback(gdb_log_callback, connection);
 
+                                               if (gdb_con->mem_write_error)
+                                               {
+                                                       LOG_ERROR("Memory write failure!");
+
+                                                       /* now that we have reported the memory write error, we can clear the condition */
+                                                       gdb_con->mem_write_error = false;
+                                               }
+
                                                bool nostep = false;
+                                               bool already_running = false;
                                                if (target->state == TARGET_RUNNING)
                                                {
-                                                       LOG_WARNING("The target is already running. Halt target before stepi/continue.");
-                                                       retval = target_halt(target);
-                                                       if (retval == ERROR_OK)
-                                                               retval = target_wait_state(target, TARGET_HALTED, 100);
+                                                       LOG_WARNING("WARNING! The target is already running. "
+                                                                       "All changes GDB did to registers will be discarded! "
+                                                                       "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, stepi/continue ignored.");
@@ -2209,7 +2286,7 @@ static int gdb_input_inner(struct connection *connection)
                                                }
                                                gdb_con->sync = false;
 
-                                               if ((retval!=ERROR_OK) || nostep)
+                                               if ((retval!=ERROR_OK) || (!already_running && nostep))
                                                {
                                                        /* Either the target isn't in the halted state, then we can't
                                                         * step/continue. This might be early setup, etc.
@@ -2229,11 +2306,15 @@ static int gdb_input_inner(struct connection *connection)
                                                         */
                                                        gdb_con->frontend_state = TARGET_RUNNING;
                                                        target_call_event_callbacks(target, TARGET_EVENT_GDB_START);
-                                                       int retval = gdb_step_continue_packet(connection, target, packet, packet_size);
-                                                       if (retval != ERROR_OK)
+
+                                                       if (!already_running)
                                                        {
-                                                               /* we'll never receive a halted condition... issue a false one.. */
-                                                               gdb_frontend_halted(target, connection);
+                                                               int retval = gdb_step_continue_packet(connection, target, packet, packet_size);
+                                                               if (retval != ERROR_OK)
+                                                               {
+                                                                       /* we'll never receive a halted condition... issue a false one.. */
+                                                                       gdb_frontend_halted(target, connection);
+                                                               }
                                                        }
                                                }
                                        }
@@ -2422,26 +2503,29 @@ COMMAND_HANDLER(handle_gdb_port_command)
 
 COMMAND_HANDLER(handle_gdb_memory_map_command)
 {
-       if (CMD_ARGC == 1)
-               COMMAND_PARSE_ENABLE(CMD_ARGV[0], gdb_use_memory_map);
+       if (CMD_ARGC != 1)
+               return ERROR_COMMAND_SYNTAX_ERROR;
 
-       return ERROR_COMMAND_SYNTAX_ERROR;
+       COMMAND_PARSE_ENABLE(CMD_ARGV[0], gdb_use_memory_map);
+       return ERROR_OK;
 }
 
 COMMAND_HANDLER(handle_gdb_flash_program_command)
 {
-       if (CMD_ARGC == 1)
-               COMMAND_PARSE_ENABLE(CMD_ARGV[0], gdb_flash_program);
+       if (CMD_ARGC != 1)
+               return ERROR_COMMAND_SYNTAX_ERROR;
 
-       return ERROR_COMMAND_SYNTAX_ERROR;
+       COMMAND_PARSE_ENABLE(CMD_ARGV[0], gdb_flash_program);
+       return ERROR_OK;
 }
 
 COMMAND_HANDLER(handle_gdb_report_data_abort_command)
 {
-       if (CMD_ARGC == 1)
-               COMMAND_PARSE_ENABLE(CMD_ARGV[0], gdb_report_data_abort);
+       if (CMD_ARGC != 1)
+               return ERROR_COMMAND_SYNTAX_ERROR;
 
-       return ERROR_COMMAND_SYNTAX_ERROR;
+       COMMAND_PARSE_ENABLE(CMD_ARGV[0], gdb_report_data_abort);
+       return ERROR_OK;
 }
 
 /* gdb_breakpoint_override */