| Message ID | 20191025113921.9412-9-richard.henderson@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | linux-user sparc fixes | expand |
Le 25/10/2019 à 13:39, Richard Henderson a écrit : > Instructions are always 4 bytes; use uint32_t not abi_ulong. > > Signed-off-by: Richard Henderson <richard.henderson@linaro.org> > --- > linux-user/sparc/signal.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c > index efb0df7e2b..ecfdf937e4 100644 > --- a/linux-user/sparc/signal.c > +++ b/linux-user/sparc/signal.c > @@ -87,7 +87,7 @@ struct target_signal_frame { > struct sparc_stackf ss; > __siginfo_t info; > abi_ulong fpu_save; > - abi_ulong insns[2] __attribute__ ((aligned (8))); > + uint32_t insns[2] __attribute__ ((aligned (8))); > abi_ulong extramask[TARGET_NSIG_WORDS - 1]; > abi_ulong extra_size; /* Should be 0 */ > qemu_siginfo_fpu_t fpu_state; > @@ -98,7 +98,7 @@ struct target_rt_signal_frame { > abi_ulong regs[20]; > sigset_t mask; > abi_ulong fpu_save; > - unsigned int insns[2]; > + uint32_t insns[2]; > stack_t stack; > unsigned int extra_size; /* Should be 0 */ > qemu_siginfo_fpu_t fpu_state; > This definition is used by sparc and sparc64 (sparc64/signal.c includes sparc/signal.c), so the definition was valid before your changes for sparc and not good for sparc64. Moreover rt_signal_frame for sparc64 doesn't look like this one (and signal_frame doesn't exist). Perhaps you should consider to introduce a specific file for sparc64? Thanks, Laurent
On 10/25/19 8:47 AM, Laurent Vivier wrote: > Le 25/10/2019 à 13:39, Richard Henderson a écrit : >> Instructions are always 4 bytes; use uint32_t not abi_ulong. >> >> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >> --- >> linux-user/sparc/signal.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c >> index efb0df7e2b..ecfdf937e4 100644 >> --- a/linux-user/sparc/signal.c >> +++ b/linux-user/sparc/signal.c >> @@ -87,7 +87,7 @@ struct target_signal_frame { >> struct sparc_stackf ss; >> __siginfo_t info; >> abi_ulong fpu_save; >> - abi_ulong insns[2] __attribute__ ((aligned (8))); >> + uint32_t insns[2] __attribute__ ((aligned (8))); >> abi_ulong extramask[TARGET_NSIG_WORDS - 1]; >> abi_ulong extra_size; /* Should be 0 */ >> qemu_siginfo_fpu_t fpu_state; >> @@ -98,7 +98,7 @@ struct target_rt_signal_frame { >> abi_ulong regs[20]; >> sigset_t mask; >> abi_ulong fpu_save; >> - unsigned int insns[2]; >> + uint32_t insns[2]; >> stack_t stack; >> unsigned int extra_size; /* Should be 0 */ >> qemu_siginfo_fpu_t fpu_state; >> > > This definition is used by sparc and sparc64 (sparc64/signal.c includes > sparc/signal.c), so the definition was valid before your changes for > sparc and not good for sparc64. Moreover rt_signal_frame for sparc64 > doesn't look like this one (and signal_frame doesn't exist). You're right that target_rt_signal_frame isn't correct for sparc64. But we also don't implement setup_rt_frame yet, so it's also currently unused. What's here is just good enough to make setup_frame work, and that is correct for both sparc and sparc64. r~
Le 25/10/2019 à 15:38, Richard Henderson a écrit : > On 10/25/19 8:47 AM, Laurent Vivier wrote: >> Le 25/10/2019 à 13:39, Richard Henderson a écrit : >>> Instructions are always 4 bytes; use uint32_t not abi_ulong. >>> >>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> >>> --- >>> linux-user/sparc/signal.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c >>> index efb0df7e2b..ecfdf937e4 100644 >>> --- a/linux-user/sparc/signal.c >>> +++ b/linux-user/sparc/signal.c >>> @@ -87,7 +87,7 @@ struct target_signal_frame { >>> struct sparc_stackf ss; >>> __siginfo_t info; >>> abi_ulong fpu_save; >>> - abi_ulong insns[2] __attribute__ ((aligned (8))); >>> + uint32_t insns[2] __attribute__ ((aligned (8))); >>> abi_ulong extramask[TARGET_NSIG_WORDS - 1]; >>> abi_ulong extra_size; /* Should be 0 */ >>> qemu_siginfo_fpu_t fpu_state; >>> @@ -98,7 +98,7 @@ struct target_rt_signal_frame { >>> abi_ulong regs[20]; >>> sigset_t mask; >>> abi_ulong fpu_save; >>> - unsigned int insns[2]; >>> + uint32_t insns[2]; >>> stack_t stack; >>> unsigned int extra_size; /* Should be 0 */ >>> qemu_siginfo_fpu_t fpu_state; >>> >> >> This definition is used by sparc and sparc64 (sparc64/signal.c includes >> sparc/signal.c), so the definition was valid before your changes for >> sparc and not good for sparc64. Moreover rt_signal_frame for sparc64 >> doesn't look like this one (and signal_frame doesn't exist). > > You're right that target_rt_signal_frame isn't correct for sparc64. But we > also don't implement setup_rt_frame yet, so it's also currently unused. > > What's here is just good enough to make setup_frame work, and that is correct > for both sparc and sparc64. ok Reviewed-by: Laurent Vivier <laurent@vivier.eu>
diff --git a/linux-user/sparc/signal.c b/linux-user/sparc/signal.c index efb0df7e2b..ecfdf937e4 100644 --- a/linux-user/sparc/signal.c +++ b/linux-user/sparc/signal.c @@ -87,7 +87,7 @@ struct target_signal_frame { struct sparc_stackf ss; __siginfo_t info; abi_ulong fpu_save; - abi_ulong insns[2] __attribute__ ((aligned (8))); + uint32_t insns[2] __attribute__ ((aligned (8))); abi_ulong extramask[TARGET_NSIG_WORDS - 1]; abi_ulong extra_size; /* Should be 0 */ qemu_siginfo_fpu_t fpu_state; @@ -98,7 +98,7 @@ struct target_rt_signal_frame { abi_ulong regs[20]; sigset_t mask; abi_ulong fpu_save; - unsigned int insns[2]; + uint32_t insns[2]; stack_t stack; unsigned int extra_size; /* Should be 0 */ qemu_siginfo_fpu_t fpu_state;
Instructions are always 4 bytes; use uint32_t not abi_ulong. Signed-off-by: Richard Henderson <richard.henderson@linaro.org> --- linux-user/sparc/signal.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.17.1