Unlike the kernel macros, our __get_user() and __put_user() do not
return a failure code. Kernel code typically has a style of
err |= __get_user(...); err |= __get_user(...);
and then checking err at the end. In sparc64_get_context() our
version of the code dropped the accumulating into err but left the
"if (err) goto do_sigsegv" checks, which will never be taken. Delete
unnecessary if()s.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201106152738.26026-3-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
The handling of the FPU state in sparc64_get_context() and
sparc64_set_context() is not the same as what the kernel actually
does: we unconditionally read and write the FP registers and the
FSR, GSR and FPRS, but the kernel logic is more complicated:
* in get_context the kernel has code for saving FPU registers,
but it is hidden inside an "if (fenab) condition and the
fenab flag is always set to 0 (inside an "#if 1" which has
been in the kernel for over 15 years). So the effect is that
the FPU state part is always written as zeroes.
* in set_context the kernel looks at the fenab field in the
structure from the guest, and only restores the state if
it is set; it also looks at the structure's FPRS to see
whether either the upper or lower or both halves of the
register file have valid data.
Bring our implementations into line with the kernel:
* in get_context:
- clear the entire target_ucontext at the top of the
function (as the kernel does)
- then don't write the FPU state, so those fields remain zero
- this fixes Coverity issue CID 1432305 by deleting the code
it was complaining about
* in set_context:
- check the fenab and the fpsr to decide which parts of
the FPU data to restore, if any
- instead of setting the FPU registers by doing two
32-bit loads and filling in the .upper and .lower parts
of the CPU_Double union separately, just do a 64-bit
load of the whole register at once. This fixes Coverity
issue CID 1432303 because we now access the dregs[] part
of the mcfpu_fregs union rather than the sregs[] part
(which is not large enough to actually cover the whole of
the data, so we were accessing off the end of sregs[])
We change both functions in a single commit to avoid potentially
breaking bisection.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201106152738.26026-2-peter.maydell@linaro.org>
[lv: fix FPRS_DU loop s/31/32/]
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
The function do_sigreturn() tries to store the PC, NPC and PSR in
uint32_t local variables, which implicitly drops the high half of
these fields for 64-bit guests.
The usual effect was that a guest which used signals would crash on
return from a signal unless it was lucky enough to take it while the
PC was in the low 4GB of the address space. In particular, Debian
/bin/dash and /bin/bash would segfault after executing external
commands.
Use abi_ulong, which is the type these fields all have in the
__siginfo_t struct.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201105212314.9628-4-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Because QEMU's user-mode emulation just directly accesses guest CPU
state, for SPARC the guest register window state is not the same in
the sparc64_get_context() and sparc64_set_context() functions as it
is for the real kernel's versions of those functions. Specifically,
for the kernel it has saved the user space state such that the O*
registers go into a pt_regs struct as UREG_I*, and the I* registers
have been spilled onto the userspace stack. For QEMU, we haven't
done that, so the guest's O* registers are still in WREG_O* and the
I* registers in WREG_I*.
The code was already accessing the O* registers correctly for QEMU,
but had copied the kernel code for accessing the I* registers off the
userspace stack. Replace this with direct accesses to fp and i7 in
the CPU state, and add a comment explaining why we differ from the
kernel code here.
This fix is sufficient to get bash to a shell prompt.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201105212314.9628-3-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
The various structs that make up the SPARC target_ucontext had some
errors:
* target structures must not include fields which are host pointers,
which might be the wrong size. These should be abi_ulong instead
* because we don't have the 'long double' part of the mcfpu_fregs
union in our version of the target_mc_fpu struct, we need to
manually force it to be 16-aligned
In particular, the lack of 16-alignment caused sparc64_get_context()
and sparc64_set_context() to read and write all the registers at the
wrong offset, which triggered a guest glibc stack check in
siglongjmp:
*** longjmp causes uninitialized stack frame ***: terminated
when trying to run bash.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20201105212314.9628-2-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Use WREG_I0 not WREG_O0 in order to properly save the "ins".
The "outs" were saved separately in setup___siginfo.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191106113318.10226-8-richard.henderson@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
s/UREG_FP/WREG_SP/g
This is non-obvious because the UREG_FP constant is fact wrong.
However, the previous search-and-replace patch made it clear that
UREG_FP expands to WREG_O6, and we can see from the enumeration in
target/sparc/cpu.h that WREG_O6 is in fact WREG_SP, the stack pointer.
The UREG_SP define is unused; remove it.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20191106113318.10226-7-richard.henderson@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
This is non-obvious because the UREG constants are in fact wrong.
s/UREG_I/WREG_O/g
s/UREG_O/WREG_I/g
s/UREG_L/WREG_L/g
These substitutions have identical integer values.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20191106113318.10226-6-richard.henderson@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Coverity complains (CID 1390847) about some dead code in
do_sigreturn(). This is an if (err) clause that can never be
true, copied from the kernel (where __get_user returns an error).
The one code path that could report an error is in the
currently commented-out pseudocode for handling FPU register
restoring, so move the if into that comment (and fix the
broken indent in the comment in the process).
(The new position for the error check is also the semantically
correct one -- we should not restore the signal mask from
the signal frame if we get an error here, so the check must
be done before set_sigmask(), not after.)
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-id: 20181115114616.26265-1-peter.maydell@linaro.org
Remove some dead code spotted by Coverity (CID 1009855,
1390854, 1390847). The underlying cause in all these cases
is the same: QEMU's put_user operations can't result in
errors, but the kernel's equivalent does. So when code
was copied from the kernel signal-frame-setup/teardown
code, checks on error flags that were needed in the kernel
became dead code for us.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Laurent Vivier <laurent@vivier.eu>
Message-Id: <20181019161715.12122-1-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Remove a comment suggesting that we need to call tb_flush()
after writing the SPARC signal frame trampoline insns.
This isn't necessary in QEMU, because (even if the guest
architecture requires explicit icache maintenance) we
ensure that memory writes result in invalidation of
translated code from that memory.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20181009184017.15675-1-peter.maydell@linaro.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
This can still be reported using the "-d unimp" command line option.
Code change produced with:
git ls-files linux-user | \
xargs sed -i -E 's/fprintf\(stderr,\s?(".*not implemented\\n")\);/qemu_log_mask(LOG_UNIMP, \1);/g'
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20180706155127.7483-3-f4bug@amsat.org>
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
No code change, only move code from signal.c to
sparc/signal.c, except adding includes and
exporting setup_frame() and setup_rt_frame().
sparc64/signal.c includes sparc/signal.c
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20180424192635.6027-18-laurent@vivier.eu>
Create a signal-common.h for future use by these new files
and use it in the existing signal.c
Signed-off-by: Laurent Vivier <laurent@vivier.eu>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20180424192635.6027-2-laurent@vivier.eu>
