Bug 156015 – Fails to build in Linux / PowerPC due to different ucontext_t definition

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Alberto Garcia]

/«PKGBUILDDIR»/Source/JavaScriptCore/heap/MachineStackMarker.cpp: In function 'void pthreadSignalHandlerSuspendResume(int, siginfo_t*, void*)':
/«PKGBUILDDIR»/Source/JavaScriptCore/heap/MachineStackMarker.cpp:89:37: error: no match for 'operator=' (operand types are 'mcontext_t' and 'ucontext::uc_regs_ptr')
     thread->suspendedMachineContext = userContext->uc_mcontext;
                                     ^
In file included from /usr/include/signal.h:326:0,
                 from /«PKGBUILDDIR»/Source/JavaScriptCore/heap/MachineStackMarker.h:36,
                 from /«PKGBUILDDIR»/Source/JavaScriptCore/heap/MachineStackMarker.cpp:23:
/usr/include/powerpc-linux-gnu/sys/ucontext.h:60:3: note: candidate: mcontext_t& mcontext_t::operator=(const mcontext_t&)
 } mcontext_t;
   ^
/usr/include/powerpc-linux-gnu/sys/ucontext.h:60:3: note:   no known conversion for argument 1 from 'ucontext::uc_regs_ptr' to 'const mcontext_t&'
/usr/include/powerpc-linux-gnu/sys/ucontext.h:60:3: note: candidate: mcontext_t& mcontext_t::operator=(mcontext_t&&)
/usr/include/powerpc-linux-gnu/sys/ucontext.h:60:3: note:   no known conversion for argument 1 from 'ucontext::uc_regs_ptr' to 'mcontext_t&&'
Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/build.make:9801: recipe for target 'Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/heap/MachineStackMarker.cpp.o' failed
make[3]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/heap/MachineStackMarker.cpp.o] Error 1
s

[Alberto Garcia]

[ somehow the previous comment is incomplete, please ignore it ]

I reproduced this with WebKitGTK+ 2.12.0.

It seems that the definition of ucontext_t is different in Linux PowerPC, producing this build error:

/«PKGBUILDDIR»/Source/JavaScriptCore/heap/MachineStackMarker.cpp: In function 'void pthreadSignalHandlerSuspendResume(int, siginfo_t*, void*)':
/«PKGBUILDDIR»/Source/JavaScriptCore/heap/MachineStackMarker.cpp:89:37: error: no match for 'operator=' (operand types are 'mcontext_t' and 'ucontext::uc_regs_ptr')
     thread->suspendedMachineContext = userContext->uc_mcontext;
                                     ^
In file included from /usr/include/signal.h:326:0,
                 from /«PKGBUILDDIR»/Source/JavaScriptCore/heap/MachineStackMarker.h:36,
                 from /«PKGBUILDDIR»/Source/JavaScriptCore/heap/MachineStackMarker.cpp:23:
/usr/include/powerpc-linux-gnu/sys/ucontext.h:60:3: note: candidate: mcontext_t& mcontext_t::operator=(const mcontext_t&)
 } mcontext_t;
   ^
/usr/include/powerpc-linux-gnu/sys/ucontext.h:60:3: note:   no known conversion for argument 1 from 'ucontext::uc_regs_ptr' to 'const mcontext_t&'
/usr/include/powerpc-linux-gnu/sys/ucontext.h:60:3: note: candidate: mcontext_t& mcontext_t::operator=(mcontext_t&&)
/usr/include/powerpc-linux-gnu/sys/ucontext.h:60:3: note:   no known conversion for argument 1 from 'ucontext::uc_regs_ptr' to 'mcontext_t&&'
Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/build.make:9801: recipe for target 'Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/heap/MachineStackMarker.cpp.o' failed
make[3]: *** [Source/JavaScriptCore/CMakeFiles/JavaScriptCore.dir/heap/MachineStackMarker.cpp.o] Error 1

Here's how it is defined:

    typedef struct ucontext {
        struct ucontext *uc_link;
        sigset_t         uc_sigmask;
        stack_t          uc_stack;
        mcontext_t       uc_mcontext;
        ...
    } ucontext_t;


Here's how it is defined in powerpc:

    typedef struct ucontext {
        struct ucontext *uc_link;
        sigset_t         uc_sigmask;
        stack_t          uc_stack;
        union uc_regs_ptr {
            struct pt_regs *regs;
            mcontext_t *uc_regs;
        } uc_mcontext;
        ...
    } ucontext_t;

Full definition here:

https://sources.debian.net/src/glibc/2.21-9/sysdeps/unix/sysv/linux/powerpc/sys/ucontext.h/?hl=139#L139

According to the setcontext(3) man page, this was removed in POSIX.1-2008 because of portability reasons.

[Yusuke Suzuki]

Created attachment 275275 [details]
Patch

[Yusuke Suzuki]

Could you ensure that the uploaded patch fixes the issue? I don't have any PowerPC machine.

[Alberto Garcia]

(In reply to comment #3)
> Could you ensure that the uploaded patch fixes the issue? I don't have any
> PowerPC machine.

Hey, thanks for the patch!

I'm currently building it and will test it later. I noticed however
that with this patch what suspendedMachineContext stores is not an
mcontext_t but a pointer to an mcontext_t. If I understand it
correctly the actual mcontext_t data in the PowerPC case is stored in
ucontext_t.uc_reg_space.

I wonder if this patch is correct then? If not maybe there's no
portable way to do this...

[Yusuke Suzuki]

(In reply to comment #4)
> (In reply to comment #3)
> > Could you ensure that the uploaded patch fixes the issue? I don't have any
> > PowerPC machine.
> 
> Hey, thanks for the patch!
> 
> I'm currently building it and will test it later. I noticed however
> that with this patch what suspendedMachineContext stores is not an
> mcontext_t but a pointer to an mcontext_t. If I understand it
> correctly the actual mcontext_t data in the PowerPC case is stored in
> ucontext_t.uc_reg_space.
> 
> I wonder if this patch is correct then? If not maybe there's no
> portable way to do this...

Oops, that's right. I'll change the patch soon.

[Yusuke Suzuki]

Created attachment 275295 [details]
Patch

[Yusuke Suzuki]

(In reply to comment #6)
> Created attachment 275295 [details]
> Patch

OK, could you try it again?

[Alberto Garcia]

(In reply to comment #7)
> (In reply to comment #6)
> > Created attachment 275295 [details]
> > Patch
> 
> OK, could you try it again?

Yeah, that works.

[Michael Catanzaro]

Comment on attachment 275295 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=275295&action=review

> Source/JavaScriptCore/heap/MachineStackMarker.cpp:90
> +    // Since glib in PowerPC does not have mcontext_t in ucontext_t::uc_mcontext, we specially handle this case here.

I would omit this comment; the conditional compilation makes it obvious that the data structure is architecture-specific.

[Yusuke Suzuki]

Comment on attachment 275295 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=275295&action=review

>> Source/JavaScriptCore/heap/MachineStackMarker.cpp:90
>> +    // Since glib in PowerPC does not have mcontext_t in ucontext_t::uc_mcontext, we specially handle this case here.
> 
> I would omit this comment; the conditional compilation makes it obvious that the data structure is architecture-specific.

OK, thanks. Omitted.

[Yusuke Suzuki]

Committed r198919: <http://trac.webkit.org/changeset/198919>