gdbserver: gdb cmds returning failure on success
[openocd.git] / src / server / gdb_server.c
index 6ed7243d28498f1889caca7c4d72936b4628367c..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;
@@ -1666,7 +1694,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,14 +1710,15 @@ 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);
                        return retval;
                }
@@ -1701,29 +1729,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;
        }
 
@@ -2196,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.");
@@ -2218,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.
@@ -2238,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);
+                                                               }
                                                        }
                                                }
                                        }
@@ -2431,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 */

Linking to existing account procedure

If you already have an account and want to add another login method you MUST first sign in with your existing account and then change URL to read https://review.openocd.org/login/?link to get to this page again but this time it'll work for linking. Thank you.

SSH host keys fingerprints

1024 SHA256:YKx8b7u5ZWdcbp7/4AeXNaqElP49m6QrwfXaqQGJAOk gerrit-code-review@openocd.zylin.com (DSA)
384 SHA256:jHIbSQa4REvwCFG4cq5LBlBLxmxSqelQPem/EXIrxjk gerrit-code-review@openocd.org (ECDSA)
521 SHA256:UAOPYkU9Fjtcao0Ul/Rrlnj/OsQvt+pgdYSZ4jOYdgs gerrit-code-review@openocd.org (ECDSA)
256 SHA256:A13M5QlnozFOvTllybRZH6vm7iSt0XLxbA48yfc2yfY gerrit-code-review@openocd.org (ECDSA)
256 SHA256:spYMBqEYoAOtK7yZBrcwE8ZpYt6b68Cfh9yEVetvbXg gerrit-code-review@openocd.org (ED25519)
+--[ED25519 256]--+
|=..              |
|+o..   .         |
|*.o   . .        |
|+B . . .         |
|Bo. = o S        |
|Oo.+ + =         |
|oB=.* = . o      |
| =+=.+   + E     |
|. .=o   . o      |
+----[SHA256]-----+
2048 SHA256:0Onrb7/PHjpo6iVZ7xQX2riKN83FJ3KGU0TvI0TaFG4 gerrit-code-review@openocd.zylin.com (RSA)