[Qemu-devel] sigaltstack bug in qemu (testing on qemu-ppc)

[Qemu-devel] sigaltstack bug in qemu (testing on qemu-ppc)

[Alex Barcelo]

I am barely able to understand this inline function:

static inline int sas_ss_flags(unsigned long sp)
{
    return (target_sigaltstack_used.ss_size == 0 ? SS_DISABLE
            : on_sig_stack(sp) ? SS_ONSTACK : 0);
}
(signal.c @97)

... and it seems wrong to me when used in the following function. I
have a test program that uses sigaltstack to do some stack
manipulation. It doesn't work.

The function:
    ...
    if ((ka->sa_flags & TARGET_SA_ONSTACK) &&
        (/* here maybe a "!" */ sas_ss_flags(oldsp))) {
    ....
(signal.c, get_sigframe @4121)

Forcing a true value makes everything work (not that I'm claiming it
as the solution, obviously).

I think that it lacks an "!". Either flag SS_DISABLE or SS_ONSTACK are
flags that should *disable* the stack change, so 1 (SS_ONSTACK) and 2
(SS_DISABLE) should not enter the if. And a 0 value means that it
should be ok to do a stack change. It makes sense, but I'm not sure if
I'm oversimplifying things too much. And I wasn't sure if just sending
a patch was ok, given that I don't fully understand the code.

Re: [Qemu-devel] sigaltstack bug in qemu (testing on qemu-ppc)

[Peter Maydell]

On 4 February 2012 14:26, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
> I am barely able to understand this inline function:
>
> static inline int sas_ss_flags(unsigned long sp)
> {
>    return (target_sigaltstack_used.ss_size == 0 ? SS_DISABLE
>            : on_sig_stack(sp) ? SS_ONSTACK : 0);
> }
> (signal.c @97)
>
> ... and it seems wrong to me when used in the following function. I
> have a test program that uses sigaltstack to do some stack
> manipulation. It doesn't work.
>
> The function:
>    ...
>    if ((ka->sa_flags & TARGET_SA_ONSTACK) &&
>        (/* here maybe a "!" */ sas_ss_flags(oldsp))) {
>    ....
> (signal.c, get_sigframe @4121)

This looks like a bug, yes -- the other architectures have the !
(or equivalent code) in their get_sigframe() implementations so
probably ppc is just wrong here.

There are a few other architectures whose get_sigframe doesn't call
sas_ss_flags() at all -- they might also be wrong but I haven't checked.

-- PMM

Re: [Qemu-devel] sigaltstack bug in qemu (testing on qemu-ppc)

[Alex Barcelo]

On Sun, Feb 5, 2012 at 00:00, Peter Maydell <peter.maydell@linaro.org> wrote:
> On 4 February 2012 14:26, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
>> (...)
> This looks like a bug, yes -- the other architectures have the !
> (or equivalent code) in their get_sigframe() implementations so
> probably ppc is just wrong here.
Yes, indeed. They do the check in some fancy ways (with a "!", with a
"==0") but they all do the same.

> There are a few other architectures whose get_sigframe doesn't call
> sas_ss_flags() at all -- they might also be wrong but I haven't checked.
I checked some of them, and it seemed me correct because not all
architectures have sigaltstack enabled. But I have not done a thorough
check.

Just sent a trivial patch, hope is ok.

Re: [Qemu-devel] sigaltstack bug in qemu (testing on qemu-ppc)

[Alexander Graf]

On 09.02.2012, at 19:36, Alex Barcelo wrote:

> On Sun, Feb 5, 2012 at 00:00, Peter Maydell <peter.maydell@linaro.org> wrote:
>> On 4 February 2012 14:26, Alex Barcelo <abarcelo@ac.upc.edu> wrote:
>>> (...)
>> This looks like a bug, yes -- the other architectures have the !
>> (or equivalent code) in their get_sigframe() implementations so
>> probably ppc is just wrong here.
> Yes, indeed. They do the check in some fancy ways (with a "!", with a
> "==0") but they all do the same.
> 
>> There are a few other architectures whose get_sigframe doesn't call
>> sas_ss_flags() at all -- they might also be wrong but I haven't checked.
> I checked some of them, and it seemed me correct because not all
> architectures have sigaltstack enabled. But I have not done a thorough
> check.
> 
> Just sent a trivial patch, hope is ok.

Thanks a lot for sending the patch out :). Please rework it according to the comments and I'll happily apply it.

Alex