On 09.09.2018 at 22:28, Stanislav Malyshev wrote:
> So, I am still not sure what the course of action for #76510 is. It's
> still not fixed, original author of the patch (Xinchen Hui) did not
> respond to comments on
> https://github.com/php/php-src/commit/513b0093c2b480bb752fb354012f42c446769486
> and we still have 7.3 with non-working phar. I don't think it's possible
> to release it this way, but I got pushback from some people about
> reverting it. So, what's the alternative course of action?
There does not appear to be anything wrong with this commit. There is
simply a bug in the strlen() optimization of GCC 8[1] (it didn't
recognize the `char c[1]` struct hack, and optimized strlen(…+1) to 0),
which is already fixed in GCC, and in the progress of being rolled out
to distros[2]. I have already suggested to add a respective check to
configure[3], and if the check fails, to disable `optimize-strlen`. My
autoconf-fu is weak, though, so I hope someone will come up with a
patch. Otherwise we could simply disable `optimize-strlen` for GCC 8
unconditionally until we have a better workaround.
The valgrind issues that came up in the bug report still need further
investigation. The invalid reads caused by allocations in lex_scan()'s
callees are definitely related to doc blocks; I don't know where the
conditional jump or move depending on uninitialised value(s) stem from,
though (help of engine and SPL experts, respectively, would be
appreciated). Anyhow, there is no hint so far that these have been
introduced with the mentioned commit, so “not guilty till proven
otherwise” should apply. In other words, I don't see any reason to
revert (parts of) the mentioned commit.
[1] <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86914>
[2]
<https://tracker.debian.org/news/984783/accepted-gcc-8-820-5-source-into-unstable/>
[3]
<https://github.com/php/php-src/commit/513b0093c2b480bb752fb354012f42c446769486#commitcomment-30423099>
--
Christoph M. Becker
