Welcome! Log In Create A New Profile

Advanced

[PHP-DEV] GCC -foptimize-strlen and bug #76510

Posted by Michael Moravec 
Michael Moravec
[PHP-DEV] GCC -foptimize-strlen and bug #76510
August 04, 2018 10:50PM
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.
Dan Ackroyd
Re: [PHP-DEV] GCC -foptimize-strlen and bug #76510
August 04, 2018 11:20PM
On 4 August 2018 at 21:46, Michael Moravec <[email protected]> wrote:
>
> 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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Nikita Popov
Re: [PHP-DEV] GCC -foptimize-strlen and bug #76510
August 05, 2018 12:00AM
On Sat, Aug 4, 2018 at 11:18 PM, Dan Ackroyd <[email protected]> wrote:

> On 4 August 2018 at 21:46, Michael Moravec <[email protected]> wrote:
> >
> > 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
Sebastian Bergmann
Re: [PHP-DEV] GCC -foptimize-strlen and bug #76510
September 08, 2018 10:10AM
On 08/04/2018 11:51 PM, Nikita Popov wrote:
> 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?

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Christoph M. Becker
Re: [PHP-DEV] GCC -foptimize-strlen and bug #76510
September 08, 2018 01:20PM
On 08.09.2018 at 09:59, Sebastian Bergmann wrote:

> On 08/04/2018 11:51 PM, Nikita Popov wrote:
>> 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.

[1]
<https://github.com/php/php-src/commit/513b0093c2b480bb752fb354012f42c446769486#commitcomment-30359209>ff

--
Christoph M. Becker

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sebastian Bergmann
Re: [PHP-DEV] GCC -foptimize-strlen and bug #76510
September 08, 2018 02:00PM
On 09/08/2018 01:14 PM, Christoph M. Becker wrote:
> 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.

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Stanislav Malyshev
Re: [PHP-DEV] GCC -foptimize-strlen and bug #76510
September 09, 2018 10:30PM
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Christoph M. Becker
Re: [PHP-DEV] GCC -foptimize-strlen and bug #76510
September 10, 2018 01:40AM
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

--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Sorry, only registered users may post in this forum.

Click here to login