Skip to content

Commit

Permalink
Add an INI setting to control uint64_t -> zend_long overflow behavior
Browse files Browse the repository at this point in the history
  • Loading branch information
jbboehr committed Apr 7, 2024
1 parent 0e7dede commit f973dd8
Show file tree
Hide file tree
Showing 19 changed files with 307 additions and 127 deletions.
26 changes: 20 additions & 6 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ jobs:
- name: Install dependencies
run: composer install --prefer-dist --no-progress --ignore-platform-req=php+ --ignore-platform-req=ext-perfidious

- run: php -l perfidious.stub.php && php perfidious.stub.php

- name: phpcs
run: php vendor/bin/phpcs

Expand All @@ -57,13 +59,18 @@ jobs:

test:
runs-on: ubuntu-latest
name: "Test | PHP ${{ matrix.php-version }}"
name: "Test | PHP ${{ matrix.php-version }} ${{ matrix.debug }}"
strategy:
matrix:
php-version:
- "8.1"
- "8.2"
- "8.3"
debug:
- ""
include:
- php-version: "8.1"
debug: "debug"
steps:
- uses: actions/checkout@v4

Expand All @@ -80,11 +87,11 @@ jobs:
php-version: "${{ matrix.php-version }}"
tools: composer:v2

- run: php -l perfidious.stub.php && php perfidious.stub.php

- run: phpize

- run: ./configure --enable-compile-warnings=error
- run: ./configure --enable-compile-warnings=error "${EXTRA_FLAGS}"
env:
EXTRA_FLAGS: ${{ matrix.debug == 'debug' && '--enable-perfidious-debug' || '' }}

- run: make

Expand All @@ -107,13 +114,18 @@ jobs:

coverage:
runs-on: ubuntu-latest
name: "Coverage | PHP ${{ matrix.php-version }}"
name: "Coverage | PHP ${{ matrix.php-version }} ${{ matrix.debug }}"
strategy:
matrix:
php-version:
- "8.1"
- "8.2"
- "8.3"
debug:
- ""
include:
- php-version: "8.1"
debug: "debug"
steps:
- uses: actions/checkout@v4

Expand All @@ -132,7 +144,9 @@ jobs:

- run: phpize

- run: ./configure --enable-compile-warnings=error CFLAGS="-fprofile-arcs -ftest-coverage ${CFLAGS}" LDFLAGS="--coverage ${LDFLAGS}"
- run: ./configure --enable-compile-warnings=error "${EXTRA_FLAGS}" CFLAGS="-fprofile-arcs -ftest-coverage ${CFLAGS}" LDFLAGS="--coverage ${LDFLAGS}"
env:
EXTRA_FLAGS: ${{ matrix.debug == 'debug' && '--enable-perfidious-debug' || '' }}

- run: lcov --directory . --zerocounters

Expand Down
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ Some notable generic perf events are:

| Name | Default | Changeable | Description |
| --------------------- | -------- | ----------- | ------------ |
| `perfidious.overflow_mode` | `0` | `PHP_INI_SYSTEM` | Sets the overflow behavior when casting counters from `uint64_t` to `zend_long`. See the constants `Perfidious\OVERFLOW_*` for other values. Note that when set to `Perfidious\OVERFLOW_WARN`, `read` and `readArray` may return `NULL`, despite their type signatures indicating otherwise. |
| `perfidious.global.enable` | `0` | `PHP_INI_SYSTEM` | Set to `1` to enable the global handle. This handle is kept open between requests. You can read from this handle via e.g. `var_dump(Perfidious\global_handle()?->read());`. |
| `perfidious.global.metrics` | `perf::PERF_COUNT_HW_CPU_CYCLES:u`, `perf::PERF_COUNT_HW_INSTRUCTIONS:u` | `PHP_INI_SYSTEM` | The metrics to monitor with the global handle. |
| `perfidious.request.enable` | `0` | `PHP_INI_SYSTEM` | Set to `1` to enable the per-request handle. This handle is kept open between requests, but reset before and after. You can read from this handle via e.g. `var_dump(Perfidious\request_handle()?->read());` |
Expand Down
38 changes: 28 additions & 10 deletions flake.nix
Original file line number Diff line number Diff line change
Expand Up @@ -79,10 +79,12 @@
stdenv ? pkgs.stdenv,
php ? pkgs.php,
libpfm ? pkgs.libpfm,
debugSupport ? false,
}:
pkgs.callPackage ./nix/derivation.nix {
inherit src;
inherit stdenv php libpfm;
inherit debugSupport;
buildPecl = pkgs.callPackage (nixpkgs + "/pkgs/build-support/php/build-pecl.nix") {
inherit php stdenv;
};
Expand Down Expand Up @@ -146,7 +148,7 @@
# in opcache and relies on mkWrapper to load extensions
export TEST_PHP_ARGS='-c ${package.php.phpIni}'
# php.unwrapped from the buildDeps is overwriting php
export PATH="${package.php}/bin:$PATH"
export PATH="${package.php}/bin:./vendor/bin:$PATH"
'';
};

Expand Down Expand Up @@ -200,21 +202,31 @@
};

# @see https://github.com/NixOS/nixpkgs/pull/110787
buildConfs = lib.cartesianProductOfSets {
php = ["php81" "php82" "php83"];
stdenv = [
"gcc"
"clang"
# totally broken
# "musl"
buildConfs =
(lib.cartesianProductOfSets {
php = ["php81" "php82" "php83"];
stdenv = [
"gcc"
"clang"
# totally broken
# "musl"
];
libpfm = ["libpfm" "libpfm-unstable"];
})
++ [
{
php = "php81";
stdenv = "gcc";
libpfm = "libpfm";
debugSupport = true;
}
];
libpfm = ["libpfm" "libpfm-unstable"];
};

buildFn = {
php,
libpfm,
stdenv,
debugSupport ? false,
}:
lib.nameValuePair
(lib.concatStringsSep "-" (lib.filter (v: v != "") [
Expand All @@ -226,12 +238,18 @@
then ""
else "${libpfm}"
)
(
if debugSupport
then "debug"
else ""
)
]))
(
makePackage {
php = matrix.php.${php};
libpfm = matrix.libpfm.${libpfm};
stdenv = matrix.stdenv.${stdenv};
inherit debugSupport;
}
);

Expand Down
4 changes: 3 additions & 1 deletion nix/derivation.nix
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
buildPecl,
src,
checkSupport ? false,
debugSupport ? false,
WerrorSupport ? checkSupport,
valgrindSupport ? true,
}:
Expand All @@ -32,7 +33,8 @@ buildPecl rec {
configureFlags =
[]
++ lib.optional WerrorSupport "--enable-compile-warnings=error"
++ lib.optionals (!WerrorSupport) ["--enable-compile-warnings=yes" "--disable-Werror"];
++ lib.optionals (!WerrorSupport) ["--enable-compile-warnings=yes" "--disable-Werror"]
++ lib.optional debugSupport "--enable-perfidious-debug";

makeFlags = ["phpincludedir=$(dev)/include"];
outputs = ["out" "dev"];
Expand Down
16 changes: 15 additions & 1 deletion perfidious.stub.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

const VERSION = "0.1.0";

const OVERFLOW_THROW = 0;
const OVERFLOW_WARN = 1;
const OVERFLOW_SATURATE = 2;
const OVERFLOW_WRAP = 3;

/**
* @throws PmuNotFoundException
Expand Down Expand Up @@ -99,6 +103,7 @@ final public function disable(): self

/**
* Get a raw byte stream from the handle's file descriptor
*
* @note closing this resource will cause subsequent calls to read to fail
* @return resource
*/
Expand All @@ -107,6 +112,11 @@ final public function rawStream()
}

/**
* @note If perfidious.overflow_mode is set to Perfidious\OVERFLOW_WARN, this method can return null, despite its
* typehint. If perfidious.overflow_mode is set to any value other than Perfidious\OVERFLOW_THROW, this
* method will *not* throw an OverflowException.
*
* @return ReadResult
* @throws OverflowException|IOException
*
* @phpstan-return ReadResult<T>
Expand All @@ -116,7 +126,11 @@ final public function read(): ReadResult
}

/**
* @return array<string, int>
* @note If perfidious.overflow_mode is set to Perfidious\OVERFLOW_WARN, this method can return null, despite its
* typehint. If perfidious.overflow_mode is set to any value other than Perfidious\OVERFLOW_THROW, this
* method will *not* throw an OverflowException.
*
* @return array
* @throws OverflowException|IOException
*
* @phpstan-return array<value-of<T>, int>
Expand Down
11 changes: 11 additions & 0 deletions php_perfidious.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,15 @@ enum perfidious_error_mode
PERFIDIOUS_ERROR_MODE_WARNING = 1,
};

enum perfidious_overflow_mode
{
PERFIDIOUS_OVERFLOW_THROW = 0,
PERFIDIOUS_OVERFLOW_WARN = 1,
PERFIDIOUS_OVERFLOW_SATURATE = 2,
PERFIDIOUS_OVERFLOW_WRAP = 3,
PERFIDIOUS_OVERFLOW_MAX = 3,
};

PERFIDIOUS_PUBLIC extern zend_class_entry *perfidious_exception_interface_ce;
PERFIDIOUS_PUBLIC extern zend_class_entry *perfidious_pmu_not_found_exception_ce;
PERFIDIOUS_PUBLIC extern zend_class_entry *perfidious_pmu_event_not_found_exception_ce;
Expand All @@ -103,6 +112,7 @@ ZEND_BEGIN_MODULE_GLOBALS(perfidious)
struct perfidious_handle *request_handle;

enum perfidious_error_mode error_mode;
enum perfidious_overflow_mode overflow_mode;
ZEND_END_MODULE_GLOBALS(perfidious)

ZEND_EXTERN_MODULE_GLOBALS(perfidious);
Expand Down Expand Up @@ -171,6 +181,7 @@ zend_result perfidious_handle_read_to_array_with_times(
ZEND_HOT
PERFIDIOUS_PUBLIC
PERFIDIOUS_ATTR_NONNULL_ALL
PERFIDIOUS_ATTR_WARN_UNUSED_RESULT
zend_result
perfidious_handle_read_to_result(const struct perfidious_handle *restrict handle, zval *restrict return_value);

Expand Down
40 changes: 24 additions & 16 deletions src/extension.c
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
#include "ext/standard/php_string.h"

#include "php_perfidious.h"
#include "functions.h"
#include "handle.h"
#include "private.h"

Expand All @@ -63,6 +62,7 @@ static ZEND_INI_MH(OnUpdateStr)

// clang-format off
PHP_INI_BEGIN()
STD_PHP_INI_ENTRY(PHP_PERFIDIOUS_NAME ".overflow_mode", "0", PHP_INI_SYSTEM, OnUpdateLong, overflow_mode, zend_perfidious_globals, perfidious_globals)
STD_PHP_INI_ENTRY(PHP_PERFIDIOUS_NAME ".global.enable", "0", PHP_INI_SYSTEM, OnUpdateBool, global_enable, zend_perfidious_globals, perfidious_globals)
STD_PHP_INI_ENTRY(PHP_PERFIDIOUS_NAME ".global.metrics", DEFAULT_METRICS, PHP_INI_SYSTEM, OnUpdateStr, global_metrics, zend_perfidious_globals, perfidious_globals)
STD_PHP_INI_ENTRY(PHP_PERFIDIOUS_NAME ".request.enable", "0", PHP_INI_SYSTEM, OnUpdateBool, request_enable, zend_perfidious_globals, perfidious_globals)
Expand Down Expand Up @@ -152,10 +152,28 @@ static PHP_MINIT_FUNCTION(perfidious)
return FAILURE;
}

REGISTER_INI_ENTRIES();

#ifdef PERFIDIOUS_DEBUG
REGISTER_BOOL_CONSTANT(PHP_PERFIDIOUS_NAMESPACE "\\DEBUG", (zend_bool) PERFIDIOUS_DEBUG, flags);
#else
REGISTER_BOOL_CONSTANT(PHP_PERFIDIOUS_NAMESPACE "\\DEBUG", false, flags);
#endif
REGISTER_STRING_CONSTANT(PHP_PERFIDIOUS_NAMESPACE "\\VERSION", (char *) PHP_PERFIDIOUS_VERSION, flags);

REGISTER_LONG_CONSTANT(PHP_PERFIDIOUS_NAMESPACE "\\OVERFLOW_THROW", PERFIDIOUS_OVERFLOW_THROW, flags);
REGISTER_LONG_CONSTANT(PHP_PERFIDIOUS_NAMESPACE "\\OVERFLOW_WARN", PERFIDIOUS_OVERFLOW_WARN, flags);
REGISTER_LONG_CONSTANT(PHP_PERFIDIOUS_NAMESPACE "\\OVERFLOW_SATURATE", PERFIDIOUS_OVERFLOW_SATURATE, flags);
REGISTER_LONG_CONSTANT(PHP_PERFIDIOUS_NAMESPACE "\\OVERFLOW_WRAP", PERFIDIOUS_OVERFLOW_WRAP, flags);

#ifdef PERFIDIOUS_DEBUG
do {
char buf[128];
snprintf(buf, sizeof(buf), "%" PRIu64, UINT64_MAX);
REGISTER_STRING_CONSTANT(PHP_PERFIDIOUS_NAMESPACE "\\UINT64_MAX", buf, flags);
} while (false);
#endif

REGISTER_INI_ENTRIES();

perfidious_exceptions_minit();
perfidious_handle_minit();
perfidious_pmu_event_info_minit();
Expand Down Expand Up @@ -284,25 +302,15 @@ static PHP_GINIT_FUNCTION(perfidious)
perfidious_globals->error_mode = PERFIDIOUS_ERROR_MODE_THROW;
}

// clang-format off
PERFIDIOUS_LOCAL
const zend_function_entry perfidious_functions[] = {
ZEND_RAW_FENTRY(PHP_PERFIDIOUS_NAMESPACE "\\get_pmu_info", ZEND_FN(perfidious_get_pmu_info), perfidious_get_pmu_info_arginfo, 0)
ZEND_RAW_FENTRY(PHP_PERFIDIOUS_NAMESPACE "\\global_handle", ZEND_FN(perfidious_global_handle), perfidious_global_handle_arginfo, 0)
ZEND_RAW_FENTRY(PHP_PERFIDIOUS_NAMESPACE "\\list_pmus", ZEND_FN(perfidious_list_pmus), perfidious_list_pmus_arginfo, 0)
ZEND_RAW_FENTRY(PHP_PERFIDIOUS_NAMESPACE "\\list_pmu_events", ZEND_FN(perfidious_list_pmu_events), perfidious_list_pmu_events_arginfo, 0)
ZEND_RAW_FENTRY(PHP_PERFIDIOUS_NAMESPACE "\\open", ZEND_FN(perfidious_open), perfidious_open_arginfo, 0)
ZEND_RAW_FENTRY(PHP_PERFIDIOUS_NAMESPACE "\\request_handle", ZEND_FN(perfidious_request_handle), perfidious_request_handle_arginfo, 0)
PHP_FE_END
};
// clang-format on

static const zend_module_dep perfidious_deps[] = {
{"spl", NULL, NULL, MODULE_DEP_REQUIRED},
{"opcache", NULL, NULL, MODULE_DEP_OPTIONAL},
ZEND_MOD_END,
};

PERFIDIOUS_LOCAL
extern const zend_function_entry perfidious_functions[];

zend_module_entry perfidious_module_entry = {
STANDARD_MODULE_HEADER_EX,
NULL,
Expand Down
Loading

0 comments on commit f973dd8

Please sign in to comment.