Skip to content
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.

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

Segfaults on user level (syntax) errors #12396

Closed
stamster opened this issue Nov 7, 2016 · 28 comments
Assignees
Labels

Comments

@stamster
Copy link
Contributor

@stamster stamster commented Nov 7, 2016

Expected and Actual Behavior

PHP fatal/syntax error.

Describe what you are trying to achieve and what goes wrong.

From any controller, trying to call simple facade method defined in ControllerBase or wherever, making a mistake which should cause only syntax error.
Instead, nginx throws 502 Bad Gateway HTTP response, while PHP-FPM throws:
[480094.083490] php-fpm7.0[30488]: segfault at 1c ip 00005563990e8d28 sp 00007ffdf56be410 error 4 in php-fpm7.0[556398e93000+3a9000]

Backtrace:

Core was generated by `php-fpm: pool middleware-php7                                                '.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00005563990e8d28 in _object_and_properties_init ()
(gdb) bt
#0  0x00005563990e8d28 in _object_and_properties_init ()
#1  0x00007f53984f47f7 in zim_Phalcon_Mvc_Micro_LazyLoader___call () from /usr/lib/php/20151012/phalcon.so
#2  0x00005563990d62fa in dtrace_execute_internal ()
#3  0x000055639916a2b3 in ?? ()
#4  0x00005563991265eb in execute_ex ()
#5  0x00005563990d6191 in dtrace_execute_ex ()
#6  0x00005563990d7df9 in zend_call_function ()
#7  0x00007f5398263b39 in zephir_call_user_func_array_noex () from /usr/lib/php/20151012/phalcon.so
#8  0x00007f53984f3199 in zim_Phalcon_Mvc_Micro_handle () from /usr/lib/php/20151012/phalcon.so
#9  0x00005563990d62fa in dtrace_execute_internal ()
#10 0x000055639916afa0 in ?? ()
#11 0x00005563991265eb in execute_ex ()
#12 0x00005563990d6191 in dtrace_execute_ex ()
#13 0x000055639917a3b7 in zend_execute ()
#14 0x00005563990e6393 in zend_execute_scripts ()
#15 0x0000556399086c00 in php_execute_script ()
#16 0x0000556398f6d626 in main ()

Provide minimal script to reproduce the issue

var_dump(
            $this->auth('   contact@my(site).com', '        mytestpass123'),
        );

(if I remove syntax error - comma sign at the end of the statement, stack works fine)

This can happen anytime developer will omit closing } or comma sign.
Those are user level errors and should not crash application server. :/

Details

  • Phalcon version: 3.0.1
  • PHP Version: PHP 7.0.8-0ubuntu0.16.04.3
  • Operating System: Ubuntu Xerus 16.04 LTS amd64
  • Installation type: Compiling from source
  • Zephir version (if any): Version 0.9.4a-dev-7e304ba18c
  • Server: Nginx
  • Other related info (Database, table schema): MariaDB 10.1.18 (unrelated here since no DB query is being executed)
@Jurigag

This comment has been minimized.

Copy link
Member

@Jurigag Jurigag commented Nov 7, 2016

Hmmm, idk, maybe zephir doesn't check if syntax is correct and calls it anyway ? Not sure what's the cause.

@sergeyklay

This comment has been minimized.

Copy link
Member

@sergeyklay sergeyklay commented Nov 7, 2016

Can't see any segfault and reproduce.

[Mon Nov 07 22:28:31.778021 2016] [:error] [pid 82:tid 140580109641472] [client 172.26.0.1:60470] FastCGI: server "/usr/lib/cgi-bin/php-fcgi" stderr: PHP message: PHP Parse error:  syntax error, unexpected ')' in /project/controllers/ControllerBase.php on line 68, referer: http://test.dev/oauth/login
[httpd:access] test.dev:80 172.26.0.1 - - [07/Nov/2016:22:28:31 +0000] "GET / HTTP/1.1" 500 bytesIn:794 bytesOut:199 reqTime:0
[Mon Nov 07 22:28:32.121095 2016] [:error] [pid 82:tid 140580084463360] [client 172.26.0.1:60474] FastCGI: server "/usr/lib/cgi-bin/php-fcgi" stderr: PHP message: PHP Parse error:  syntax error, unexpected ')' in /project/controllers/ControllerBase.php on line 68, referer: http://test.dev/
[httpd:access] test.dev:80 172.26.0.1 - - [07/Nov/2016:22:28:31 +0000] "GET /favicon.ico HTTP/1.1" 500 bytesIn:693 bytesOut:199 reqTime:0
[php:access] 127.0.0.1 -  07/Nov/2016:22:28:31 +0000 "GET /public/index.php" 500 /project/public/index.php cpu:14.75% mem:2 reqTime:0.136
[php:access] 127.0.0.1 -  07/Nov/2016:22:28:31 +0000 "GET /public/index.php?_url=/favicon.ico" 500 /project/public/index.php cpu:6.98% mem:2 reqTime:0.143
[php:fpm] [07-Nov-2016 22:28:31] WARNING: [pool www] child 1938 said into stderr: "NOTICE: PHP message: PHP Parse error:  syntax error, unexpected ')' in /project/controllers/ControllerBase.php on line 68"
[php:fpm] [07-Nov-2016 22:28:32] WARNING: [pool www] child 1937 said into stderr: "NOTICE: PHP message: PHP Parse error:  syntax error, unexpected ')' in /project/controllers/ControllerBase.php on line 68"

Not related directly to the Phalcon

@sergeyklay sergeyklay closed this Nov 7, 2016
@sergeyklay sergeyklay added the Not a bug label Nov 7, 2016
@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Nov 9, 2016

It crashes only when used with Micro lazy loader (as shown in back trace).

So from such controller which extends MVC controller, whenever you run var_dump() or even print_r() with syntax error, it will crash.
Funny thing, making plain fatal errors like nonExistentFunction() it does NOT crash app server.

So the conclusion is - it will only crash whenever you have syntax error, fatal errors are not a problem. So this is clearly a bug @sergeyklay which is easy to reproduce.

One more BT:

#0  0x00005563990e8d28 in _object_and_properties_init ()
#1  0x00007f53984f47f7 in zim_Phalcon_Mvc_Micro_LazyLoader___call () from /usr/lib/php/20151012/phalcon.so
#2  0x00005563990d62fa in dtrace_execute_internal ()
#3  0x000055639916a2b3 in ?? ()
#4  0x00005563991265eb in execute_ex ()
#5  0x00005563990d6191 in dtrace_execute_ex ()
#6  0x00005563990d7df9 in zend_call_function ()
#7  0x00007f5398263b39 in zephir_call_user_func_array_noex () from /usr/lib/php/20151012/phalcon.so
#8  0x00007f53984f3199 in zim_Phalcon_Mvc_Micro_handle () from /usr/lib/php/20151012/phalcon.so
#9  0x00005563990d62fa in dtrace_execute_internal ()
#10 0x000055639916afa0 in ?? ()
#11 0x00005563991265eb in execute_ex ()
#12 0x00005563990d6191 in dtrace_execute_ex ()
#13 0x000055639917a3b7 in zend_execute ()
#14 0x00005563990e6393 in zend_execute_scripts ()
#15 0x0000556399086c00 in php_execute_script ()
#16 0x0000556398f6d626 in main ()

Like @Jurigag said, it seems that Phalcon_Mvc_Micro_LazyLoader does not check the syntax before lazy loading.

@sergeyklay sergeyklay removed the Not a bug label Nov 9, 2016
@sergeyklay sergeyklay reopened this Nov 9, 2016
@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Nov 9, 2016

So the definite steps are:

  • use MicroCollection for routes
  • use lazy loading for controllers of the collection
  • cause syntax error anywhere on lazy loaded controller
@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Nov 13, 2016

Our team makes 10-15 segfaults per day on this :)
Any news out there for a fix? @sergeyklay

@sergeyklay

This comment has been minimized.

Copy link
Member

@sergeyklay sergeyklay commented Nov 13, 2016

I'll try to sort out as soon as I can

@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Nov 15, 2016

Thanks @sergeyklay

I can confirm that this bug is NOT present on PHP 5.6.x (tested on manually compiled 5.6.27 and .28).
So this report should have PHP7 flag/label.

So on PHP 5.6.x:

syntax error, unexpected ')'

On PHP 7.0.x - segfault.

@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Nov 24, 2016

@sergeyklay should this fix be put in a milestone for 3.0.2?

@sergeyklay sergeyklay added this to the 3.0.3 milestone Nov 25, 2016
@sergeyklay

This comment has been minimized.

Copy link
Member

@sergeyklay sergeyklay commented Nov 25, 2016

The 3.0.2 version will be released on weekend

@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Nov 26, 2016

OKAY, great, but this bug does not seem to be fixed in this release? I guess it's hard to find the root cause of it. :/

@Jurigag

This comment has been minimized.

Copy link
Member

@Jurigag Jurigag commented Nov 27, 2016

You see 3.0.3 milestone or not ?

@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Nov 27, 2016

I missed that somehow. Thanks for pointing out.

@sergeyklay

This comment has been minimized.

Copy link
Member

@sergeyklay sergeyklay commented Dec 18, 2016

@stamster did you use php -l before push/deploy? :)

@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Dec 19, 2016

@sergeyklay sorry, I don't get it? Before which deploy?

-l Syntax check only (lint)
Well, for sure I did not, since PhpStorm is used by me and other team members :)

I just tested it.

php -l AdminController.php 
PHP Parse error:  syntax error, unexpected ')' in AdminController.php on line 598
Errors parsing AdminController.php

If I access it via web browser as usual, it will trigger segfault + 502 HTTP (nginx) status, which is, in this case expected when Phalcon tries to lazy load the class/controller.

With PHP CLI I target directly that one specific file, and thus it reports syntax error.

@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Dec 19, 2016

@sergeyklay I edited previous post :)

@sergeyklay sergeyklay modified the milestones: 3.0.3, 3.0.4 Dec 23, 2016
@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Dec 27, 2016

New release is out, but still no fix for this :/

Do we have a clue at least where the bug resides in Zephir?

Thanks!

@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Jan 16, 2017

@sergeyklay do we have some info about this issue? Thanks! :)

@sergeyklay sergeyklay modified the milestones: 3.1.x, 3.1.0 Feb 12, 2017
@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Mar 22, 2017

3.1.0 has been released, any news about this issue?

@sergeyklay

This comment has been minimized.

Copy link
Member

@sergeyklay sergeyklay commented Mar 22, 2017

the lowest priority :) I'll try to sort out in 3.1.x

@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Mar 22, 2017

What I meant - is this issue present in 3.1.0 as well? :) I'll compile it tmr.
Thanks @sergeyklay

@sergeyklay sergeyklay modified the milestones: 3.1.x, 3.1.1 Mar 23, 2017
@sergeyklay sergeyklay modified the milestones: 3.1.1, 3.2.x Apr 5, 2017
@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Jun 24, 2017

This bug is still present on 3.2.x, FYI.
It's just that it seems little bit different (back trace / gdb).

#0  0x00007f7d6be26250 in ?? () from /usr/lib/php/20151012/phalcon.so
#1  0x00007f0000000000 in ?? ()
#2  0x00007f7d76650bc8 in ?? ()
#3  0x0000000000000477 in ?? ()
#4  0x00007f7d76653000 in ?? ()
#5  0x00007f7d746c7d80 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#6  0x00007f7d746d4ff8 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#7  0x00007f7d764ca64b in do_lookup_x (undef_name=0x7f7d74287c0c "fclose", new_hash=4245278497, old_hash=0x7f7d746d4ff8, ref=0x7f7d74285c28, 
    result=0x7f7d746c7d80, scope=<optimized out>, i=140733529437492, version=0x7fff1407b600, flags=1986333640, skip=0x0, type_class=23, 
    undef_map=0x7f7d76653b40) at dl-lookup.c:423
#8  0x00007f7d746c7d80 in ?? () from /lib/x86_64-linux-gnu/libc.so.6
#9  0x00007fff1407b534 in ?? ()
#10 0x00007fff1407b600 in ?? ()
#11 0x00007f7d76650bc8 in ?? ()
#12 0x0000000000000000 in ?? ()

One more, looks more logical...

Core was generated by `php-fpm: master process (/etc/php/7.0/fpm/php-fpm.conf)                      '.
Program terminated with signal SIGSEGV, Segmentation fault.
#0  0x00007f7d6be23b56 in ?? () from /usr/lib/php/20151012/phalcon.so
(gdb) bt
#0  0x00007f7d6be23b56 in ?? () from /usr/lib/php/20151012/phalcon.so
#1  0x0000555ef2ba4840 in ?? ()
#2  0x0000555ef413ece0 in ?? ()
#3  0x0000555ef2ba4840 in ?? ()
#4  0x0000555ef2ba4220 in ?? ()
#5  0x00007fff1407afc0 in ?? ()
#6  0x0000555ef27af585 in zend_shutdown ()
#7  0x0000555ef274e86b in php_module_shutdown ()
#8  0x0000555ef284fef9 in ?? ()
#9  0x0000555ef2848bc6 in fpm_cleanups_run ()
#10 0x0000555ef2850923 in ?? ()
#11 0x0000555ef28517a0 in fpm_pctl_child_exited ()
#12 0x0000555ef28486cf in fpm_children_bury ()
#13 0x0000555ef284cfa6 in ?? ()
#14 0x0000555ef28571f6 in ?? ()
#15 0x0000555ef284d83c in fpm_event_loop ()
#16 0x0000555ef2847fd7 in fpm_run ()
#17 0x0000555ef263329a in main ()
@Jurigag

This comment has been minimized.

Copy link
Member

@Jurigag Jurigag commented Jun 24, 2017

Well this backtrace doesn't tell anything really.

@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Jun 24, 2017

Slightly changed output - IDK why, Phalcon/Zephir/PHP all have been upgraded, but the original issue is there - syntax errors are root cause.

@sergeyklay sergeyklay modified the milestones: 3.2.x, 3.3.x Oct 14, 2017
@sergeyklay sergeyklay modified the milestones: 3.3.x, 3.4.x Mar 24, 2018
@sergeyklay sergeyklay removed this from the 3.4.x milestone Jun 1, 2018
@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Jun 11, 2018

3.4 released, was this fixed? 10x

@stale

This comment has been minimized.

Copy link

@stale stale bot commented Sep 9, 2018

Thank you for contributing to this issue. As it has been 90 days since the last activity, we are automatically closing the issue. This is often because the request was already solved in some way and it just wasn't updated or it's no longer applicable. If that's not the case, please feel free to either reopen this issue or open a new one. We will be more than happy to look at it again! You can read more here: https://blog.phalconphp.com/post/github-closing-old-issues

@stale stale bot added the stale label Sep 9, 2018
@stale stale bot closed this Sep 10, 2018
@davidofferman davidofferman referenced this issue Jul 9, 2019
2 of 5 tasks complete
@sergeyklay

This comment has been minimized.

Copy link
Member

@sergeyklay sergeyklay commented Jul 10, 2019

Fixed in:

@stamster

This comment has been minimized.

Copy link
Contributor Author

@stamster stamster commented Jul 11, 2019

🎆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.