Add Debugging Interface

[G-Yong]

Add a debugging interface to the existing architecture.
Using the added debugging interface, we implemented debugging for QuickJS in VSCode. The project address is:[QuickJS-Debugger]

[saghul]

At a quick glance, this looks better than the other approaches I've seen, kudos!

Now, since this will have a performance impact, I'd say we want it gated with a compile time time macro.

@bnoordhuis thoughts?

[G-Yong]

Thanks for the feedback @saghul! I've added a compile-time macro QJS_ENABLE_DEBUGGER to gate all the debug-related code. Here's a summary of the changes:

Compile-time gating (QJS_ENABLE_DEBUGGER)

• All debug fields in JSContext, all debug API implementations (JS_SetOPChangedHandler, JS_GetStackDepth , JS_GetLocalVariablesAtLevel, JS_FreeLocalVariables ) , and the per-opcode callback in JS_CallInternal are now wrapped in #ifdef QJS_ENABLE_DEBUGGER.
• The declarations in quickjs.h are similarly guarded.
• When QJS_ENABLE_DEBUGGER is defined, DIRECT_DISPATCH is automatically set to 0 (alongside the existing EMSCRIPTEN and _MSC_VER conditions), so the switch-based dispatch is used and the opcode callback fires correctly.
• A new xoption(QJS_ENABLE_DEBUGGER ...) has been added to CMakeLists.txt , defaulting to OFF. When not enabled, there is zero overhead — no extra struct fields, no callback checks in the hot path, and DIRECT_DISPATCH remains unaffected.

Other cleanups

• Renamed JSLocalVar to JSDebugLocalVar to avoid confusion with the internal JSVarDef.
• Fixed a potential memory leak where funcname was only freed inside if (filename).
• Removed leftover debug fprintf comments and unnecessary braces in the hot path.

Let me know if there's anything else you'd like adjusted.

[saghul]

@bnoordhuis WDYT?

[bnoordhuis]

Behind a compile-time flag would be good. The diff mostly looks good to me but there are a couple of changes I'd like to see:

• the API functions should always be available but return an error when compiled without debugger support

• can you rename the new emit_source_loc calls to something like emit_source_loc_debug and turn that into a no-op when there's no debugger support?

• I don't love the name JSOPChangedHandler. Suggestions for a better one? Something like JSBytecodeTraceFunc perhaps?

I'm assuming this good enough as a building block to assemble a functional debugger from? I can see how it lets you single-step through code, inspect stack frames, set breakpoints, etc., so I'm guessing... yes?

[G-Yong]

Thanks for the review @bnoordhuis! I've addressed all your feedback:

1. API functions always available

Type definitions JSBytecodeTraceFunc, JSDebugLocalVar and all function declarations are now outside #ifdef QJS_ENABLE_DEBUGGER, so they're always visible. When compiled without debugger support, stub implementations are provided:

JS_SetBytecodeTraceHandler() — no-op
JS_GetStackDepth() — returns -1
JS_GetLocalVariablesAtLevel()— sets *pcount = 0, returns NULL
JS_FreeLocalVariables() — no-op
This way user code can link against these APIs unconditionally without #ifdef.

2. emit_source_loc_debug

Added a macro after emit_source_loc():

All 10 new call sites (for return, let/const, var, if, for, break/continue, switch, try/finally) now use emit_source_loc_debug(s). The 4 pre-existing emit_source_loc(s)calls (new-expression, function call, binary expression, throw) are left unchanged.

3. Renamed JSOPChangedHandler → JSBytecodeTraceFunc

Agreed, much better name. Full rename:

Old	New
JSOPChangedHandler	JSBytecodeTraceFunc
JS_SetOPChangedHandler	JS_SetBytecodeTraceHandler
ctx->operation_changed	ctx->bytecode_trace
ctx->oc_opaque	ctx->trace_opaque

4. Re: building block for a functional debugger

Yes — the current API covers the essential building blocks: single-step, breakpoints, call stack inspection, and local variable read access. I've built a working VSCode debugger on top of it QuickJS-Debugger

For a more complete debugger experience, a few additional APIs could be added later (as separate PRs):

• JS_EvalInStackFrame() — eval an expression in a specific frame's scope
• Closure variable access (captured vars from outer scopes)
• Exception hook (pause on throw)

But these can be built incrementally. The current PR is a solid minimal foundation.

[bnoordhuis]

I'm reasonably confident we can support this at runtime, no compile-time flags, without significant slowdowns when the debugger is inactive:

1. add a new OP_debug opcode
2. spray the generated bytecode with OP_debugs (but only when debugging is enabled)
3. when the interpreter hits an OP_debug opcode, invoke a debugger callback

Proof of concept, very WIP: bnoordhuis/quickjs@f5fbb5b

[G-Yong]

Thanks @bnoordhuis for the OP_debug proposal and the POC (f5fbb5b)! I've fully adopted this architecture.

Here's a summary of what's been done, and a few design choices I'd like to explain:

What we adopted from your POC

• OP_debug opcode: DEF(debug, 1, 0, 0, none) right after OP_invalid — a real opcode that survives all optimization phases.
• Runtime gating, no compile-time flags: Removed QJS_ENABLE_DEBUGGER entirely. DIRECT_DISPATCH is no longer forced to 0 by the debugger.
• JS_NewDebugContext(rt, cb): Creates a debug-enabled context. s->emit_debug = (ctx->debug_break != NULL) controls OP_debug emission during parsing.
• emit_source_loc emits OP_debug: When s->emit_debug is true, emit_source_loc() automatically appends OP_debug after OP_source_loc. Every source location point becomes a debug breakpoint.
• code_match skips OP_debug: Pass 4 peephole optimizer transparently skips OP_debug during pattern matching, same as your POC.
• emit_return instrumented: Added emit_source_loc(s) before OP_return_async and OP_return/OP_return_undef, matching your POC.
• JS_GetContextOpaque for user data: No separate opaque parameter — the callback retrieves user data via JS_GetContextOpaque(ctx), exactly as in your POC.

Additional work beyond the POC

Since the POC was intentionally minimal ("very WIP"), I filled in the remaining pieces:

• Phase 3 optimizer: Added explicit case OP_debug: with add_pc2line_info to preserve line info, then emit OP_debug to the output buffer.
• code_has_label / find_jump_target: Updated to skip OP_debug (same pattern as OP_source_loc and OP_label).
• Broad emit_source_loc coverage: 16 call sites covering return, let/const, var, if, for, break/continue, switch, try/finally, throw, expression statements, new expressions, function calls, binary expressions, and emit_return.
• Debug APIs: JS_GetStackDepth(), JS_GetLocalVariablesAtLevel(), JS_FreeLocalVariables() — always available (no #ifdef guards).

Where we diverged (and why)

Callback signature

Your POC passes raw call-frame values:

typedef void JSDebugBreakFunc(JSContext *ctx, JSValueConst func_obj,
                              JSValueConst this_obj, JSValueConst new_target,
                              int argc, JSValueConst *argv);

We use resolved debug info instead:

typedef int JSDebugBreakFunc(JSContext *ctx,
                             const char *filename, const char *funcname,
                             int line, int col);

Reason: A debugger needs filename/line/col at every break point. With the raw-values signature, the callback would need public APIs to extract the current PC's line number from the bytecode — but find_line_num, pc2line, and the PC itself are all internal. Rather than exposing interpreter internals, we resolve the info inside CASE(OP_debug) where everything is already accessible and pass it directly. This keeps the public API surface minimal.

int return type

Our callback returns int (0 = continue, non-zero = raise exception). This gives the debugger a clean way to abort execution (e.g., user clicks "Stop" in VS Code) without requiring the callback to throw via JS_Throw + signal mechanism.

---

Happy to adjust any of this based on your feedback. The working VSCode debugger built on top of this is at QuickJS-Debugger (branch OP_debug).

[saghul]

Left some comments! I like where this is going!

[saghul]

Since the bytecode needs to be regeneratred, can you bump the BC_VERSION please? You'll need to update the fuxxing tests too.

[G-Yong]

Done — bumped BC_VERSION from 25 to 26 and updated the fuzz corpus in tests/test_bjson.js (version byte in the base64-encoded blobs).

[saghul]

Any chance you can add a small test in api-test.c ?

[G-Yong]

Added a debug_trace test in api-test.c that covers:

1. No handler — eval works, callback is never invoked
2. Set handler — callback fires for each statement, receives correct filename
3. Stack depth — JS_GetStackDepth() returns ≥ 1 inside nested calls
4. Local variables — JS_GetLocalVariablesAtLevel() / JS_FreeLocalVariables() see ≥ 2 locals inside f(a, b)
5. Abort execution — returning non-zero from the callback raises an exception
6. Clear handler — passing NULL to JS_SetDebugTraceHandler stops callbacks

[saghul]

Looks like you didn't regenerate the bundled bytecode after bumping it, and tests are failing due to that.

[G-Yong]

Fixed — ran make codegen to regenerate all bundled bytecode files under gen/ (repl.c, standalone.c, hello.c, hello_module.c, test_fib.c, function_source.c) and the builtin-*.h headers with the updated BC_VERSION 26.

The reason I didn't include the codegen step earlier was intentional: regenerating these files produces a large diff across multiple generated files, which would have buried the actual debugging interface changes and made the PR much harder to review. Now that the core changes have been reviewed, I've added the regenerated files in a separate commit.

[saghul]

@bnoordhuis any chance you can take a look?

[bnoordhuis]

I intentionally suggested emitting OP_debug conditionally because doing it unconditionally a) makes the bytecode bigger, and b) slows down the interpreter dispatch loop. Can you at least check that https://github.com/quickjs-ng/web-tooling-benchmark doesn't regress?

[saghul]

Ah sorry I think that's my bad, I misunderstood what you meant 😢

[G-Yong]

I intentionally suggested emitting OP_debug conditionally because doing it unconditionally a) makes the bytecode bigger, and b) slows down the interpreter dispatch loop. Can you at least check that https://github.com/quickjs-ng/web-tooling-benchmark doesn't regress?

Okay, I will first conduct a comparative test based on the tools you provided to see the actual impact.

[G-Yong]

Web Tooling Benchmark Performance Test Results

I ran web-tooling-benchmark (17 real-world JavaScript workloads) to measure the performance impact of this PR's OP_debug changes.

Test Environment

Item	Detail
OS	Windows 10.0.26200
Compiler	MSVC 19.29.30159.0 (Visual Studio 2019)
Build	CMake Release mode
Upstream	quickjs-ng/quickjs master (v0.14.0)
Fork	G-Yong/quickjs (this PR branch, with unconditional OP_debug)
Runs	3 per version, results averaged

Detailed Results (runs/s, higher is better)

Benchmark	Upstream (avg)	Fork (avg)	Diff
acorn	0.59	0.57	-3.4%
babel	1.70	1.62	-4.7%
babel-minify	2.38	2.24	-5.9%
babylon	1.20	1.11	-7.5%
buble	1.99	1.95	-2.0%
chai	2.45	2.40	-2.0%
espree	0.44	0.42	-4.5%
esprima	0.81	0.78	-3.7%
jshint	1.95	1.90	-2.6%
lebab	1.52	1.54	+1.3%
postcss	1.55	1.47	-5.2%
prepack	2.08	1.93	-7.2%
prettier	0.81	0.78	-3.7%
source-map	1.04	1.02	-1.9%
terser	4.59	4.42	-3.7%
typescript	1.86	1.80	-3.2%
uglify-js	1.29	1.24	-3.9%
Geometric mean	1.43	1.38	-3.5%

Raw Run Data

Upstream: 1.43, 1.42, 1.45 (avg: 1.433)
Fork: 1.36, 1.39, 1.39 (avg: 1.380)

Summary

The unconditional OP_debug emission causes a ~3.5% overall performance regression (geometric mean). The impact is not uniform — parser-heavy benchmarks like babylon (-7.5%) and prepack (-7.2%) are hit harder, likely due to higher bytecode volume and more frequent OP_debug dispatch in tight loops.

[G-Yong]

I'm a bit confused. @bnoordhuis, do you mean we should keep OP_debug but compile it conditionally? Also, regarding the emit_source_loc_debug issue mentioned earlier, there might be a problem. If we adopt emit_source_loc_debug, we not only need to add it at specific locations, but also replace the existing emit_source_loc with emit_source_loc_debug. Otherwise, the statements represented by the original emit_source_loc won't respond to OP_debug.

[saghul]

Sorry @G-Yong i added to the confusion.

As your benchmarks show, just by having the debug opcodes things get slower so it should be opt-in.

Not sure if compile time or config, @bnoordhuis what's on your mind?

[G-Yong]

I didn't quite understand why emit_source_loc_debug was needed before. Now I get it a little bit— the newly added emit_source_loc calls are debug-specific (for stepping through if/for/return/etc.), unlike the original 5 upstream calls which are always needed for error reporting. Having emit_source_loc_debug as a no-op without debugger support keeps the bytecode identical to upstream and avoids the performance overhead.

[bnoordhuis]

Having emit_source_loc_debug as a no-op without debugger support keeps the bytecode identical to upstream and avoids the performance overhead.

Yes, exactly. :-)

The reason I suggested it is because it lets the debugger be a runtime option instead of a compile-time option.

[G-Yong]

Thanks @bnoordhuis, I now understand the intent! I've refactored the implementation to make the debugger a runtime option instead of a compile-time option:

Key changes:

• Removed all #ifdef QJS_ENABLE_DEBUGGER guards and the CMake option entirely
• OP_debug is now always defined in the opcode table
• emit_source_loc_debug() checks ctx->debug_trace at runtime — when no trace handler is set, it's a no-op, so the emitted bytecode is identical to upstream with zero performance overhead
• When a user calls JS_SetDebugTraceHandler(ctx, cb), subsequent JS compilation will emit OP_debug opcodes and the debugger becomes active
• All interpreter/optimizer code that handles OP_debug is always compiled in (no #ifdef guards needed — if OP_debug isn't emitted, those code paths are never reached)

This means a single QuickJS binary can serve both as a normal engine and a debug-capable engine, controlled entirely at runtime by whether JS_SetDebugTraceHandler has been called before JS code is compiled.

[G-Yong]

Thanks for the thorough review @saghul (and Claude)! I've pushed 1011587 on top of the branch addressing the issues raised. Summary:

OP_debug callback (JS_CallInternal)

• Use JS_AtomToCString instead of a 64-byte stack buffer so filename / funcname are passed at full length (fixes the silent truncation bug).
• If the callback returns non-zero without raising an exception, the engine now throws a default InternalError("aborted by debugger"), so JS_GetException() never returns the JS_UNINITIALIZED sentinel.
• If the callback calls JS_Throw* but returns 0, we still goto exception (checked via JS_HasException), so a pending exception can never silently leak into the next opcode / next eval.

JS_GetLocalVariablesAtLevel

• Skip compiler-generated names starting with < (e.g. , <this_active_func>).
• Substitute JS_UNINITIALIZED slots (let/const TDZ) with JS_UNDEFINED so the internal sentinel never escapes to C callers.
• On JS_AtomToCString OOM, free what was already allocated, clear the pending exception, set *pcount = 0 and return NULL (no more leaked exception state).

Duplicate / stray OP_debug emission

• Removed the unconditional dbuf_putc(bc, OP_debug) from emit_source_loc — it was firing inside expression parsers and producing multiple OP_debug per statement. OP_debug is now only emitted from emit_source_loc_debug, which is called strictly at statement boundaries.
• Moved the emit_source_loc_debug for TOK_LET / TOK_CONST out of the haslet: label so the fall-through into TOK_VAR no longer fires the trace twice for let/const.
• Added the missing emit_source_loc_debug at the top of throw and the default expression-statement path.
• Dropped dead emit_source_loc_debug calls after emit_break, before label_finally, and before the final OP_ret of try/catch/finally (those sites were unreachable / used stale source positions).

Optimizer

• find_jump_target and code_has_label no longer skip OP_debug. It is an executable opcode with a side effect (the user callback) and must not be elided when the optimizer follows a jump.

Misc

• Re-indented the stray hard-tab line in next_token with spaces.

quickjs.h

• Rewrote the doc comments on JSDebugTraceFunc / JS_SetDebugTraceHandler to match the implementation contract: either a non-zero return or raising via JS_Throw* aborts execution; the engine synthesizes a default exception when needed; filename/funcname are only valid for the duration of the callback; and OP_debug is only emitted for code parsed after the handler has been installed (so the handler must be set before evaluating application code).

I have not yet tackled the broader "handler attached after parse-time is silently a no-op" point as a behavior change — that one is now explicitly documented in the header. Happy to also implement option (a) (always emit OP_debug at statement boundaries, gate the dispatch at runtime) if you'd prefer that over the documented contract.

[G-Yong]

Before changing the design I ran the web-tooling-benchmark v0.5.3 suite to compare both options. Setup: Windows / MSVC 2019 Release / x64, upstream c707cf5, three back-to-back runs each, geomean of runs/s.

Option (a) — always emit OP_source_loc + OP_debug, gate at runtime in OP_debug case

• Upstream: 1.39 / 1.38 / 1.32 → 1.363
• Fork (always-emit): 1.28 / 1.22 / 1.23 → 1.245
• Geomean regression: −8.7 %
• Worst hits: prepack −16.2 %, jshint −14.5 %, postcss −13.1 %, babylon −11.7 %, terser −11.4 %, esprima −10.6 %. Every project regressed.

Current PR design — parse-time gate (emit_source_loc_debug checks ctx->debug_trace once, at parse time)

• Upstream: 1.34 / 1.38 / 1.36 → 1.360
• Fork (gated): 1.32 / 1.34 / 1.36 → 1.340
• Geomean Δ: −1.5 % — i.e. within run-to-run noise; per-benchmark deltas are roughly half positive, half negative.

[saghul]

Can you rebase one last time please?

[G-Yong]

I’ve used the ‘Sync fork’ option on GitHub. It seems to be just a merge operation. You could ‘merge --squash’ my pull request, as there are too many changes to make a rebase feasible. 😂

[saghul]

👍 can you take another look to see if more changes are necessary after explicit context management landed?

[G-Yong]

👍 can you take another look to see if more changes are necessary after explicit context management landed?

For the time being, we have made all the changes we could think of and had them reviewed by the AI. To the best of our ability and experience, the revisions are now largely complete.

[saghul]

_{Reviewed with tuicr.}

There are still changes necessary.

[saghul]

The commit fixing the column numbers landed! Can you tell the clanker to rebase / merge?

[saghul]

Btw, you can cleanup your branch like so:

git checkout your-branch
git fetch origin
git reset --soft $(git merge-base origin/master HEAD)
git commit -m "Add debugger interface"
git push --force

[G-Yong]

Btw, you can cleanup your branch like so:

git checkout your-branch
git fetch origin
git reset --soft $(git merge-base origin/master HEAD)
git commit -m "Add debugger interface"
git push --force

Got it. I really appreciate your patient guidance. I’ll sort it out tomorrow. (Today is a bit tight for me – hope you understand. 🙏)

[saghul]

No rush, thank you!

[G-Yong]

@saghul It’s done. I’ve finished the rebase.

[jprendes]

Could the API use JSAtom instead of char * for filename and funcname?
It would be up to the embedder to convert those to char * if they need to.
When debugging is enabled this is done for every statement and can be costly.
The embedder could choose to compare the JSAtom and avoid the allocation and convertion cost.

[saghul]

That's a good idea! What are you thinking, that the embedded would perhaps keep a LRU cache of atoms?

[jprendes]

I was thinking on a hashmap of breakpoints keyed by the tuple (JSAtom, line, column), and trying to reduce the cost of not hitting a breakpoint.
It would also be nice to have an API to query valid breakpoint locations, but that should be a different PR.

[jprendes]

I would like to eventually be able to implement the Chrome devtools debugger protocol using the debugger hooks

[saghul]

IIRC that's what the original author did? Is that how the VSCode extension works @G-Yong ?

[G-Yong]

The reason we do not implement the CDP (Chrome DevTools Protocol) or other debugging interfaces directly in quickjs.c/quickjs.h is that they are quite complex (involving a large amount of code, multithreading, socket communication, JSON parsing,etc.), and incorporating them directly would likely lead to system instability. Therefore, we have implemented a minimal debugging interface in quickjs, leaving the specific debug protocol to be implemented by third parties or by the user.

[G-Yong]

Could the API use JSAtom instead of char * for filename and funcname? It would be up to the embedder to convert those to char * if they need to. When debugging is enabled this is done for every statement and can be costly. The embedder could choose to compare the JSAtom and avoid the allocation and convertion cost.

What a brilliant idea!👍

[saghul]

The reason we do not implement the CDP (Chrome DevTools Protocol) or other debugging interfaces directly in quickjs.c/quickjs.h is that they are quite complex (involving a large amount of code, multithreading, socket communication, JSON parsing,etc.), and incorporating them directly would likely lead to system instability. Therefore, we have implemented a minimal debugging interface in quickjs, leaving the specific debug protocol to be implemented by third parties or by the user.

That was a good decision indeed!

[jprendes]

I think not implementing the CDP in QuickJS is the right approach.
However, I would like a few more hooks so that we can implement more of the CDP using those hooks.
I don't want to block this PR, this is good work, and we can build on top of this once it's merged :-)

[saghul]

Sounds good, thanks for the feedback!

[G-Yong]

@jprendes

I think this PR is a good start. Other things that would be nice to have is setting variable values on different stack frames, capturing closure variables in the stack (not just locals), eval-ing in a stack frame. Also the devtools debugger protocol has hooks for script parsed, and script failed to parse.
Also, support for the debugger statement :-D

Thank you for your suggestions. Most of the features you mentioned have just been implemented in the latest commit.

• setting variable values on different stack frames
• capturing closure variables in the stack (not just locals)
• eval-ing in a stack frame
• support for the debugger statement

As for the devtools debugger protocol has hooks for script parsed, and script failed to parse, this is implemented in the project G-Yong/QuickJS-Debugger (in main.c)

If you think there are changes that need to be made, you can actually submit a pull request with the updated code to the repository I’ve forked. Once it’s merged, your commit will be retained in my repository. When the official repository merges this pull request (provided they don’t use merge --squash), everyone’s commits will be merged into the official branch together. No one’s commits will be lost. ;-)

[buke]

gentle ping

[dblnz]

Any update on this, is this in a ready-to-merge state?

[saghul]

Sorry, I haven't been able to get back at this yet.