From eae01caea2209f7edf341bf01f2df052d9aa6db4 Mon Sep 17 00:00:00 2001 From: Dimitris Karakasilis Date: Sat, 15 Aug 2026 21:10:45 +0300 Subject: [PATCH 1/2] build: make the tree compile on gcc 16, and add a CI lane that keeps it that way Six files used a POSIX symbol without including its header and compiled only because an older libstdc++ pulled the header in transitively. gcc 16 does not, so they fail. One is src/vllm/entrypoints/openai/server_main.cpp, i.e. a SHIPPED binary that does not build on a current distro. This is the second time this exact break has landed. Three files were fixed after a gfx1151 report on #41; the pattern came back in five more, because every Linux CI lane installs the distro g++ (gcc 13 on ubuntu-latest) and no lane uses anything newer. A point fix does not hold, so this adds the missing coverage rather than only the includes: * build-newest-gcc: a COMPILE-ONLY lane in the gcc:16 container, tests configured ON so the test tree is compiled too. It deliberately does not run ctest -- build-test-cpu already owns execution, and this lane exists to catch compile-time portability for roughly one extra build per PR. * tests/support/process_id.h: ONE portable process id, next to the existing test_env.h that exists for the same reason (#603). The tests name temporary directories after the process; ::getpid() is POSIX and MSVC ships no , so the naive include fixes gcc 16 and breaks both windows-msvc lanes. A per-file #ifdef would be copied into the next loader test exactly as the broken idiom was, so the portable spelling lands once. * cmake/CompilerWarnings.cmake: on gcc >= 16 only, -Warray-bounds is reported but no longer fatal. gcc 16 emits it inside libstdc++ and vendored nlohmann for correct code, and no change to the calling code avoids it: after inlining, _Sp_counted_base::_M_release() is identical for every shared_ptr type, so GCC attributes one instantiation's destructor to another's allocation size. libstdc++ carries its own #pragma suppressions around that destructor; upstream is GCC PR tree-optimization/122197. gcc <= 15 is unchanged and still fails the build on a real out-of-bounds. * examples/ltx2_gen/main.cpp: the retake knobs become a NAMED array instead of a braced-init-list iterated in place, which is the second thing the lane found. Both forms are correct -- that backing array IS lifetime-extended for the loop -- but the in-place form draws -Wdangling-reference on gcc 16. The named array has automatic storage, so no reference binds to a temporary and the false positive cannot arise; nothing is suppressed. Noted because it cost a build to learn: making the LOOP VARIABLE a copy does not help, the diagnostic is about the range reference rather than the element. * src/vllm/entrypoints/openai/server_main.cpp: the include sits inside the file's existing non-Windows block rather than at the top, because MSVC ships no . That hunk is Ettore Di Giacinto's, from the review of this pull request, folded into this commit rather than left standing alone: check-doc-checkpoint requires every commit touching src/vllm/entrypoints/ to update docs/USAGE.md, and it was repairing a regression this commit had introduced in the first place. * docs/USAGE.md: a Host compilers section stating which compilers build the tree and what the gcc 16 warning policy is. Required by check-doc-checkpoint, which classifies src/vllm/entrypoints/ as a user-facing surface and has no bypass by design. Verified on gcc 16.1.1 (Arch): full CPU configure + build with tests ON, no command-line workaround, 0 errors. The workflow file itself is NOT verified locally -- a GitHub Actions lane cannot be executed off the forge -- but its first run on this pull request already earned its keep: it found a sixth file (::sysconf in qwen3_5.cpp) that no other lane can see. Windows is NOT verified here either: there is no MSVC on this machine, so the two windows-msvc lanes on this pull request are the first execution of the portable header. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: Claude-Code:claude-opus-5 [Claude Code] Signed-off-by: Dimitris Karakasilis --- .github/workflows/ci.yml | 40 ++++++++++++++++ cmake/CompilerWarnings.cmake | 27 ++++++++++- docs/USAGE.md | 16 +++++++ examples/ltx2_gen/main.cpp | 15 +++++- src/vllm/entrypoints/openai/server_main.cpp | 1 + tests/support/process_id.h | 47 +++++++++++++++++++ .../models/test_indextts2_s2mel_loader.cpp | 3 +- .../models/test_indextts2_talker_loader.cpp | 3 +- tests/vllm/models/test_ltx2_text_encoder.cpp | 3 +- .../models/test_minimax_music3_loader.cpp | 9 ++-- 10 files changed, 154 insertions(+), 10 deletions(-) create mode 100644 tests/support/process_id.h diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index fd3f97520..a797f95d0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -958,6 +958,46 @@ jobs: echo "Running test_ops_matmul_elem with VT_CPU_MATMUL_TIER=${tier}" VT_CPU_MATMUL_TIER="${tier}" build/tests/test_ops_matmul_elem done + build-newest-gcc: + # COMPILE-ONLY coverage on a compiler NEWER than any other lane's. Every + # other Linux lane installs the distro `g++`, which on ubuntu-latest is + # gcc 13, so a header that only compiles because of a TRANSITIVE include + # goes green here and red on a current distro. That is not hypothetical: + # ::getpid without was fixed once and came back in five more + # files, one of them src/vllm/entrypoints/openai/server_main.cpp — a + # SHIPPED binary that does not build on gcc 16 (reported on issue #41). + # + # Deliberately does NOT run ctest: this lane exists to catch compile-time + # portability, and build-test-cpu already owns execution. Keeping it to a + # build is roughly one extra compile per PR. + concurrency: + group: ci-build-newest-gcc-${{ github.event_name }}-${{ github.ref }}-${{ github.repository }} + cancel-in-progress: ${{ github.event_name != 'schedule' && github.event_name != 'workflow_dispatch' }} + if: github.event.action != 'closed' + runs-on: ubuntu-latest + timeout-minutes: 90 + container: gcc:16 + steps: + - name: Install build tools + # The gcc image ships the toolchain only; git is needed BEFORE checkout + # so actions/checkout uses git rather than the slower tarball path. + run: | + apt-get update + DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends \ + ca-certificates cmake git ninja-build python3 + rm -rf /var/lib/apt/lists/* + - uses: actions/checkout@v4 + - name: Report the compiler this lane is actually pinning + run: g++ --version + - name: Configure + run: | + cmake -S . -B build-newest-gcc -G Ninja \ + -DCMAKE_BUILD_TYPE=Release \ + -DVLLM_CPP_BUILD_TESTS=ON + - name: Build + # Same bounded parallelism as build-test-cpu: a bare -j OOM-kills the + # runner during the parallel link of the test executables. + run: cmake --build build-newest-gcc -j 2 build-test-cpu-arm64: # Independent Arm execution evidence: the x86 lane cannot prove HWCAP # dispatch, Arm instructions, or the host ABI. The native runner exercises diff --git a/cmake/CompilerWarnings.cmake b/cmake/CompilerWarnings.cmake index 6a56933f6..0c7918306 100644 --- a/cmake/CompilerWarnings.cmake +++ b/cmake/CompilerWarnings.cmake @@ -15,12 +15,37 @@ function(vllm_cpp_set_warnings target) if(NOT VLLM_CPP_SANITIZE STREQUAL "OFF") set(_vllm_cpp_werror "") endif() + + # GCC >= 16 reports -Warray-bounds inside LIBSTDC++ and the vendored nlohmann + # json for code that is correct, so the diagnostic stays VISIBLE but stops + # being fatal on those compilers only. Everything <= 15 is unchanged and still + # fails the build on a real out-of-bounds. + # + # It is the same false-positive class this file already documents above for + # the sanitizer lanes, and it is not something the calling code can avoid: + # `_Sp_counted_base::_M_release()` is identical machine code for every + # shared_ptr type, so after inlining GCC attributes ONE instantiation's + # destructor to ANOTHER instantiation's allocation size. On gcc 16.1.1 that + # presents as "array subscript 'std::mutex[0]' is partly outside array bounds + # of 'unsigned char [32]'" pointing at a json.dump() call, in a translation + # unit that contains no shared_ptr at all. + # + # libstdc++ carries its own `#pragma GCC diagnostic` suppressions around that + # very destructor (bits/shared_ptr_base.h), i.e. the standard library treats + # this as a warning to silence rather than a bug to fix in user code. Upstream + # is GCC PR tree-optimization/122197; Eigen, assimp and CMSSW all disable the + # check the same way on the affected releases. + set(_vllm_cpp_array_bounds "") + if(CMAKE_CXX_COMPILER_ID STREQUAL "GNU" AND + CMAKE_CXX_COMPILER_VERSION VERSION_GREATER_EQUAL 16) + set(_vllm_cpp_array_bounds -Wno-error=array-bounds) + endif() if(MSVC) target_compile_options(${target} PRIVATE $<$:/W4 /WX>) else() target_compile_options(${target} PRIVATE - $<$:-Wall -Wextra ${_vllm_cpp_werror}> + $<$:-Wall -Wextra ${_vllm_cpp_werror} ${_vllm_cpp_array_bounds}> # OBJCXX (.mm — the Metal backend) is a SEPARATE COMPILE_LANGUAGE from CXX, # so the CXX genex above does not reach it. Without this line the Metal TUs # would be the only unwarned code in the tree (BACKEND-METAL-MLX W0). diff --git a/docs/USAGE.md b/docs/USAGE.md index 4838477f8..e9d4972ef 100644 --- a/docs/USAGE.md +++ b/docs/USAGE.md @@ -22,6 +22,22 @@ example targets are named after the directories they are built from, so an in-source build makes the linker write each executable over its own source directory (issue #85). +### Host compilers + +gcc 13 and 14 and clang are exercised by CI, and **gcc 16 builds the tree, +including the OpenAI server**. Before this it did not: several files, one of +them the server's own `main`, called `getpid()` without including `` +and compiled only because an older libstdc++ happened to pull that header in +for them. A compile-only CI lane on the newest released gcc now guards this, +because every other Linux lane uses the distro compiler and cannot see it. + +On gcc 16 the `array-bounds` warning is reported but is **not** treated as an +error, unlike on every earlier gcc. That release emits it inside libstdc++ and +the vendored JSON library for code that is correct, and no change to the +calling code avoids it (`cmake/CompilerWarnings.cmake` explains the mechanism +and cites the upstream gcc bug). A genuine out-of-bounds still fails the build +on gcc 15 and earlier, which is what the rest of CI enforces. + ### Setting the compiled build identity `vllm-server --version` reports the CMake project version by default. Release diff --git a/examples/ltx2_gen/main.cpp b/examples/ltx2_gen/main.cpp index b75b70df1..b447798e5 100644 --- a/examples/ltx2_gen/main.cpp +++ b/examples/ltx2_gen/main.cpp @@ -425,7 +425,17 @@ int main(int argc, char** argv) { // Each retake knob rides the SAME per-generation array. Supplying one without // the window is refused by the engine rather than ignored, so a half-typed // retake reports what is missing instead of rendering the ordinary path. - for (const auto& kv : {std::make_pair("retake_start_time", &retake_start), + // The knobs are a NAMED array rather than a braced-init-list iterated in + // place. Both are correct -- a braced-init-list bound to the range variable + // has its backing array lifetime-extended for the whole loop -- but the + // in-place form draws -Wdangling-reference from gcc 16, which + // `build-newest-gcc` found on its first run. The warning is a false positive + // and it is not silenced: the range now has automatic storage and a name, so + // no reference binds to a temporary and the question does not arise. Making + // the loop variable a copy does NOT help; the diagnostic is about the range + // reference, not the element. + const std::pair retake_knobs[] = { + std::make_pair("retake_start_time", &retake_start), std::make_pair("retake_end_time", &retake_end), std::make_pair("retake_frame_rate", &retake_fps), std::make_pair("regenerate_video", ®en_video), @@ -452,7 +462,8 @@ int main(int argc, char** argv) { std::make_pair("video_skip_step", &video_skip_step), std::make_pair("video_stg_blocks", &video_stg_blocks), std::make_pair("a2v_guidance_scale", &a2v_scale), - std::make_pair("v2a_guidance_scale", &v2a_scale)}) { + std::make_pair("v2a_guidance_scale", &v2a_scale)}; + for (const auto& kv : retake_knobs) { if (kv.second->empty()) continue; gen_keys.emplace_back(kv.first); gen_values.push_back(*kv.second); diff --git a/src/vllm/entrypoints/openai/server_main.cpp b/src/vllm/entrypoints/openai/server_main.cpp index 291e6bb21..550e0bab1 100644 --- a/src/vllm/entrypoints/openai/server_main.cpp +++ b/src/vllm/entrypoints/openai/server_main.cpp @@ -57,6 +57,7 @@ #include #include #include +#include // ::getpid below; guarded because MSVC has no such header #include #endif diff --git a/tests/support/process_id.h b/tests/support/process_id.h new file mode 100644 index 000000000..03488b411 --- /dev/null +++ b/tests/support/process_id.h @@ -0,0 +1,47 @@ +// vllm.cpp original (test harness); no upstream mirror. +// +// ONE portable process id for the whole test tree. +// +// WHY IT EXISTS. Tests name their temporary directories after the running +// process so two concurrent runs cannot collide. `::getpid()` is POSIX, and its +// declaration lives in ``, which MSVC does not ship at all. So the +// obvious spelling does not fail on Windows — it does not COMPILE, and it takes +// both `windows-msvc-*` lanes down with it. Exactly the shape of issue #603, +// which is why `tests/support/test_env.h` next door exists. +// +// It also fails on a CURRENT POSIX toolchain for the opposite reason. Several +// files called `::getpid()` while including nothing that declares it, and +// compiled anyway because an older libstdc++ pulled `` in for them. +// gcc 16 does not: +// +// error: '::getpid' has not been declared; did you mean 'getpt'? +// +// That was fixed once in three files and came back in five more, because each +// new loader test copies the temp-directory helper from the last one. A per-file +// `#ifdef` would be copied just as faithfully and get it wrong again, so the +// portable spelling lands ONCE, here, and new tests include it. +#pragma once + +#if defined(_WIN32) +#include +#else +#include +#endif + +namespace vllm_test { + +// The current process id, for building a name no other process will pick. +// +// NOT for anything but naming. The two platforms agree that this is a unique +// live-process identifier and agree on nothing else about it, so it is used for +// uniqueness only, never compared against a recorded value or reused after the +// process exits. +inline int ProcessId() { +#if defined(_WIN32) + return ::_getpid(); +#else + return static_cast(::getpid()); +#endif +} + +} // namespace vllm_test diff --git a/tests/vllm/models/test_indextts2_s2mel_loader.cpp b/tests/vllm/models/test_indextts2_s2mel_loader.cpp index 43ec72a4d..5df4d05cb 100644 --- a/tests/vllm/models/test_indextts2_s2mel_loader.cpp +++ b/tests/vllm/models/test_indextts2_s2mel_loader.cpp @@ -20,6 +20,7 @@ #include "vllm/model_executor/models/indextts2_config.h" #include "vllm/model_executor/models/indextts2_s2mel_loader.h" +#include "support/process_id.h" namespace { std::string U64Le(uint64_t v) { @@ -117,7 +118,7 @@ std::string BuildTail(int64_t hidden = 8, int64_t in_ch = 4, int64_t wn = 8, std::string WriteTemp(const std::string& bytes, const std::string& tag) { const std::filesystem::path path = std::filesystem::temp_directory_path() / - ("indextts2_s2mel_" + tag + "_" + std::to_string(::getpid()) + ".safetensors"); + ("indextts2_s2mel_" + tag + "_" + std::to_string(vllm_test::ProcessId()) + ".safetensors"); std::ofstream out(path, std::ios::binary); out.write(bytes.data(), static_cast(bytes.size())); out.close(); diff --git a/tests/vllm/models/test_indextts2_talker_loader.cpp b/tests/vllm/models/test_indextts2_talker_loader.cpp index e8920c124..bbdf56fa7 100644 --- a/tests/vllm/models/test_indextts2_talker_loader.cpp +++ b/tests/vllm/models/test_indextts2_talker_loader.cpp @@ -16,6 +16,7 @@ #include "vllm/model_executor/models/indextts2_config.h" #include "vllm/model_executor/models/indextts2_talker_loader.h" +#include "support/process_id.h" namespace { std::string U64Le(uint64_t v) { @@ -79,7 +80,7 @@ std::string BuildTalker(int64_t H = 8, int64_t L = 2, int64_t I = 16, int64_t TV std::string WriteTemp(const std::string& bytes, const std::string& tag) { const std::filesystem::path p = std::filesystem::temp_directory_path() / - ("indextts2_talker_" + tag + "_" + std::to_string(::getpid()) + ".safetensors"); + ("indextts2_talker_" + tag + "_" + std::to_string(vllm_test::ProcessId()) + ".safetensors"); std::ofstream out(p, std::ios::binary); out.write(bytes.data(), static_cast(bytes.size())); return p.string(); diff --git a/tests/vllm/models/test_ltx2_text_encoder.cpp b/tests/vllm/models/test_ltx2_text_encoder.cpp index b36f0b0f4..9ce58a802 100644 --- a/tests/vllm/models/test_ltx2_text_encoder.cpp +++ b/tests/vllm/models/test_ltx2_text_encoder.cpp @@ -37,6 +37,7 @@ #include "doctest/doctest.h" #include "support/max_abs_diff.h" +#include "support/process_id.h" #include "vllm/model_executor/model_loader/safetensors_reader.h" #include "vllm/model_executor/models/gemma4.h" #include "vllm/model_executor/models/ltx2_loader.h" @@ -1457,7 +1458,7 @@ TEST_CASE("ltx2 text: a non-f32 compute dtype is REFUSED, never silently widened TEST_CASE("ltx2 text: the tokenizer and HF sidecars come out of TENSORS, not files") { const fs::path dir = - fs::temp_directory_path() / ("ltx2_text_assets_" + std::to_string(::getpid())); + fs::temp_directory_path() / ("ltx2_text_assets_" + std::to_string(vllm_test::ProcessId())); fs::create_directories(dir); const std::string tokenizer = R"({"version":"1.0","model":{"type":"BPE"}})"; diff --git a/tests/vllm/models/test_minimax_music3_loader.cpp b/tests/vllm/models/test_minimax_music3_loader.cpp index 847ebca41..2afaef2d6 100644 --- a/tests/vllm/models/test_minimax_music3_loader.cpp +++ b/tests/vllm/models/test_minimax_music3_loader.cpp @@ -43,6 +43,7 @@ #include "vllm/model_executor/models/minimax_music3_loader.h" #include "vllm/model_executor/models/vocoder1d.h" +#include "support/process_id.h" using vllm::MiniMaxMusic3AccountReport; using vllm::MiniMaxMusic3AccountTensors; using vllm::MiniMaxMusic3Config; @@ -164,7 +165,7 @@ std::vector EntriesFor(const std::vector& spec std::string TempPath(const char* stem) { return (std::filesystem::temp_directory_path() / - (std::string("music3_") + stem + "_" + std::to_string(::getpid()) + ".safetensors")) + (std::string("music3_") + stem + "_" + std::to_string(vllm_test::ProcessId()) + ".safetensors")) .string(); } @@ -733,7 +734,7 @@ TEST_CASE("music3 load: a component whose file lost a tensor throws BY NAME") { TEST_CASE("music3 resolve: a NATIVE-arm checkpoint is refused, naming what is missing") { const std::filesystem::path root = std::filesystem::temp_directory_path() / - ("music3_native_" + std::to_string(::getpid())); + ("music3_native_" + std::to_string(vllm_test::ProcessId())); std::filesystem::remove_all(root); // The layout convert_minimax_music3_to_diffusers.py:30,34,38 reads, and the // one sglang_omni/models/minimax_music3/checkpoint.py:35-56 serves. @@ -776,7 +777,7 @@ TEST_CASE("music3 resolve: ONE native marker is enough to be diagnosed as the na for (const char* marker : {"flowmatching_vae.pth", "dav.pth", "qwen_7B"}) { const std::filesystem::path root = std::filesystem::temp_directory_path() / - ("music3_partial_" + std::string(marker) + "_" + std::to_string(::getpid())); + ("music3_partial_" + std::string(marker) + "_" + std::to_string(vllm_test::ProcessId())); std::filesystem::remove_all(root); std::filesystem::create_directories(root); if (std::string(marker) == "qwen_7B") { @@ -804,7 +805,7 @@ TEST_CASE("music3 resolve: ONE native marker is enough to be diagnosed as the na TEST_CASE("music3 resolve: a tree that is NEITHER arm names the components it lacks") { const std::filesystem::path root = - std::filesystem::temp_directory_path() / ("music3_empty_" + std::to_string(::getpid())); + std::filesystem::temp_directory_path() / ("music3_empty_" + std::to_string(vllm_test::ProcessId())); std::filesystem::remove_all(root); std::filesystem::create_directories(root); CHECK(!vllm::MiniMaxMusic3IsNativeArm(root.string())); From 316389d1a8f01ad112c5130976ce6613622a0851 Mon Sep 17 00:00:00 2001 From: Ettore Di Giacinto Date: Mon, 17 Aug 2026 22:29:34 +0000 Subject: [PATCH 2/2] fix(qwen3.5): the gcc-16 lane found a sixth file, on its first run MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `build-newest-gcc` failed on the very pull request that adds it, which is the lane working rather than the lane breaking: src/vllm/model_executor/models/qwen3_5.cpp:5296:27: error: '::sysconf' has not been declared; did you mean 'swscanf'? src/vllm/model_executor/models/qwen3_5.cpp:5296:35: error: '_SC_PAGESIZE' was not declared in this scope `g++ (GCC) 16.2.0`, container `gcc:16`. This is the class the lane exists for, one file further than the five this PR already repairs, and a different symbol pair from the same header — so a `::getpid` sweep would not have found it. `` was already included for the same block and carried the file this far on gcc 13 through a transitive declaration. The include goes inside the `#if defined(__unix__)` guard the file already has at the top, which is the same condition guarding the `::sysconf` call at `:5294`. `qwen3_5.cpp` is not in `REQUIRED_CPP` in `scripts/check-windows-portability.py` and is not built by `scripts/build-windows-release.ps1`, so no Windows arm sees it either way; `check-windows-portability.py` returns `Windows portability contract OK`. FOLLOWING_AGENTS_PROTOCOL Following-Agents-Protocol: true AI-Assisted: true Assisted-by: ClaudeCode:claude-opus-5 [ClaudeCode] Signed-off-by: Dimitris Karakasilis --- src/vllm/model_executor/models/qwen3_5.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/vllm/model_executor/models/qwen3_5.cpp b/src/vllm/model_executor/models/qwen3_5.cpp index f24db5807..bc4702d21 100644 --- a/src/vllm/model_executor/models/qwen3_5.cpp +++ b/src/vllm/model_executor/models/qwen3_5.cpp @@ -1,5 +1,6 @@ #if defined(__unix__) #include +#include // ::sysconf(_SC_PAGESIZE) in the readahead hint below #endif // vllm.cpp original; see qwen3_5.h. Forward math mirrored 1:1 from the pinned // upstream (qwen3_next.py::Qwen3NextDecoderLayer / Qwen3NextModel.forward,