gdb: -Wshadow warning fixes
[fw/openocd] / src / server / gdb_server.c
index 6ed7243d28498f1889caca7c4d72936b4628367c..a1ff7c8ed669e112d2f6d26f5db8d8c8448040a1 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,
@@ -1190,29 +1219,14 @@ static int gdb_set_register_packet(struct connection *connection,
        return ERROR_OK;
 }
 
+/* No attempt is made to translate the "retval" to
+ * GDB speak. This has to be done at the calling
+ * site as no mapping really exists.
+ */
 static int gdb_error(struct connection *connection, int retval)
 {
-       switch (retval)
-       {
-               case ERROR_TARGET_DATA_ABORT:
-                       gdb_send_error(connection, EIO);
-                       break;
-               case ERROR_TARGET_TRANSLATION_FAULT:
-                       gdb_send_error(connection, EFAULT);
-                       break;
-               case ERROR_TARGET_UNALIGNED_ACCESS:
-                       gdb_send_error(connection, EFAULT);
-                       break;
-               case ERROR_TARGET_NOT_HALTED:
-                       gdb_send_error(connection, EFAULT);
-                       break;
-               default:
-                       /* This could be that the target reset itself. */
-                       LOG_ERROR("unexpected error %i", retval);
-                       gdb_send_error(connection, EFAULT);
-                       break;
-       }
-
+       LOG_DEBUG("Reporting %i to GDB as generic error", retval);
+       gdb_send_error(connection, EFAULT);
        return ERROR_OK;
 }
 
@@ -1361,7 +1375,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 +1396,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 +1418,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 +1642,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;
@@ -1666,7 +1679,6 @@ static int gdb_memory_map(struct connection *connection,
        int offset;
        int length;
        char *separator;
-       int blocksize;
        uint32_t ram_start = 0;
        int i;
 
@@ -1683,15 +1695,16 @@ static int gdb_memory_map(struct connection *connection,
         * 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++) {
-               p = get_flash_bank_by_num(i);
-               if (p == NULL) {
+               retval = get_flash_bank_by_num(i, &p);
+               if (retval != ERROR_OK)
+               {
                        free(banks);
-                       retval = ERROR_FAIL;
-                       gdb_send_error(connection, retval);
+                       gdb_error(connection, retval);
                        return retval;
                }
                banks[i] = p;
@@ -1701,29 +1714,60 @@ static int gdb_memory_map(struct connection *connection,
                        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);
+                               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
-                *
-                * FIXME Don't lie about flash regions with different
-                * sector sizes; just tell GDB about each region as
-                * if it were a separate flash device.
+               /* 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.
                 */
-               blocksize = gdb_calc_blocksize(p);
+               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;
+               }
 
-               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;
        }
 
@@ -1742,7 +1786,7 @@ static int gdb_memory_map(struct connection *connection,
        xml_printf(&retval, &xml, &pos, &size, "</memory-map>\n");
 
        if (retval != ERROR_OK) {
-               gdb_send_error(connection, retval);
+               gdb_error(connection, retval);
                return retval;
        }
 
@@ -1893,7 +1937,7 @@ static int gdb_query_packet(struct connection *connection,
 
                if (retval != ERROR_OK)
                {
-                       gdb_send_error(connection, retval);
+                       gdb_error(connection, retval);
                        return retval;
                }
 
@@ -2107,7 +2151,17 @@ static int gdb_input_inner(struct connection *connection)
        struct gdb_connection *gdb_con = connection->priv;
        static int extended_protocol = 0;
 
-       /* drain input buffer */
+       /* drain input buffer. If one of the packets fail, then an error
+        * packet is replied, if applicable.
+        *
+        * This loop will terminate and the error code is returned.
+        *
+        * The calling fn will check if this error is something that
+        * can be recovered from, or if the connection must be closed.
+        *
+        * If the error is recoverable, this fn is called again to
+        * drain the rest of the buffer.
+        */
        do
        {
                packet_size = GDB_BUFFER_SIZE-1;
@@ -2191,18 +2245,24 @@ static int gdb_input_inner(struct connection *connection)
                                case 'c':
                                case 's':
                                        {
-                                               int retval = ERROR_OK;
-
-                                               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.");
@@ -2218,7 +2278,7 @@ static int gdb_input_inner(struct connection *connection)
                                                }
                                                gdb_con->sync = false;
 
-                                               if ((retval!=ERROR_OK) || nostep)
+                                               if (!already_running && nostep)
                                                {
                                                        /* Either the target isn't in the halted state, then we can't
                                                         * step/continue. This might be early setup, etc.
@@ -2238,11 +2298,17 @@ 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);
+                                                               /* Here we don't want packet processing to stop even if this fails,
+                                                                * so we use a local variable instead of retval. */
+                                                               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);
+                                                               }
                                                        }
                                                }
                                        }
@@ -2431,26 +2497,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 */