Skip to content

Commit 836a162

Browse files
cmb69arnaud-lb
andauthored
Don't fiddle with NDEBUG in C code (GH-16511)
* Don't fiddle with NDEBUG in C code It is way to late to do this in php.h, since assert.h has already been included. Even pushing that down to zend_portability.h may not have the desired effect. Instead we define or undefine NDEBUG as CFLAG, so that it works in all circumstances. As a last resort we fail at build time, if `NDEBUG` is defined when `ZEND_DEBUG` is enabled. We also remove the useless workaround in zend_test to include assert.h again, since that usually won't have any effect anyway. Co-authored-by: Arnaud Le Blanc <[email protected]>
1 parent 361fb40 commit 836a162

File tree

5 files changed

+10
-14
lines changed

5 files changed

+10
-14
lines changed

Zend/zend_portability.h

+3
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,9 @@
4747
#include "../TSRM/TSRM.h"
4848

4949
#include <stdio.h>
50+
#if ZEND_DEBUG && defined(NDEBUG)
51+
# error "NDEBUG must not be defined when ZEND_DEBUG is enabled"
52+
#endif
5053
#include <assert.h>
5154
#include <math.h>
5255

configure.ac

+6
Original file line numberDiff line numberDiff line change
@@ -799,6 +799,8 @@ PHP_ARG_ENABLE([debug],
799799
AS_VAR_IF([PHP_DEBUG], [yes], [
800800
PHP_DEBUG=1
801801
ZEND_DEBUG=yes
802+
CFLAGS="$CFLAGS -UNDEBUG"
803+
CXXFLAGS="$CXXFLAGS -UNDEBUG"
802804
PHP_REMOVE_OPTIMIZATION_FLAGS
803805
dnl Add -O0 only if GCC or ICC is used.
804806
if test "$GCC" = "yes" || test "$ICC" = "yes"; then
@@ -817,6 +819,8 @@ AS_VAR_IF([PHP_DEBUG], [yes], [
817819
], [
818820
PHP_DEBUG=0
819821
ZEND_DEBUG=no
822+
CFLAGS="$CFLAGS -DNDEBUG"
823+
CXXFLAGS="$CXXFLAGS -DNDEBUG"
820824
])
821825

822826
PHP_ARG_ENABLE([debug-assertions],
@@ -829,6 +833,8 @@ PHP_ARG_ENABLE([debug-assertions],
829833
AS_VAR_IF([PHP_DEBUG_ASSERTIONS], [yes], [
830834
PHP_DEBUG=1
831835
ZEND_DEBUG=yes
836+
CFLAGS="$CFLAGS -UNDEBUG"
837+
CXXFLAGS="$CXXFLAGS -UNDEBUG"
832838
])
833839

834840
AC_ARG_ENABLE([zts],

ext/zend_test/test.c

-6
Original file line numberDiff line numberDiff line change
@@ -38,12 +38,6 @@
3838
#include "zend_exceptions.h"
3939
#include "zend_mm_custom_handlers.h"
4040

41-
// `php.h` sets `NDEBUG` when not `PHP_DEBUG` which will make `assert()` from
42-
// assert.h a no-op. In order to have `assert()` working on NDEBUG builds, we
43-
// undefine `NDEBUG` and re-include assert.h
44-
#undef NDEBUG
45-
#include "assert.h"
46-
4741
#if defined(HAVE_LIBXML) && !defined(PHP_WIN32)
4842
# include <libxml/globals.h>
4943
# include <libxml/parser.h>

main/php.h

-7
Original file line numberDiff line numberDiff line change
@@ -109,13 +109,6 @@ typedef int pid_t;
109109
# endif
110110
#endif
111111

112-
#if PHP_DEBUG
113-
#undef NDEBUG
114-
#else
115-
#ifndef NDEBUG
116-
#define NDEBUG
117-
#endif
118-
#endif
119112
#include <assert.h>
120113

121114
#ifdef HAVE_UNIX_H

win32/build/confutils.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -3429,7 +3429,7 @@ function toolset_setup_common_libs()
34293429
function toolset_setup_build_mode()
34303430
{
34313431
if (PHP_DEBUG == "yes") {
3432-
ADD_FLAG("CFLAGS", "/LDd /MDd /Od /D ZEND_DEBUG=1 " +
3432+
ADD_FLAG("CFLAGS", "/LDd /MDd /Od /U NDebug /U NDEBUG /D ZEND_DEBUG=1 " +
34333433
(TARGET_ARCH == 'x86'?"/ZI":"/Zi"));
34343434
ADD_FLAG("LDFLAGS", "/debug");
34353435
// Avoid problems when linking to release libraries that use the release

0 commit comments

Comments
 (0)