Skip to content

Fix build against PHP 8.6 (php_verror params argument removed) - #629

Closed
andypost wants to merge 1 commit into
krakjoe:masterfrom
andypost:fix-php-verror-php86
Closed

Fix build against PHP 8.6 (php_verror params argument removed)#629
andypost wants to merge 1 commit into
krakjoe:masterfrom
andypost:fix-php-verror-php86

Conversation

@andypost

Copy link
Copy Markdown
Contributor

php-src commit 0a12b3e8268 (php/php-src#22865, merged 2026-07-24) removed the docref params argument from php_verror(), changing its signature from 5 to 4 arguments. Since then APCu fails to compile against master:

/tmp/apcu/apc.c:44:17: error: too many arguments to function 'php_verror'; expected 4, have 5
   44 |                 php_verror(NULL, "", verbosity, format, args);

This adds a version-guarded apc_php_verror() wrapper macro (a #if cannot live inside the APC_PRINT_FUNCTION macro body), so the code builds against both PHP 8.6-dev and older versions.

🤖 Generated with Claude Code

https://claude.ai/code/session_01JNDQDyaiHPPMtsno9Kyo4t

php-src commit 0a12b3e8268 (php/php-src#22865) removed the docref params
argument from php_verror(), changing its signature from 5 to 4 arguments.
Since a PHP_VERSION_ID guard cannot distinguish 8.6-dev snapshots taken
before and after that change, format the message with vspprintf() and
report it through php_error_docref(), whose signature is stable across
all supported versions. Passing NULL docref and empty params to
php_verror() was equivalent to php_error_docref(NULL, ...) anyway.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JNDQDyaiHPPMtsno9Kyo4t
@andypost
andypost force-pushed the fix-php-verror-php86 branch from f2566fd to eac054a Compare July 24, 2026 14:53
@andypost

Copy link
Copy Markdown
Contributor Author

Faced it at Drupal's nightly pipeline https://git.drupalcode.org/project/drupalci_environments/-/jobs/11143560

Comment thread apc.c
Comment on lines 37 to 59
/* console display functions */
#define APC_PRINT_FUNCTION(name, verbosity) \
void apc_##name(const char *format, ...) \
{ \
va_list args; \
char *buf; \
\
va_start(args, format); \
php_verror(NULL, "", verbosity, format, args); \
vspprintf(&buf, 0, format, args); \
va_end(args); \
php_error_docref(NULL, verbosity, "%s", buf); \
efree(buf); \
}

APC_PRINT_FUNCTION(error, E_ERROR)
APC_PRINT_FUNCTION(warning, E_WARNING)
APC_PRINT_FUNCTION(notice, E_NOTICE)

#ifdef APC_DEBUG
APC_PRINT_FUNCTION(debug, E_NOTICE)
#else
void apc_debug(const char *format, ...) {}
#endif

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Generating functions with a macro-mechanism seems unnecessarily complex here. Especially since all the generated functions do pretty much exactly what php_error_docref() does. I know it was like that before, but I think it's time to change it, now.

It's probably better to remove these functions / macros completely from apc.c and replace the definitions in apc.h with macros that propagate the variadic data directly to php_error_docref().

Therefore, i propose to replace this...

apcu/apc.h

Lines 80 to 84 in bc9e296

/* console display functions */
PHP_APCU_API void apc_error(const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2);
PHP_APCU_API void apc_warning(const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2);
PHP_APCU_API void apc_notice(const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2);
PHP_APCU_API void apc_debug(const char *format, ...) ZEND_ATTRIBUTE_FORMAT(printf, 1, 2);

by something like this:

/* convenience macros for console output */
#define apc_error(...) php_error_docref(NULL, E_ERROR, __VA_ARGS__)
#define apc_warning(...) php_error_docref(NULL, E_WARNING, __VA_ARGS__)
#define apc_notice(...) php_error_docref(NULL, E_NOTICE, __VA_ARGS__)

#ifdef APC_DEBUG
# define apc_debug(...) php_error_docref(NULL, E_NOTICE, __VA_ARGS__)
#else
# define apc_debug(...) do {} while (0)
#endif

@andypost andypost Jul 25, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Sounds like great idea and clean-up but it breaks ABI so not clear if it allowed

@andypost

Copy link
Copy Markdown
Contributor Author

Opened #630 as an alternative that implements @madmajestro's review suggestion: replace the generated console functions with macros forwarding to php_error_docref(). It fixes the same 8.6 build break and supersedes this PR. Trade-off (removes four PHP_APCU_API exported symbols) is written up there — committer's call which approach to take.

@andypost

Copy link
Copy Markdown
Contributor Author

After a closer look I recommend closing this PR in favor of #630.

Reviewing my own diff here turned up a structural defect: for apc_error the verbosity is E_ERROR, and php_error_docref() on that level ends in zend_bailout() — a longjmp that never returns (main/main.c, the E_ERROR case). So the efree(buf) after the call is unreachable on that path and the vspprintf-allocated buffer leaks. Harmless in practice (request-scoped emalloc, reclaimed at shutdown right after — the request is terminating anyway), but a debug-ZMM build or Valgrind run of the error path will flag it.

The defect can't be patched within this PR's shape: the buffer can't be freed before the call (it is the argument) nor after (bailout jumped away). The only fix is to not allocate — i.e. forward format+args directly to php_error_docref(), which is exactly what #630 does per @madmajestro's suggestion. Any repair of this PR converges on that one.

Both PRs are green across the full CI matrix including the 8.6 job. Leaving this open only as the minimal fallback if the PHP_APCU_API symbol removal in #630 is unwanted — otherwise #630 is the better change and this can be closed.

@madmajestro

Copy link
Copy Markdown
Collaborator

I am closing this as it has been superseded by #630, which was recently merged.

@andypost
andypost deleted the fix-php-verror-php86 branch July 26, 2026 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants