Fibers are broken on alpine x86 with clang

[arnaud-lb]

Description

Found while looking at #9735 (comment)

Most fibers tests fail when building with clang on alpine x86 in non-debug.

This Dockerfile allows to reproduce the issue:

FROM i386/alpine:3.17.1
RUN apk add gcc clang autoconf make bison re2c git musl-dev
RUN git clone  https://github.com/php/php-src --branch master --depth 1
WORKDIR php-src
RUN ./buildconf --force && \
    CC=clang \
    CXX=clang++ \
    ./configure --disable-all --with-pic --build i586-alpine-linux-musl && \
    make -j$(nproc)
RUN TESTS=Zend/tests/fibers make test || true
CMD bash

Also happens in an alpine VM:

CC=clang CXX=clang++ ./configure --disable-all
TESTS=Zend/tests/fibers make test

Tests pass when building with gcc, or with -O0.

PHP Version

8.1 f673449

Operating System

Alpine 3.17.1 x86

[arnaud-lb]

May be related: #9566

[andypost]

Thank you for filing issue. Sadly I have no idea how to tackle it

[andypost]

FYI I'm building only PHP 8.2 with clang, php81 package using gcc

[nielsdos]

I'm taking a brief look at this, it's a segfault in zend_fibers.c:350:

php-src/Zend/zend_fibers.c

Line 350 in 3ed5264

zend_fiber_transfer transfer = *data.transfer;

[nielsdos]

The problem seems to be in how the pointers (that are in the argument's struct) are passed from the assembly to the function.

[nielsdos]

The following patch fixes it for me. I guess more platforms than just Alpine are actually affected by this:

diff --git a/Zend/asm/make_i386_sysv_elf_gas.S b/Zend/asm/make_i386_sysv_elf_gas.S
index b76de260..5568f57b 100644
--- a/Zend/asm/make_i386_sysv_elf_gas.S
+++ b/Zend/asm/make_i386_sysv_elf_gas.S
@@ -82,8 +82,8 @@ make_fcontext:
 
 trampoline:
     /* move transport_t for entering context-function */
-    movl  %edi, (%esp)
-    movl  %esi, 0x4(%esp)
+    pushl %esi
+    pushl %edi
     pushl %ebp
     /* jump to context-function */
     jmp *%ebx

Can someone else please also verify if this fixes it for them? Thanks :)

Note: idk why the stack overwriting was used instead of pushing, I guess to save stack space? If that's the case we can do that too but the solution becomes a little more complicated.

[nielsdos]

I don't have a Linux 32-bit install handy, and I want to check whether it breaks other platforms or not. So I'm going to create a draft PR so that CI can run through this.

EDIT: oh also, I guess if this fix works then we can get rid of the fiber tests in that ignorelist you mentioned here: #9735 (comment) ?