node:vm: report compileFunction body lines relative to lineOffset when it is 0

src/jsc/bindings/ErrorStackFrame.cpp

@@ -1,6 +1,7 @@
 #include "root.h"
 #include "JavaScriptCore/CodeBlock.h"
 #include "headers-handwritten.h"
+#include "NodeVMScriptFetcher.h"
 #include "JavaScriptCore/BytecodeIndex.h"
 #include "wtf/Assertions.h"
 #include "wtf/text/OrdinalNumber.h"
@@ -95,6 +96,11 @@ ZigStackFramePosition getAdjustedPositionForBytecode(JSC::CodeBlock* code, JSC::
         break;
     }
 
+    if (auto* provider = code->source().provider()) {
+        if (unsigned wrapperColumns = NodeVMScriptFetcher::wrapperColumnsOnLine(*provider, pos.line_zero_based))
+            pos.column_zero_based = static_cast<int32_t>(std::max<int64_t>(static_cast<int64_t>(pos.column_zero_based) - wrapperColumns, 0));
+    }
+
     return pos;
 }
 

src/jsc/bindings/FormatStackTraceForJS.cpp

@@ -22,6 +22,7 @@
 #include "BunClientData.h"
 #include "CallSite.h"
 #include "ErrorStackTrace.h"
+#include "NodeVMScriptFetcher.h"
 #include "headers-handwritten.h"
 
 #include <wtf/Scope.h>
@@ -265,6 +266,13 @@ WTF::String formatStackTrace(
         if (!frame.hasLineAndColumnInfo()) continue;
 
         originalLineColumns[i] = frame.computeLineAndColumn();
+        if (auto* codeBlock = frame.codeBlock()) {
+            if (auto* provider = codeBlock->source().provider()) {
+                LineColumn& lineColumn = originalLineColumns[i];
+                if (unsigned wrapperColumns = NodeVMScriptFetcher::wrapperColumnsOnLine(*provider, static_cast<int>(lineColumn.line) - 1))
+                    lineColumn.column = lineColumn.column > wrapperColumns ? lineColumn.column - wrapperColumns : 1;
+            }
+        }
 
         JSC::JSGlobalObject* globalObjectForFrame = lexicalGlobalObject;
         if (auto* callee = frame.callee()) {

src/jsc/bindings/NodeVM.cpp

@@ -183,23 +183,23 @@ JSC::JSFunction* constructAnonymousFunction(JSC::JSGlobalObject* globalObject, c
     }
 
     // wrap the arguments in an anonymous function expression
-    int startOffset = 0;
-    String code = stringifyAnonymousFunction(globalObject, args, throwScope, &startOffset);
+    int wrapperPrefixLength = 0;
+    String code = stringifyAnonymousFunction(globalObject, args, throwScope, &wrapperPrefixLength);
     EXCEPTION_ASSERT(!!throwScope.exception() == code.isNull());
+    RETURN_IF_EXCEPTION(throwScope, nullptr);
 
-    // The user's body starts on line 2 of the wrapped program (after the
-    // "(function () {\n" prefix). Shift the provider's start position up one
-    // line so reported positions line up with the body the way V8's
-    // CompileFunction does: body line 1 reports as lineOffset+1. JSC clamps
-    // non-positive provider start positions to zero, so once the input is
-    // already <= 0 there is nothing to gain by going further negative; clamp
-    // there to keep the value bounded for downstream arithmetic.
-    int lineZeroBased = position.m_line.zeroBasedInt();
-    TextPosition wrappedPosition(OrdinalNumber::fromZeroBasedInt(lineZeroBased > 0 ? lineZeroBased - 1 : lineZeroBased), position.m_column);
+    // JSC counts the wrapper in line 1's columns and cannot start a source at a negative column: apply what exceeds the wrapper here, take the rest off when positions are reported.
+    int columnOffset = position.m_column.zeroBasedInt();
+    int appliedColumnOffset = columnOffset > wrapperPrefixLength ? columnOffset - wrapperPrefixLength : 0;
+    unsigned wrapperColumns = static_cast<unsigned>(static_cast<int64_t>(wrapperPrefixLength) + appliedColumnOffset - columnOffset);
+    TextPosition wrappedPosition(position.m_line, OrdinalNumber::fromZeroBasedInt(appliedColumnOffset));
 
-    SourceCode sourceCode(
-        JSC::StringSourceProvider::create(code, sourceOrigin, WTF::move(options.filename), sourceTaintOrigin, wrappedPosition, SourceProviderSourceType::Program),
-        wrappedPosition.m_line.oneBasedInt(), wrappedPosition.m_column.oneBasedInt());
+    Ref<JSC::SourceProvider> provider = JSC::StringSourceProvider::create(code, sourceOrigin, WTF::move(options.filename), sourceTaintOrigin, wrappedPosition, SourceProviderSourceType::Program);
+
+    if (auto* fetcher = sourceOrigin.fetcher(); fetcher && fetcher->fetcherType() == ScriptFetcher::Type::NodeVM)
+        static_cast<NodeVMScriptFetcher*>(fetcher)->setWrapper(provider.get(), static_cast<unsigned>(wrapperPrefixLength), wrapperColumns);
+
+    SourceCode sourceCode(WTF::move(provider), wrappedPosition.m_line.oneBasedInt(), wrappedPosition.m_column.oneBasedInt());
 
     CodeCache* cache = vm.codeCache();
     ProgramExecutable* programExecutable = ProgramExecutable::create(globalObject, sourceCode);
@@ -380,21 +380,22 @@ static JSPromise* importModuleInner(JSGlobalObject* globalObject, JSString* modu
     RELEASE_AND_RETURN(scope, JSPromise::resolvedPromise(globalObject, thenResult));
 }
 
-// Helper function to create an anonymous function expression with parameters
+// The body deliberately shares the wrapper's line: JSC clamps a source's first line to 1, so a wrapper line could not be compensated for at lineOffset 0.
 String stringifyAnonymousFunction(JSGlobalObject* globalObject, const ArgList& args, ThrowScope& scope, int* outOffset)
 {
     // How we stringify functions is important for creating anonymous function expressions
     String program;
     if (args.isEmpty()) {
         // No arguments, just an empty function body
-        program = "(function () {\n\n})"_s;
+        program = "(function () {\n})"_s;
+        *outOffset = "(function () {"_s.length();
     } else if (args.size() == 1) {
         // Just the function body
         auto body = args.at(0).toWTFString(globalObject);
         RETURN_IF_EXCEPTION(scope, {});
 
-        program = tryMakeString("(function () {\n"_s, body, "\n})"_s);
-        *outOffset = "(function () {\n"_s.length();
+        program = tryMakeString("(function () {"_s, body, "\n})"_s);
+        *outOffset = "(function () {"_s.length();
 
         if (!program) [[unlikely]] {
             throwOutOfMemoryError(globalObject, scope);
@@ -419,8 +420,8 @@ String stringifyAnonymousFunction(JSGlobalObject* globalObject, const ArgList& a
         auto body = args.at(parameterCount).toWTFString(globalObject);
         RETURN_IF_EXCEPTION(scope, {});
 
-        program = tryMakeString("(function ("_s, paramString.toString(), ") {\n"_s, body, "\n})"_s);
-        *outOffset = "(function ("_s.length() + paramString.length() + ") {\n"_s.length();
+        program = tryMakeString("(function ("_s, paramString.toString(), ") {"_s, body, "\n})"_s);
+        *outOffset = "(function ("_s.length() + paramString.length() + ") {"_s.length();
 
         if (!program) [[unlikely]] {
             throwOutOfMemoryError(globalObject, scope);
@@ -490,7 +491,8 @@ JSC::EncodedJSValue createCachedData(JSGlobalObject* globalObject, const JSC::So
 // AppendExceptionLine helpers shared by the runtime (handleException) and
 // compile-time (decorateParseErrorStack) paths — a single implementation of
 // Node's arrow-header format so the two call sites cannot drift.
-static String nthSourceLineForArrowHeader(StringView source, int64_t physicalLine1Based)
+// firstLineSkip: compileFunction's wrapper text in front of the user's first line.
+static String nthSourceLineForArrowHeader(StringView source, int64_t physicalLine1Based, unsigned firstLineSkip = 0)
 {
     if (physicalLine1Based < 1 || physicalLine1Based > static_cast<int64_t>(source.length()) + 1)
         return {};
@@ -505,6 +507,8 @@ static String nthSourceLineForArrowHeader(StringView source, int64_t physicalLin
     if (lineEnd == WTF::notFound)
         lineEnd = source.length();
     StringView lineView = source.substring(lineStart, lineEnd - lineStart);
+    if (physicalLine1Based == 1)
+        lineView = lineView.substring(firstLineSkip);
     if (lineView.endsWith('\r'))
         lineView = lineView.left(lineView.length() - 1);
     // Like Node, skip the decoration for excessively long lines.
@@ -574,14 +578,19 @@ bool handleException(JSGlobalObject* globalObject, VM& vm, NakedPtr<JSC::Excepti
         unsigned caretColumn = 0;
         if (JSC::CodeBlock* codeBlock = stack_frame.codeBlock()) {
             if (JSC::SourceProvider* provider = codeBlock->source().provider()) {
+                unsigned wrapperPrefixLength = NodeVMScriptFetcher::wrapperTextLength(*provider);
+
                 int64_t startLineZeroBased = provider->startPosition().m_line.zeroBasedInt();
                 int64_t physicalLine = static_cast<int64_t>(line_and_column.line) - startLineZeroBased;
-                sourceLineText = nthSourceLineForArrowHeader(provider->source(), physicalLine);
+                sourceLineText = nthSourceLineForArrowHeader(provider->source(), physicalLine, wrapperPrefixLength);
                 if (!sourceLineText.isNull()) {
                     caretColumn = line_and_column.column;
-                    unsigned startColumnZeroBased = static_cast<unsigned>(provider->startPosition().m_column.zeroBasedInt());
-                    if (physicalLine == 1 && caretColumn > startColumnZeroBased)
-                        caretColumn -= startColumnZeroBased;
+                    if (physicalLine == 1) {
+                        // SourceCode clamps a negative start column to 0, so that is what the frame's column includes.
+                        unsigned startColumnZeroBased = static_cast<unsigned>(std::max(0, provider->startPosition().m_column.zeroBasedInt()));
+                        unsigned firstLineShift = startColumnZeroBased + wrapperPrefixLength;
+                        caretColumn = caretColumn > firstLineShift ? caretColumn - firstLineShift : 0;
+                    }
                 }
             }
         }

src/jsc/bindings/NodeVMScriptFetcher.h

@@ -3,6 +3,7 @@
 #include "root.h"
 
 #include <JavaScriptCore/ScriptFetcher.h>
+#include <JavaScriptCore/SourceProvider.h>
 #include <JavaScriptCore/Weak.h>
 #include <JavaScriptCore/WeakInlines.h>
 #include <wtf/Scope.h>
@@ -41,7 +42,45 @@ class NodeVMScriptFetcher : public JSC::ScriptFetcher {
         });
     }
 
+    // The compileFunction wrapper (see stringifyAnonymousFunction) shares the program's first line with the body.
+    void setWrapper(JSC::SourceProvider& program, unsigned textLength, unsigned columnsBeyondNode)
+    {
+        m_wrapperSourceID = program.asID();
+        m_wrapperTextLength = textLength;
+        m_wrapperColumns = columnsBeyondNode;
+    }
+
+    // Wrapper text starting the first line; 0 unless `provider` is a compileFunction program.
+    static unsigned wrapperTextLength(JSC::SourceProvider& provider)
+    {
+        auto* fetcher = wrapperFetcherFor(provider);
+        return fetcher ? fetcher->m_wrapperTextLength : 0;
+    }
+
+    // By how much JSC's columns exceed Node's on this line; 0 unless it is a compileFunction program's first line.
+    static unsigned wrapperColumnsOnLine(JSC::SourceProvider& provider, int lineZeroBased)
+    {
+        auto* fetcher = wrapperFetcherFor(provider);
+        if (!fetcher)
+            return 0;
+        // SourceCode clamps the start line to the first line, so JSC reports the first physical line as max(lineOffset, 0).
+        int firstLine = std::max(0, provider.startPosition().m_line.zeroBasedInt());
+        return lineZeroBased == firstLine ? fetcher->m_wrapperColumns : 0;
+    }
+
 private:
+    // Matched by provider: eval() and new Function() code inside the body inherits this fetcher without the wrapper.
+    static NodeVMScriptFetcher* wrapperFetcherFor(JSC::SourceProvider& provider)
+    {
+        auto* fetcher = provider.sourceOrigin().fetcher();
+        if (!fetcher || fetcher->fetcherType() != Type::NodeVM)
+            return nullptr;
+        auto* vmFetcher = static_cast<NodeVMScriptFetcher*>(fetcher);
+        if (!vmFetcher->m_wrapperTextLength || provider.asID() != vmFetcher->m_wrapperSourceID)
+            return nullptr;
+        return vmFetcher;
+    }
+
     JSC::Strong<JSC::Unknown> m_dynamicImportCallback;
     // m_owner is the NodeVMScript / JSFunction / module wrapper that holds this
     // fetcher via m_source -> SourceProvider -> SourceOrigin -> RefPtr<fetcher>.
@@ -50,6 +89,9 @@ class NodeVMScriptFetcher : public JSC::ScriptFetcher {
     // as a GC root). Use Weak instead: when the owner is collected its
     // SourceCode chain drops the last RefPtr to this fetcher.
     JSC::Weak<JSC::JSCell> m_owner;
+    JSC::SourceID m_wrapperSourceID = 0;
+    unsigned m_wrapperTextLength = 0;
+    unsigned m_wrapperColumns = 0;
     bool m_isUsingDefaultLoader = false;
 
     NodeVMScriptFetcher(JSC::VM& vm, JSC::JSValue dynamicImportCallback, JSC::JSValue owner)

src/jsc/bindings/ZigException.cpp

@@ -43,6 +43,7 @@
 
 #include "ErrorStackFrame.h"
 #include "ErrorStackTrace.h"
+#include "NodeVMScriptFetcher.h"
 #include "ObjectBindings.h"
 
 #include <JavaScriptCore/VMInlines.h>
@@ -161,6 +162,9 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr
         return;
 
     if (source_lines_count > 1 && source_lines != nullptr && sourceString.is8Bit()) {
+        // vm.compileFunction's wrapper starts the first line; it is not part of the user's source.
+        unsigned wrapperTextLength = Bun::NodeVMScriptFetcher::wrapperTextLength(*provider);
+
         // Search for the beginning of the line
         unsigned int lineStart = location.byte_position;
         while (lineStart > 0 && sourceString[lineStart] != '\n') {
@@ -185,7 +189,8 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr
             (*referenced_source_provider)->deref();
         }
         *referenced_source_provider = provider;
-        source_lines[0] = Bun::toStringView(sourceString.substring(lineStart, lineEnd - lineStart));
+        unsigned int textStart = lineStart == 0 ? std::min(wrapperTextLength, lineEnd) : lineStart;
+        source_lines[0] = Bun::toStringView(sourceString.substring(textStart, lineEnd - textStart));
         source_line_numbers[0] = location.line();
 
         if (lineStart > 0) {
@@ -211,7 +216,8 @@ static void populateStackFramePosition(const JSC::StackFrame& stackFrame, BunStr
                 }
 
                 // We are at the beginning of the line
-                source_lines[source_line_i] = Bun::toStringView(sourceString.substring(byte_offset_in_source_string, end_of_line_offset - byte_offset_in_source_string + 1));
+                unsigned int contextStart = byte_offset_in_source_string == 0 ? std::min(wrapperTextLength, end_of_line_offset + 1) : byte_offset_in_source_string;
+                source_lines[source_line_i] = Bun::toStringView(sourceString.substring(contextStart, end_of_line_offset + 1 - contextStart));
 
                 source_line_numbers[source_line_i] = location.line().fromZeroBasedInt(location.line().zeroBasedInt() - source_line_i);
                 source_line_i++;

test/js/node/test/parallel/test-vm-basic.js

@@ -131,9 +131,14 @@ const vm = require('vm');
 
 // vm.compileFunction
 {
+  // Bun compiles the body on the same line as the wrapper (the engine cannot
+  // start a source at line 0, so a wrapper line would shift every body line),
+  // and toString() returns the text as compiled.
   assert.strictEqual(
     vm.compileFunction('console.log("Hello, World!")').toString(),
-    'function () {\nconsole.log("Hello, World!")\n}'
+    typeof Bun === 'undefined'
+      ? 'function () {\nconsole.log("Hello, World!")\n}'
+      : 'function () {console.log("Hello, World!")\n}'
   );
 
   assert.strictEqual(
@@ -277,17 +282,17 @@ const vm = require('vm');
   // Setting value to run the last three tests
   Error.stackTraceLimit = 1;
 
-  // Bun's compileFunction stack frames differ from Node's: JSC attributes
-  // the throw to a different column (and columnOffset is not applied), the
-  // wrapper costs one line when lineOffset is 0, and the anonymous source is
-  // labeled differently.
+  // Bun's compileFunction stack frames differ from Node's in two ways: JSC
+  // attributes the throw to the call's `(` where V8 uses the `new` keyword
+  // (hence column 16 where Node has 7), and the anonymous source is labeled
+  // differently. Lines and offsets match Node.
   assert.throws(() => {
     vm.compileFunction('throw new Error("Sample Error")')();
   }, {
     message: 'Sample Error',
     stack: typeof Bun === 'undefined'
       ? 'Error: Sample Error\n    at <anonymous>:1:7'
-      : 'Error: Sample Error\n    at <anonymous> (file:///:2:16)'
+      : 'Error: Sample Error\n    at <anonymous> (file:///:1:16)'
   });
 
   assert.throws(() => {
@@ -313,7 +318,7 @@ const vm = require('vm');
     message: 'Sample Error',
     stack: typeof Bun === 'undefined'
       ? 'Error: Sample Error\n    at <anonymous>:1:10'
-      : 'Error: Sample Error\n    at <anonymous> (file:///:2:16)'
+      : 'Error: Sample Error\n    at <anonymous> (file:///:1:19)'
   });
 
   assert.strictEqual(
@@ -337,7 +342,7 @@ const vm = require('vm');
     // Bun's compileFunction stack frames differ from Node's (see above).
     stack: typeof Bun === 'undefined'
       ? 'ReferenceError: varInContext is not defined\n    at <anonymous>:1:1'
-      : 'ReferenceError: varInContext is not defined\n    at <anonymous> (file:///:2:20)'
+      : 'ReferenceError: varInContext is not defined\n    at <anonymous> (file:///:1:20)'
   });
 
   assert.notDeepStrictEqual(