Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114559
Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 14.0 \(3654.80.0.2.43\))
In-Reply-To: <CAAKU0ukenZ2Uj2k01CT3cH+k4K=+dU+2uirE0DAqHxmdWjSSQQ@mail.gmail.com>
Date: Fri, 21 May 2021 17:30:10 -0300
Cc: PHP internals <internals@lists.php.net>
Content-Transfer-Encoding: quoted-printable
Message-ID: <08D4E892-342E-47F7-B6CA-D8EEB882B3C2@cmpct.info>
References: <194DA850-19F7-41C4-97CF-944F13B57AFD@cmpct.info>
 <CAAKU0ukenZ2Uj2k01CT3cH+k4K=+dU+2uirE0DAqHxmdWjSSQQ@mail.gmail.com>
To: Levi Morrison <levi.morrison@datadoghq.com>
Subject: Re: [PHP-DEV] Using clang-analyzer with PHP: experiences?
From: calvin@cmpct.info (Calvin Buckley)


> On May 21, 2021, at 4:48 PM, Levi Morrison =
<levi.morrison@datadoghq.com> wrote:
>=20
> Just to check: are you setting the environment variable USE_ZEND_ALLOC
> to 0? This causes the engine to use malloc:
> https://heap.space/xref/PHP-7.4/Zend/zend_alloc.c?r=3D600402d9#2738.
>=20
> For what it's worth, I was recently annoyed _again_ by valgrind being
> so noisy because zend_string_equal_val intentionally reads past the
> end of a zend_string. The allocator ensures that memory was allocated,
> but it isn't guaranteed to be initialized. We should find some way to
> initialize this memory for future releases -- maybe add a function
> which null terminates a zend string by adding not 1 null byte but as
> many as necessary to reach the end of the allocation. This should be
> trivial enough in cost to do, compared to some other solutions like
> always zero'ing out the whole memory block or initializing the
> trailing bytes at zend_string_alloc time.
>=20
> Also, I'm not sure this read-past-the-end technique is actually safe,
> such as when USE_ZEND_ALLOC is set to zero and we use malloc directly,
> which does not make the same guarantees about alignment and padding on
> the string...
>=20
> Nikita pushed up this change only today, but it would theoretically
> help with valgrind being used a runtime but not compiled with valgrind
> support: =
https://github.com/php/php-src/commit/a0c44fbaf19841164c7984a6c21b364d391f=
3750.
> I say theoretically only because I haven't tested it yet.

WRT Valgrind: Yes, I do run it with that environment variable, because =
run-tests sets it for you when using Valgrind. Unfortunately there=E2=80=99=
s some more false positives, but it=E2=80=99s coming from the library =
the extension is wrapping. I=E2=80=99d need to take some time to figure =
out if that=E2=80=99s a false positive, a leak caused by me, or worse, a =
leak in the library itself. The additional improvements to Valgrind =
support are welcome though.=