Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .github/actions/build-extension/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@ inputs:
required: false
default: 'ON'
description: 'Link the C++ driver statically (ON bundles it into the .so)'
sanitize_address:
required: false
default: 'OFF'
description: 'Build with AddressSanitizer (ON enables ENABLE_SANITIZERS + SANITIZE_ADDRESS)'

runs:
using: composite
Expand All @@ -43,6 +47,9 @@ runs:
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache \
-DPHP_SCYLLADB_STATIC="${{ inputs.static_driver }}" \
-DPHP_SCYLLADB_BACKEND="${BACKEND}" \
-DENABLE_SANITIZERS="${{ inputs.sanitize_address }}" \
-DSANITIZE_ADDRESS="${{ inputs.sanitize_address }}" \
-DSANITIZE_UNDEFINED=OFF \
-DENABLE_LTO="${{ inputs.enable_lto }}"

- name: Build
Expand Down
100 changes: 100 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,106 @@ jobs:
SCYLLADB_HOSTS: "127.0.0.1"
SCYLLADB_PHP_BACKEND: ${{ steps.driver.outputs.backend }}

test-asan:
name: "ASan: PHP ${{ matrix.php }}-nts / scylladb (memory-safety guard)"
runs-on: ubuntu-24.04

services:
scylladb:
image: scylladb/scylla:2026.1
options: >-
--health-cmd "cqlsh -e 'SELECT cluster_name FROM system.local' 2>/dev/null"
--health-interval 10s
--health-timeout 5s
--health-retries 30
--health-start-period 60s
ports:
- 9042:9042

strategy:
fail-fast: false
matrix:
php:
- "8.4"
- "8.5"

steps:
- uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
with:
fetch-depth: 1
submodules: recursive

- name: Install system dependencies
run: |
sudo apt-get update -q
sudo apt-get install -y --no-install-recommends \
pkg-config build-essential ninja-build libuv1-dev

- uses: shivammathur/setup-php@f3e473d116dcccaddc5834248c87452386958240 # v2
with:
php-version: ${{ matrix.php }}
ini-file: development
coverage: none
tools: composer, php-config, phpize
extensions: pcntl, mbstring, curl, intl
env:
phpts: nts
debug: true
fail-fast: true

- name: Setup CMake
uses: jwlawson/actions-setup-cmake@0d6a7d60b009d01c9e7523be22153ff8f19460d3 # v2
with:
cmake-version: "3.30"

- name: Setup ccache
uses: hendrikmuhs/ccache-action@5ebbd400eff9e74630f759d94ddd7b6c26299639 # v1.2
with:
key: asan-${{ runner.os }}-${{ matrix.php }}-nts-scylladb
restore-keys: |
asan-${{ runner.os }}-${{ matrix.php }}-nts-scylladb
asan-${{ runner.os }}-${{ matrix.php }}-
max-size: 500M
append-timestamp: false

- uses: ./.github/actions/install-cpp-driver
id: driver
with:
driver: scylladb

# AddressSanitizer-instrumented build. This is the regression guard for
# the cycle-collector use-after-free (a shared type's CassDataType freed
# by a mis-accounted get_gc): the whole suite was clean under ASan only
# after the get_gc handlers were fixed. A plain test run is not a reliable
# guard — the fault is heap-layout dependent and need not crash on x86.
- uses: ./.github/actions/build-extension
with:
driver: scylladb
cpp_driver_pkg_config_path: ${{ steps.driver.outputs.pkg_config_path }}
sanitize_address: 'ON'

- name: Install Composer dependencies
run: |
composer install --no-ansi --no-interaction --no-progress --prefer-dist

- name: Run tests under AddressSanitizer
# The extension is ASan-instrumented but the setup-php binary is not, so
# the ASan runtime is LD_PRELOADed and USE_ZEND_ALLOC=0 routes Zend
# allocations through it. detect_leaks=0 because PHP intentionally leaks
# persistent allocations at shutdown; detect_odr_violation=0 avoids false
# positives from the statically bundled C++ driver. halt_on_error=1 fails
# the job on the first heap-use-after-free / overflow.
run: |
ASAN_RT="$(gcc -print-file-name=libasan.so)"
echo "ASan runtime: ${ASAN_RT}"
LD_PRELOAD="${ASAN_RT}" \
USE_ZEND_ALLOC=0 \
ASAN_OPTIONS="detect_leaks=0:detect_odr_violation=0:verify_asan_link_order=0:abort_on_error=1:halt_on_error=1" \
php -d extension=cassandra ./vendor/bin/pest --colors=always
env:
SCYLLADB_HOSTS: "127.0.0.1"
SCYLLADB_PHP_BACKEND: ${{ steps.driver.outputs.backend }}

pie-build:
name: PIE / PHP ${{ matrix.php }}-nts / ${{ matrix.driver }}
runs-on: ubuntu-24.04
Expand Down
23 changes: 23 additions & 0 deletions cmake/FindLibuv.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,29 @@ else ()

if (LINK_LIBUV_STATIC)
pkg_check_modules(LIBUV REQUIRED IMPORTED_TARGET libuv-static)

# libuv-static.pc lists the archive as "-l:libuv.a" (GNU ld name-form).
# GNU ld resolves it, but Apple's ld64 cannot parse the "-l:name" form
# and fails with: ld: library ':libuv.a' not found. On Apple, resolve
# the archive to an absolute path and substitute it into the imported
# target's link interface so the bare name never reaches the linker.
if (APPLE)
find_library(LIBUV_STATIC_ARCHIVE
NAMES libuv.a uv_a
HINTS ${LIBUV_STATIC_LIBRARY_DIRS} ${LIBUV_LIBRARY_DIRS})
if (NOT LIBUV_STATIC_ARCHIVE)
message(FATAL_ERROR
"LINK_LIBUV_STATIC=ON but no libuv static archive was found in "
"'${LIBUV_STATIC_LIBRARY_DIRS}'. Install a static libuv "
"or configure with -DLINK_LIBUV_STATIC=OFF.")
endif ()
get_target_property(_libuv_link_libs PkgConfig::LIBUV INTERFACE_LINK_LIBRARIES)
list(TRANSFORM _libuv_link_libs
REPLACE "^(-l)?:?libuv\\.a$" "${LIBUV_STATIC_ARCHIVE}")
set_target_properties(PkgConfig::LIBUV PROPERTIES
INTERFACE_LINK_LIBRARIES "${_libuv_link_libs}")
unset(_libuv_link_libs)
endif ()
else ()
pkg_check_modules(LIBUV REQUIRED IMPORTED_TARGET libuv)
endif ()
Expand Down
10 changes: 9 additions & 1 deletion src/Cluster/BuilderHandlers.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@ extern zend_object_handlers php_scylladb_cluster_builder_handlers;

HashTable *php_scylladb_cluster_builder_gc(zend_object *object, zval **table, int *n)
{
return zend_std_get_gc(object, table, n);
/* Expose the only zval the builder holds (default_timeout) via the GC
* buffer. Going through zend_std_get_gc would instead invoke the
* side-effecting get_properties (it rebuilds object->properties on every
* call), which corrupts refcounts across the collector's mark/scan passes. */
auto self = php_scylladb_cluster_builder_object_fetch(object);
zend_get_gc_buffer *buffer = zend_get_gc_buffer_create();
zend_get_gc_buffer_add_zval(buffer, &self->default_timeout);
zend_get_gc_buffer_use(buffer, table, n);
return nullptr;
}
HashTable *php_scylladb_cluster_builder_properties(zend_object *object)
{
Expand Down
10 changes: 9 additions & 1 deletion src/Collection.c
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,15 @@ PHP_METHOD(Cassandra_Collection, remove)
HashTable *
php_scylladb_collection_gc(zend_object *object, zval** table, int *n)
{
return zend_std_get_gc(object, table, n);
auto self = php_scylladb_collection_object_fetch(object);
zend_get_gc_buffer *buffer = zend_get_gc_buffer_create();
zend_get_gc_buffer_add_zval(buffer, &self->type);
zval *current;
ZEND_HASH_FOREACH_VAL(&self->values, current) {
zend_get_gc_buffer_add_zval(buffer, current);
} ZEND_HASH_FOREACH_END();
zend_get_gc_buffer_use(buffer, table, n);
return nullptr;
}

HashTable *
Expand Down
8 changes: 4 additions & 4 deletions src/DefaultSession.c
Original file line number Diff line number Diff line change
Expand Up @@ -484,7 +484,7 @@ ZEND_METHOD(Cassandra_DefaultSession, execute) {
ZEND_PARSE_PARAMETERS_START(1, 2)
Z_PARAM_ZVAL(statement)
Z_PARAM_OPTIONAL
Z_PARAM_ZVAL(options)
Z_PARAM_ZVAL_OR_NULL(options)
ZEND_PARSE_PARAMETERS_END();

self = PHP_SCYLLADB_GET_SESSION(getThis());
Expand Down Expand Up @@ -634,7 +634,7 @@ ZEND_METHOD(Cassandra_DefaultSession, executeAsync) {
ZEND_PARSE_PARAMETERS_START(1, 2)
Z_PARAM_ZVAL(statement)
Z_PARAM_OPTIONAL
Z_PARAM_ZVAL(options)
Z_PARAM_ZVAL_OR_NULL(options)
ZEND_PARSE_PARAMETERS_END();

self = PHP_SCYLLADB_GET_SESSION(getThis());
Expand Down Expand Up @@ -736,7 +736,7 @@ ZEND_METHOD(Cassandra_DefaultSession, prepare) {
ZEND_PARSE_PARAMETERS_START(1, 2)
Z_PARAM_ZVAL(cql)
Z_PARAM_OPTIONAL
Z_PARAM_ZVAL(options)
Z_PARAM_ZVAL_OR_NULL(options)
ZEND_PARSE_PARAMETERS_END();

self = PHP_SCYLLADB_GET_SESSION(getThis());
Expand Down Expand Up @@ -843,7 +843,7 @@ ZEND_METHOD(Cassandra_DefaultSession, prepareAsync) {
ZEND_PARSE_PARAMETERS_START(1, 2)
Z_PARAM_ZVAL(cql)
Z_PARAM_OPTIONAL
Z_PARAM_ZVAL(options)
Z_PARAM_ZVAL_OR_NULL(options)
ZEND_PARSE_PARAMETERS_END();

self = PHP_SCYLLADB_GET_SESSION(getThis());
Expand Down
12 changes: 11 additions & 1 deletion src/Map.c
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,17 @@ PHP_METHOD(Cassandra_Map, offsetExists)
HashTable *
php_scylladb_map_gc(zend_object *object, zval** table, int *n)
{
return zend_std_get_gc(object, table, n);
auto self = php_scylladb_map_object_fetch(object);
zend_get_gc_buffer *buffer = zend_get_gc_buffer_create();
zend_get_gc_buffer_add_zval(buffer, &self->type);
php_scylladb_map_entry *curr, *temp;
HASH_ITER(hh, self->entries, curr, temp)
{
zend_get_gc_buffer_add_zval(buffer, &curr->key);
zend_get_gc_buffer_add_zval(buffer, &curr->value);
}
zend_get_gc_buffer_use(buffer, table, n);
return nullptr;
}

HashTable *
Expand Down
6 changes: 5 additions & 1 deletion src/Numbers/Bigint.c
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,11 @@ void php_scylladb_bigint_init(INTERNAL_FUNCTION_PARAMETERS)
return;
}

self->data.bigint.value = (cass_int64_t)Z_DVAL_P(value);
/* zend_dval_to_lval gives PHP's well-defined double->int64 conversion.
* A raw (int64_t) cast is undefined when double_value == 2^63 (which
* (double)INT64_MAX rounds up to, so the bound check above lets it
* through) and saturates differently per platform. */
self->data.bigint.value = zend_dval_to_lval(double_value);
}
else if (Z_TYPE_P(value) == IS_STRING)
{
Expand Down
8 changes: 7 additions & 1 deletion src/Numbers/Decimal.c
Original file line number Diff line number Diff line change
Expand Up @@ -524,7 +524,13 @@ HashTable *php_scylladb_decimal_gc(
#endif
zval** table, int *n)
{
return zend_std_get_gc(object, table, n);
/* Holds no zvals (mpz_t + scale only). Report no roots directly instead of
* going through zend_std_get_gc, which would invoke the side-effecting
* get_properties (it rebuilds object->properties on every call) and corrupt
* refcounts across the collector's mark/scan passes. */
*table = nullptr;
*n = 0;
return nullptr;
}

HashTable *php_scylladb_decimal_properties(
Expand Down
7 changes: 5 additions & 2 deletions src/Numbers/NumberParser.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,8 +137,11 @@ php_scylladb_parse_int(char* in, int in_len, cass_int32_t* number )
}

if (errno == ERANGE) {
zend_throw_exception_ex(php_scylladb_range_exception_ce, 0 ,
"value must be between %d and %d, %s given", INT_MIN, INT_MAX, in);
/* Signal the overflow through errno (still ERANGE here) and return without
* throwing. The only callers are Tinyint/Smallint, which emit their own
* narrower "must be between ..." range error; throwing here would both
* report the wrong (32-bit) bounds and clobber errno via the exception
* machinery before the caller can inspect it. */
return 0;
}

Expand Down
8 changes: 4 additions & 4 deletions src/Numbers/Smallint.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,10 +107,10 @@ void php_scylladb_smallint_init(INTERNAL_FUNCTION_PARAMETERS)
if (!php_scylladb_parse_int(Z_STRVAL_P(value), Z_STRLEN_P(value), &number ))
{

// If the parsing function fails, it would have set an exception. If it's
// a range error, the error message would be wrong because the parsing
// function supports all 32-bit values, so the "valid" range it reports would
// be too large for Smallint. Reset the exception in that case.
// parse_int reports a 32-bit overflow via errno == ERANGE without
// throwing (its 32-bit bounds are too wide for Smallint); emit the
// narrower range error here. Any other failure already carries its
// own exception.

if (errno == ERANGE)
{
Expand Down
8 changes: 4 additions & 4 deletions src/Numbers/Tinyint.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,10 +104,10 @@ void php_scylladb_tinyint_init(INTERNAL_FUNCTION_PARAMETERS)
{
if (!php_scylladb_parse_int(Z_STRVAL_P(value), Z_STRLEN_P(value), &number ))
{
// If the parsing function fails, it would have set an exception. If it's
// a range error, the error message would be wrong because the parsing
// function supports all 32-bit values, so the "valid" range it reports would
// be too large for Tinyint. Reset the exception in that case.
// parse_int reports a 32-bit overflow via errno == ERANGE without
// throwing (its 32-bit bounds are too wide for Tinyint); emit the
// narrower range error here. Any other failure already carries its
// own exception.

if (errno == ERANGE)
{
Expand Down
8 changes: 7 additions & 1 deletion src/Numbers/Varint.c
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,13 @@ HashTable *php_scylladb_varint_gc(
#endif
zval** table, int *n )
{
return zend_std_get_gc(object, table, n);
/* Holds no zvals (mpz_t only). Report no roots directly instead of going
* through zend_std_get_gc, which would invoke the side-effecting
* get_properties (it rebuilds object->properties on every call) and corrupt
* refcounts across the collector's mark/scan passes. */
*table = nullptr;
*n = 0;
return nullptr;
}

HashTable *php_scylladb_varint_properties(
Expand Down
11 changes: 10 additions & 1 deletion src/Set.c
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,16 @@ PHP_METHOD(Cassandra_Set, rewind)
HashTable*
php_scylladb_set_gc(zend_object* object, zval** table, int* n)
{
return zend_std_get_gc(object, table, n);
auto self = php_scylladb_set_object_fetch(object);
zend_get_gc_buffer *buffer = zend_get_gc_buffer_create();
zend_get_gc_buffer_add_zval(buffer, &self->type);
php_scylladb_set_entry *curr, *temp;
HASH_ITER(hh, self->entries, curr, temp)
{
zend_get_gc_buffer_add_zval(buffer, &curr->value);
}
zend_get_gc_buffer_use(buffer, table, n);
return nullptr;
}

HashTable*
Expand Down
17 changes: 16 additions & 1 deletion src/Tuple.c
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,22 @@ ZEND_METHOD(Cassandra_Tuple, rewind)
HashTable *
php_scylladb_tuple_gc(zend_object *object, zval** table, int *n)
{
return zend_std_get_gc(object, table, n);
/* Expose the internal zvals (the tuple type and every element value) to the
* cycle collector. These live in the C struct, not in std properties, so the
* default handler would hide them and the GC could prematurely free a still
* referenced type/value. */
auto self = php_scylladb_tuple_object_fetch(object);
zend_get_gc_buffer *buffer = zend_get_gc_buffer_create();

zend_get_gc_buffer_add_zval(buffer, &self->type);

zval *current;
ZEND_HASH_FOREACH_VAL(&self->values, current) {
zend_get_gc_buffer_add_zval(buffer, current);
} ZEND_HASH_FOREACH_END();

zend_get_gc_buffer_use(buffer, table, n);
return nullptr;
}

HashTable *
Expand Down
6 changes: 4 additions & 2 deletions src/Type/Collection.c
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,10 @@ HashTable* php_scylladb_type_collection_gc(
zendObject* object,
#endif
zval** table, int* n) {
*table = nullptr;
*n = 0;
auto self = php_scylladb_type_object_fetch(object);
zend_get_gc_buffer *buffer = zend_get_gc_buffer_create();
zend_get_gc_buffer_add_zval(buffer, &self->data.collection.value_type);
zend_get_gc_buffer_use(buffer, table, n);
return nullptr;
}

Expand Down
Loading
Loading