Skip to content

Add Debugging Interface#1421

Open
G-Yong wants to merge 44 commits intoquickjs-ng:masterfrom
G-Yong:master
Open

Add Debugging Interface#1421
G-Yong wants to merge 44 commits intoquickjs-ng:masterfrom
G-Yong:master

Conversation

@G-Yong
Copy link
Copy Markdown

@G-Yong G-Yong commented Mar 26, 2026

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]
debugInVSCode

添加位置变动(操作变动)回调接口,为外部实现调试功能实现可能
Introduces JS_SetOPChangedHandler to allow setting a callback for operation changes in the JSContext. Also adds calls to emit_source_loc in various statement parsing locations to improve source location tracking during parsing.
假如没有,位置跟踪会发生异常。
解决在函数内出现静态错误时,返回的堆栈信息中的列号错误的bug。
Introduces functions to get stack depth and retrieve local variables at a specific stack frame level, along with a struct for local variable info and a function to free the allocated array. Also updates the JSOPChangedHandler signature to include JSContext for improved debugging capabilities.
假如采用旧的代码,会发生下面的错误:

function add(a, b){
    return a + b;

    var b  // OP_return会出现在这里
    while(1){}
}

add(1, 2)
@saghul
Copy link
Copy Markdown
Contributor

saghul commented Mar 26, 2026

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
Copy link
Copy Markdown
Author

G-Yong commented Mar 28, 2026

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
Copy link
Copy Markdown
Contributor

saghul commented Mar 28, 2026

@bnoordhuis WDYT?

Copy link
Copy Markdown
Contributor

@bnoordhuis bnoordhuis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 added 3 commits March 30, 2026 09:50
Rename the old operation_changed/JSOPChangedHandler to bytecode_trace/JSBytecodeTraceFunc and replace JS_SetOPChangedHandler with JS_SetBytecodeTraceHandler. Add conditional compilation guards so debugger-related code is compiled only when QJS_ENABLE_DEBUGGER is set (including stack depth, local-variable APIs, and freeing logic). Introduce emit_source_loc_debug no-op macro when debugger is disabled and make JS_GetStackDepth return -1 without the debugger. Update public header comments to reflect the new API and behavior.
@G-Yong
Copy link
Copy Markdown
Author

G-Yong commented Mar 30, 2026

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.

@G-Yong G-Yong requested a review from bnoordhuis March 31, 2026 09:47
@bnoordhuis
Copy link
Copy Markdown
Contributor

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

Introduce a per-context debugging mechanism: add OP_debug opcode and a JS_NewDebugContext API that accepts a JSDebugBreakFunc callback. Debug opcodes are emitted at statement/source boundaries only when a context is created with a non-NULL debug callback (s->emit_debug is set from ctx->debug_break). The interpreter now handles OP_debug by invoking ctx->debug_break with filename/funcname/line/col and can raise an exception if the callback returns non-zero. The implementation records pc2line info for OP_debug during resolve_labels so source locations can be resolved at runtime. Removed the old build-time QJS_ENABLE_DEBUGGER gate and the bytecode-trace API (JS_SetBytecodeTraceHandler and related fields), and dropped the CMake option QJS_ENABLE_DEBUGGER. Updated headers and opcodes (DEF(debug)) and adjusted parsing/codegen to emit OP_debug where appropriate.
@G-Yong
Copy link
Copy Markdown
Author

G-Yong commented Apr 1, 2026

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).

G-Yong and others added 2 commits April 1, 2026 13:18
* Regenerate pre-compiled bytecode files after OP_debug opcode addition

The commit 1e05bd7 added OP_debug to quickjs-opcode.h but did not
regenerate the pre-compiled bytecode files. This shifted all opcode
numbers by +1, causing the interpreter to misparse the bytecode
stream and trigger "invalid atom index" errors.

Regenerated all bytecode files using `make codegen` with the updated
qjsc compiler that includes the OP_debug opcode.

Agent-Logs-Url: https://github.com/G-Yong/quickjs/sessions/9ded3eef-eaab-421f-a1d7-0fca19fee48d

Co-authored-by: G-Yong <21030893+G-Yong@users.noreply.github.com>

* Move OP_debug to end of opcode list to avoid shifting upstream opcodes

Instead of inserting OP_debug at position 1 (which shifted all
subsequent opcode numbers and broke pre-compiled bytecode), place
it after all short opcodes at the end of the opcode list.

This way:
- All regular and short opcodes keep their upstream positions
- OP_debug gets a new unique index (246) that doesn't conflict
- Future upstream merges won't cause bytecode incompatibilities
- The codegen check passes with no stale bytecode

Agent-Logs-Url: https://github.com/G-Yong/quickjs/sessions/96e96c96-17d4-40c3-8131-ec6375bb30c8

Co-authored-by: G-Yong <21030893+G-Yong@users.noreply.github.com>

* Revert bytecode regeneration, keep only DEF(debug) move to end of opcode list

Place the new OP_debug opcode after all short opcodes instead of
right after OP_invalid.  Inserting it near the top of the table
shifts every subsequent opcode number by one, which silently
invalidates all pre-compiled bytecode files (gen/*.c, builtin-*.h)
and causes "invalid atom index" errors at runtime.

By appending it at the end of the DEF list (but before the
temporary 'def' opcodes), no existing opcode value changes, so
the checked-in bytecode files remain valid and no regeneration
step is required.

OP_debug is only emitted at compile time when a debug context is
active (JS_NewDebugContext) and never appears in pre-compiled
bytecode, so its exact numeric value is irrelevant to stored
bytecode compatibility.

Agent-Logs-Url: https://github.com/G-Yong/quickjs/sessions/33150476-2312-48c9-98b4-113ff7039512

Co-authored-by: G-Yong <21030893+G-Yong@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: G-Yong <21030893+G-Yong@users.noreply.github.com>
Copy link
Copy Markdown
Contributor

@saghul saghul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments! I like where this is going!

G-Yong added 7 commits April 1, 2026 17:10
Replace JS_AtomToCString/JS_FreeCString usage with JS_AtomGetStr into fixed-size buffers when calling ctx->debug_break in quickjs.c, and always compute the source line number (removed the pc2line_buf guard). This avoids temporary allocations/frees for filename/funcname passed to the debug callback. Also simplify JSDebugLocalVar comments in quickjs.h by removing notes about freeing the name and value.
Replace the old JS_NewDebugContext API with JS_SetDebugBreakHandler(ctx, cb) so callers can set or clear a debug-break callback on any existing context. Update header docs to clarify that OP_debug opcodes are always emitted at statement boundaries and the callback is only invoked when set. Remove the emit_debug flag from JSParseState and always emit OP_debug in emit_source_loc; initialization no longer toggles emit_debug. This decouples bytecode emission of debug traps from whether a handler was present at context creation, allowing handlers to be attached later.
OP_debug is a transparent no-op (0 pop, 0 push) used solely for
debugger breakpoints. Setting last_opcode_pos to point at OP_debug
breaks all peephole optimizations that rely on get_prev_opcode(),
because they see OP_debug instead of the real preceding opcode.

Affected code paths include:
- js_is_live_code(): misidentifies dead code as live
- set_object_name(): fails to match OP_set_name / OP_set_class_name
- lvalue parsing: falls into the default (invalid lvalue) branch
- set_object_name_computed(): fails to rewrite opcodes

This caused CI test failures when OP_debug was always emitted.

Fix: stop updating last_opcode_pos in emit_source_loc(), making
OP_debug transparent to the peephole optimizer, just like
OP_source_loc already is.
@saghul
Copy link
Copy Markdown
Contributor

saghul commented Apr 2, 2026

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
Copy link
Copy Markdown
Author

G-Yong commented Apr 2, 2026

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).

G-Yong added 2 commits April 2, 2026 17:10
Rename debug break callback and related symbols to use "trace" naming: JSDebugBreakFunc -> JSDebugTraceFunc, ctx->debug_break -> ctx->debug_trace, and JS_SetDebugBreakHandler -> JS_SetDebugTraceHandler; update call sites in the bytecode interpreter accordingly. Bump BC_VERSION from 25 to 26 to reflect the bytecode changes and update tests/test_bjson.js base64 fixtures (Gf -> Gv) to match the new bytecode format. Also adjust the JS_GetStackDepth comment wording.
@saghul
Copy link
Copy Markdown
Contributor

saghul commented Apr 2, 2026

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

@G-Yong
Copy link
Copy Markdown
Author

G-Yong commented Apr 2, 2026

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 depthJS_GetStackDepth() returns ≥ 1 inside nested calls
  4. Local variablesJS_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
Copy link
Copy Markdown
Contributor

saghul commented Apr 2, 2026

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

…mismatch (25 -> 26) (#4)

Agent-Logs-Url: https://github.com/G-Yong/quickjs/sessions/22cd13bd-1c2e-44b2-af85-79b7cf479498

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: G-Yong <21030893+G-Yong@users.noreply.github.com>
@G-Yong
Copy link
Copy Markdown
Author

G-Yong commented Apr 3, 2026

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants