Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:75944 Return-Path: Mailing-List: contact internals-help@lists.php.net; run by ezmlm Delivered-To: mailing list internals@lists.php.net Received: (qmail 11858 invoked from network); 23 Jul 2014 09:45:45 -0000 Received: from unknown (HELO lists.php.net) (127.0.0.1) by localhost with SMTP; 23 Jul 2014 09:45:45 -0000 Authentication-Results: pb1.pair.com header.from=bobwei9@hotmail.com; sender-id=pass Authentication-Results: pb1.pair.com smtp.mail=bobwei9@hotmail.com; spf=pass; sender-id=pass Received-SPF: pass (pb1.pair.com: domain hotmail.com designates 65.55.111.112 as permitted sender) X-PHP-List-Original-Sender: bobwei9@hotmail.com X-Host-Fingerprint: 65.55.111.112 blu004-omc2s37.hotmail.com Received: from [65.55.111.112] ([65.55.111.112:49548] helo=BLU004-OMC2S37.hotmail.com) by pb1.pair.com (ecelerity 2.1.1.9-wez r(12769M)) with ESMTP id BC/E6-21666-8448FC35 for ; Wed, 23 Jul 2014 05:45:44 -0400 Received: from BLU436-SMTP130 ([65.55.111.71]) by BLU004-OMC2S37.hotmail.com with Microsoft SMTPSVC(7.5.7601.22712); Wed, 23 Jul 2014 02:45:44 -0700 X-TMN: [60smXYg4IId7KVOOhREVPtv38rmF1TrK] X-Originating-Email: [bobwei9@hotmail.com] Message-ID: Received: from bobweinandsimac.fritz.box ([78.141.132.157]) by BLU436-SMTP130.smtp.hotmail.com over TLS secured channel with Microsoft SMTPSVC(8.0.9200.16384); Wed, 23 Jul 2014 02:45:41 -0700 Content-Type: multipart/alternative; boundary="Apple-Mail=_DFFB4AA8-5EB8-4BCC-930B-CDA917B9F21B" MIME-Version: 1.0 (Mac OS X Mail 7.3 \(1878.2\)) In-Reply-To: Date: Wed, 23 Jul 2014 11:45:37 +0200 CC: Nikita Popov , Stas Malyshev , Julien Pauli , PHP Internals References: To: Dmitry Stogov X-Mailer: Apple Mail (2.1878.2) X-OriginalArrivalTime: 23 Jul 2014 09:45:41.0782 (UTC) FILETIME=[DDF06360:01CFA65A] Subject: Re: [PHP-DEV] Weird constant expression syntax and bug From: bobwei9@hotmail.com (Bob Weinand) --Apple-Mail=_DFFB4AA8-5EB8-4BCC-930B-CDA917B9F21B Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="windows-1252" Am 23.7.2014 um 11:34 schrieb Dmitry Stogov : > On Wed, Jul 23, 2014 at 1:20 PM, Bob Weinand = wrote: >=20 >> Yes. Did you see my thoughts before? >>=20 >> 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. >>>=20 >>=20 >> ^ these? >>=20 >=20 > 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=85 >> I don't even remember why I even added that restriction there. >>=20 >=20 > 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. >=20 >>=20 >> Bob >>=20 >> Am 23.7.2014 um 10:47 schrieb Dmitry Stogov : >>=20 >> On Wed, Jul 23, 2014 at 12:16 PM, Bob Weinand = wrote: >>=20 >>> Hey, thank you for looking into it :-) >>>=20 >>> Am 23.7.2014 um 00:23 schrieb Dmitry Stogov : >>>> hi Bob, >>>>=20 >>>> 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. >>>>=20 >>>> I see few issues: >>>>=20 >>>> 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. >>>>=20 >>>> >>> 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. >>>>=20 >>>> >>> 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 >>=20 >> Yeah all the issues above (1-3) are actually a single inconsistency. >> You may find it logical, but I think differently. >>=20 >>=20 >>> 4) Zend/tests/constant_expressions_arrays.phpt crashes whit >>>> opcache.protect_memory=3D1 (that indicates petential SHM memory >>> corruption) >>>>=20 >>>> This may be fixed with the following patch: >>>>=20 >>>> 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 >>=20 >> Yes. Of course. >>=20 >>=20 >>> If you can fix it, please apply the patch, I'm not so deep into = opcache >>> to take responsibility for that one. >>>=20 >>=20 >> OK. This part of the patch must be safe. I'll apply it later. >>=20 >>=20 >>>=20 >>>> 5) Circular constant references crash (found by Nikita) >>>>=20 >>>> >>> 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 >>> >> 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 >>=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 >>=20 >> Thanks. Dmitry. >>=20 >>=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) >>>>=20 >>>> 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: >>>>=20 >>>> I spent few hours trying to find a solution, but failed. May be my = ideas >>>> could lead you to something... >>>>=20 >>>> Otherwise, I would recommend to remove this feature from PHP-5.6. >>>>=20 >>>> Thanks. Dmitry. >>>=20 >>>=20 >>> Bob >>>=20 >>>> On Tue, Jul 22, 2014 at 10:00 AM, Dmitry Stogov >>> wrote: >>>>=20 >>>>> Hi Bob, >>>>>=20 >>>>> Now I think it's not fixable by design :( >>>>>=20 >>>>> I'll try to think about it later today. >>>>> Could you please collect all related issues. >>>>>=20 >>>>> Thanks. Dmitry. >>>>>=20 >>>>>=20 >>>>> On Mon, Jul 21, 2014 at 8:36 PM, Bob Weinand >>> wrote: >>>>>=20 >>>>>> Am 2.7.2014 um 15:43 schrieb Dmitry Stogov : >>>>>>=20 >>>>>> I don't have good ideas out of the box and I probably won't be = able to >>>>>> look into this before next week. >>>>>>=20 >>>>>>=20 >>>>>> Hey, I still have no real idea how to solve it without breaking >>> opcache. >>>>>>=20 >>>>>> This one seems to be considered like a blocking bug for 5.6. >>>>>>=20 >>>>>> Could you please try to fix this in a sane manner? >>>>>>=20 >>>>>> Bob >>>=20 >>=20 Bob --Apple-Mail=_DFFB4AA8-5EB8-4BCC-930B-CDA917B9F21B--