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.
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
