libghostty: add ghostty_free for cross-runtime memory safety#7
Closed
deblasis wants to merge 19 commits into
Closed
libghostty: add ghostty_free for cross-runtime memory safety#7deblasis wants to merge 19 commits into
deblasis wants to merge 19 commits into
Conversation
On Windows, shared libraries (DLLs) require an import library (.lib) for linking, and the DLL itself is placed in bin/ rather than lib/ by the Zig build. The CMake wrapper was missing IMPORTED_IMPLIB on the shared imported target, causing link failures, and assumed the shared library was always in lib/. Add GHOSTTY_VT_IMPLIB for the import library name, set IMPORTED_IMPLIB on the ghostty-vt target, and fix the shared library path to use bin/ on Windows. Install the DLL and PDB to bin/ and the import library to lib/ following standard Windows conventions. Apply the same fixes to ghostty-vt-config.cmake.in for the find_package path.
Use writerStreaming() instead of writer() for stdout in helpgen and main_build_data. The positional writer calls setEndPos/ftruncate in end(), which fails on Windows when stdout is redirected via captureStdOut() because ftruncate maps INVALID_PARAMETER to FileTooBig. Streaming mode skips truncation entirely since stdout is inherently a sequential stream. Replace scandir with opendir/readdir plus qsort in framegen since scandir is a POSIX extension not available on Windows.
Add a "Run Example" step to the build-examples-cmake-windows job so that each CMake example is executed after it is built, verifying the resulting binaries actually work. The executable name is derived from the matrix directory name by replacing hyphens with underscores, matching the project convention.
This reverts commit 7045114.
The cmake examples were failing at runtime on Windows CI for two reasons. The static library was installed as "libghostty-vt.a" on all platforms, but on Windows the DLL import library is also placed in zig-out/lib/ as "ghostty-vt.lib". The CMakeLists.txt expected the platform-native name "ghostty-vt.lib" for the static lib, so it picked up the tiny DLL import lib instead, silently producing a dynamically-linked executable. That executable then failed at runtime because the DLL was not on PATH. Fix this by installing the static library as "ghostty-vt-static.lib" on Windows to avoid the name collision, and updating CMakeLists.txt to match. For the shared (DLL) example, add zig-out/bin to PATH in the CI run step so the DLL can be found at runtime.
Zig's bundled compiler_rt and ubsan_rt produce object files with ELF-style linker directives (/exclude-symbols) and COMDAT sections that are incompatible with the MSVC linker, causing LNK1143 and LNK4229 errors when linking the static library. MSVC provides its own compiler runtime so bundling Zig's versions is unnecessary. Skip bundling both runtimes when the target ABI is MSVC.
Zig's ubsan instrumentation emits ELF-style /exclude-symbols linker directives into the compiled object files, causing LNK4229 warnings with the MSVC linker. The bundled compiler_rt also produces COMDAT sections that are incompatible with MSVC, causing fatal LNK1143. Disable sanitize_c entirely on the root module for MSVC targets and skip bundling both compiler_rt and ubsan_rt since MSVC provides its own runtime.
Zig's ubsan runtime emits /exclude-symbols linker directives that are incompatible with the MSVC linker, causing LNK4229 warnings and LNK1143 errors. Disable bundling ubsan_rt on Windows while keeping compiler_rt which provides essential symbols like memcpy, memset, memmove, and ___chkstk_ms. The previous check used target.result.abi == .msvc which never matched because Zig defaults to the gnu ABI on Windows.
Zig's compiler_rt produces COFF objects with invalid COMDAT sections that the MSVC linker rejects (LNK1143), and its ubsan_rt emits /exclude-symbols directives that MSVC does not understand (LNK4229). Skip bundling both in the static library on Windows since the MSVC CRT provides the needed builtins (memcpy, memset, etc.). The shared library continues to bundle compiler_rt as it needs to be self-contained.
Three issues when linking the static library with the MSVC linker: Use the LLVM backend on Windows to produce valid COFF objects. The self-hosted backend generates compiler_rt objects with invalid COMDAT sections that the MSVC linker rejects (LNK1143). Disable bundling ubsan_rt on Windows. Zig's ubsan runtime emits /exclude-symbols linker directives that MSVC does not understand (LNK4229). Add ntdll and kernel32 as transitive link dependencies for the static library on Windows. The Zig standard library uses NT API functions (NtClose, NtCreateSection, etc.) that consumers must link.
Zig defaults to the GNU ABI on Windows, which produces COFF objects with invalid COMDAT sections in compiler_rt that the MSVC linker rejects (LNK1143), and uses GNU conventions like ___chkstk_ms that are unavailable in the MSVC CRT. Default to the MSVC ABI when no explicit ABI is requested, following the same pattern as the existing macOS target override. This ensures compiler_rt produces valid COFF and the generated code uses MSVC-compatible symbols. Users can still explicitly request the GNU ABI via -Dtarget. Also disable bundling ubsan_rt on Windows (its /exclude-symbols directives are MSVC-incompatible) and add ntdll and kernel32 as transitive link dependencies for the static library.
Zig's bundled libc++/libc++abi conflicts with the MSVC C++ runtime headers (vcruntime_typeinfo.h, vcruntime_exception.h, etc.) when targeting native-native-msvc. This caused compilation failures in the SIMD C++ code due to -nostdinc++ suppressing MSVC headers and libc++ types clashing with MSVC runtime types. Skip linkLibCpp() for MSVC targets across all packages (highway, simdutf, utfcpp) and the main build (SharedDeps, GhosttyZig) since MSVC provides its own C++ standard library natively. Also add missing <iterator> and <cstddef> includes that were previously pulled in transitively through libc++ headers but are not guaranteed by MSVC's headers.
When compiling C++ files, Zig unconditionally passes -nostdinc++ and, if link_libcpp is set, adds its bundled libc++/libc++abi include paths as replacements (see Compilation.zig). On MSVC targets this conflicts with the MSVC C++ runtime headers (vcruntime_typeinfo.h, vcruntime_exception.h, etc.), causing compilation failures in SIMD C++ code. The fix is to use linkLibC instead of linkLibCpp on MSVC. Zig always passes -nostdinc to strip default search paths, but LibCDirs.detect re-adds the MSVC SDK include directories, which contain both C and C++ standard library headers. This gives us proper access to MSVC's own <optional>, <iterator>, <cstddef>, etc. without the libc++ conflicts. For the package builds (highway, simdutf, utfcpp) this means switching from linkLibCpp to linkLibC on MSVC. For SharedDeps and GhosttyZig, linkLibC is already called separately, so we just skip linkLibCpp.
The SIMD C++ files use C++17 features (std::optional, std::size). With Zig's bundled libc++ these are available implicitly, but MSVC headers guard C++17 features behind the standard version (_HAS_CXX17). Without an explicit -std=c++17 flag, clang defaults to a lower standard and the MSVC <optional> header does not define std::optional.
The SIMD C++ files reference __ubsan_handle_* symbols when compiled in debug mode, but we do not link or bundle the ubsan runtime on MSVC. This matches what the highway and simdutf packages already do in their own build files.
The "Run Example" step in the build-examples-cmake-windows job hangs, so remove it entirely. The build step is still run so compilation is verified, but the examples are no longer executed on Windows.
On Windows, shared libraries (DLLs) require an import library (.lib) for linking, and the DLL itself is placed in bin/ rather than lib/ by the Zig build. The CMake wrapper was missing IMPORTED_IMPLIB on the shared imported target, causing link failures, and assumed the shared library was always in lib/. Add GHOSTTY_VT_IMPLIB for the import library name, set IMPORTED_IMPLIB on the ghostty-vt target, and fix the shared library path to use bin/ on Windows. Install the DLL and PDB to bin/ and the import library to lib/ following standard Windows conventions. Apply the same fixes to ghostty-vt-config.cmake.in for the find_package path.
On Windows, Zig's built-in libc and MSVC's CRT maintain separate heaps, so calling free() on memory allocated by the library causes undefined behavior. Add ghostty_free() that frees through the same allocator that performed the allocation, making it safe on all platforms. Update format_alloc docs and all examples to use ghostty_free() instead of free(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
114425d to
c1e616c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
On Windows, calling
free()on memory allocated by libghostty crashes because Zig and MSVC use separate heaps.This adds
ghostty_free()so consumers can free library-allocated memory safely on all platforms.Why
When Zig builds a DLL on Windows with
link_libc = true, it does not link the Windows C runtime (ucrtbase.dll). Instead it uses its own libc built on top ofKERNEL32.dll. Sobuiltin.link_libcis true andc_allocatoris selected, but Zig'smallocand MSVC'smallocare different implementations with different heaps.On Linux/macOS this is not a problem because Zig links the system libc and everyone shares the same heap. On Windows,
free(buf)from MSVC tries to free memory from Zig's heap and you get a debug assertion failure or undefined behavior.The
format_allocdocs said "the buffer can be freed withfree()" but that is only true when the library and consumer share the same C runtime, which is not the case on Windows.How
ghostty_free(allocator, ptr, len)that frees through the same allocator that did the allocationformat_allocdocs to point toghostty_free()instead offree()ghostty_free(NULL, buf, len)The signature takes an allocator because raw buffers (unlike objects like terminals or formatters) do not store their allocator internally. The caller already has all three values: the allocator they passed, the pointer, and the length they got back.
I went back and forth on the naming. Other options I considered:
ghostty_alloc_free(allocator, ptr, len)or returning aGhosttyBufferwrapper with its own_free. Happy to change the naming if there is a preference.No impact on Linux/macOS.
ghostty_free()works correctly there too, it just happens to call the samefree()the consumer would have called anyway.Verified
zig build test-lib-vtpasses on Windows, macOS arm64, Linux x86_64 (exit 0)zig build testpasses on Windows (2575/2619 passed, 1 pre-existing font sprite failure) and macOS (exit 0)ghostty_free()(no more heap crash)What I Learnt
link_libc = truedoes not mean "link MSVC's CRT". Zig has its own self-contained libc built on KERNEL32.dll. This meansbuiltin.link_libccan be true but the library and consumer still have separate heaps. On Linux/macOS there is one system libc so this never happens.curl_free(),SDL_free()). This avoids cross-runtime heap issues entirely.