Hi,
yesterday I was digging into the bug #76510:
https://bugs.php.net/bug.php?id=76510
A strange PHAR- and streams-related issue that only happens for some
users/binaries.
I was able to trace it down to a GCC optimization "optimize-strlen".
Here's when the bug does and does not reproduce:
gcc-8 with -O0 works
gcc-8 with -O1 works
gcc-8 with -O2 does not work
gcc-8 with -O3 does not work
gcc-8 with -Og works
gcc-8 with -Os works
With some further testing, I was able to find a workaround: -O2
-fno-optimize-strlen
I bisected this issue to the commit
513b0093c2b480bb752fb354012f42c446769486:
Refactor php_url struct to save memory dup in common cases
https://github.com/php/php-src/commit/513b0093c2b480bb752fb354012f42c446769486
(bisect log is in the bug report)
To confirm:
git checkout 513b0093c2b480bb752fb354012f42c446769486 + gcc-8 + -O2: Fatal
error
git checkout 513b0093c2b480bb752fb354012f42c446769486^ + gcc-8 + -O2: works
Unfortunately this is where my journey ends - my C/GCC knowledge is not
sufficient enough to analyse this further.
I'd like to ask someone to take over from here so we can see this issue
fixed in the next 7.3 pre-release.
Thank you,
M.
Unfortunately this is where my journey ends - my C/GCC knowledge is not
sufficient enough to analyse this further.
Hi Michael,
If you can reproduce the issue, it would be appropriate for you to
open a bug report with the GCC people, presumably at
https://gcc.gnu.org/bugzilla/
What can be done to work around optimisation bugs is quite limited.
cheers
Dan
Ack
Unfortunately this is where my journey ends - my C/GCC knowledge is not
sufficient enough to analyse this further.Hi Michael,
If you can reproduce the issue, it would be appropriate for you to
open a bug report with the GCC people, presumably at
https://gcc.gnu.org/bugzilla/What can be done to work around optimisation bugs is quite limited.
More likely than not this is a bug on our side triggered by this
optimization, not a bug in GCC. A run through valgrind might reveal
something.
Nikita
More likely than not this is a bug on our side triggered by this
optimization, not a bug in GCC. A run through valgrind might reveal
something.
Valgrind information has been added to
https://bugs.php.net/bug.php?id=76510.
Can an optimization such as "optimize-strlen" be disabled on a per-file
(or better, on a per-function) basis as a (hopefully) short-term workaround?
More likely than not this is a bug on our side triggered by this
optimization, not a bug in GCC. A run through valgrind might reveal
something.Valgrind information has been added to
https://bugs.php.net/bug.php?id=76510.Can an optimization such as "optimize-strlen" be disabled on a per-file
(or better, on a per-function) basis as a (hopefully) short-term workaround?
The actual bug (not was is reported by valgrind) is caused by a compiler
optimization bug in GCC[1]; even some distros are already deploying
fixes. Since this bug is likely to affect other parts of php-src and
even extensions, I don't think it makes sense to disable optimize-strlen
for individual files only, but I rather suggest to check whether
optimize-strlen is broken, and if so to add -fno-optimize-strlen to the
CFLAGS (etc.)
Regardless how we work around the optimization bug, we should do it
ASAP. php-7.3.0RC1 is scheduled for tagging on Tuesday. A patch/PR
would be appreciated.
--
Christoph M. Becker
I don't think it makes sense to disable optimize-strlen
for individual files only, but I rather suggest to check whether
optimize-strlen is broken, and if so to add -fno-optimize-strlen to the
CFLAGS (etc.)
Makes sense to me.
Hi!
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?
Stas Malyshev
smalyshev@gmail.com
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