On Sat, Sep 14, 2019 at 03:08:10PM -0400, George Koehler wrote:

> On Thu, 12 Sep 2019 16:19:18 +0200
> Otto Moerbeek <[email protected]> wrote:
> 
> > On Thu, Sep 12, 2019 at 12:54:47AM -0400, George Koehler wrote:
> > > The broken diff follows.
> > 
> > The good news is that is is not broken for my use-case: PowerDNS
> > Recursor.  It does not use ontop_fcontext. Thanks a lot for working on
> > this! I am wondering if there any users of ontop_fcontext in our tree...
> > 
> >     -Otto
> 
> Here's a new diff with 3 more fixes:
> 
>  1. It changes ontop_fcontext, so the fibonacci example now works.
> 
>  2. It changes make_fcontext to align the stack pointer to 16 bytes.
>     (Most code can run well or slightly slow with a 4-aligned stack
>     pointer, but altivec vectors might cause a problem.)
> 
>  3. Our patch-boost_context_pooled_fixedsize_stack_hpp used a wrong
>     variable name, so any program that tried to #include
>     <boost/context/pooled_fixedsize_stack.hpp> would get an error.
>     The diff changes the variable name and bumps REVISION-main; this
>     is the only part of the diff to affect arches other than powerpc.
> 
> I have no code using pooled_fixedsize_stack, but one of the examples
> in boost includes the header via <boost/context/all.hpp>.
> 
> I broke the fibonacci example because I caused ontop_fcontext to leave
> a stack frame, but didn't provide an .eh_frame for C++ exceptions.
> Then fibonacci threw an exception, but the unwinder can't remove the
> frame, so it didn't reach the code to catch the exception.
> 
> To fix fibonacci, I go back to having ontop_fcontext make a tail call
> to the ontop-function without leaving a stack frame, like it does on
> Linux.  I then cheat by placing an 8-byte transfer_t on the *other*
> stack; the existing code uses a similar cheat on Linux.  This cheat
> will break if the program resumes the other stack before the
> ontop-function returns, but this is already broken on Linux.
> 
> The diff doesn't fix 2 other bugs:
> 
>  1. The handling of register %r13 is wrong, but this seems not to
>     matter on OpenBSD, so I'm not trying to fix it.
> 
>  2. The call to _exit(0) in make_fcontext is wrong for systems using
>     the secure PLT, like OpenBSD.  I have no code that reaches this
>     call, but I would expect it to crash because it fails to set r30
>     to the global offset table.
> 
> I have stopped work on this diff.  My next task is to report an issue
> to GitHub boost/context, about the multiple problems with ppc32.

This PowerDNS Recursor is still happy and the
boost_context_pooled_fixedsize_stack_hpp fix looks ok as well.

I'd say this is good to go in. Thanks,

        -Otto

> 
> Index: Makefile
> ===================================================================
> RCS file: /cvs/ports/devel/boost/Makefile,v
> retrieving revision 1.89
> diff -u -p -r1.89 Makefile
> --- Makefile  9 Aug 2019 11:25:29 -0000       1.89
> +++ Makefile  14 Sep 2019 00:56:15 -0000
> @@ -16,8 +16,8 @@ MASTER_SITES=       ${MASTER_SITE_SOURCEFORGE:
>  EXTRACT_SUFX=        .tar.bz2
>  FIX_EXTRACT_PERMISSIONS =    Yes
>  
> -REVISION-main=       6
> -REVISION-md= 1
> +REVISION-main=       7
> +REVISION-md= 2
>  
>  SO_VERSION=  9.0
>  BOOST_LIBS=  boost_atomic-mt \
> Index: patches/patch-boost_context_pooled_fixedsize_stack_hpp
> ===================================================================
> RCS file: 
> /cvs/ports/devel/boost/patches/patch-boost_context_pooled_fixedsize_stack_hpp,v
> retrieving revision 1.1
> diff -u -p -r1.1 patch-boost_context_pooled_fixedsize_stack_hpp
> --- patches/patch-boost_context_pooled_fixedsize_stack_hpp    13 Dec 2018 
> 19:52:46 -0000      1.1
> +++ patches/patch-boost_context_pooled_fixedsize_stack_hpp    14 Sep 2019 
> 00:56:15 -0000
> @@ -18,7 +18,7 @@ Index: boost/context/pooled_fixedsize_st
>           stack_context allocate() {
>  -            void * vp = storage_.malloc();
>  -            if ( ! vp) {
> -+            void * vp = mmap(NULL, size_, PROT_READ | PROT_WRITE, 
> MAP_PRIVATE | MAP_ANON | MAP_STACK, -1, 0);
> ++            void * vp = mmap(NULL, stack_size_, PROT_READ | PROT_WRITE, 
> MAP_PRIVATE | MAP_ANON | MAP_STACK, -1, 0);
>  +            if ( vp == MAP_FAILED ) {
>                   throw std::bad_alloc();
>               }
> Index: patches/patch-libs_context_build_Jamfile_v2
> ===================================================================
> RCS file: patches/patch-libs_context_build_Jamfile_v2
> diff -N patches/patch-libs_context_build_Jamfile_v2
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-libs_context_build_Jamfile_v2       14 Sep 2019 00:56:15 
> -0000
> @@ -0,0 +1,17 @@
> +$OpenBSD$
> +
> +ppc32_sysv_elf has 2 instances of "<toolset>clang".
> +The second "clang" should be "gcc".
> +
> +Index: libs/context/build/Jamfile.v2
> +--- libs/context/build/Jamfile.v2.orig
> ++++ libs/context/build/Jamfile.v2
> +@@ -326,7 +326,7 @@ alias asm_sources
> +      <address-model>32
> +      <architecture>power
> +      <binary-format>elf
> +-     <toolset>clang
> ++     <toolset>gcc
> +    ;
> + 
> + alias asm_sources
> Index: patches/patch-libs_context_src_asm_jump_ppc32_sysv_elf_gas_S
> ===================================================================
> RCS file: patches/patch-libs_context_src_asm_jump_ppc32_sysv_elf_gas_S
> diff -N patches/patch-libs_context_src_asm_jump_ppc32_sysv_elf_gas_S
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-libs_context_src_asm_jump_ppc32_sysv_elf_gas_S      14 Sep 
> 2019 00:56:15 -0000
> @@ -0,0 +1,66 @@
> +$OpenBSD$
> +
> +ELF systems other than Linux use a different convention to return a
> +small struct like transfer_t.
> +
> +Index: libs/context/src/asm/jump_ppc32_sysv_elf_gas.S
> +--- libs/context/src/asm/jump_ppc32_sysv_elf_gas.S.orig
> ++++ libs/context/src/asm/jump_ppc32_sysv_elf_gas.S
> +@@ -78,6 +78,9 @@
> + .align 2
> + .type jump_fcontext,@function
> + jump_fcontext:
> ++    # Linux: jump_fcontext( hidden transfer_t * %r3, %r4, %r5)
> ++    # Other: transfer_t %r3:%r4 = jump_fcontext( %r3, %r4)
> ++
> +     # reserve space on stack
> +     subi  %r1, %r1, 244
> + 
> +@@ -121,7 +124,9 @@ jump_fcontext:
> +     stw  %r29, 216(%r1)  # save R29
> +     stw  %r30, 220(%r1)  # save R30
> +     stw  %r31, 224(%r1)  # save R31
> ++#ifdef __Linux__
> +     stw  %r3,  228(%r1)  # save hidden
> ++#endif
> + 
> +     # save CR
> +     mfcr  %r0
> +@@ -135,8 +140,12 @@ jump_fcontext:
> +     # store RSP (pointing to context-data) in R6
> +     mr  %r6, %r1
> + 
> +-    # restore RSP (pointing to context-data) from R4
> ++    # restore RSP (pointing to context-data) from R4/R3
> ++#ifdef __Linux__
> +     mr  %r1, %r4
> ++#else
> ++    mr  %r1, %r3
> ++#endif
> + 
> +     lfd  %f14, 0(%r1)  # restore F14
> +     lfd  %f15, 8(%r1)  # restore F15
> +@@ -178,7 +187,9 @@ jump_fcontext:
> +     lwz  %r29, 216(%r1)  # restore R29
> +     lwz  %r30, 220(%r1)  # restore R30
> +     lwz  %r31, 224(%r1)  # restore R31
> ++#ifdef __Linux__
> +     lwz  %r3,  228(%r1)  # restore hidden
> ++#endif
> + 
> +     # restore CR
> +     lwz   %r0, 232(%r1)
> +@@ -195,8 +206,13 @@ jump_fcontext:
> +     addi  %r1, %r1, 244
> + 
> +     # return transfer_t 
> ++#ifdef __Linux__
> +     stw  %r6, 0(%r3)
> +     stw  %r5, 4(%r3)
> ++#else
> ++    mr   %r3, %r6
> ++    #    %r4, %r4
> ++#endif
> + 
> +     # jump to context
> +     bctr
> Index: patches/patch-libs_context_src_asm_make_ppc32_sysv_elf_gas_S
> ===================================================================
> RCS file: patches/patch-libs_context_src_asm_make_ppc32_sysv_elf_gas_S
> diff -N patches/patch-libs_context_src_asm_make_ppc32_sysv_elf_gas_S
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-libs_context_src_asm_make_ppc32_sysv_elf_gas_S      14 Sep 
> 2019 00:56:15 -0000
> @@ -0,0 +1,78 @@
> +$OpenBSD$
> +
> +Stack should have alignment 16 after jump_fcontext drops 244 bytes.
> +
> +ELF systems other than Linux use a different convention to return a
> +small struct like transfer_t.
> +
> +Index: libs/context/src/asm/make_ppc32_sysv_elf_gas.S
> +--- libs/context/src/asm/make_ppc32_sysv_elf_gas.S.orig
> ++++ libs/context/src/asm/make_ppc32_sysv_elf_gas.S
> +@@ -85,12 +85,19 @@ make_fcontext:
> +     # shift address in R3 to lower 16 byte boundary
> +     clrrwi  %r3, %r3, 4
> + 
> +-    # reserve space for context-data on context-stack
> +-    # including 64 byte of linkage + parameter area (R1 % 16 == 0)
> +-    subi  %r3, %r3, 336
> ++    # reserve space on context-stack, including 16 bytes of linkage
> ++    # and parameter area + 244 bytes of context-data; jump_fcontext
> ++    # will drop 244 bytes to align the stack (244 % 16 != 0)
> ++    subi  %r3, %r3, 16 + 244
> + 
> +     # third arg of make_fcontext() == address of context-function
> ++#ifdef __Linux__
> ++    # save context-function as PC
> +     stw  %r5, 240(%r3)
> ++#else
> ++    # save context-function for trampoline
> ++    stw  %r5, 252(%r3)
> ++#endif
> + 
> +     # set back-chain to zero
> +     li   %r0, 0
> +@@ -99,10 +106,12 @@ make_fcontext:
> +     mffs  %f0  # load FPSCR
> +     stfd  %f0, 144(%r3)  # save FPSCR
> + 
> ++#ifdef __Linux__
> +     # compute address of returned transfer_t
> +     addi  %r0, %r3, 252
> +     mr    %r4, %r0 
> +     stw   %r4, 228(%r3) 
> ++#endif
> + 
> +     # load LR
> +     mflr  %r0
> +@@ -111,6 +120,11 @@ make_fcontext:
> + 1:
> +     # load LR into R4
> +     mflr  %r4
> ++#ifndef __Linux__
> ++    # compute abs address of trampoline; use as PC
> ++    addi  %r7, %r4, trampoline - 1b
> ++    stw   %r7, 240(%r3)
> ++#endif
> +     # compute abs address of label finish
> +     addi  %r4, %r4, finish - 1b
> +     # restore LR
> +@@ -123,6 +137,19 @@ make_fcontext:
> +     mtlr  %r6
> + 
> +     blr  # return pointer to context-data
> ++
> ++#ifndef __Linux__
> ++trampoline:
> ++    # On systems other than Linux, jump_fcontext is returning the
> ++    # transfer_t in %r3:%r4, but we need to pass transfer_t * %r3 to
> ++    # our context-function.
> ++    lwz   %r0, 8(%r1)   # address of context-function
> ++    mtctr %r0
> ++    stw   %r3, 8(%r1)
> ++    stw   %r4, 12(%r1)  # move transfer_t to stack
> ++    la    %r3, 8(%r1)   # address of transfer_t
> ++    bctr
> ++#endif
> + 
> + finish:
> +     # save return address into R0
> Index: patches/patch-libs_context_src_asm_ontop_ppc32_sysv_elf_gas_S
> ===================================================================
> RCS file: patches/patch-libs_context_src_asm_ontop_ppc32_sysv_elf_gas_S
> diff -N patches/patch-libs_context_src_asm_ontop_ppc32_sysv_elf_gas_S
> --- /dev/null 1 Jan 1970 00:00:00 -0000
> +++ patches/patch-libs_context_src_asm_ontop_ppc32_sysv_elf_gas_S     14 Sep 
> 2019 00:56:15 -0000
> @@ -0,0 +1,75 @@
> +$OpenBSD$
> +
> +ELF systems other than Linux use a different convention to return a
> +small struct like transfer_t.
> +
> +Index: libs/context/src/asm/ontop_ppc32_sysv_elf_gas.S
> +--- libs/context/src/asm/ontop_ppc32_sysv_elf_gas.S.orig
> ++++ libs/context/src/asm/ontop_ppc32_sysv_elf_gas.S
> +@@ -78,6 +78,9 @@
> + .align 2
> + .type ontop_fcontext,@function
> + ontop_fcontext:
> ++    # Linux: ontop_fcontext( hidden transfer_t * %r3, %r4, %r5, %r6)
> ++    # Other: transfer_t %r3:%r4 = ontop_fcontext( %r3, %r4, %r5)
> ++
> +     # reserve space on stack
> +     subi  %r1, %r1, 244
> + 
> +@@ -121,7 +124,9 @@ ontop_fcontext:
> +     stw  %r29, 216(%r1)  # save R29
> +     stw  %r30, 220(%r1)  # save R30
> +     stw  %r31, 224(%r1)  # save R31
> ++#ifdef __Linux__
> +     stw  %r3,  228(%r1)  # save hidden
> ++#endif
> + 
> +     # save CR
> +     mfcr  %r0
> +@@ -135,8 +140,12 @@ ontop_fcontext:
> +     # store RSP (pointing to context-data) in R7
> +     mr  %r7, %r1
> + 
> +-    # restore RSP (pointing to context-data) from R4
> ++    # restore RSP (pointing to context-data) from R4/R3
> ++#ifdef __Linux__
> +     mr  %r1, %r4
> ++#else
> ++    mr  %r1, %r3
> ++#endif
> + 
> +     lfd  %f14, 0(%r1)  # restore F14
> +     lfd  %f15, 8(%r1)  # restore F15
> +@@ -178,7 +187,9 @@ ontop_fcontext:
> +     lwz  %r29, 216(%r1)  # restore R29
> +     lwz  %r30, 220(%r1)  # restore R30
> +     lwz  %r31, 224(%r1)  # restore R31
> ++#ifdef __Linux__
> +     lwz  %r4,  228(%r1)  # restore hidden
> ++#endif
> + 
> +     # restore CR
> +     lwz   %r0, 232(%r1)
> +@@ -191,12 +202,22 @@ ontop_fcontext:
> +     # adjust stack
> +     addi  %r1, %r1, 244
> + 
> ++#ifdef __Linux__
> +     # return transfer_t 
> +     stw  %r7, 0(%r4)
> +     stw  %r5, 4(%r4)
> + 
> +     # restore CTR
> +     mtctr %r6
> ++#else
> ++    # On systems other than Linux, we allocate a transfer_t on the
> ++    # other stack, below its stack pointer %r7.
> ++    stw  %r7, -8(%r7)
> ++    stw  %r4, -4(%r7)
> ++    la   %r3, -8(%r7)  # address of transfer_t
> ++
> ++    mtctr %r5
> ++#endif
> + 
> +     # jump to ontop-function
> +     bctr
> 

Reply via email to