Nikita Popov [Wed, 11 Nov 2020 10:51:20 +0000 (11:51 +0100)]
Retain reference to share handle from curl handle
Not keeping a reference will not result in use after free, because
curl protects against it, but it will result in a memory leak,
because curl_share_cleanup() will fail. We should make sure that
the share handle object stays alive as long as the curl handles
use it.
If Zip operations fails due to PHP error conditions before libzip even
has been called, there is no meaningful indication what failed; the
functions just return false, and the Zip status indicated that no error
did occur. Therefore we raise `E_WARNING` in these cases.
Nikita Popov [Thu, 5 Nov 2020 10:58:31 +0000 (11:58 +0100)]
Fix multiple trait fixup
If a trait method is inherited, preloading trait fixup might be
performed on it multiple times. Usually this is fine, because
the opcodes pointer will have already been updated, and will thus
not be found in the xlat table.
However, it can happen that the new opcodes pointer is the same
as one of the old opcodes pointers, if the pointer has been reused
by the allocator. In this case we will look up the wrong op array
and overwrite the trait method with an unrelated trait method.
We fix this by indexing the xlat table not by the opcodes pointer,
but by the refcount pointer. The refcount pointer is not changed
during optimization, and accurately represents which op arrays
should use the same opcodes.
Fixes bug #80307. The test case does not reproduce the bug, because
this depends on a lot of "luck" with the allocator. The test case
merely illustrates a case where orig_op_array would have been NULL
in the original code.
Fix #80266: parse_url silently drops port number 0
As of commit 81b2f3e[1], `parse_url()` accepts URLs with a zero port,
but does not report that port, what is wrong in hindsight.
Since the port number is stored as `unsigned short` there is no way to
distinguish between port zero and no port. For BC reasons, we thus
introduce `parse_url_ex2()` which accepts an output parameter that
allows that distinction, and use the new function to fix the behavior.
The introduction of `parse_url_ex2()` has been suggested by Nikita.
Nikita Popov [Wed, 4 Nov 2020 13:51:44 +0000 (14:51 +0100)]
Assert that references are not persisted
There should not be any need to persist references, and it's unlikely
that persisting a reference will behave correctly at runtime, because
we don't have a concept of an immutable reference.
Nikita Popov [Tue, 3 Nov 2020 15:45:13 +0000 (16:45 +0100)]
Don't disable early binding during preloading script
We should only disable early binding during the opcache_compile_file()
calls, not inside the preloading script or anything it includes.
The right condition to check for is whether we compile the file
without execution, as declaring classes is "execution".
Nikita Popov [Tue, 3 Nov 2020 13:15:05 +0000 (14:15 +0100)]
Fix variance checks on resolved union types
This is a bit annoying: When preloading is used, types might be
resolved during inheritance checks, so we need to deal with CE
types rather than just NAME types everywhere.
Nikita Popov [Tue, 3 Nov 2020 10:50:14 +0000 (11:50 +0100)]
Don't ignore internal classes during preloading
When preloading, it's fine to make use of internal class information,
as we do not support Windows. It is also necessary to allow proper
variance checks against internal classes.
Alex Dowad [Thu, 29 Oct 2020 21:10:04 +0000 (23:10 +0200)]
Convert numeric string array keys to integers correctly in JITted code
While fixing bugs in mbstring, one of my new test cases failed with a strange
error message stating: 'Warning: Undefined array key 1...', when clearly the
array key had been set properly.
GDB'd that sucker and found that JIT'd PHP code was calling directly into
`zend_hash_add_new` (which was not converting the numeric string key to an
integer properly). But where was that code coming from? I examined the disasm,
looked up symbols to figure out where call instructions were going, then grepped
the codebase for those function names. It soon became clear that the disasm I
was looking at was compiled from `zend_jit_fetch_dim_w_helper`.
Nikita Popov [Fri, 30 Oct 2020 10:11:16 +0000 (11:11 +0100)]
Fixed bug #80290
Dropping the dtor arg args[3] rather than using STR_COPY: Since
PHP 8, we no longer support separation in call_user_function(),
so we also don't need to worry about things like arguments being
replaced with references.
Fix bug #72413: Segfault with get_result and PS cursors
We cannot simply switch to use_result here, because the fetch_row
methods in get_result mode and in use_result/store_result mode
are different: In one case it accepts a statement, in the other
a return value zval. Thus, doing a switch to use_result results
in a segfault when trying to fetch a row.
Actually supporting get_result with cursors would require adding
cursor support in mysqlnd_result, not just mysqlnd_ps. That would
be a significant amount of effort and, given the age of the issue,
does not appear to be particularly likely to happen soon.
As such, we simply generate an error when using get_result()
with cursors, which is much better than causing a segfault.
Instead, parameter binding needs to be used.
Nikita Popov [Thu, 29 Oct 2020 13:07:08 +0000 (14:07 +0100)]
Handle errors during PDO row fetch
The EOF flag also gets set on error, so we always end up ignoring
errors here.
However, we should only check errors for unbuffered results. For
buffered results, this function is guaranteed not to error, and
querying the errno may return an unrelated error.
Fix #44618: Fetching may rely on uninitialized data
Unless `SQLGetData()` returns `SQL_SUCCESS` or `SQL_SUCCESS_WITH_INFO`,
the `StrLen_or_IndPtr` output argument is not guaranteed to be properly
set. Thus we handle retrieval failure other than `SQL_ERROR` by
yielding `false` for those column values and raising a warning.
Nikita Popov [Wed, 28 Oct 2020 16:12:35 +0000 (17:12 +0100)]
Fixed bug #65825
Set error_info when we fail to read a packet, instead of throwing
a warning. Additionally we also need to populate the right
error_info in rowp_read -- we'll later take the error from the
packet, not the connection.
No test case, as this is hard to reliably test. I'm using the
test case from:
https://github.com/php/php-src/pull/2131#issuecomment-538374838
projects / php / log