[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 22/24] bsd-user/arm/target_arch_signal.h: arm set_mcontext

From: Warner Losh
Subject: Re: [PATCH 22/24] bsd-user/arm/target_arch_signal.h: arm set_mcontext
Date: Thu, 28 Oct 2021 22:34:47 -0600

On Thu, Oct 28, 2021 at 6:07 PM Warner Losh <imp@bsdimp.com> wrote:

On Thu, Oct 28, 2021 at 11:53 AM Richard Henderson <richard.henderson@linaro.org> wrote:
On 10/19/21 9:44 AM, Warner Losh wrote:
> +    regs->regs[15] = tswap32(gr[TARGET_REG_PC]);
> +    cpsr = tswap32(gr[TARGET_REG_CPSR]);
> +    cpsr_write(regs, cpsr, CPSR_USER | CPSR_EXEC, CPSRWriteByInstr);

Hmm.  What's the expected behaviour if the saved CPSR state does not match the PC state
wrt thumb?

I'm ok if this should fail in spectacular ways, I just wanna know.

I *think* what will happen at the moment is that qemu will go into a whacky state in which
the translator will read and interpret unaligned data.

I have a pending patch set for arm to raise unaligned exceptions for mis-aligned pc.  For
arm32 mode, this is fine, and we'll raise the exception.  But for thumb mode, this is
architecturally impossible, and the translator will assert.

The assert is going to be a problem.  There are a couple of options:

(1) TARGET_REG_PC wins: unset PC[0] and adjust CPSR[T] to match.

(2) CPSR_T wins: unset pc[0] if CPSR[T] is set, unchanged otherwise.  (In the Arm ARM
psueodcode, pc[0] is hardwired to 0 in thumb mode.)

(3) Don't worry about matching PC[0] and CPSR[T], but do prevent an impossible situation
and unset PC[0] always.  If PC[1] is set, and CPSR[T] is unset, then we'll raise unaligned
when the cpu restarts.

Consider this program:
#include <signal.h>
#include <stdio.h>
#include <unistd.h>
int g;
void fortytwo(int arg) { g = 42; }
int main(int argc, char **argv) {
        g = 123;
        signal(SIGALRM, fortytwo); alarm(1); pause();
        printf("G is %d\n", g);

Built for 'arm' I get
G is 42
Build -mthumb I get
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault

So even without your new assert, there are some issues I need to look into before I can
get to this very interesting case :(. These are all good questions, though. I clearly have
some work to do here...

Turns out I just needed to filter things correctly, and the changes to bsd-user/arm/target_arch_thread.h
made the thumb signals work. I've not yet written tests that run T32 instructions and get a A32
signal (or vice versa). I've also not tried to do the same with T32 and A32 threads (well, threads
executing in those two modes and switching between them). That is beyond the scope of this
set of patches, though.

Real FreeBSD blindly sets these values and hopes the processor generates a fault for invalid states.
With the filtering I added for the next round, we'll at least ensure that PC[0] == CPSR[T]. This ensures
consistency, but I don't know how well it will fare in the real world. FreeBSD's thumb support wrt
mcontext and thumb has only recently become more robust, but it doesn't check in the kernel.
And, finally, you're missing the mc_vfp_* handling.

Yes. We don't really support that at the moment, but I'll look into how hard that might be
to add.

I've added it here and in get_mcontext too.

I'll also include an up-to-date copy of a pointer to the tip of the bsd-user fork so you can
see the current state of thigns like signal.c, which I have penciled in for after the aarch
and riscv64 patches that I have lined up (but haven't done the common errors between the
archs yet). Since I'd either need to seen a super-large review or delay things somewhat
for signal.c, I'd like to get the other architectures in since they are almost ready unless there's
a compelling reason to do signal.c and all its dependencies next. But that's getting a bit far
afield from this one patch....

And thank you for finding this and the other hard issues with this series! It's been instructive
and gives me a few things to double check on the other, unsent series.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]