Re: Weird constant expression syntax and bug

PHP-5.6 is frozen for new features for a long time. Adding new features after RC is not a good idea. And we will need some kind of RFC and voting. I help you technically, but you know my opinion about this feature.. anyway, lets fix bugs first, I got an idea that may work... Thanks. Dmitry. On Wed, Jul 23, 2014 at 1:45 PM, Bob Weinand <bobwei9@hotmail.com> wrote: > Am 23.7.2014 um 11:34 schrieb Dmitry Stogov <dmitry@zend.com>: > > On Wed, Jul 23, 2014 at 1:20 PM, Bob Weinand <bobwei9@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 <dmitry@zend.com>: > > On Wed, Jul 23, 2014 at 12:16 PM, Bob Weinand <bobwei9@hotmail.com> wrote: > > Hey, thank you for looking into it :-) > > Am 23.7.2014 um 00:23 schrieb Dmitry Stogov <dmitry@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 <dmitry@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 <bobwei9@hotmail.com> > > wrote: > > > Am 2.7.2014 um 15:43 schrieb Dmitry Stogov <dmitry@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 > > > > >