Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:108349
Content-Type: text/plain;
	charset=us-ascii
Content-Transfer-Encoding: quoted-printable
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\))
Date: Mon, 3 Feb 2020 15:16:39 +0100
References: <CAF+90c8_0ZXn65uUOLcLNAyiVe9RfDSgJd7Ri=BGZeipMmRtBw@mail.gmail.com>
To: PHP internals <internals@lists.php.net>
In-Reply-To: <CAF+90c8_0ZXn65uUOLcLNAyiVe9RfDSgJd7Ri=BGZeipMmRtBw@mail.gmail.com>
Message-ID: <7270D83F-7C65-4399-9D64-FD54499B2CDC@php.net>
Subject: Re: [PHP-DEV] VM reentry limit
From: ruudboon@php.net (Ruud Boon)


> On 3 Feb 2020, at 14:29, Nikita Popov <nikita.ppv@gmail.com> wrote:
>=20
> Hi internals,
>=20
> We have a long-standing issue (tracked at
> https://bugs.php.net/bug.php?id=3D64196 and the very numerous =
duplicates)
> that certain types of infinite recursion can lead to a stack overflow.
> While for us it is easy to diagnose this, end users will only see a
> "segmentation fault" and will be unable to correlate this with =
infinite
> recursion as the root cause.
>=20
> To provide some technical context, recursion in PHP usually occurs on =
the
> virtual machine stack, in which case unbounded recursion is supported, =
as
> long as the stack size does not exceed your memory limit. However, =
some
> types of calls (in particular magic methods and certain callbacks) go
> through an internal function and have to reenter the virtual machine. =
This
> uses up space on the C stack and may ultimately result in a stack =
overflow.
>=20
> I would like to propose the introduction of a zend.vm_reentry_limit =
ini
> option as a solution to this problem, implemented in
> https://github.com/php/php-src/pull/5135. This ini setting will limit =
the
> number of nested VM reentries that are allowed before an Error is =
thrown.
>=20
> It should be noted that this is (intentionally) not a general =
recursion
> limit. Deep recursion can happen legitimately (e.g. during AST =
processing)
> and it is hard to put a reasonable upper limit on it that both detects
> unintentional infinite recursion while allowing legitimate deep =
recursion.
> The limit implemented here exists specifically to prevent stack =
overflows
> and give the programmer a more obvious indication of the cause of the
> problem.
>=20
> Regards,
> Nikita
>=20
> PS: Some extensions will force all function calls to occur via VM =
reentry.
> Such extensions should probably either disable the option (by setting =
it to
> -1), or significantly increase the limit, to avoid false positives.

I would love to see this addition, I think it can be really helpful for =
a lot of users.
Can we add a function to report the number of VM reentries as well =
(get_vm_reentry_count). This could be helpful for determining a =
reasonable ini value.

Cheers,
Ruud