Skip to content

Apple M1/Big Sur - Segfault on RETURN_ZVAL in redis.c when in MULTI or PIPELINE mode #1917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
stefan-valnet opened this issue Jan 26, 2021 · 9 comments
Closed

Apple M1/Big Sur - Segfault on RETURN_ZVAL in redis.c when in MULTI or PIPELINE mode #1917

stefan-valnet opened this issue Jan 26, 2021 · 9 comments
Assignees
@michael-grunder

Comments

@stefan-valnet
Copy link

stefan-valnet commented Jan 26, 2021 •
edited
Loading

Expected behaviour

 $ php redisdebug.php
Array
(
    [0] => value1
    [1] => value2
)

Actual behaviour

 $ php redisdebug.php
zsh: segmentation fault  php redisdebug.php

I'm seeing this behaviour on

  • OS: MacOS Big Sur 11.1 | 11.2 on Apple Silicon
  • Redis: 6.0.10 | 5.0.0
  • PHP: 8.0.1 | 7.4.14 (Compiled from source and via homebrew)
  • phpredis: 5.3.2 | git develop

Steps to reproduce, backtrace or example script

<?php
$redis = new Redis();
$redis->connect('127.0.0.1', 6379, 1, NULL, 100);
$pipe = $redis->multi(Redis::PIPELINE);
$pipe->get("key1");
$pipe->get("key2");
$pipeData = $pipe->exec();
print_r($pipeData);

Interestingly enough, if $pipe contains just one command, the exec succeeds.

Segfault happens in redis.c

PHP_METHOD(Redis, multi)
{
...
    } else {
        php_error_docref(NULL, E_WARNING, "Unknown mode sent to Redis::multi");
        RETURN_FALSE;
    }

--> RETURN_ZVAL(getThis(), 1, 0);
}

I've checked

  • [X ] There is no similar issue from other users
  • [ X] Issue isn't fixed in develop branch
@michael-grunder michael-grunder self-assigned this Jan 26, 2021
@michael-grunder
Copy link
Member

Hi @stefan-valnet,

I actually have access to an M1 mac for another project so I will attempt to replicate the crash!

@stefan-valnet
Copy link
Author

Thanks @michael-grunder ,

Tried a recompile on Big Sur 11.2 - still there.

Are you able to reproduce the crash?

Anything else I can do to help?

@michael-grunder
Copy link
Member

michael-grunder commented Feb 2, 2021 •
edited
Loading

Hi @stefan-valnet, I am able to replicate the problem.

Unfortunately, debugging native code in a console is something of a nightmare on M1 macs presently. GDB no longer works at all, and lldb crashes a lot (e.g. printing a variable usually segfaults lldb 11.0).

I have tracked it down to a small area:

frame #2: 0x00000001003999cc php`redis_read_variant_reply(execute_data=0x0000000104a17120, return_value=0x000000016fdfe510, redis_sock=0x0000000104a7e000, z_tab=0x0000000000000350, ctx=0x0000000000000000) at library.c:3353:12
frame #3: 0x0000000100373e28 php`redis_sock_read_multibulk_multi_reply_loop(execute_data=0x0000000104a17120, return_value=0x000000016fdfe510, redis_sock=0x0000000104a7e000, z_tab=0x000000016fdfe510, numElems=2) at redis.c:2653:13

It may have to do with the fact that return_value and z_tab point to the same address in the above call, resulting in z_tab being clobbered. I'm just speculating though.

I'll try to get a fix up today.

Update: I've determined what's causing the problem. It has to do with how we're calling the function pointers during exec. The fix shouldn't be too tough.

michael-grunder added a commit that referenced this issue Feb 3, 2021
@michael-grunder
Normalize Redis callback prototypes and stop typecasting.
e0796d4 
Convert every single Redis callback function to the form:

int callback(INTERNAL_FUNCTION_PARAMETERS, RedisSock*, zval*, void*);

Additionally, stop typcasting the callback to void* which was
suppressing warnings and exposing a segfault on Apple silicon (#1917).
@michael-grunder
Copy link
Member

@stefan-valnet Give the linked branch a try if you can. The issue had to do with differences between the x86 and AARCH64 calling conventions, and that we were typecasting a function pointer to void* which suppressed the warnings we should have been seeing.

Your example now works for me on my rented M1 and tests are passing.

@michael-grunder
Copy link
Member

cc @yatsukhnenko Working branch for changes

@stefan-valnet
Copy link
Author

Thanks @michael-grunder!

Confirmed working over here.

@fawzanm
Copy link

fawzanm commented Feb 4, 2021

Excellent, I was having SEG FAULT with Redis and Laravel Horizon. @michael-grunder's suggested branch seems to fix this issue.

@brendt
Copy link

brendt commented Feb 24, 2021 •
edited
Loading

Can confirm https://github.com/phpredis/phpredis/tree/issue.1917-callback-segfault works as intended (tested on Laravel + Horizon)

If anyone stumbles upon this issue before it's permanently fixed, follow the install instructions in https://github.com/phpredis/phpredis/blob/issue.1917-callback-segfault/INSTALL.markdown and you're done

@michael-grunder michael-grunder mentioned this issue Feb 24, 2021
Merged
@yatsukhnenko
Copy link
Member

@michael-grunder since you merge your changes can we close this issue?

@michael-grunder michael-grunder mentioned this issue Mar 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@michael-grunder michael-grunder

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@michael-grunder @yatsukhnenko @fawzanm @brendt @stefan-valnet