Add internal API for Error Notification Callbacks

[beberlei]

PHP Extensions are able to hook into the error mechanism of the Zend Engine by overwriting the zend_error_cb callback. This callback has a lot of responsibilities though (display, repeated error supression, logging, exception conversion), so that overwriting is quite difficult. A few extensions also overwrite the error handler in a way that previous handlers are not called anymore (xdebug), making it unreliable.

This PR introduces a new API that allows extensions to register global error notification handlers.

• These handlers are only responsible for processing errors and notifying third party systems with a guarantee that they are called. Stopping propagation is explicitly not the idea of this API. Handlers are supposed to have no side effects on further error processing.
• These handlers are implemented as zend_llist, so no care has to be taken to call previous handlers, making it more robust than chaining function pointers.
• The notification callbacks are triggered outside of php_error_cb, so that it is guaranteed they are always called when an error occurs, even when zend_error_cb is overwritten. This also means that the notification happens before repeatable errors are ignored and last_error is updated.
• This API is internal and not exposed to userland.

Example

As an example I was able to refactor the inlined DTrace notification handling into the new API.

report_zend_debug code is also be a candidate for being a notification callback and was migrated. This debug code is not used anywhere, maybe it should be removed though?

Usage Example:

void dtrace_error_notify_cb(int type, const char *error_filename, uint32_t error_lineno, zend_string *message)
{
	if (DTRACE_ERROR_ENABLED()) {
		DTRACE_ERROR(ZSTR_VAL(message), (char *)error_filename, error_lineno);
	}
}

zend_register_error_notify_callback(dtrace_error_notify_cb);

Questions

• Should this be part of the proposed new zend_instrument API? zend_instrument_install_error_callback(cb)

Out of scope

The error mechanism can use a larger cleanup as discussed in the Improved Error Callbacks Mechanism RFC that was never implemented.

This PR is only about a reliable notification callback for now. Other changes may follow this.

[nikic]

One thing to note here is that there's some places that directly call zend_error_cb.

[nikic]

Needs a rebase after #5639.

[nikic]

This looks reasonable to me.

[nikic]

Here and elsewhere, do not use const uint32_t, just uint32_t. This makes particularly little sense inside signatures.

[beberlei]

zend_error_cb also has const uint32_t in signature, thats where I copied it from. Probably good to have another PR to clean that up, because it affects at least ext/soap

[morrisonlevi]

I'm a bit busy so I went through it quickly. Checking one thing:

This also means that the notification happens before repeatable errors are ignored and last_error is updated.

Can you give me an example? I have avoided the previous hooks because I was aware of the unreliability, so I'm not familiar this case.

[beberlei]

@morrisonlevi In php_error_cb there is code that checks the ini setting ignore_repeated_errors. If that is On, then the log_errors + display_errors code is not executed, when the previous error (based on the full message!) is exactly the same. This is mostly important for warnings, notices and deprecations.

[beberlei]

@krakjoe @nikic @Girgias @morrisonlevi i am going to merge this next week unless you have comments.

[beberlei]

merged with 6c8b94e

[twose]

Should it be ZEND_API?

[nikic]

Yeah, it should.

[beberlei]

oops, will fix it

[beberlei]

Ah you already committed this yesterday, thanks