JosephFerano/flan
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

A trap dies where it stands, and does not wait for a reader first

Browse Source 
rt_die began with fflush(stdout) and ended with exit(134), and under a merged
flan dev both could hang. The flush waits on a 64K pipe whose reader is busy;
exit runs the atexit chain and the ELF destructors, which want the loader lock
a dlopening listener thread may be holding, and in that build the chain also
holds OCaml's shutdown. A bounds failure that hangs is worse than one that
kills the process, because nothing is printed and nothing exits.

So the flush every trap does first goes through rt_flush_out, which puts fd 1
into non-blocking mode and accepts a truncated tail, and rt_die takes _exit
and unlinks FLAN_DEV_SOCK by hand -- the same shape as die_now in
flan_agent.c, which argued all of this first and is quoted rather than
diverged from. On a terminal, a file, or a pipe with room, O_NONBLOCK changes
nothing, and the acceptance corpus diffs that output.

One consequence worth knowing: _exit skips ASan's exit-time leak report, so a
trapping program under @sanitize no longer reports leaks.
This commit is contained in:
Joseph Ferano 2026-09-18 07:35:56 +07:00
parent c1ccc318f7
commit 0c0eadb424
2 changed files with 83 additions and 22 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
lib/dev.ml
View File

@ -3146,9 +3146,15 @@ int flan_merged_program_state(void) {
* message has already sent two investigations in this repository to the wrong
* place. Gone is the honest state, and the client already has words for it.
*
* [atexit] covers exit(3), which is what flan_rt.c's rt_die uses. _exit and
* abort skip it by design, so the places that take those routes unlink for
* themselves; see die_now in flan_agent.c. */
* [atexit] covers exit(3), which is the compiler thread's own way out and the
* way out of a program that somehow returns from main. It does NOT cover the
* two places a program dies where it stands: rt_die in flan_rt.c and die_now
* in flan_agent.c both take _exit, because the atexit chain and the ELF
* destructors want the loader lock a dlopening listener thread may be holding
* — and in this build that chain also holds OCaml's shutdown. Both of them
* therefore unlink this path by hand, which is why this function's body is
* written out three times in the tree rather than shared: it is four lines, and
* the alternative is a runtime that has to link against the daemon. */
static void flan_merged_unlink_sock(void) {
const char *s = getenv("FLAN_DEV_SOCK");
if (s != NULL && *s != '\0') unlink(s);

93
runtime/flan_rt.c
View File

@ -158,8 +158,11 @@ void flan_rt_init(int32_t argc, char **argv) {
}
/* Defined below with the rest of the non-local exits, and forward-declared
* here because the argument vector is built long before them. */
* here because the argument vector is built long before them. [rt_flush_out]
* is the flush every one of those paths does first; its own note says why it
* is not [fflush(stdout)]. */
static _Noreturn void rt_die(void);
static void rt_flush_out(void);
/* The one malloc in this file that is not an allocator's, because the argument
* vector belongs to the process rather than to any region a Flan program named.
@ -172,7 +175,7 @@ void flan_argv(flan_slice *out) {
if (rt_args == NULL && rt_argc > 0) {
rt_args = (flan_slice *)malloc(sizeof(flan_slice) * (size_t)rt_argc);
if (rt_args == NULL) {
fflush(stdout);
rt_flush_out();
fprintf(stderr,
"flan: out of memory building the argument vector for %d "
"arguments\n",
@ -391,15 +394,67 @@ void flan_escape_bytes(const uint8_t *p, int64_t n, flan_slice *out) {
* redirected stdout is not, so without this the error appears above the output
* that led to it. */
#include <fcntl.h>
#include <unistd.h>
/* That flush, made incapable of waiting.
*
* Under a merged `flan dev' stdout is a 64K pipe into the compiler thread,
* which is not reading it while it is answering a request. A trap taken by a
* program that had filled that pipe therefore began by blocking in the flush
* meant to order its own last words — and a bounds failure that hangs instead
* of dying is the worst shape a trap can take, because the person watching has
* no message and no exit status and no reason to think anything happened.
*
* So fd 1 is put into non-blocking mode first and the flush is best-effort.
* What that costs is a truncated tail: whatever no longer fits in the pipe is
* dropped rather than waited for. What it buys is that the trap always reaches
* its message and its exit. On a terminal, a file, or a pipe with room —
* which is every run that is not this one pathological case — O_NONBLOCK
* changes nothing at all, and the acceptance corpus diffs this output.
*
* The return value is ignored deliberately, twice over: a failed fcntl leaves
* the old blocking behaviour, which is what this code did before, and a flush
* that reports EAGAIN has done as much as it is going to. There is nothing a
* dying process can do about either. */
static void rt_flush_out(void) {
int flags = fcntl(1, F_GETFL, 0);
if (flags >= 0) (void)fcntl(1, F_SETFL, flags | O_NONBLOCK);
(void)fflush(stdout);
}
/* [_exit] and not [exit], for the reason die_now gives in
* vendor/agent/flan_agent.c and gives at length: this runs on the game thread,
* the dev agent's listener thread may be inside [dlopen] holding the loader
* lock, and [exit] runs the atexit chain and the ELF destructors, which want
* that same lock. In a merged build that chain also holds OCaml's shutdown,
* and it would be run from a thread that is not OCaml's. A trap that deadlocks
* in the runtime's teardown is the same failure as a trap that hangs on a full
* pipe, reached a few instructions later.
*
* What [_exit] skips is the atexit handler that removes the editor's socket —
* so, exactly as die_now does, this removes it by hand. A socket file left on
* disk with nothing accepting on it answers the next client with
* ECONNREFUSED, which reads like a daemon that is there and refusing rather
* than one that died; that message has already sent two investigations in this
* repository to the wrong place. FLAN_DEV_SOCK is unset in an ordinary run and
* then this does nothing.
*
* The two functions are kept saying the same thing on purpose. They are the
* two ways a Flan program dies where it stands, and a difference between them
* would be a difference nobody could predict from the outside. */
static _Noreturn void rt_die(void) {
fflush(stdout);
const char *sock;
rt_flush_out();
fflush(stderr);
exit(134);
sock = getenv("FLAN_DEV_SOCK");
if (sock != NULL && *sock != '\0') unlink(sock);
_exit(134);
}
_Noreturn void flan_bounds_fail(const uint8_t *loc, int64_t loclen,
int64_t idx, int64_t len) {
fflush(stdout);
rt_flush_out();
fprintf(stderr, "%.*s: index %lld is out of bounds for length %lld\n",
(int)loclen, (const char *)loc, (long long)idx, (long long)len);
rt_die();
@ -458,7 +513,7 @@ void flan_error(uint32_t type_id, void *condition, void *xfer,
flan_break_hook(name, namelen, condition, xfer);
if (*(void **)xfer != NULL) return;
}
fflush(stdout);
rt_flush_out();
fprintf(stderr, "unhandled %.*s\n", (int)namelen, (const char *)name);
rt_die();
}
@ -468,7 +523,7 @@ void flan_error(uint32_t type_id, void *condition, void *xfer,
* there is nowhere to resume, so there is nothing else to do. */
_Noreturn void flan_restart_fail(const uint8_t *loc, int64_t loclen,
const uint8_t *name, int64_t namelen) {
fflush(stdout);
rt_flush_out();
fprintf(stderr, "%.*s: no restart named %.*s is active\n",
(int)loclen, (const char *)loc, (int)namelen, (const char *)name);
rt_die();
@ -483,7 +538,7 @@ _Noreturn void flan_restart_args_fail(const uint8_t *loc, int64_t loclen,
const uint8_t *name, int64_t namelen,
const uint8_t *want, int64_t wantlen,
const uint8_t *got, int64_t gotlen) {
fflush(stdout);
rt_flush_out();
fprintf(stderr, "%.*s: restart %.*s takes %.*s, given %.*s\n",
(int)loclen, (const char *)loc, (int)namelen, (const char *)name,
(int)wantlen, (const char *)want, (int)gotlen, (const char *)got);
@ -498,7 +553,7 @@ _Noreturn void flan_restart_args_fail(const uint8_t *loc, int64_t loclen,
_Noreturn void flan_restart_unarmed(const uint8_t *loc, int64_t loclen,
const uint8_t *name, int64_t namelen,
const uint8_t *want, int64_t wantlen) {
fflush(stdout);
rt_flush_out();
fprintf(stderr,
"%.*s: restart %.*s takes %.*s, and whatever took it supplied no "
"arguments — a restart with parameters cannot be taken from the "
@ -514,7 +569,7 @@ _Noreturn void flan_restart_unarmed(const uint8_t *loc, int64_t loclen,
* lexical case is refused by the checker; this is the one that reaches a
* function through a call, where nothing static could see it. */
_Noreturn void flan_transfer_fail(const uint8_t *loc, int64_t loclen) {
fflush(stdout);
rt_flush_out();
fprintf(stderr,
"%.*s: a defer invoked a restart, which a defer may not do — it is "
"the cleanup a transfer runs on its way out\n",
@ -524,7 +579,7 @@ _Noreturn void flan_transfer_fail(const uint8_t *loc, int64_t loclen) {
_Noreturn void flan_slice_fail(const uint8_t *loc, int64_t loclen,
int64_t lo, int64_t hi, int64_t len) {
fflush(stdout);
rt_flush_out();
fprintf(stderr, "%.*s: slice [%lld %lld) is out of bounds for length %lld\n",
(int)loclen, (const char *)loc, (long long)lo, (long long)hi,
(long long)len);
@ -632,7 +687,7 @@ void flan_slice_error(const uint8_t *loc, int64_t loclen, int64_t lo,
* testing high <= length would wave the failure through. */
_Noreturn void flan_slice_promise_fail(const uint8_t *loc, int64_t loclen,
int64_t n) {
fflush(stdout);
rt_flush_out();
fprintf(stderr,
"%.*s: slice-from-ptr was promised %lld elements behind the pointer, "
"and a count of elements is never negative\n",
@ -706,7 +761,7 @@ static const uint8_t flan_arith_name[] = "ArithError";
* formatting is the unhandled path's job, and this is the unhandled path. */
static void flan_arith_fail(const uint8_t *loc, int64_t loclen, int32_t op,
int64_t lhs, int64_t rhs) {
fflush(stdout);
rt_flush_out();
switch (op) {
case FLAN_ARITH_DIV_ZERO:
fprintf(stderr, "%.*s: divide by zero: (/ %lld 0)\n", (int)loclen,
@ -1206,7 +1261,7 @@ void flan_alloc_free_all(flan_allocator *a, const uint8_t *loc, int64_t loclen)
}
_Noreturn void flan_null_alloc_fail(const uint8_t *loc, int64_t loclen) {
fflush(stdout);
rt_flush_out();
fprintf(stderr,
"%.*s: this allocator is null — a zeroed Allocator was never given "
"one\n",
@ -1215,7 +1270,7 @@ _Noreturn void flan_null_alloc_fail(const uint8_t *loc, int64_t loclen) {
}
_Noreturn void flan_free_all_fail(const uint8_t *loc, int64_t loclen) {
fflush(stdout);
rt_flush_out();
fprintf(stderr,
"%.*s: this allocator does not offer free-all — it has no region to "
"release, and releasing nothing is not the same as releasing "
@ -1271,7 +1326,7 @@ void flan_alloc_region_only(flan_allocator *a, const uint8_t *loc,
}
_Noreturn void flan_region_only_fail(const uint8_t *loc, int64_t loclen) {
fflush(stdout);
rt_flush_out();
fprintf(stderr,
"%.*s: this container's elements own storage, and this allocator can "
"free one block — so a free here would release the slots and leak "
@ -1349,7 +1404,7 @@ int64_t flan_alloc_id(flan_allocator *a) { return (int64_t)(intptr_t)a; }
_Noreturn void flan_vec_stale_fail(const uint8_t *loc, int64_t loclen,
int64_t was, int64_t now) {
fflush(stdout);
rt_flush_out();
fprintf(stderr,
"%.*s: this container's allocator was released — it was made at "
"epoch %lld and the allocator is at %lld now\n",
@ -1359,7 +1414,7 @@ _Noreturn void flan_vec_stale_fail(const uint8_t *loc, int64_t loclen,
_Noreturn void flan_vec_bounds_fail(const uint8_t *loc, int64_t loclen,
int64_t i, int64_t len) {
fflush(stdout);
rt_flush_out();
fprintf(stderr, "%.*s: index %lld is out of bounds for length %lld\n",
(int)loclen, (const char *)loc, (long long)i, (long long)len);
rt_die();
@ -2852,7 +2907,7 @@ int64_t flan_file_fail_reason(void) { return flan_file_fail; }
* so this traps naming the declare-c that was called, the way an out-of-bounds
* index traps naming its site. See lib/shim.ml, which emits the call. */
_Noreturn void flan_shim_nul_fail(const char *site) {
fflush(stdout);
rt_flush_out();
fprintf(stderr,
"%s: a string passed to C contains a NUL byte — C reads to the "
"first one, so the value this function would act on is a prefix of "