From c1abc8ae74ec1d2926b460fe4a58bcb0e712da0f Mon Sep 17 00:00:00 2001 From: Don Reid Date: Thu, 26 Jul 2018 09:02:04 -0700 Subject: [PATCH 1/5] Use local variable for read_result instead of *ret, and fix calculation of *ret for EOF case. --- src/gdbserver/semihosting.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/gdbserver/semihosting.c b/src/gdbserver/semihosting.c index 312b1517e..75fc6b1ef 100644 --- a/src/gdbserver/semihosting.c +++ b/src/gdbserver/semihosting.c @@ -307,6 +307,7 @@ int do_semihosting (stlink_t *sl, uint32_t r0, uint32_t r1, uint32_t *ret) { int fd; uint32_t buffer_len; void *buffer; + int read_result; if (mem_read(sl, r1, args, sizeof (args)) != 0 ) { DLOG("Semihosting SYS_READ error: " @@ -337,7 +338,7 @@ int do_semihosting (stlink_t *sl, uint32_t r0, uint32_t r1, uint32_t *ret) { DLOG("Semihosting: read(%d, target_addr:0x%08x, %zu)\n", fd, buffer_address, buffer_len); - *ret = (uint32_t)read(fd, buffer, buffer_len); + read_result = (uint32_t)read(fd, buffer, buffer_len); saved_errno = errno; if (*ret == (uint32_t)-1) { @@ -350,7 +351,7 @@ int do_semihosting (stlink_t *sl, uint32_t r0, uint32_t r1, uint32_t *ret) { *ret = buffer_len; return -1; } else { - *ret -= buffer_len; + *ret = buffer_len - read_result; } } From a20d4995932db827138fc6c4e235b3ed5456418f Mon Sep 17 00:00:00 2001 From: Don Reid Date: Thu, 26 Jul 2018 16:23:36 -0700 Subject: [PATCH 2/5] Found a problem when reading an odd (%4) number of bytes at the end of a file. fread (on stm32) get them (say 3 bytes), then askes for more. do_semihosting gets a read return of 0 and tries to write that. mem_write alters the address to be aligned and overwrites then 3 bytes from the last read. This change simply tells mem_write to do nothing if len is 0. --- src/gdbserver/semihosting.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/gdbserver/semihosting.c b/src/gdbserver/semihosting.c index 75fc6b1ef..bec777354 100644 --- a/src/gdbserver/semihosting.c +++ b/src/gdbserver/semihosting.c @@ -91,6 +91,10 @@ static int mem_read(stlink_t *sl, uint32_t addr, void *data, uint16_t len) static int mem_write(stlink_t *sl, uint32_t addr, void *data, uint16_t len) { + // Note: this function can write more than it is asked to! + // If addr is not an even 32 bit boundary. + if (len == 0) return 0; // Don't write anything. + int offset = addr % 4; int write_len = len + offset; @@ -344,7 +348,7 @@ int do_semihosting (stlink_t *sl, uint32_t r0, uint32_t r1, uint32_t *ret) { if (*ret == (uint32_t)-1) { *ret = buffer_len; } else { - if (mem_write(sl, buffer_address, buffer, *ret) != 0 ) { + if (mem_write(sl, buffer_address, buffer, read_result) != 0 ) { DLOG("Semihosting SYS_READ error: " "cannot write buffer to target memory\n"); free(buffer); From dc89eda98c414002f4338433187bc215cf82f19a Mon Sep 17 00:00:00 2001 From: Don Reid Date: Fri, 27 Jul 2018 07:07:23 -0700 Subject: [PATCH 3/5] Fix Issues from Fabien-Chouteau's review of my previous patch in isue #727. --- src/gdbserver/semihosting.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gdbserver/semihosting.c b/src/gdbserver/semihosting.c index bec777354..bc69eeeae 100644 --- a/src/gdbserver/semihosting.c +++ b/src/gdbserver/semihosting.c @@ -311,7 +311,7 @@ int do_semihosting (stlink_t *sl, uint32_t r0, uint32_t r1, uint32_t *ret) { int fd; uint32_t buffer_len; void *buffer; - int read_result; + ssize_t read_result; if (mem_read(sl, r1, args, sizeof (args)) != 0 ) { DLOG("Semihosting SYS_READ error: " @@ -342,10 +342,10 @@ int do_semihosting (stlink_t *sl, uint32_t r0, uint32_t r1, uint32_t *ret) { DLOG("Semihosting: read(%d, target_addr:0x%08x, %zu)\n", fd, buffer_address, buffer_len); - read_result = (uint32_t)read(fd, buffer, buffer_len); + read_result = read(fd, buffer, buffer_len); saved_errno = errno; - if (*ret == (uint32_t)-1) { + if (read_result == (uint32_t)-1) { *ret = buffer_len; } else { if (mem_write(sl, buffer_address, buffer, read_result) != 0 ) { From 90585567271ccb7419452d99c8582bd1ace74f74 Mon Sep 17 00:00:00 2001 From: Don Reid Date: Fri, 27 Jul 2018 07:21:35 -0700 Subject: [PATCH 4/5] Revert change to mem_write() so it does not confuse fixes to do_semihosting(). --- src/gdbserver/semihosting.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/gdbserver/semihosting.c b/src/gdbserver/semihosting.c index bc69eeeae..a7ba7c757 100644 --- a/src/gdbserver/semihosting.c +++ b/src/gdbserver/semihosting.c @@ -91,10 +91,6 @@ static int mem_read(stlink_t *sl, uint32_t addr, void *data, uint16_t len) static int mem_write(stlink_t *sl, uint32_t addr, void *data, uint16_t len) { - // Note: this function can write more than it is asked to! - // If addr is not an even 32 bit boundary. - if (len == 0) return 0; // Don't write anything. - int offset = addr % 4; int write_len = len + offset; From 44b33920f1f557fb39e6ff36d9abc60e3da46132 Mon Sep 17 00:00:00 2001 From: Don Reid Date: Fri, 27 Jul 2018 07:48:59 -0700 Subject: [PATCH 5/5] Add cast to avoid warning. --- src/gdbserver/semihosting.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gdbserver/semihosting.c b/src/gdbserver/semihosting.c index a7ba7c757..5fd0d669f 100644 --- a/src/gdbserver/semihosting.c +++ b/src/gdbserver/semihosting.c @@ -351,7 +351,7 @@ int do_semihosting (stlink_t *sl, uint32_t r0, uint32_t r1, uint32_t *ret) { *ret = buffer_len; return -1; } else { - *ret = buffer_len - read_result; + *ret = buffer_len - (uint32_t)read_result; } }