semihosting: Write back semihosting data before completion callback

'lock_user' allocates a host buffer to shadow a target buffer,
'unlock_user' copies that host buffer back to the target and frees the
host memory. If the completion function uses the target buffer, it
must be called after unlock_user to ensure the data are present.

This caused the arm-compatible TARGET_SYS_READC to fail as the
completion function, common_semi_readc_cb, pulled data from the target
buffer which would not have been gotten the console data.

I decided to fix all instances of this pattern instead of just the
console_read function to make things consistent and potentially fix
bugs in other cases.

Signed-off-by: Keith Packard <keithp@keithp.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20221012014822.1242170-1-keithp@keithp.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20230124180127.1881110-24-alex.bennee@linaro.org>
This commit is contained in:
Keith Packard 2023-01-24 18:01:15 +00:00 committed by Alex Bennée
parent 2c46bc240a
commit 978c2bf97b
1 changed files with 10 additions and 10 deletions

View File

@ -319,11 +319,11 @@ static void host_read(CPUState *cs, gdb_syscall_complete_cb complete,
} }
ret = RETRY_ON_EINTR(read(gf->hostfd, ptr, len)); ret = RETRY_ON_EINTR(read(gf->hostfd, ptr, len));
if (ret == -1) { if (ret == -1) {
complete(cs, -1, errno);
unlock_user(ptr, buf, 0); unlock_user(ptr, buf, 0);
complete(cs, -1, errno);
} else { } else {
complete(cs, ret, 0);
unlock_user(ptr, buf, ret); unlock_user(ptr, buf, ret);
complete(cs, ret, 0);
} }
} }
@ -339,8 +339,8 @@ static void host_write(CPUState *cs, gdb_syscall_complete_cb complete,
return; return;
} }
ret = write(gf->hostfd, ptr, len); ret = write(gf->hostfd, ptr, len);
complete(cs, ret, ret == -1 ? errno : 0);
unlock_user(ptr, buf, 0); unlock_user(ptr, buf, 0);
complete(cs, ret, ret == -1 ? errno : 0);
} }
static void host_lseek(CPUState *cs, gdb_syscall_complete_cb complete, static void host_lseek(CPUState *cs, gdb_syscall_complete_cb complete,
@ -426,8 +426,8 @@ static void host_stat(CPUState *cs, gdb_syscall_complete_cb complete,
ret = -1; ret = -1;
} }
} }
complete(cs, ret, err);
unlock_user(name, fname, 0); unlock_user(name, fname, 0);
complete(cs, ret, err);
} }
static void host_remove(CPUState *cs, gdb_syscall_complete_cb complete, static void host_remove(CPUState *cs, gdb_syscall_complete_cb complete,
@ -444,8 +444,8 @@ static void host_remove(CPUState *cs, gdb_syscall_complete_cb complete,
} }
ret = remove(p); ret = remove(p);
complete(cs, ret, ret ? errno : 0);
unlock_user(p, fname, 0); unlock_user(p, fname, 0);
complete(cs, ret, ret ? errno : 0);
} }
static void host_rename(CPUState *cs, gdb_syscall_complete_cb complete, static void host_rename(CPUState *cs, gdb_syscall_complete_cb complete,
@ -469,9 +469,9 @@ static void host_rename(CPUState *cs, gdb_syscall_complete_cb complete,
} }
ret = rename(ostr, nstr); ret = rename(ostr, nstr);
complete(cs, ret, ret ? errno : 0);
unlock_user(ostr, oname, 0); unlock_user(ostr, oname, 0);
unlock_user(nstr, nname, 0); unlock_user(nstr, nname, 0);
complete(cs, ret, ret ? errno : 0);
} }
static void host_system(CPUState *cs, gdb_syscall_complete_cb complete, static void host_system(CPUState *cs, gdb_syscall_complete_cb complete,
@ -488,8 +488,8 @@ static void host_system(CPUState *cs, gdb_syscall_complete_cb complete,
} }
ret = system(p); ret = system(p);
complete(cs, ret, ret == -1 ? errno : 0);
unlock_user(p, cmd, 0); unlock_user(p, cmd, 0);
complete(cs, ret, ret == -1 ? errno : 0);
} }
static void host_gettimeofday(CPUState *cs, gdb_syscall_complete_cb complete, static void host_gettimeofday(CPUState *cs, gdb_syscall_complete_cb complete,
@ -554,8 +554,8 @@ static void staticfile_read(CPUState *cs, gdb_syscall_complete_cb complete,
} }
memcpy(ptr, gf->staticfile.data + gf->staticfile.off, len); memcpy(ptr, gf->staticfile.data + gf->staticfile.off, len);
gf->staticfile.off += len; gf->staticfile.off += len;
complete(cs, len, 0);
unlock_user(ptr, buf, len); unlock_user(ptr, buf, len);
complete(cs, len, 0);
} }
static void staticfile_lseek(CPUState *cs, gdb_syscall_complete_cb complete, static void staticfile_lseek(CPUState *cs, gdb_syscall_complete_cb complete,
@ -608,8 +608,8 @@ static void console_read(CPUState *cs, gdb_syscall_complete_cb complete,
return; return;
} }
ret = qemu_semihosting_console_read(cs, ptr, len); ret = qemu_semihosting_console_read(cs, ptr, len);
complete(cs, ret, 0);
unlock_user(ptr, buf, ret); unlock_user(ptr, buf, ret);
complete(cs, ret, 0);
} }
static void console_write(CPUState *cs, gdb_syscall_complete_cb complete, static void console_write(CPUState *cs, gdb_syscall_complete_cb complete,
@ -624,8 +624,8 @@ static void console_write(CPUState *cs, gdb_syscall_complete_cb complete,
return; return;
} }
ret = qemu_semihosting_console_write(ptr, len); ret = qemu_semihosting_console_write(ptr, len);
complete(cs, ret ? ret : -1, ret ? 0 : EIO);
unlock_user(ptr, buf, 0); unlock_user(ptr, buf, 0);
complete(cs, ret ? ret : -1, ret ? 0 : EIO);
} }
static void console_fstat(CPUState *cs, gdb_syscall_complete_cb complete, static void console_fstat(CPUState *cs, gdb_syscall_complete_cb complete,