From f476c9eec42d551bc9015089abc47b09058f06b5 Mon Sep 17 00:00:00 2001 From: Antonio Borneo Date: Thu, 17 Oct 2019 18:02:38 +0200 Subject: [PATCH] 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 Reviewed-on: http://openocd.zylin.com/5322 Tested-by: jenkins Reviewed-by: Steven Stallion Reviewed-by: Muhammad Omair Javaid Reviewed-by: Tomas Vanek --- src/server/gdb_server.c | 10 +++++----- src/target/nds32.c | 10 +++++----- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c index 49939a5cb..12c03a515 100644 --- 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 4115ea4f2..f40ce534b 100644 --- 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: -- 2.47.2