Pass parameters by reference when calling bindParam

[joshuabenuck]

This PR fixes issue #143.

In the PHP 8 upgrading internals document, there is this note:

The zend_fcall_info no_separation flag has been removed, and separation is
never allowed. If you wish to pass (or allow passing) arguments by
reference, explicitly create those arguments as references using
ZEND_MAKE_REF. This removal also affects call_user_function_ex(), which
should be replaced by call_user_function().

Here are some details about what that means in practice and how it compares to previous PHP releases.

PHP 8.0 emits a warning when a reference is needed and wasn't provided. The runtime still creates a reference, it is just noisy about having to do it.

Prior to PHP 8.0, a flag could be passed in which indicated whether the runtime should create a reference in such scenarios. If a reference was needed and one wasn't provided, the behavior is essentially what PHP 8.0 does (warning, but creates a reference anyway).

In our pdo instrumentation, we call nr_php_call to invoke bindParam. This uses PHP's call_user_function. At this particular call site, a reference is needed and since we don't provide one, a warning is emitted.

The first attempt blindly created a reference for all parameters, but this breaks things as it introduces references where they are not expected. To truly copy the previous behavior, we would need to call ARG_SHOULD_BE_SENT_BY_REF which requires that we resolve the function before invoking call_user_function which will have to resolve the function again. This introduces extra overhead which would be best to avoid.

We may be able to invoke a call_user_function variant that requires the function handle instead of the function name, but this is a more invasive change.

The approach in the latest revision chooses to require each call site make the determination about whether a reference is needed before calling nr_php_call. If there are places where we use nr_php_call and friends to make calls to arbitrary functions, such call sites will need to be reworked to programmatically determine whether a reference is needed. In practice, we usually just call a known function. We just now need to be sure to add references, where they are needed, before making such calls.

The reported issue was found using Doctrine. The call path creates a prepared statement and calls bindValue for the parameters. I was unable to reproduce the warning message using bindParam despite that being what we instrument. I can only assume that bindValue somehow depends on bindParam.

The added integration test runs a prepared statement and calls bindValue on a single parameter which enough to trigger the warning when the fix is not present.

[joshuabenuck]

Tests that call file_get_contents or that touch curl are not passing on the 8.0 PR jenkins job. The instrumentation for these functions makes use of nr_php_call so they would be impacted by these changes.

Additionally, the valgrind run of the unit tests on the GHA 8.0 builds looks to be consistently failing with a curl related problem.

[kneitinger]

Looks good, thank you for tracking this down and fixing it!