gdb_server: fix string length with semihosting_fileio

The GDB file-I/O remote protocol extension, used for implementing
the semihosting file I/O, requires the length of strings to
include the trailing zero character, as explicitly stated inside a
comment in GDB source code [1]:
	/* 1. Parameter: Ptr to pathname / length incl. trailing zero.  */

ARM specification for semihosting [2] requires the string length
to not include the trailing zero character, e.g. in SYS_OPEN
specifications:
	"field 3: An integer that gives the length of the string
	 pointed to by field 1. The length does not include the
	 terminating null character that must be present."

The mismatch above requires OpenOCD to add "one" to the string
length before passing it to GDB. Such conversion is missing
either in the generic semihosting provider of the data, the
function semihosting_common(), and in the consumer of the data,
the gdb_server function gdb_fileio_reply().
The conversion is already implemented in the target specific
function nds32_get_gdb_fileio_info(), but it's not the preferred
place for such GDB specific requirement.

This issue affects the semihosting calls "open", "unlink",
"rename" and "system".

Remove the "+1" conversion from nds32_get_gdb_fileio_info().
Add the "+1" conversion in gdb_fileio_reply().

[1] http://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;f=gdb/remote-fileio.c;h=11c141e42c4d#l381
[2] "Semihosting for AArch32 and AArch64, Release 2.0"
    https://static.docs.arm.com/100863/0200/semihosting.pdf

Change-Id: I35461bcb30f734fe2d51f7f0d418e3d04b4af506
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: http://openocd.zylin.com/5322
Tested-by: jenkins
Reviewed-by: Steven Stallion <sstallion@gmail.com>
Reviewed-by: Muhammad Omair Javaid <omair.javaid@linaro.org>
Reviewed-by: Tomas Vanek <vanekt@fbl.cz>

diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index 49939a5cba22a98e2a684ea36a3dab820ce972e0..12c03a51547fa0d1896c000d14910ef012dc5484 100644 (file)
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -800,7 +800,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)
@@ -824,13 +824,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,
@@ -850,7 +850,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. */

diff --git a/src/target/nds32.c b/src/target/nds32.c
index 4115ea4f24f7553041859e7c56a63403efa8934f..f40ce534b2707eefe77819aed038c79f74df8d60 100644 (file)
--- a/src/target/nds32.c
+++ b/src/target/nds32.c
@@ -2361,7 +2361,7 @@ int nds32_get_gdb_fileio_info(struct target *target, struct gdb_fileio_info *fil
                                fileio_info->param_4 = reg_r2;
 
                                target->type->read_buffer(target, reg_r0, 256, filename);
-                               fileio_info->param_2 = strlen((char *)filename) + 1;
+                               fileio_info->param_2 = strlen((char *)filename);
                        }
                        break;
                case NDS32_SYSCALL_CLOSE:
@@ -2399,7 +2399,7 @@ int nds32_get_gdb_fileio_info(struct target *target, struct gdb_fileio_info *fil
                                /* reserve fileio_info->param_2 for length of path */
 
                                target->type->read_buffer(target, reg_r0, 256, filename);
-                               fileio_info->param_2 = strlen((char *)filename) + 1;
+                               fileio_info->param_2 = strlen((char *)filename);
                        }
                        break;
                case NDS32_SYSCALL_RENAME:
@@ -2413,10 +2413,10 @@ int nds32_get_gdb_fileio_info(struct target *target, struct gdb_fileio_info *fil
                                /* reserve fileio_info->param_4 for length of new path */
 
                                target->type->read_buffer(target, reg_r0, 256, filename);
-                               fileio_info->param_2 = strlen((char *)filename) + 1;
+                               fileio_info->param_2 = strlen((char *)filename);
 
                                target->type->read_buffer(target, reg_r1, 256, filename);
-                               fileio_info->param_4 = strlen((char *)filename) + 1;
+                               fileio_info->param_4 = strlen((char *)filename);
                        }
                        break;
                case NDS32_SYSCALL_FSTAT:
@@ -2458,7 +2458,7 @@ int nds32_get_gdb_fileio_info(struct target *target, struct gdb_fileio_info *fil
                                /* reserve fileio_info->param_2 for length of old path */
 
                                target->type->read_buffer(target, reg_r0, 256, command);
-                               fileio_info->param_2 = strlen((char *)command) + 1;
+                               fileio_info->param_2 = strlen((char *)command);
                        }
                        break;
                case NDS32_SYSCALL_ERRNO: