Copilot commented on code in PR #13670: URL: https://github.com/apache/trafficserver/pull/13670#discussion_r3997564470
########## tools/benchmark/benchmark_Regex.cc: ########## @@ -0,0 +1,637 @@ +/** @file + + Benchmarks for the tsutil Regex wrapper: time per operation and the number and size + of heap allocations each operation makes. + + The allocation half matters as much as the timing half. PCRE2 routes every allocation + it makes for a compile or a match through the callbacks the wrapper installs, and those + call the system allocator, so counting calls to malloc across a region counts exactly + what the wrapper caused. Under the just-in-time engine a match should reach the system + allocator zero times, because the match data comes out of the caller's own buffer. The + interpreter is the exception: it allocates a backtracking frames vector through the same + allocator, so a match that runs interpreted does show up in the count. + + Interposing malloc is only wired up on Linux, where defining these symbols in the + executable is enough. Elsewhere the counters stay at zero and the report says so, so a + run on another platform still gives timings without quietly reporting zero allocations + as a result. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include <cstddef> +#include <cstdint> +#include <cstdio> +#include <cstdlib> +#include <cstring> +#include <string> +#include <string_view> +#include <vector> + +#define CATCH_CONFIG_ENABLE_BENCHMARKING +#include <catch2/catch_test_macros.hpp> +#include <catch2/benchmark/catch_benchmark.hpp> + +#include "tsutil/Regex.h" + +#define PCRE2_CODE_UNIT_WIDTH 8 +#include <pcre2.h> + +// --------------------------------------------------------------------------- +// Allocation counting +// --------------------------------------------------------------------------- + +namespace +{ +struct AllocStats { + unsigned long calls = 0; + unsigned long bytes = 0; +}; + +// Counting is per thread so a benchmark that spawns threads does not race the counters. +// These benchmarks are single threaded; the qualifier is here so the numbers stay honest +// if one is added later. +thread_local AllocStats alloc_stats; +thread_local bool alloc_counting = false; + +class CountAllocations +{ +public: + CountAllocations() + { + alloc_stats = AllocStats{}; + alloc_counting = true; + } + ~CountAllocations() { alloc_counting = false; } + + AllocStats + stats() const + { + return alloc_stats; + } +}; + +#if defined(__linux__) +constexpr bool ALLOC_COUNTING_AVAILABLE = true; +#else +constexpr bool ALLOC_COUNTING_AVAILABLE = false; +#endif + +} // namespace + +#if defined(__linux__) +#include <dlfcn.h> + +// Interpose the system allocator. Defining these in the executable takes precedence over +// libc for every caller in the process, which is what makes the count cover PCRE2's own +// allocations as well as the wrapper's. +namespace +{ +using malloc_fn = void *(*)(size_t); +using free_fn = void (*)(void *); +using calloc_fn = void *(*)(size_t, size_t); +using realloc_fn = void *(*)(void *, size_t); + +malloc_fn real_malloc = nullptr; +free_fn real_free = nullptr; +calloc_fn real_calloc = nullptr; +realloc_fn real_realloc = nullptr; + +// dlsym() itself can allocate while the real pointers are still being resolved. Hand +// those few allocations out of a static buffer rather than recursing. +// +// Each block is preceded by a header holding its size, so a realloc of one can copy the +// old contents rather than silently returning uninitialised storage. The header is one +// max_align_t wide so the pointer handed back keeps the alignment malloc promises. +constexpr size_t BOOTSTRAP_HEADER = alignof(std::max_align_t); +static_assert(BOOTSTRAP_HEADER >= sizeof(size_t), "the bootstrap header must hold a size"); + +alignas(std::max_align_t) char bootstrap_buffer[16384]; +size_t bootstrap_used = 0; +bool resolving = false; + +bool +from_bootstrap(void *p) +{ + return p >= static_cast<void *>(bootstrap_buffer) && p < static_cast<void *>(bootstrap_buffer + sizeof(bootstrap_buffer)); +} + +void * +bootstrap_alloc(size_t size) +{ + // Check the request against what is left before rounding it up. Rounding first would let + // a huge size wrap to a small payload, pass the capacity test, and hand back storage far + // smaller than asked for. These wrappers stand in for malloc for every library in the + // process while dlsym resolves, so that block would corrupt somebody else's startup. + size_t const remaining = sizeof(bootstrap_buffer) - bootstrap_used; + if (remaining <= BOOTSTRAP_HEADER || size > remaining - BOOTSTRAP_HEADER) { + return nullptr; + } + + size_t const payload = (size + alignof(std::max_align_t) - 1) & ~(alignof(std::max_align_t) - 1); + if (payload > remaining - BOOTSTRAP_HEADER) { + return nullptr; + } + char *block = bootstrap_buffer + bootstrap_used; + memcpy(block, &size, sizeof(size)); + bootstrap_used += BOOTSTRAP_HEADER + payload; + return block + BOOTSTRAP_HEADER; +} + +size_t +bootstrap_size(void *p) +{ + size_t size = 0; + memcpy(&size, static_cast<char *>(p) - BOOTSTRAP_HEADER, sizeof(size)); + return size; +} + +// Resolve all four into locals and publish them together, with real_malloc last. dlsym() +// may allocate or free while these lookups are in progress, which re-enters the wrappers +// below; they test their own pointer and fall back to the bootstrap path while it is still +// null, so no wrapper can reach a half-resolved table. +void +resolve_real_allocators() +{ + if (real_malloc != nullptr || resolving) { + return; + } + resolving = true; + + auto *m = reinterpret_cast<malloc_fn>(dlsym(RTLD_NEXT, "malloc")); + auto *f = reinterpret_cast<free_fn>(dlsym(RTLD_NEXT, "free")); + auto *c = reinterpret_cast<calloc_fn>(dlsym(RTLD_NEXT, "calloc")); + auto *r = reinterpret_cast<realloc_fn>(dlsym(RTLD_NEXT, "realloc")); + + real_free = f; + real_calloc = c; + real_realloc = r; + real_malloc = m; // published last: this is the pointer the early return above tests + + resolving = false; +} + +void +record(size_t size) +{ + if (alloc_counting) { + ++alloc_stats.calls; + alloc_stats.bytes += size; + } +} +} // namespace + +extern "C" void * +malloc(size_t size) noexcept +{ + if (real_malloc == nullptr) { + resolve_real_allocators(); + if (real_malloc == nullptr) { + return bootstrap_alloc(size); + } + } + record(size); + return real_malloc(size); +} + +extern "C" void +free(void *p) noexcept +{ + if (p == nullptr || from_bootstrap(p)) { + return; + } + if (real_free == nullptr) { + resolve_real_allocators(); + if (real_free == nullptr) { + // Still resolving, so there is nothing to free through. Leaking the few blocks the + // loader turns over during startup is better than calling through a null pointer. + return; + } + } + real_free(p); +} + +extern "C" void * +calloc(size_t n, size_t size) noexcept +{ + if (real_calloc == nullptr) { + resolve_real_allocators(); + if (real_calloc == nullptr) { + // n * size can wrap, which would ask the bootstrap buffer for a small block and then + // memset a huge one. Refuse rather than compute it. + if (size != 0 && n > SIZE_MAX / size) { + return nullptr; + } + size_t const total = n * size; + void *p = bootstrap_alloc(total); + if (p != nullptr) { + memset(p, 0, total); + } + return p; + } + } + record(n * size); + return real_calloc(n, size); +} + +extern "C" void * +realloc(void *p, size_t size) noexcept +{ + if (real_realloc == nullptr) { + resolve_real_allocators(); + } + + // A block handed out by bootstrap_alloc() is not one the system allocator knows, so it + // cannot be passed to the real realloc. Move it instead, carrying the old contents over: + // the loader reallocs while resolving symbols, and handing back uninitialised storage + // there makes the lookup fail in a way that is very hard to read. + if (from_bootstrap(p)) { + size_t const old = bootstrap_size(p); + size_t const copy = size < old ? size : old; + + if (real_malloc == nullptr) { + void *moved = bootstrap_alloc(size); + if (moved != nullptr) { + memcpy(moved, p, copy); + } + return moved; + } + + record(size); + void *moved = real_malloc(size); + if (moved != nullptr) { + memcpy(moved, p, copy); + } + return moved; + } + + if (real_realloc == nullptr) { + // Still resolving and this is not a bootstrap block, so there is nothing safe to do + // with it other than hand back a fresh one. + return bootstrap_alloc(size); + } + + record(size); + return real_realloc(p, size); +} +#endif // __linux__ + +// --------------------------------------------------------------------------- +// Corpus +// +// Patterns and subjects taken from what the tree actually matches: remap rules, a host +// allowlist, an extension test, and the crash-guard rule from #5762. +// --------------------------------------------------------------------------- + +namespace +{ +char const *const PATTERN_PATH = R"(^/([^/]+)/([^/]+)/(.*)$)"; +char const *const PATTERN_HOST = R"(^(?:[a-z0-9-]+\.)*example\.com$)"; +char const *const PATTERN_EXTENSION = R"(\.(jpg|jpeg|png|gif|css|js)$)"; +char const *const PATTERN_QUERY = R"(^/alpha/bravo/[?]((?!action=(newsfeed|calendar|contacts|notepad)).)*$)"; + +std::string_view const SUBJECT_PATH = "/images/2026/summer/header.jpg"; +std::string_view const SUBJECT_HOST = "cdn.edge.example.com"; +std::string_view const SUBJECT_EXTENSION = "/images/2026/summer/header.jpg"; +std::string_view const SUBJECT_MISS = "/no/match/here/at/all"; + +// A set of host patterns, the shape a rule list has when a caller scans one in order. +std::vector<std::string> +host_patterns(int count) +{ + std::vector<std::string> patterns; + patterns.reserve(count); + for (int i = 0; i < count; ++i) { + patterns.emplace_back("^(?:[a-z0-9-]+\\.)*host" + std::to_string(i) + "\\.example\\.com$"); + } + return patterns; +} + +// Whether this PCRE2 produced machine code for a pattern. The project requires only +// libpcre2-8, and a build can be configured without the just-in-time compiler or refuse an +// individual pattern, in which case a subject sized to exhaust the JIT stack instead runs +// to completion on the interpreter. That measures something else entirely, and far more +// slowly, so the cases that depend on the JIT ask first. +bool +pattern_has_jit(char const *pattern) +{ + int errnum = 0; + PCRE2_SIZE erroffset = 0; + pcre2_code *code = pcre2_compile(reinterpret_cast<PCRE2_SPTR>(pattern), PCRE2_ZERO_TERMINATED, 0, &errnum, &erroffset, nullptr); + if (code == nullptr) { + return false; + } + pcre2_jit_compile(code, PCRE2_JIT_COMPLETE); + size_t jit_size = 0; + pcre2_pattern_info(code, PCRE2_INFO_JITSIZE, &jit_size); + pcre2_code_free(code); + return jit_size > 0; Review Comment: `jit_size > 0` only proves that PCRE2 generated JIT code; it does not prove the 64 KiB subject reaches `PCRE2_ERROR_JIT_STACKLIMIT`. On a release with different JIT stack usage, both callers below will benchmark a normal match while labeling it as the stack-exhaustion case and report its allocations under that label. Probe the actual result (or keep increasing the subject) and skip unless the stack-limit error is observed. ########## tools/benchmark/benchmark_Regex.cc: ########## @@ -0,0 +1,637 @@ +/** @file + + Benchmarks for the tsutil Regex wrapper: time per operation and the number and size + of heap allocations each operation makes. + + The allocation half matters as much as the timing half. PCRE2 routes every allocation + it makes for a compile or a match through the callbacks the wrapper installs, and those + call the system allocator, so counting calls to malloc across a region counts exactly + what the wrapper caused. Under the just-in-time engine a match should reach the system + allocator zero times, because the match data comes out of the caller's own buffer. The + interpreter is the exception: it allocates a backtracking frames vector through the same + allocator, so a match that runs interpreted does show up in the count. + + Interposing malloc is only wired up on Linux, where defining these symbols in the + executable is enough. Elsewhere the counters stay at zero and the report says so, so a + run on another platform still gives timings without quietly reporting zero allocations + as a result. + + @section license License + + Licensed to the Apache Software Foundation (ASF) under one + or more contributor license agreements. See the NOTICE file + distributed with this work for additional information + regarding copyright ownership. The ASF licenses this file + to you under the Apache License, Version 2.0 (the + "License"); you may not use this file except in compliance + with the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. + */ + +#include <cstddef> +#include <cstdint> +#include <cstdio> +#include <cstdlib> +#include <cstring> +#include <string> +#include <string_view> +#include <vector> + +#define CATCH_CONFIG_ENABLE_BENCHMARKING +#include <catch2/catch_test_macros.hpp> +#include <catch2/benchmark/catch_benchmark.hpp> + +#include "tsutil/Regex.h" + +#define PCRE2_CODE_UNIT_WIDTH 8 +#include <pcre2.h> + +// --------------------------------------------------------------------------- +// Allocation counting +// --------------------------------------------------------------------------- + +namespace +{ +struct AllocStats { + unsigned long calls = 0; + unsigned long bytes = 0; +}; + +// Counting is per thread so a benchmark that spawns threads does not race the counters. +// These benchmarks are single threaded; the qualifier is here so the numbers stay honest +// if one is added later. +thread_local AllocStats alloc_stats; +thread_local bool alloc_counting = false; + +class CountAllocations +{ +public: + CountAllocations() + { + alloc_stats = AllocStats{}; + alloc_counting = true; + } + ~CountAllocations() { alloc_counting = false; } + + AllocStats + stats() const + { + return alloc_stats; + } +}; + +#if defined(__linux__) +constexpr bool ALLOC_COUNTING_AVAILABLE = true; +#else +constexpr bool ALLOC_COUNTING_AVAILABLE = false; +#endif + +} // namespace + +#if defined(__linux__) +#include <dlfcn.h> + +// Interpose the system allocator. Defining these in the executable takes precedence over +// libc for every caller in the process, which is what makes the count cover PCRE2's own +// allocations as well as the wrapper's. +namespace +{ +using malloc_fn = void *(*)(size_t); +using free_fn = void (*)(void *); +using calloc_fn = void *(*)(size_t, size_t); +using realloc_fn = void *(*)(void *, size_t); + +malloc_fn real_malloc = nullptr; +free_fn real_free = nullptr; +calloc_fn real_calloc = nullptr; +realloc_fn real_realloc = nullptr; + +// dlsym() itself can allocate while the real pointers are still being resolved. Hand +// those few allocations out of a static buffer rather than recursing. +// +// Each block is preceded by a header holding its size, so a realloc of one can copy the +// old contents rather than silently returning uninitialised storage. The header is one +// max_align_t wide so the pointer handed back keeps the alignment malloc promises. +constexpr size_t BOOTSTRAP_HEADER = alignof(std::max_align_t); +static_assert(BOOTSTRAP_HEADER >= sizeof(size_t), "the bootstrap header must hold a size"); + +alignas(std::max_align_t) char bootstrap_buffer[16384]; +size_t bootstrap_used = 0; +bool resolving = false; + +bool +from_bootstrap(void *p) +{ + return p >= static_cast<void *>(bootstrap_buffer) && p < static_cast<void *>(bootstrap_buffer + sizeof(bootstrap_buffer)); +} + +void * +bootstrap_alloc(size_t size) +{ + // Check the request against what is left before rounding it up. Rounding first would let + // a huge size wrap to a small payload, pass the capacity test, and hand back storage far + // smaller than asked for. These wrappers stand in for malloc for every library in the + // process while dlsym resolves, so that block would corrupt somebody else's startup. + size_t const remaining = sizeof(bootstrap_buffer) - bootstrap_used; + if (remaining <= BOOTSTRAP_HEADER || size > remaining - BOOTSTRAP_HEADER) { + return nullptr; + } + + size_t const payload = (size + alignof(std::max_align_t) - 1) & ~(alignof(std::max_align_t) - 1); + if (payload > remaining - BOOTSTRAP_HEADER) { + return nullptr; + } + char *block = bootstrap_buffer + bootstrap_used; + memcpy(block, &size, sizeof(size)); + bootstrap_used += BOOTSTRAP_HEADER + payload; + return block + BOOTSTRAP_HEADER; +} + +size_t +bootstrap_size(void *p) +{ + size_t size = 0; + memcpy(&size, static_cast<char *>(p) - BOOTSTRAP_HEADER, sizeof(size)); + return size; +} + +// Resolve all four into locals and publish them together, with real_malloc last. dlsym() +// may allocate or free while these lookups are in progress, which re-enters the wrappers +// below; they test their own pointer and fall back to the bootstrap path while it is still +// null, so no wrapper can reach a half-resolved table. +void +resolve_real_allocators() +{ + if (real_malloc != nullptr || resolving) { + return; + } + resolving = true; + + auto *m = reinterpret_cast<malloc_fn>(dlsym(RTLD_NEXT, "malloc")); + auto *f = reinterpret_cast<free_fn>(dlsym(RTLD_NEXT, "free")); + auto *c = reinterpret_cast<calloc_fn>(dlsym(RTLD_NEXT, "calloc")); + auto *r = reinterpret_cast<realloc_fn>(dlsym(RTLD_NEXT, "realloc")); + + real_free = f; + real_calloc = c; + real_realloc = r; + real_malloc = m; // published last: this is the pointer the early return above tests + + resolving = false; +} + +void +record(size_t size) +{ + if (alloc_counting) { + ++alloc_stats.calls; + alloc_stats.bytes += size; + } +} +} // namespace + +extern "C" void * +malloc(size_t size) noexcept +{ + if (real_malloc == nullptr) { + resolve_real_allocators(); + if (real_malloc == nullptr) { + return bootstrap_alloc(size); + } + } + record(size); + return real_malloc(size); +} + +extern "C" void +free(void *p) noexcept +{ + if (p == nullptr || from_bootstrap(p)) { + return; + } + if (real_free == nullptr) { + resolve_real_allocators(); + if (real_free == nullptr) { + // Still resolving, so there is nothing to free through. Leaking the few blocks the + // loader turns over during startup is better than calling through a null pointer. + return; + } + } + real_free(p); +} + +extern "C" void * +calloc(size_t n, size_t size) noexcept +{ + if (real_calloc == nullptr) { + resolve_real_allocators(); + if (real_calloc == nullptr) { + // n * size can wrap, which would ask the bootstrap buffer for a small block and then + // memset a huge one. Refuse rather than compute it. + if (size != 0 && n > SIZE_MAX / size) { + return nullptr; + } + size_t const total = n * size; + void *p = bootstrap_alloc(total); + if (p != nullptr) { + memset(p, 0, total); + } + return p; + } + } + record(n * size); + return real_calloc(n, size); +} + +extern "C" void * +realloc(void *p, size_t size) noexcept +{ + if (real_realloc == nullptr) { + resolve_real_allocators(); + } + + // A block handed out by bootstrap_alloc() is not one the system allocator knows, so it + // cannot be passed to the real realloc. Move it instead, carrying the old contents over: + // the loader reallocs while resolving symbols, and handing back uninitialised storage + // there makes the lookup fail in a way that is very hard to read. + if (from_bootstrap(p)) { + size_t const old = bootstrap_size(p); + size_t const copy = size < old ? size : old; + + if (real_malloc == nullptr) { + void *moved = bootstrap_alloc(size); + if (moved != nullptr) { + memcpy(moved, p, copy); + } + return moved; + } + + record(size); + void *moved = real_malloc(size); + if (moved != nullptr) { + memcpy(moved, p, copy); + } + return moved; + } + + if (real_realloc == nullptr) { + // Still resolving and this is not a bootstrap block, so there is nothing safe to do + // with it other than hand back a fresh one. + return bootstrap_alloc(size); Review Comment: This fallback still violates `realloc` semantics for a non-bootstrap pointer: if a loader reallocates an allocation that was not issued by `bootstrap_alloc()` while symbol resolution is in progress, this returns unrelated uninitialized storage and leaves the original pointer untouched. That can corrupt the loader's state; do not return a fresh bootstrap block here—fail without replacing the original, or otherwise route the request to the real allocator once resolution is safe. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
