Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:75939
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain hotmail.com designates 65.55.111.95 as permitted sender)
Message-ID: <BLU436-SMTP167808268E21EDECAB34E34E2FE0@phx.gbl>
Content-Type: multipart/alternative;
	boundary="Apple-Mail=_021C6C52-472E-487E-890D-2BAA349F1F27"
MIME-Version: 1.0 (Mac OS X Mail 7.3 \(1878.2\))
In-Reply-To: <CA+9eiLsWkWxDFSmrqru8ue9A6fZasEqy_xfmjCanvx+mkLXfMA@mail.gmail.com>
Date: Wed, 23 Jul 2014 11:20:42 +0200
CC: Nikita Popov <nikita.ppv@gmail.com>,
 Stas Malyshev <smalyshev@sugarcrm.com>,
 Julien Pauli <jpauli@php.net>,
 PHP Internals <internals@lists.php.net>
References: <CA+9eiLvLhAWCfRmJ05nm46KdDymFmA=g-mkc0uuJeXbVppXnPA@mail.gmail.com> <BLU436-SMTP143809EBE997F38F5F7EF18E2FE0@phx.gbl> <CA+9eiLsWkWxDFSmrqru8ue9A6fZasEqy_xfmjCanvx+mkLXfMA@mail.gmail.com>
To: Dmitry Stogov <dmitry@zend.com>
Subject: Re: [PHP-DEV] Weird constant expression syntax and bug
From: bobwei9@hotmail.com (Bob Weinand)

--Apple-Mail=_021C6C52-472E-487E-890D-2BAA349F1F27
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="windows-1252"

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?)
>=20
> Then we can fix the ast and don't have to take care of opcache at run =
time (=3D 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?


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=85
I don't even remember why I even added that restriction there.


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 :-)
>=20
> 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 =3D [1];
> > }
> > ?>
>=20
> Because it's actually valid. You don't use it in non-static scalar =
context.
>=20
> > 2) In some cases array constants may be used, but not in the others.
> >
> > <?php
> > class Foo {
> >    const BAR =3D [0];
> >    static $a =3D Foo::BAR; // constant array usage
> > }
> > var_dump(Foo::$a);     // prints array
> > var_dump(Foo::BAR);  // emits fatal error
> > ?>
>=20
> They can only be used in static scalar contexts.
>=20
> 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...)
>=20
> > 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".
>=20
> See above...
>=20
> Yeah all the issues above (1-3) are actually a single inconsistency.
> You may find it logical, but I think differently.
>=20
> > 4) Zend/tests/constant_expressions_arrays.phpt crashes whit
> > opcache.protect_memory=3D1 (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 =3D *tmp_ptr;
> >        } else {
> >                INIT_PZVAL_COPY(&c.value, val);
> > +               if (Z_TYPE(c.value) =3D=3D 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) =3D ht;
> > +               } else {
> > +                       zval_copy_ctor(&c.value);
> > +               }
> >                zval_copy_ctor(&c.value);
> >        }
> >        c.flags =3D CONST_CS; /* non persistent, case sensetive */
>=20
> I assume you wanted to patch zend_vm_def.h, not zend_vm_execute.h.
>=20
> Yes. Of course.
> =20
> If you can fix it, please apply the patch, I'm not so deep into =
opcache to take responsibility for that one.
>=20
> OK. This part of the patch must be safe. I'll apply it later.
> =20
>=20
> > 5) Circular constant references crash (found by Nikita)
> >
> > <?php
> > class A {
> >    const FOO =3D [self::BAR];
> >    const BAR =3D [self::FOO];
> > }
> > var_dump(A::FOO); // crashes because of infinity recursion
> > ?>
>=20
> That isn't a specific problems with arrays:
>=20
> <?php
> class test {
>     const BAR =3D 0 + self::FOO;
>     const FOO =3D 0 + self::BAR;
> }
> var_dump(test::BAR);
>=20
> just segfaults too because of the exact same issue
>=20
> 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.
>=20
> Thanks. Dmitry.
> =20
>=20
> > 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 =3D *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 =3D=3D =
ZEND_RECV_INIT ||
> > +                            (*EG(opline_ptr))->opcode =3D=3D
> > ZEND_DECLARE_CONST)) {
> > +                               *result =3D *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 =3D *ast->u.val;
> > +                               } else {
> > +                                       *result =3D *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.
>=20
>=20
> Bob
>=20
> > 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

--Apple-Mail=_021C6C52-472E-487E-890D-2BAA349F1F27--