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

PHP7, array_push *RECURSION* #1140

Closed
mruz opened this issue Dec 5, 2015 · 9 comments
Closed

PHP7, array_push *RECURSION* #1140

mruz opened this issue Dec 5, 2015 · 9 comments
Labels

Comments

@mruz
Copy link
Contributor

mruz commented Dec 5, 2015

I have psr-4 loader:

namespace Ice;

class Loader
{

    protected prefixes = [];

    /**
     * Register loader with SPL autoloader stack.
     * 
     * @return void
     */
    public function register()
    {
        spl_autoload_register([this, "loadClass"]);
    }

    /**
     * Adds a base directory for a namespace prefix.
     *
     * @param string prefix The namespace prefix
     * @param string baseDir A base directory for class files in the namespace
     * @param bool prepend If true, prepend the base directory to the stack instead of appending it; this causes it to
     * be searched first rather than last
     * @return Loader
     */
    public function addNamespace(string prefix, string baseDir, boolean prepend = false)
    {
        // normalize namespace prefix
        let prefix = trim(prefix, "\\") . "\\";

        // normalize the base directory with a trailing separator
        let baseDir = rtrim(baseDir, "/") . DIRECTORY_SEPARATOR,
            baseDir = rtrim(baseDir, DIRECTORY_SEPARATOR) . "/";

        // initialize the namespace prefix array
        if !isset this->prefixes[prefix] {
            let this->prefixes[prefix] = [];
        }

        // retain the base directory for the namespace prefix
        if prepend {
            array_unshift(this->prefixes[prefix], utf8_encode(baseDir));
        } else {
            array_push(this->prefixes[prefix], utf8_encode(baseDir));
        }
        return this;
    }

    /**
     * Loads the class file for a given class name.
     *
     * @param string className The fully-qualified class name
     * @return mixed The mapped file name on success, or boolean false on failure
     */
    public function loadClass(string className)
    {
        var prefix, pos, relativeClass, mappedFile;

        // the current namespace prefix
        let prefix = className;

        // work backwards through the namespace names of the fully-qualified
        // class name to find a mapped file name
        let pos = strrpos(prefix, "\\");
        while pos !== false {
            // retain the trailing namespace separator in the prefix
            let prefix = substr(className, 0, pos + 1);

            // the rest is the relative class name
            let relativeClass = substr(className, pos + 1);

            // try to load a mapped file for the prefix and relative class
            let mappedFile = this->loadMappedFile(prefix, relativeClass);
            if mappedFile {
                return mappedFile;
            }

            // remove the trailing namespace separator for the next iteration
            // of strrpos()
            let prefix = rtrim(prefix, "\\");
            let pos = strrpos(prefix, "\\");
        }

        // never found a mapped file
        return false;
    }

    /**
     * Load the mapped file for a namespace prefix and relative class.
     * 
     * @param string $prefix The namespace prefix
     * @param string $relative_class The relative class name
     * @return mixed Boolean false if no mapped file can be loaded, or the name of the mapped file that was loaded
     */
    protected function loadMappedFile(string prefix, string relativeClass)
    {
        var baseDir, file;

        // are there any base directories for this namespace prefix?
        if !isset this->prefixes[prefix] {
            return false;
        }

        // look through base directories for this namespace prefix
        for baseDir in this->prefixes[prefix] {
            // replace the namespace prefix with the base directory,
            // replace namespace separators with directory separators
            // in the relative class name, append with .php
            let file = baseDir . str_replace("\\", DIRECTORY_SEPARATOR, relativeClass) . ".php",
                file = baseDir . str_replace("\\", "/", relativeClass) . ".php";

            // if the mapped file exists, require it
            if this->requireFile(file) {
                // yes, we"re done
                return file;
            }
        }

        // never found it
        return false;
    }

    /**
     * If a file exists, require it from the file system.
     * 
     * @param string $file The file to require
     * @return bool True if the file exists, false if not
     */
    protected function requireFile(string file)
    {
        if file_exists(file) {
            require file;
            return true;
        }
        return false;
    }
}

If I'm trying to add namespace:

$loader = new Ice\Loader();
$loader
    ->addNamespace('App', __ROOT__ . '/app/boot')
    ->register();
var_dump($loader);

Expected (works on PHP5):

object(Ice\Loader)#1 (1) {
  ["prefixes":protected]=>
  array(1) {
    ["App\"]=>
    array(1) {
      [0]=>
      string(40) "/home/mruz/Dropbox/git/farmigo/app/boot/"
    }
  }
}

On PHP7:

object(Ice\Loader)#1 (1) {
  ["prefixes":protected]=>
  array(1) {
    ["App\"]=>
    array(1) {
      ["*prefixes"]=>
      *RECURSION*
    }
  }
}

Program received signal SIGSEGV, Segmentation fault.
0x00005555558d9c9f in zend_make_printable_zval ()
(gdb) bt
#0  0x00005555558d9c9f in zend_make_printable_zval ()
#1  0x00007ffff313c197 in zephir_concat_vvs.constprop.57 () from /usr/lib64/php7/extensions/ice.so
#2  0x00007ffff305b286 in zim_Ice_Loader_loadMappedFile () from /usr/lib64/php7/extensions/ice.so
#3  0x00007ffff311c098 in zephir_call_user_function.constprop.106 () from /usr/lib64/php7/extensions/ice.so
#4  0x00007ffff311d126 in zephir_call_class_method_aparams () from /usr/lib64/php7/extensions/ice.so
#5  0x00007ffff30d1ca5 in zim_Ice_Loader_loadClass () from /usr/lib64/php7/extensions/ice.so
#6  0x00005555558bfc8d in zend_call_function ()
#7  0x00005555559181a8 in zend_call_method ()
#8  0x00005555556df42c in zif_spl_autoload_call ()
#9  0x00005555558bfc8d in zend_call_function ()
#10 0x00005555558c0b76 in zend_lookup_class_ex ()
#11 0x00005555558c1916 in zend_fetch_class_by_name ()
#12 0x0000555555979352 in ZEND_NEW_SPEC_CONST_HANDLER ()
#13 0x0000555555967d70 in execute_ex ()
#14 0x00005555559686e4 in zend_execute ()
#15 0x00005555558dc7be in zend_execute_scripts ()
#16 0x000055555582ff30 in php_execute_script ()
#17 0x0000555555a40e65 in do_cli ()
#18 0x0000555555a42230 in main ()
PHP_METHOD(Ice_Loader, addNamespace) {

        int ZEPHIR_LAST_CALL_STATUS;
        zephir_fcall_cache_entry *_14 = NULL;
        zend_bool prepend;
        zval *prefix_param = NULL, *baseDir_param = NULL, *prepend_param = NULL, _0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10$$3, _11$$4, _12$$4, _13$$4, _15$$5, _16$$5, _17$$5;
        zval prefix, baseDir;
                zval this_zv;
        zval *this_ptr = getThis();
        if (EXPECTED(this_ptr)) {
                ZVAL_OBJ(&this_zv, Z_OBJ_P(this_ptr));
                this_ptr = &this_zv;
        } else this_ptr = NULL;

        ZVAL_UNDEF(&prefix);
        ZVAL_UNDEF(&baseDir);
        ZVAL_UNDEF(&_0);
        ZVAL_UNDEF(&_1);
        ZVAL_UNDEF(&_2);
        ZVAL_UNDEF(&_3);
        ZVAL_UNDEF(&_4);
        ZVAL_UNDEF(&_5);
        ZVAL_UNDEF(&_6);
        ZVAL_UNDEF(&_7);
        ZVAL_UNDEF(&_8);
        ZVAL_UNDEF(&_9);
        ZVAL_UNDEF(&_10$$3);
        ZVAL_UNDEF(&_11$$4);
        ZVAL_UNDEF(&_12$$4);
        ZVAL_UNDEF(&_13$$4);
        ZVAL_UNDEF(&_15$$5);
        ZVAL_UNDEF(&_16$$5);
        ZVAL_UNDEF(&_17$$5);

        ZEPHIR_MM_GROW();
        zephir_fetch_params(1, 2, 1, &prefix_param, &baseDir_param, &prepend_param);

        zephir_get_strval(&prefix, prefix_param);
        zephir_get_strval(&baseDir, baseDir_param);
        if (!prepend_param) {
                prepend = 0;
        } else {
                prepend = zephir_get_boolval(prepend_param);
        }

        ZEPHIR_INIT_VAR(&_0);
        ZEPHIR_INIT_VAR(&_1);
        ZVAL_STRING(&_1, "\\");
        zephir_fast_trim(&_0, &prefix, &_1, ZEPHIR_TRIM_BOTH TSRMLS_CC);
        ZEPHIR_INIT_VAR(&_2);
        ZEPHIR_CONCAT_VS(&_2, &_0, "\\");
        zephir_get_strval(&prefix, &_2);
        ZEPHIR_INIT_VAR(&_3);
        ZEPHIR_INIT_VAR(&_4);
        ZVAL_STRING(&_4, "/");
        zephir_fast_trim(&_3, &baseDir, &_4, ZEPHIR_TRIM_RIGHT TSRMLS_CC);
        ZEPHIR_INIT_VAR(&_5);
        ZEPHIR_CONCAT_VS(&_5, &_3, "/");
        zephir_get_strval(&baseDir, &_5);
        ZEPHIR_INIT_VAR(&_6);
        ZEPHIR_INIT_VAR(&_7);
        ZVAL_STRING(&_7, "/");
        zephir_fast_trim(&_6, &baseDir, &_7, ZEPHIR_TRIM_RIGHT TSRMLS_CC);
        ZEPHIR_INIT_VAR(&_8);
        ZEPHIR_CONCAT_VS(&_8, &_6, "/");
        zephir_get_strval(&baseDir, &_8);
        zephir_read_property(&_9, this_ptr, SL("prefixes"), PH_NOISY_CC | PH_READONLY);
        if (!(zephir_array_isset(&_9, &prefix))) {
                ZEPHIR_INIT_VAR(&_10$$3);
                array_init(&_10$$3);
                zephir_update_property_array(this_ptr, SL("prefixes"), &prefix, &_10$$3 TSRMLS_CC);
        }
        if (prepend) {
                zephir_read_property(&_11$$4, this_ptr, SL("prefixes"), PH_NOISY_CC | PH_READONLY);
                zephir_array_fetch(&_12$$4, &_11$$4, &prefix, PH_NOISY | PH_READONLY, "ice/loader.zep", 53 TSRMLS_CC);
                ZEPHIR_CALL_FUNCTION(&_13$$4, "utf8_encode", &_14, 112, &baseDir);
                zephir_check_call_status();
                ZEPHIR_MAKE_REF(&_12$$4);
                ZEPHIR_CALL_FUNCTION(NULL, "array_unshift", NULL, 113, &_12$$4, &_13$$4);
                ZEPHIR_UNREF(&_12$$4);
                zephir_check_call_status();
        } else {
                zephir_read_property(&_15$$5, this_ptr, SL("prefixes"), PH_NOISY_CC | PH_READONLY);
                zephir_array_fetch(&_16$$5, &_15$$5, &prefix, PH_NOISY | PH_READONLY, "ice/loader.zep", 55 TSRMLS_CC);
                ZEPHIR_CALL_FUNCTION(&_17$$5, "utf8_encode", &_14, 112, &baseDir);
                zephir_check_call_status();
                ZEPHIR_MAKE_REF(&_16$$5);
                ZEPHIR_CALL_FUNCTION(NULL, "array_push", NULL, 114, &_16$$5, &_17$$5);
                ZEPHIR_UNREF(&_16$$5);
                zephir_check_call_status();
        }
        RETURN_THIS();

}
@steffengy steffengy added the bug label Dec 5, 2015
@steffengy
Copy link
Contributor

First off thanks for helping making PHP7 support better :)
I'll try to take a look at this soon

@steffengy
Copy link
Contributor

steffengy commented Dec 5, 2015

minimal sample:

 public function addNamespace(string prefix, string baseDir)
    {
        if !isset this->prefixes[prefix] {
            let this->prefixes[prefix] = [];
        }
        array_push(this->prefixes[prefix], baseDir);
    }

This is very tricky to fix, won't happen soon.

@mruz
Copy link
Contributor Author

mruz commented Dec 6, 2015

Bad to hear, maybe it's related to fetching array. I had a problem with #1126 (comment)

let handler = data["routeMap"][j][0],
    varNames = data["routeMap"][j][1];

@steffengy
Copy link
Contributor

It is indeed related to array fetching.
If you're interested in an explanation:

if !isset this->prefixes[prefix] {
            let this->prefixes[prefix] = [];
        }

causes the refcount of the new array to be 2
(1 for the current method scope and 1 for being assigned as array member of an objects property)
which only gets lowered at the end of the method
(since then the memory frames are "garbage collected").

array_push(this->prefixes[prefix], baseDir);

Here the array with refcount 2 is separated (since the refcount should actually be 1,
which would only be possible if we had garbage collection after each scope or if the compiler would determine to generate code that reuses the array created above) ->
a new_array is created (and the current array gets refcount 1 as a result of separation)
which is filled with your values but never used (since it has no "connection" to object properties)
Then at then end of the method, values are "garbage collected" and the old array (which is still an property-array member), get's destroyed since the refcount is only 1.
The method now points to uninitizalized memory, which is what you see in your var_dump.

I cannot quite think of a way to fix this without the ways described above,
which are very tricky to implement...

@fanybook
Copy link

use your code (php7)

when is start apache, has a PHP Warning

PHP Warning: PHP Startup: Unable to load dynamic library '/usr/local/php/lib/php/extensions/no-debug-non-zts-20151012/ice.so' - /usr/local/php/lib/php/extensions/no-debug-non-zts-20151012/ice.so: undefined symbol: zend_std_call_user_call in Unknown on line 0

@mruz
Copy link
Contributor Author

mruz commented Mar 17, 2016

@andresgutierrez @ovr any idea to resolve this?

@mruz
Copy link
Contributor Author

mruz commented Mar 18, 2016

@steffengy this is temporary fix for me, but I kept it open

        // initialize the namespace prefix array
        if !isset this->prefixes[prefix] {
            let this->prefixes[prefix] = [utf8_encode(baseDir)];
            return this;
        }

@niden niden added this to the 2017-Q1 milestone Jan 14, 2017
@niden niden modified the milestones: 2017-Q1, 2017-Q2 Apr 14, 2017
@sergeyklay sergeyklay self-assigned this Oct 27, 2017
@sergeyklay sergeyklay removed this from the 2017-Q2 milestone Apr 9, 2018
@sergeyklay sergeyklay removed their assignment Dec 11, 2018
@sergeyklay
Copy link
Contributor

@dreamsxin Could you please take a look

@dreamsxin dreamsxin mentioned this issue Feb 26, 2019
3 tasks
@sergeyklay
Copy link
Contributor

Fixed in the 0.11.11 version. Feel free to open a new issue if the problem appears again. Thank you for contributing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants