Skip to content

Commit ff9d4db

Browse files
Route bx asserts and SEH crashes to stderr (#1741)
## What bx''s `installExceptionHandler()` and `BX_ASSERT` both dispatch through bx''s assert handler. The built-in default writes the failure banner and native callstack **only** to `bx::getDebugOut()` (`OutputDebugString` on Windows) and then tears the process down via `TerminateProcess(1)`. In a non-headless console / CI run that means: - the native callstack never reaches the log, - the finish line never prints (atexit is skipped), - and the only signal is a bare exit code `1` — indistinguishable from an uncaught JS error. This is exactly what happened in a recent CI Validation Tests failure: the process died mid-render with exit code 1 and **no** `--- BN: ... ---` banner. ## Change Install a Playground assert handler (`bx::setAssertHandler`) right after `bx::installExceptionHandler()` that routes **both** paths through `Diagnostics::DumpFailure` — which writes to **stderr** (captured by CI) as well as `OutputDebugString` — then exits deterministically with code `3`, matching the other hard-failure handlers in this file. - Exception-filter invocations (`location.line == UINT32_MAX`) are labeled `CRASH`. - Real `BX_ASSERT` sites keep their file/line and are labeled `ASSERT`. - Re-entrancy guarded; breaks into the debugger when one is attached. Single file, +48 lines (`Apps/Playground/Shared/Diagnostics.cpp`). ## Validation Forced an access violation (the same mid-render SEH path) via a temporary env-gated injection (reverted before commit) and confirmed stderr now shows: ``` --- BN: CRASH --- Access violation. Exception Code c0000005 Callstack (8): 1: .../Apps/Playground/Win32/App.cpp 166 wWinMain ... Build info: MSVC 17.0, ... C++20 ... --- END --- ``` Exit code `3`, finish line printed. ## Notes - This is debuggability infrastructure, independent of the JSI mid-render crash itself (a separate pre-existing/flaky issue) — this change simply makes such crashes **visible** in CI logs instead of swallowing them. - Trade-off: the self-terminate path does not produce a WER `.dmp` (today''s default path produces none either). Preserving `.dmp` on real crashes can be a follow-up. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 34186fe commit ff9d4db

1 file changed

Lines changed: 56 additions & 0 deletions

File tree

Apps/Playground/Shared/Diagnostics.cpp

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#include "Diagnostics.h"
22

3+
#include <bx/bx.h>
34
#include <bx/debug.h>
45
#include <bx/error.h>
56
#include <bx/platform.h>
@@ -110,6 +111,57 @@ namespace
110111
std::_Exit(3);
111112
}
112113
#endif
114+
115+
// bx routes BOTH BX_ASSERT failures and the Windows SEH top-level
116+
// exception filter (bx::installExceptionHandler) through its assert
117+
// handler. bx's built-in default writes the banner only to
118+
// bx::getDebugOut() (OutputDebugString), which is invisible in a console /
119+
// CI run, and then tears the process down via TerminateProcess -- so the
120+
// native callstack never reaches the log and atexit (the finish line)
121+
// never runs. Route it through DumpFailure instead, which writes to stderr
122+
// (captured by CI), then exit deterministically with code 3 like the other
123+
// hard-failure handlers above.
124+
bool OnBxAssert(const bx::Location& location, uint32_t skip, const char* format, va_list args)
125+
{
126+
// Guard against re-entry if formatting/stack-walking itself faults.
127+
// Returning false here would tell bx to *continue* past the failed
128+
// assert / propagate the exception, i.e. keep running in an
129+
// already-failing state. Instead fail fast deterministically: skip the
130+
// (faulting) formatting path, emit a minimal marker, and exit.
131+
static std::atomic<bool> s_inHandler{false};
132+
bool expected = false;
133+
if (!s_inHandler.compare_exchange_strong(expected, true))
134+
{
135+
std::fputs("\n--- BN: CRASH (re-entrant) ---\n", stderr);
136+
std::fflush(stderr);
137+
Diagnostics::SetExitCode(3);
138+
Diagnostics::PrintFinishLine();
139+
std::_Exit(3);
140+
}
141+
142+
// bx's exception-handler call sites (SEH filter, pure-call) pass
143+
// UINT32_MAX as the line and a synthetic file ("Exception Handler");
144+
// real BX_ASSERT sites pass an actual file/line.
145+
const bool isException = (location.line == UINT32_MAX);
146+
Diagnostics::DumpFailureV(
147+
isException ? "CRASH" : "ASSERT",
148+
isException ? nullptr : location.filePath,
149+
isException ? 0 : static_cast<int>(location.line),
150+
skip,
151+
format,
152+
args);
153+
154+
#if defined(_MSC_VER)
155+
if (::IsDebuggerPresent())
156+
{
157+
bx::debugBreak();
158+
}
159+
#endif
160+
161+
Diagnostics::SetExitCode(3);
162+
Diagnostics::PrintFinishLine();
163+
std::_Exit(3);
164+
}
113165
}
114166

115167
namespace Diagnostics
@@ -124,6 +176,10 @@ namespace Diagnostics
124176

125177
bx::installExceptionHandler();
126178

179+
// Route bx asserts + the SEH top-level exception filter to DumpFailure
180+
// (stderr-visible) instead of bx's OutputDebugString-only default.
181+
bx::setAssertHandler(&OnBxAssert);
182+
127183
#if defined(_MSC_VER)
128184
// Route assert() to stderr instead of UCRT's modal dialog. Covers the
129185
// direct assert() codepath; _CrtSetReportMode below covers _CRT_*.

0 commit comments

Comments
 (0)