Am 23.7.2014 um 11:34 schrieb Dmitry Stogov <dmi...@zend.com>:
> On Wed, Jul 23, 2014 at 1:20 PM, Bob Weinand <bobw...@hotmail.com> wrote:
>
>> Yes. Did you see my thoughts before?
>>
>> I'm just wondering if we can't somehow deeply copy the asts for opcache
>>> between compile time and run time in pass_two() (If I'm not wrong
>>> pass_two() has some hook for zend extensions?)
>>>
>>> Then we can fix the ast and don't have to take care of opcache at run
>>> time (= when the (dynamic) asts will be evaluated). It'd maybe even be a
>>> bit faster as it then doesn't have to copy so much at run-time.
>>>
>>
>> ^ these?
>>
>
> Yes, I saw, but it not always possible. At least some pointers to AST are
> kept in shared memory. So AST may not be duplicated at all.
Not sure, but I think that each AST actually has maximum one pointer to it
after compilation time.
We actually don't need to refcount ASTs, so I think it's safe to just e.g.
create a HashTable with which AST is linked to which zval pointer?
>> I'm also not very happy with it.
>> I think I just should remove that restriction of run-time completely. It's
>> just causing more confusion than it helps…
>> I don't even remember why I even added that restriction there.
>>
>
> It was a restriction to not support arrays in constant context. It seems
> like nobody can remember why it was introduced.
> However, I think it's too dangerous to break it in last minute before
> release.
> Actually, you already broke it, and just prohibited usage at run-time.
Actually, it's not yet *last minute*, AFAIK, we're still having another RC
after this bug fix.
It is actually safe to remove that restriction. (I've tested it already).
> Thanks. Dmitry.
>
>>
>> Bob
>>
>> Am 23.7.2014 um 10:47 schrieb Dmitry Stogov <dmi...@zend.com>:
>>
>> On Wed, Jul 23, 2014 at 12:16 PM, Bob Weinand <bobw...@hotmail.com> wrote:
>>
>>> Hey, thank you for looking into it :-)
>>>
>>> Am 23.7.2014 um 00:23 schrieb Dmitry Stogov <dmi...@zend.com>:
>>>> hi Bob,
>>>>
>>>> I still think that current array usage in constant expressions is not
>>>> consistent and dangerous. It "smells" to me, and I think it may bring
>>>> troubles in the future even if the existing known bugs are fixed.
>>>>
>>>> I see few issues:
>>>>
>>>> 1) It is possible to declare array class constants however they can't be
>>>> used. I can't remember why array in constants were prohibited before and
>>>> what problems they brought. The following script works without any
>>> warnings.
>>>>
>>>> <?php
>>>> class Foo {
>>>> const BAR = [1];
>>>> }
>>>> ?>
>>>
>>> Because it's actually valid. You don't use it in non-static scalar
>>> context.
>>>
>>>> 2) In some cases array constants may be used, but not in the others.
>>>>
>>>> <?php
>>>> class Foo {
>>>> const BAR = [0];
>>>> static $a = Foo::BAR; // constant array usage
>>>> }
>>>> var_dump(Foo::$a); // prints array
>>>> var_dump(Foo::BAR); // emits fatal error
>>>> ?>
>>>
>>> They can only be used in static scalar contexts.
>>>
>>> I wanted to introduce constants to be used and dereferenced also at
>>> run-time, but that requires a RFC.
>>> If anyone would allow me to introduce that still now (it'd be a
>>> relatively simple patch), I'll happily do it.
>>> The issue just was that I was a bit late to create a RFC (beta freeze
>>> etc...)
>>>
>>>> 3) The fact that constants are allowed in compile time and even stored,
>>> but
>>>> can't be used confuses me as well as the error message "PHP Fatal error:
>>>> Arrays are not allowed in constants at run-time".
>>>
>>> See above...
>>>
>>
>> Yeah all the issues above (1-3) are actually a single inconsistency.
>> You may find it logical, but I think differently.
>>
>>
>>> 4) Zend/tests/constant_expressions_arrays.phpt crashes whit
>>>> opcache.protect_memory=1 (that indicates petential SHM memory
>>> corruption)
>>>>
>>>> This may be fixed with the following patch:
>>>>
>>>> diff --git a/Zend/zend_vm_execute.h b/Zend/zend_vm_execute.h
>>>> index 144930e..f1aab9a 100644
>>>> --- a/Zend/zend_vm_execute.h
>>>> +++ b/Zend/zend_vm_execute.h
>>>> @@ -4323,6 +4323,16 @@ static int ZEND_FASTCALL
>>>> ZEND_DECLARE_CONST_SPEC_CONST_CONST_HANDLER(ZEND_OPCOD
>>>> c.value = *tmp_ptr;
>>>> } else {
>>>> INIT_PZVAL_COPY(&c.value, val);
>>>> + if (Z_TYPE(c.value) == IS_ARRAY) {
>>>> + HashTable *ht;
>>>> +
>>>> + ALLOC_HASHTABLE(ht);
>>>> + zend_hash_init(ht,
>>>> zend_hash_num_elements(Z_ARRVAL(c.value)), NULL, ZVAL_PTR_DTOR, 0);
>>>> + zend_hash_copy(ht, Z_ARRVAL(c.value),
>>>> (copy_ctor_func_t) zval_deep_copy, NULL, sizeof(zval *));
>>>> + Z_ARRVAL(c.value) = ht;
>>>> + } else {
>>>> + zval_copy_ctor(&c.value);
>>>> + }
>>>> zval_copy_ctor(&c.value);
>>>> }
>>>> c.flags = CONST_CS; /* non persistent, case sensetive */
>>>
>>> I assume you wanted to patch zend_vm_def.h, not zend_vm_execute.h.
>>>
>>
>> Yes. Of course.
>>
>>
>>> If you can fix it, please apply the patch, I'm not so deep into opcache
>>> to take responsibility for that one.
>>>
>>
>> OK. This part of the patch must be safe. I'll apply it later.
>>
>>
>>>
>>>> 5) Circular constant references crash (found by Nikita)
>>>>
>>>> <?php
>>>> class A {
>>>> const FOO = [self::BAR];
>>>> const BAR = [self::FOO];
>>>> }
>>>> var_dump(A::FOO); // crashes because of infinity recursion
>>>> ?>
>>>
>>> That isn't a specific problems with arrays:
>>>
>>> <?php
>>> class test {
>>> const BAR = 0 + self::FOO;
>>> const FOO = 0 + self::BAR;
>>> }
>>> var_dump(test::BAR);
>>>
>>> just segfaults too because of the exact same issue
>>>
>>
>> Oh... This is really bad.
>> It means we have a general AST evaluation problem.
>> It must be fixed before 5.6 release.
>> I'll try to make another attempt in the evening today or tomorrow.
>>
>>
>> Thanks. Dmitry.
>>
>>
>>>
>>>> I didn't find any useful way to fix it. One of the ideas with following
>>>> hack seemed to work, but it breaks another test
>>>> (Zend/tests/constant_expressions_classes.phpt)
>>>>
>>>> diff --git a/Zend/zend_ast.c b/Zend/zend_ast.c
>>>> index 12f9405..8798737 100644
>>>> --- a/Zend/zend_ast.c
>>>> +++ b/Zend/zend_ast.c
>>>> @@ -251,10 +251,22 @@ ZEND_API void zend_ast_evaluate(zval *result,
>>>> zend_ast *ast, zend_class_entry *s
>>>> zval_dtor(&op2);
>>>> break;
>>>> case ZEND_CONST:
>>>> - *result = *ast->u.val;
>>>> - zval_copy_ctor(result);
>>>> - if (IS_CONSTANT_TYPE(Z_TYPE_P(result))) {
>>>> - zval_update_constant_ex(&result, 1,
>>> scope
>>>> TSRMLS_CC);
>>>> + if (EG(in_execution) && EG(opline_ptr) &&
>>>> *EG(opline_ptr) &&
>>>> + ((*EG(opline_ptr))->opcode ==
>>> ZEND_RECV_INIT ||
>>>> + (*EG(opline_ptr))->opcode ==
>>>> ZEND_DECLARE_CONST)) {
>>>> + *result = *ast->u.val;
>>>> + zval_copy_ctor(result);
>>>> + if (IS_CONSTANT_TYPE(Z_TYPE_P(result)))
>>> {
>>>> +
>>> zval_update_constant_ex(&result, 1,
>>>> scope TSRMLS_CC);
>>>> + }
>>>> + } else {
>>>> + if
>>> (IS_CONSTANT_TYPE(Z_TYPE_P(ast->u.val)))
>>>> {
>>>> +
>>>> zval_update_constant_ex(&ast->u.val, 1, scope TSRMLS_CC);
>>>> + *result = *ast->u.val;
>>>> + } else {
>>>> + *result = *ast->u.val;
>>>> + zval_copy_ctor(result);
>>>> + }
>>>> }
>>>> break;
>>>> case ZEND_BOOL_AND:
>>>>
>>>> I spent few hours trying to find a solution, but failed. May be my ideas
>>>> could lead you to something...
>>>>
>>>> Otherwise, I would recommend to remove this feature from PHP-5.6.
>>>>
>>>> Thanks. Dmitry.
>>>
>>>
>>> Bob
>>>
>>>> On Tue, Jul 22, 2014 at 10:00 AM, Dmitry Stogov <dmi...@zend.com>
>>> wrote:
>>>>
>>>>> Hi Bob,
>>>>>
>>>>> Now I think it's not fixable by design :(
>>>>>
>>>>> I'll try to think about it later today.
>>>>> Could you please collect all related issues.
>>>>>
>>>>> Thanks. Dmitry.
>>>>>
>>>>>
>>>>> On Mon, Jul 21, 2014 at 8:36 PM, Bob Weinand <bobw...@hotmail.com>
>>> wrote:
>>>>>
>>>>>> Am 2.7.2014 um 15:43 schrieb Dmitry Stogov <dmi...@zend.com>:
>>>>>>
>>>>>> I don't have good ideas out of the box and I probably won't be able to
>>>>>> look into this before next week.
>>>>>>
>>>>>>
>>>>>> Hey, I still have no real idea how to solve it without breaking
>>> opcache.
>>>>>>
>>>>>> This one seems to be considered like a blocking bug for 5.6.
>>>>>>
>>>>>> Could you please try to fix this in a sane manner?
>>>>>>
>>>>>> Bob
>>>
>>
Bob
