[robobun]

Based on #830, which is based on #836 (the third commit here).

Problem

• A breakpoint or a debugger statement in a function is skipped when the function runs inlined in DFG or FTL code: 8,507 of 10,000 calls in the test.
• ByteCodeParser::inliningCost (dfg/DFGByteCodeParser.cpp:2021) refuses to inline only when the caller has the debugger's mark. The callee is not checked.
• Two callers have no mark: code of a global object without a debugger, and a generator body that is suspended when the debugger attaches.

Fix

• inliningCost also refuses a callee whose bytecode has debug hooks. A private builtin has the mark but no hooks, and stays inlinable.
• Correct because an inlined op_debug is a nop, and CodeBlock::addBreakpoint jettisons only the optimized code of its own function.
• Verified: JSTests/stress/dfg-does-not-inline-a-function-that-has-debug-hooks.js (3 callers, all fail without the change).

Background

• Debug hooks are op_debug instructions. The interpreter and the Baseline JIT call the debugger from them. The DFG compiles them to nothing.
• The debugger's mark is UnlinkedCodeBlock::wasCompiledWithDebuggingOpcodes(): the bytecode was generated for a global object with a debugger.
• [JSC] Keep the code a generator body started in, and give held code only to callers of its mode #830 makes the mark of a function reliable: a global object with a debugger always gets bytecode with debug hooks.

Downsides

• Code of a global object without a debugger runs a call to a debugged function as a real call, only while a debugger is attached to the callee's global object.
• None found without a debugger: no bytecode has debug hooks then. The new check is one load and one branch for each inlining candidate, at compile time.

Notes

jsc shell, Release with assertions, --useConcurrentJIT=0. The callee has a debugger statement, which keeps it from tiering up by itself, as a breakpoint does. $vm.dfgTrue() in the callee is true only when it runs inlined. Calls that reached the statement in the callee's own frame, of 10,000:

caller	main e8a8efc09c	#830	this PR
both global objects linked it, the debugged one generated it first, the other one runs it	10,000	1,493	10,000
the other global object generated it first	1,493	1,493	10,000
a generator body that is suspended when the debugger attaches	2,166	2,151	10,000

The first row is 10,000 on main because main hands the other global object the debugged one's marked bytecode. #830 stops that, and this PR puts the guard where it belongs.

Upstream JavaScriptCore has the same guard and the same gap.

[claude]

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline nit, I also checked that every DFG/FTL inlining path reaches the new refusal: both attemptToInlineCall (DFGByteCodeParser.cpp:2440) and handleVarargsInlining (DFGByteCodeParser.cpp:2508) go through inliningCost, and the new check sits after the baseline CodeBlock null check, so no inlining route skips it and there is no null dereference. The private-builtin exclusion matches BytecodeGenerator::shouldEmitDebugHooks (BytecodeGenerator.h:1097), which never emits op_debug for private builtins.

Extended reasoning...

The change adds one USE(BUN_JSC_ADDITIONS)-guarded refusal in ByteCodeParser::inliningCost so a callee whose bytecode carries debug hooks is never inlined, plus a new jsc-shell stress test; it touches no security-sensitive surface. The check can only make the DFG/FTL more conservative (a real call instead of an inlined one) and only while a debugger is attached, but it is core JIT inlining policy whose correctness rests on the mark semantics introduced by the stacked PRs this one builds on, so a human maintainer look is still worthwhile.

[github-actions]

Preview build of a8110c4: autobuild-preview-pr-837-a8110c45

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

I re-reviewed the latest push and found no bugs; the earlier testLoopCount nit is addressed. Since this changes the DFG inlining gate in code covered by the JSC reviewers in CODEOWNERS and depends on the base PR's debugger-mark guarantees, a human look is still worthwhile.

What was reviewed:

• The new gate in inliningCost mirrors BytecodeGenerator::shouldEmitDebugHooks (Debugger mode and not a private builtin), so it refuses exactly the callees that carry op_debug; excluding private builtins is consistent with that.
• FTL path: targetCodeBlock and the baseline block share the executable's UnlinkedCodeBlock, so checking the baseline block's mark covers both tiers; the check only ever removes inlining, so no new JIT behavior is introduced.
• The test's $vm.enableDebuggerModeWhenIdle, createGlobalObject and testLoopCount all exist in this fork's shell, and the single runDefault mode pins its options.

Extended reasoning...

The change adds one USE(BUN_JSC_ADDITIONS)-guarded early return in ByteCodeParser::inliningCost (Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp) refusing to inline a callee whose bytecode was compiled with debugging opcodes unless it is a private builtin, plus a new stress test. It touches no security-sensitive surface; the effect is strictly to inline less when a debugger is attached. Approval was withheld because /Source/JavaScriptCore and /JSTests are covered by CODEOWNERS entries and the fix's correctness rests on the debugger mark being reliable, which the author attributes to a base PR not in this diff. The latest commit addressed the only prior feedback (iteration count now comes from testLoopCount), and the bug hunt ran dry with no findings.

[claude]

Code review found no issues

No high-confidence issues detected in this change.

[claude]

I reviewed the latest push and found no bugs; the earlier testLoopCount nit is addressed. Since this changes DFG/FTL inlining policy under the JavaScriptCore codeowner paths, a human look is still worthwhile.

What was reviewed:

• The new guard in ByteCodeParser::inliningCost(): wasCompiledWithDebuggingOpcodes() && !isPrivateBuiltinFunction() is the same predicate as BytecodeGenerator::shouldEmitDebugHooks(), so it refuses exactly the callees whose bytecode contains op_debug, and it runs before the targetCodeBlock/FTL selection so both tiers are covered.
• Checked the ordering relative to the existing m_hasDebuggerEnabled and null-codeBlock checks — the callee CodeBlock is non-null when dereferenced.
• The test now takes its count from testLoopCount, fails only by throwing, and the $vm helpers it uses (dfgTrue, enableDebuggerModeWhenIdle) exist in this fork.

Extended reasoning...

The change adds a USE(BUN_JSC_ADDITIONS)-guarded 12-line check to Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp that refuses to inline a callee compiled with debugger opcodes unless it is a private builtin, plus a new JSTests/stress test covering two cross-realm callers and a suspended generator. No security-sensitive surface is touched; this is JIT inlining policy interacting with the debugger. The decision to defer rather than approve rests on the files being covered by .github/CODEOWNERS (/Source/JavaScriptCore and /JSTests) and on the change altering DFG/FTL inlining behaviour, where the test's reliance on the callee not tiering up on its own is an empirical property best confirmed by a maintainer and CI. The new commit since the last review addressed the one prior nit (testLoopCount), and no outstanding CHANGES_REQUESTED review exists in the timeline.