On Mon Sep 16, 2019 at 07:54:53PM +0200, Otto Moerbeek wrote:
> 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,
No objections here.
>
> -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
> >
>
