0.20.0: reduced unsafe pitfalls. - #48
Merged
Merged
Conversation
…est the protocol under Miri The preamble that ships inside every generated dylib now lives in a real source file (pulled into PANIC_PREAMBLE via include_str!), so the exact same unsafe panic-buffer code can be include!-ed into the test binary and executed by Miri. New tests cover both sides of the protocol: the dylib-side buffer writes (store, fallback, clamping to small caller buffers, 512-byte truncation) and the host-side read_panic_buffer decode (fn-pointer transmute, uninitialized buffer, raw-parts slice).
- Fall back to std::time::Instant under Miri: minstant's #[ctor] probes the TSC via rdtsc, which Miri cannot interpret. All timed operations are LLM calls, compilation, and evolution (ms-to-s scale), so the std clock is more than sufficient there. - Ignore integration tests under Miri: they compile dylibs with cargo and load them via dlopen, neither of which Miri supports. - Ignore the two DebuggingRecorder observability tests under Miri: crossbeam-epoch (via metrics-util) violates Stacked Borrows, a known third-party false positive.
- flake.nix: add the miri rustup component to the nightly toolchain. - CI: new miri job running the symbiont lib tests under MIRIFLAGS=-Zmiri-disable-isolation, covering the unsafe panic-buffer protocol and fn-pointer transmutes. - CAVEATS.md: document what Miri covers and what stays out of reach (the dlopen boundary, cross-dylib calls, and the mem::zeroed() placeholder return in the catch_unwind wrapper).
Preparation for enforcing a Default bound on evolvable return types: decide() returns Action, and on a caught panic the harness substitutes the return type's default value. Hold is the natural placeholder -- a crashed strategy should do nothing this candle. Previously a panicking decide() produced a mem::zeroed() Action, which is UB for this enum.
…olvable panics
The catch_unwind wrapper injected into every generated dylib used
unsafe { mem::zeroed() } as the placeholder return value after a caught
panic. All-zero bytes are undefined behaviour for return types like
String, Vec, references, or niche-optimized enums, so a panicking
evolvable with such a return type was instant UB.
The wrapper now substitutes ::core::default::Default::default() -- safe
for every type -- and the evolvable! macro enforces the required
Default bound with a compile error at the declaration site (spanned to
the offending return type), so generated dylibs always compile. Hosts
keep detecting panics via Runtime::take_panic and discard the
placeholder. A compile_fail doctest on the evolvable re-export pins the
enforcement.
Generated code is now scanned at the AST level (validation stage,
before any cargo round-trip) and rejected if it contains any unsafe
construct:
- unsafe { .. } blocks
- unsafe fn (free, impl, trait, and foreign)
- unsafe impl / unsafe trait
- extern blocks
- unsafe attributes such as #[unsafe(export_name = ..)] -- except the
exact #[unsafe(no_mangle)] export attribute the harness manages
- unsafe tokens smuggled through macro definitions or invocations
A crate-level #![forbid(unsafe_code)] in the dylib cannot do this job:
the injected panic preamble is legitimately unsafe, forbid permits no
local allow escape for it, and in edition 2024 the unsafe_code lint
fires on the #[unsafe(no_mangle)] attribute validation injects into
every evolvable function. The AST scan is also cheaper (rejects before
compiling), pinpoints the offending construct for the backpressure
prompt, and cannot be evaded with #[allow(unsafe_code)].
The rejection feeds the self-healing loop as Error::UnsafeCode with a
new 'unsafe' failure kind (metrics + EvolveFailure records), the retry
prompt names the offending construct, and the system prompt tells the
agent up front that unsafe is forbidden. Covered by unit tests for
every construct and a backpressure integration test.
…rated code
Extends the validation-stage policy scan beyond unsafe code. Rejected
unconditionally, because they break the harness's own contracts:
- static items and thread_local! -- dylib-local state silently resets
on every evolution and every retained revision gets its own copy;
state must be host-owned (the CAVEATS design rule and system-prompt
instruction are now actually enforced)
- macro_rules! definitions -- macro bodies would otherwise be a blind
spot for every other rule
- #[global_allocator], #[panic_handler], #[alloc_error_handler],
#[no_main] -- hijack the allocator/panic/entry contract between host
and dylib
- std::panic::{set_hook, take_hook, update_hook} -- replacing the hook
breaks the panic-buffer reporting protocol
Rejected by default, host-configurable via DylibConfig
(with_denied_path / with_allowed_path): references to std::process
(exit/abort kill the host, bypassing panic capture), std::thread
(spawned threads escape the feedback-loop contract), std::fs,
std::net, std::env, std::os, and std::io::stdin.
Denied paths are matched after resolving the file's use aliases
(use std as s; use std::process::exit as quit; use std::io; ...),
inside macro tokens, and glob imports of denied modules are rejected
outright. Violations surface as Error::ForbiddenConstruct with a new
'forbidden' failure kind, a dedicated retry-prompt nudge naming the
construct and reason, and an up-front system-prompt rule. This bounds
what evolvable code can name; it is guidance for the evolution loop,
not a security sandbox (documented in CAVEATS.md).
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
1 participant
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.
Changes include:
evolvablefunction must return a type withDefaultimplemented. Previouslymem::zeroed()was used after function panic was caught which was UB for some types.mirion this codebase to detect undefined behaviour in the unsafe code (panic-buffer, protocol, fn-pointer transmutes).More LLM code is being denied:
Extends the validation-stage policy scan beyond unsafe code. Rejected
unconditionally, because they break the harness's own contracts:
staticitems andthread_local!dylib-local state silently resetson every evolution and every retained revision gets its own copy;
state must be host-owned (the CAVEATS design rule and system-prompt
instruction are now actually enforced)
macro_rules!definitions: macro bodies would otherwise be a blindspot for every other rule
#[global_allocator],#[panic_handler],#[alloc_error_handler],#[no_main]: hijack the allocator/panic/entry contract between hostand dylib
std::panic::{set_hook, take_hook, update_hook}-- replacing the hookbreaks the panic-buffer reporting protocol
Rejected by default, host-configurable via
DylibConfig(
with_denied_path/with_allowed_path):std::process(exit/abort kill the host, bypassing panic capture)std::thread(spawned threads escape the feedback-loop contract)std::fsstd::netstd::envstd::osstd::io::stdin