| Message ID | 20220503194843.1379101-6-richard.henderson@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | semihosting cleanup | expand |
On Tue, 3 May 2022 at 20:57, Richard Henderson <richard.henderson@linaro.org> wrote: > > Mirror the user-only function of the same name. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > include/semihosting/softmmu-uaccess.h | 3 +++ > semihosting/uaccess.c | 29 +++++++++++++++++++++++++++ > 2 files changed, 32 insertions(+) > > diff --git a/include/semihosting/softmmu-uaccess.h b/include/semihosting/softmmu-uaccess.h > index 03300376d3..4f08dfc098 100644 > --- a/include/semihosting/softmmu-uaccess.h > +++ b/include/semihosting/softmmu-uaccess.h > @@ -53,4 +53,7 @@ void softmmu_unlock_user(CPUArchState *env, void *p, > target_ulong addr, target_ulong len); > #define unlock_user(s, args, len) softmmu_unlock_user(env, s, args, len) > > +ssize_t softmmu_strlen_user(CPUArchState *env, target_ulong addr); > +#define target_strlen(p) softmmu_strlen_user(env, p) > + > #endif /* SEMIHOSTING_SOFTMMU_UACCESS_H */ > diff --git a/semihosting/uaccess.c b/semihosting/uaccess.c > index 0d3b32b75d..3cd809122c 100644 > --- a/semihosting/uaccess.c > +++ b/semihosting/uaccess.c > @@ -23,6 +23,35 @@ void *softmmu_lock_user(CPUArchState *env, target_ulong addr, > return p; > } > > +ssize_t softmmu_strlen_user(CPUArchState *env, target_ulong addr) > +{ > + char buf[256]; > + size_t len = 0; > + > + while (1) { > + size_t chunk; > + char *p; > + > + chunk = -(addr | TARGET_PAGE_MASK); 'chunk' is unsigned but we're assigning it a negative number here... I assume this is doing some clever bit-twiddling trick but it isn't very obvious. In any case, rounding to a page boundary isn't sufficient to avoid problems, because we don't mandate that RAM-to-device MemoryRegion boundaries happen on page boundaries. I think you either need to do this at a low enough level that you can look at what MemoryRegion types you're reading from, or you need to do it byte at a time. > + chunk = MIN(chunk, sizeof(buf)); > + > + if (cpu_memory_rw_debug(env_cpu(env), addr, buf, chunk, 0)) { > + return -1; > + } > + p = memchr(buf, 0, chunk); > + if (p) { > + len += p - buf; > + return len <= INT32_MAX ? (ssize_t)len : -1; > + } > + > + len += chunk; > + addr += chunk; > + if (len > INT32_MAX) { > + return -1; > + } > + } > +} thanks -- PMM
On 5/16/22 08:11, Peter Maydell wrote: >> + chunk = -(addr | TARGET_PAGE_MASK); > > 'chunk' is unsigned but we're assigning it a negative number here... > I assume this is doing some clever bit-twiddling trick but it isn't > very obvious. Number of bytes left in page -- I'll rename the variable. > In any case, rounding to a page boundary isn't sufficient to > avoid problems, because we don't mandate that RAM-to-device > MemoryRegion boundaries happen on page boundaries. I think you > either need to do this at a low enough level that you can > look at what MemoryRegion types you're reading from, or you need > to do it byte at a time. Easy enough to use probe_access_flags. r~
diff --git a/include/semihosting/softmmu-uaccess.h b/include/semihosting/softmmu-uaccess.h index 03300376d3..4f08dfc098 100644 --- a/include/semihosting/softmmu-uaccess.h +++ b/include/semihosting/softmmu-uaccess.h @@ -53,4 +53,7 @@ void softmmu_unlock_user(CPUArchState *env, void *p, target_ulong addr, target_ulong len); #define unlock_user(s, args, len) softmmu_unlock_user(env, s, args, len) +ssize_t softmmu_strlen_user(CPUArchState *env, target_ulong addr); +#define target_strlen(p) softmmu_strlen_user(env, p) + #endif /* SEMIHOSTING_SOFTMMU_UACCESS_H */ diff --git a/semihosting/uaccess.c b/semihosting/uaccess.c index 0d3b32b75d..3cd809122c 100644 --- a/semihosting/uaccess.c +++ b/semihosting/uaccess.c @@ -23,6 +23,35 @@ void *softmmu_lock_user(CPUArchState *env, target_ulong addr, return p; } +ssize_t softmmu_strlen_user(CPUArchState *env, target_ulong addr) +{ + char buf[256]; + size_t len = 0; + + while (1) { + size_t chunk; + char *p; + + chunk = -(addr | TARGET_PAGE_MASK); + chunk = MIN(chunk, sizeof(buf)); + + if (cpu_memory_rw_debug(env_cpu(env), addr, buf, chunk, 0)) { + return -1; + } + p = memchr(buf, 0, chunk); + if (p) { + len += p - buf; + return len <= INT32_MAX ? (ssize_t)len : -1; + } + + len += chunk; + addr += chunk; + if (len > INT32_MAX) { + return -1; + } + } +} + char *softmmu_lock_user_string(CPUArchState *env, target_ulong addr) { /* TODO: Make this something that isn't fixed size. */
Mirror the user-only function of the same name. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- include/semihosting/softmmu-uaccess.h | 3 +++ semihosting/uaccess.c | 29 +++++++++++++++++++++++++++ 2 files changed, 32 insertions(+)