Skip to content

Do not remove -O0 in the middle of a flag #15828

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed

Do not remove -O0 in the middle of a flag #15828

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67d4c27
Fix removal of optimization cflags in debug builds (#9647)
arnaud-lb May 22, 2024
e624350
Do not remove -O0 in the middle of a flag
arnaud-lb Sep 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions build/php.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2735,3 +2735,17 @@ AC_DEFUN([PHP_PATCH_CONFIG_HEADERS], [
$SED -e 's/^#undef PACKAGE_[^ ]*/\/\* & \*\//g' < $srcdir/$1 \
> $srcdir/$1.tmp && mv $srcdir/$1.tmp $srcdir/$1
])

dnl
dnl PHP_REMOVE_OPTIMIZATION_FLAGS
dnl
dnl Removes known compiler optimization flags like -O, -O0, -O1, ..., -Ofast
dnl from CFLAGS and CXXFLAGS.
dnl
AC_DEFUN([PHP_REMOVE_OPTIMIZATION_FLAGS], [
changequote({,})
sed_script='s/\([\t ]\|^\)-O\([0-9gsz]\|fast\|\)\([\t ]\|$\)/\1/g'
changequote([,])
CFLAGS=`echo "$CFLAGS" | sed -e "$sed_script"`
CXXFLAGS=`echo "$CFLAGS" | sed -e "$sed_script"`
Comment on lines +2746 to +2750
Copy link
Member

@petk petk Sep 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
changequote({,})
sed_script='s/\([\t ]\|^\)-O\([0-9gsz]\|fast\|\)\([\t ]\|$\)/\1/g'
changequote([,])
CFLAGS=`echo "$CFLAGS" | sed -e "$sed_script"`
CXXFLAGS=`echo "$CFLAGS" | sed -e "$sed_script"`
sed_script='s/\([[\t ]]\|^\)-O\([[0-9gsz]]\|fast\|\)\([[\t ]]\|$\)/\1/g'
CFLAGS=$(echo "$CFLAGS" | $SED -e "$sed_script")
CXXFLAGS=$(echo "$CFLAGS" | $SED -e "$sed_script")

I've removed previous suggestion and added all in one:

  • backticks command substitution replaced with $(...)
  • $SED used instead of sed direct command so SED variable can be set at the configure step SED=../path/to/sed ./configure
  • changequote builtin M4 macro is something that ideally should be avoided for the cost of duplicating the [ and ] characters in the sed_script variable a bit.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry and another one. Probably the most important one as CXXFLAGS should be its own variable instead.

diff --git a/build/php.m4 b/build/php.m4
index dee47f6e1a..e13fc3367e 100644
--- a/build/php.m4
+++ b/build/php.m4
@@ -2745,5 +2745,5 @@ dnl
 AC_DEFUN([PHP_REMOVE_OPTIMIZATION_FLAGS], [
   sed_script='s/\([[\t ]]\|^\)-O\([[0-9gsz]]\|fast\|\)\([[\t ]]\|$\)/\1/g'
   CFLAGS=$(echo "$CFLAGS" | $SED -e "$sed_script")
-  CXXFLAGS=$(echo "$CFLAGS" | $SED -e "$sed_script")
+  CXXFLAGS=$(echo "$CXXFLAGS" | $SED -e "$sed_script")
 ])

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

])
10 changes: 2 additions & 8 deletions configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -845,10 +845,7 @@ if test "$PHP_GCOV" = "yes"; then
PHP_ADD_MAKEFILE_FRAGMENT($abs_srcdir/build/Makefile.gcov, $abs_srcdir)

dnl Remove all optimization flags from CFLAGS.
changequote({,})
CFLAGS=`echo "$CFLAGS" | "${SED}" -e 's/-O[0-9s]*//g'`
CXXFLAGS=`echo "$CXXFLAGS" | "${SED}" -e 's/-O[0-9s]*//g'`
changequote([,])
PHP_REMOVE_OPTIMIZATION_FLAGS

dnl Add the special gcc flags.
CFLAGS="$CFLAGS -O0 -fprofile-arcs -ftest-coverage"
Expand All @@ -865,10 +862,7 @@ PHP_ARG_ENABLE([debug],
if test "$PHP_DEBUG" = "yes"; then
PHP_DEBUG=1
ZEND_DEBUG=yes
changequote({,})
CFLAGS=`echo "$CFLAGS" | "${SED}" -e 's/-O[0-9s]*//g'`
CXXFLAGS=`echo "$CXXFLAGS" | "${SED}" -e 's/-O[0-9s]*//g'`
changequote([,])
PHP_REMOVE_OPTIMIZATION_FLAGS
dnl Add -O0 only if GCC or ICC is used.
if test "$GCC" = "yes" || test "$ICC" = "yes"; then
CFLAGS="$CFLAGS -O0"
Expand Down
5 changes: 1 addition & 4 deletions scripts/phpize.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,10 +115,7 @@ dnl Discard optimization flags when debugging is enabled.
if test "$PHP_DEBUG" = "yes"; then
PHP_DEBUG=1
ZEND_DEBUG=yes
changequote({,})
CFLAGS=`echo "$CFLAGS" | $SED -e 's/-O[0-9s]*//g'`
CXXFLAGS=`echo "$CXXFLAGS" | $SED -e 's/-O[0-9s]*//g'`
changequote([,])
PHP_REMOVE_OPTIMIZATION_FLAGS
dnl Add -O0 only if GCC or ICC is used.
if test "$GCC" = "yes" || test "$ICC" = "yes"; then
CFLAGS="$CFLAGS -O0"
Expand Down
Loading