Skip to content

Commit

Permalink
Improve stack overflow, show more properties in Error objects (#15985)
Browse files Browse the repository at this point in the history
Co-authored-by: Dave Caruso <[email protected]>
  • Loading branch information
Jarred-Sumner and paperclover authored Dec 26, 2024
1 parent 7317c7b commit 2b2ca32
Show file tree
Hide file tree
Showing 43 changed files with 1,730 additions and 319 deletions.
2 changes: 1 addition & 1 deletion cmake/targets/BuildBun.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -860,7 +860,7 @@ endif()

if(WIN32)
target_link_options(${bun} PUBLIC
/STACK:0x1200000,0x100000
/STACK:0x1200000,0x200000
/errorlimit:0
)
if(RELEASE)
Expand Down
7 changes: 6 additions & 1 deletion src/bit_set.zig
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
//! This is a fork of Zig standard library bit_set.zig
//! - https://github.com/ziglang/zig/pull/14129
//! - AutoBitset which optimally chooses between a dynamic or static bitset.
//! Prefer our fork over std.bit_set.zig.
//!
//! This file defines several variants of bit sets. A bit set
//! is a densely stored set of integers with a known maximum,
//! in which each integer gets a single bit. Bit sets have very
Expand Down Expand Up @@ -402,7 +407,7 @@ pub fn ArrayBitSet(comptime MaskIntType: type, comptime size: usize) type {

/// Returns true if the bit at the specified index
/// is present in the set, false otherwise.
pub inline fn isSet(self: *const Self, index: usize) bool {
pub fn isSet(self: *const Self, index: usize) bool {
if (comptime Environment.allow_assert) bun.assert(index < bit_length);
if (num_masks == 0) return false; // doesn't compile in this case
return (self.masks[maskIndex(index)] & maskBit(index)) != 0;
Expand Down
180 changes: 146 additions & 34 deletions src/bun.js/ConsoleObject.zig

Large diffs are not rendered by default.

14 changes: 10 additions & 4 deletions src/bun.js/api/BunObject.zig
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,8 @@ pub fn braces(global: *JSC.JSGlobalObject, brace_str: bun.String, opts: gen.Brac

pub fn which(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSError!JSC.JSValue {
const arguments_ = callframe.arguments_old(2);
var path_buf: bun.PathBuffer = undefined;
const path_buf = bun.PathBufferPool.get();
defer bun.PathBufferPool.put(path_buf);
var arguments = JSC.Node.ArgumentsSlice.init(globalThis.bunVM(), arguments_.slice());
defer arguments.deinit();
const path_arg = arguments.nextEat() orelse {
Expand Down Expand Up @@ -397,7 +398,7 @@ pub fn which(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.JSE
}

if (Which.which(
&path_buf,
path_buf,
path_str.slice(),
cwd_str.slice(),
bin_str.slice(),
Expand Down Expand Up @@ -506,6 +507,7 @@ pub fn inspect(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.J

// very stable memory address
var array = MutableString.init(getAllocator(globalThis), 0) catch unreachable;
defer array.deinit();
var buffered_writer_ = MutableString.BufferedWriter{ .context = &array };
var buffered_writer = &buffered_writer_;

Expand All @@ -523,15 +525,19 @@ pub fn inspect(globalThis: *JSC.JSGlobalObject, callframe: *JSC.CallFrame) bun.J
writer,
formatOptions,
);
if (globalThis.hasException()) {
return .zero;
}

buffered_writer.flush() catch {
return .undefined;
return globalThis.throwOutOfMemory();
};

// we are going to always clone to keep things simple for now
// the common case here will be stack-allocated, so it should be fine
var out = ZigString.init(array.slice()).withEncoding();
const ret = out.toJS(globalThis);
array.deinit();

return ret;
}

Expand Down
74 changes: 50 additions & 24 deletions src/bun.js/api/bun/subprocess.zig
Original file line number Diff line number Diff line change
Expand Up @@ -1703,15 +1703,58 @@ pub const Subprocess = struct {
return spawnMaybeSync(globalThis, args, secondaryArgsValue, true);
}

// This is split into a separate function to conserve stack space.
// On Windows, a single path buffer can take 64 KB.
fn getArgv0(globalThis: *JSC.JSGlobalObject, PATH: []const u8, cwd: []const u8, argv0: ?[*:0]const u8, first_cmd: JSValue, allocator: std.mem.Allocator) bun.JSError!struct {
argv0: [:0]const u8,
arg0: [:0]u8,
} {
var arg0 = try first_cmd.toSliceOrNullWithAllocator(globalThis, allocator);
defer arg0.deinit();
// Heap allocate it to ensure we don't run out of stack space.
const path_buf: *bun.PathBuffer = try bun.default_allocator.create(bun.PathBuffer);
defer bun.default_allocator.destroy(path_buf);

var actual_argv0: [:0]const u8 = "";

if (argv0 == null) {
const resolved = Which.which(path_buf, PATH, cwd, arg0.slice()) orelse {
return throwCommandNotFound(globalThis, arg0.slice());
};
actual_argv0 = try allocator.dupeZ(u8, resolved);
} else {
const resolved = Which.which(path_buf, PATH, cwd, bun.sliceTo(argv0.?, 0)) orelse {
return throwCommandNotFound(globalThis, arg0.slice());
};
actual_argv0 = try allocator.dupeZ(u8, resolved);
}

return .{
.argv0 = actual_argv0,
.arg0 = try allocator.dupeZ(u8, arg0.slice()),
};
}

pub fn spawnMaybeSync(
globalThis: *JSC.JSGlobalObject,
args_: JSValue,
secondaryArgsValue: ?JSValue,
comptime is_sync: bool,
) bun.JSError!JSValue {
if (comptime is_sync) {
// We skip this on Windows due to test failures.
if (comptime !Environment.isWindows) {
// Since the event loop is recursively called, we need to check if it's safe to recurse.
if (!bun.StackCheck.init().isSafeToRecurse()) {
globalThis.throwStackOverflow();
return error.JSError;
}
}
}

var arena = bun.ArenaAllocator.init(bun.default_allocator);
defer arena.deinit();
var allocator = arena.allocator();
const allocator = arena.allocator();

var override_env = false;
var env_array = std.ArrayListUnmanaged(?[*:0]const u8){};
Expand Down Expand Up @@ -1801,33 +1844,16 @@ pub const Subprocess = struct {
return globalThis.throwInvalidArguments("cmd must not be empty", .{});
}

{
var first_cmd = cmds_array.next().?;
var arg0 = try first_cmd.toSliceOrNullWithAllocator(globalThis, allocator);
defer arg0.deinit();

if (argv0 == null) {
var path_buf: bun.PathBuffer = undefined;
const resolved = Which.which(&path_buf, PATH, cwd, arg0.slice()) orelse {
return throwCommandNotFound(globalThis, arg0.slice());
};
argv0 = try allocator.dupeZ(u8, resolved);
} else {
var path_buf: bun.PathBuffer = undefined;
const resolved = Which.which(&path_buf, PATH, cwd, bun.sliceTo(argv0.?, 0)) orelse {
return throwCommandNotFound(globalThis, arg0.slice());
};
argv0 = try allocator.dupeZ(u8, resolved);
}

argv.appendAssumeCapacity(try allocator.dupeZ(u8, arg0.slice()));
}
const argv0_result = try getArgv0(globalThis, PATH, cwd, argv0, cmds_array.next().?, allocator);
argv0 = argv0_result.argv0.ptr;
argv.appendAssumeCapacity(argv0_result.arg0.ptr);

while (cmds_array.next()) |value| {
const arg = value.getZigString(globalThis);
const arg = try value.toBunString2(globalThis);
defer arg.deref();

// if the string is empty, ignore it, don't add it to the argv
if (arg.len == 0) {
if (arg.isEmpty()) {
continue;
}
argv.appendAssumeCapacity(try arg.toOwnedSliceZ(allocator));
Expand Down
18 changes: 18 additions & 0 deletions src/bun.js/bindings/BunString.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "ZigGlobalObject.h"
#include "IDLTypes.h"

#include <limits>
#include <wtf/Seconds.h>
#include <wtf/text/ExternalStringImpl.h>
#include <JavaScriptCore/JSONObject.h>
Expand All @@ -34,6 +35,7 @@

#include "GCDefferalContext.h"
#include "wtf/text/StringImpl.h"
#include "wtf/text/StringToIntegerConversion.h"

extern "C" void mi_free(void* ptr);

Expand Down Expand Up @@ -90,6 +92,22 @@ extern "C" BunString BunString__tryCreateAtom(const char* bytes, size_t length)
return { BunStringTag::Dead, {} };
}

// int64_t max to say "not a number"
extern "C" int64_t BunString__toInt32(BunString* bunString)
{
if (bunString->tag == BunStringTag::Empty || bunString->tag == BunStringTag::Dead) {
return std::numeric_limits<int64_t>::max();
}

String str = bunString->toWTFString();
auto val = WTF::parseIntegerAllowingTrailingJunk<int32_t>(str);
if (val) {
return val.value();
}

return std::numeric_limits<int64_t>::max();
}

namespace Bun {
JSC::JSValue toJS(JSC::JSGlobalObject* globalObject, BunString bunString)
{
Expand Down
3 changes: 3 additions & 0 deletions src/bun.js/bindings/ErrorCode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,7 @@ export default [
["ERR_ASYNC_TYPE", TypeError],
["ERR_INVALID_ASYNC_ID", RangeError],
["ERR_ASYNC_CALLBACK", TypeError],

// Postgres
["ERR_POSTGRES_ERROR_RESPONSE", Error, "PostgresError"],
] as ErrorCodeMapping;
116 changes: 110 additions & 6 deletions src/bun.js/bindings/JSPropertyIterator.cpp
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@


#include "root.h"

#include "BunClientData.h"
#include "ZigGlobalObject.h"
#include "JavaScriptCore/JSType.h"
#include "JavaScriptCore/EnumerationMode.h"
#include "JavaScriptCore/ExceptionScope.h"
#include "JavaScriptCore/ThrowScope.h"
Expand All @@ -11,6 +12,7 @@
#include "wtf/Assertions.h"
#include "wtf/FastMalloc.h"
#include "headers-handwritten.h"
#include "ObjectBindings.h"

namespace Bun {
using namespace JSC;
Expand All @@ -25,6 +27,7 @@ class JSPropertyIterator {

RefPtr<JSC::PropertyNameArrayData> properties;
Ref<JSC::VM> vm;
bool isSpecialProxy = false;
static JSPropertyIterator* create(JSC::VM& vm, RefPtr<JSC::PropertyNameArrayData> data)
{
return new JSPropertyIterator(vm, data);
Expand All @@ -33,15 +36,50 @@ class JSPropertyIterator {
WTF_MAKE_FAST_ALLOCATED;
};

extern "C" JSPropertyIterator* Bun__JSPropertyIterator__create(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue encodedValue, size_t* count)
extern "C" JSPropertyIterator* Bun__JSPropertyIterator__create(JSC::JSGlobalObject* globalObject, JSC::EncodedJSValue encodedValue, size_t* count, bool own_properties_only, bool only_non_index_properties)
{
JSC::VM& vm = globalObject->vm();
JSC::JSValue value = JSValue::decode(encodedValue);
JSC::JSObject* object = value.getObject();

auto scope = DECLARE_THROW_SCOPE(vm);
JSC::PropertyNameArray array(vm, PropertyNameMode::StringsAndSymbols, PrivateSymbolMode::Exclude);
object->getPropertyNames(globalObject, array, DontEnumPropertiesMode::Exclude);

#if OS(WINDOWS)
if (UNLIKELY(object->type() == JSC::ProxyObjectType)) {
// Check if we're actually iterating through the JSEnvironmentVariableMap's proxy.
auto* zigGlobal = defaultGlobalObject(globalObject);
if (zigGlobal->m_processEnvObject.isInitialized()) {
if (object == zigGlobal->m_processEnvObject.get(zigGlobal)) {
object->methodTable()->getOwnPropertyNames(
object,
globalObject,
array,
DontEnumPropertiesMode::Exclude);
RETURN_IF_EXCEPTION(scope, nullptr);

*count = array.size();
if (array.size() == 0) {
return nullptr;
}

auto* iter = JSPropertyIterator::create(vm, array.releaseData());
iter->isSpecialProxy = true;
return iter;
}
}
}
#endif

if (own_properties_only) {
if (only_non_index_properties) {
object->getOwnNonIndexPropertyNames(globalObject, array, DontEnumPropertiesMode::Exclude);
} else {
object->getOwnPropertyNames(object, globalObject, array, DontEnumPropertiesMode::Exclude);
}
} else {
object->getPropertyNames(globalObject, array, DontEnumPropertiesMode::Exclude);
}
RETURN_IF_EXCEPTION(scope, nullptr);

*count = array.size();
Expand All @@ -52,13 +90,79 @@ extern "C" JSPropertyIterator* Bun__JSPropertyIterator__create(JSC::JSGlobalObje
return JSPropertyIterator::create(vm, array.releaseData());
}

extern "C" size_t Bun__JSPropertyIterator__getLongestPropertyName(JSPropertyIterator* iter, JSC::JSGlobalObject* globalObject, JSC::JSObject* object)
{
size_t longest = 0;
for (const auto& prop : iter->properties->propertyNameVector()) {
if (prop.length() > longest) {
longest = prop.length();
}
}

return longest;
}

static EncodedJSValue getOwnProxyObject(JSPropertyIterator* iter, JSObject* object, const JSC::Identifier& prop, BunString* propertyName)
{
auto& vm = iter->vm;
auto scope = DECLARE_THROW_SCOPE(vm);

PropertySlot slot(object, PropertySlot::InternalMethodType::GetOwnProperty, nullptr);
auto* globalObject = object->globalObject();
if (!object->methodTable()->getOwnPropertySlot(object, globalObject, prop, slot)) {
return {};
}
RETURN_IF_EXCEPTION(scope, {});

JSValue result = slot.getValue(globalObject, prop);
RETURN_IF_EXCEPTION(scope, {});

*propertyName = Bun::toString(prop.impl());
return JSValue::encode(result);
}

extern "C" EncodedJSValue Bun__JSPropertyIterator__getNameAndValue(JSPropertyIterator* iter, JSC::JSGlobalObject* globalObject, JSC::JSObject* object, BunString* propertyName, size_t i)
{
const auto& prop = iter->properties->propertyNameVector()[i];
if (UNLIKELY(iter->isSpecialProxy)) {
return getOwnProxyObject(iter, object, prop, propertyName);
}

auto& vm = iter->vm;
auto scope = DECLARE_THROW_SCOPE(vm);
PropertySlot slot(object, PropertySlot::InternalMethodType::GetOwnProperty);
if (!object->getOwnPropertySlot(object, globalObject, prop, slot)) {
return {};
}
RETURN_IF_EXCEPTION(scope, {});

auto scope = DECLARE_THROW_SCOPE(iter->vm);
JSValue result = object->get(globalObject, prop);
JSValue result = slot.getValue(globalObject, prop);
RETURN_IF_EXCEPTION(scope, {});

*propertyName = Bun::toString(prop.impl());
return JSValue::encode(result);
}

extern "C" EncodedJSValue Bun__JSPropertyIterator__getNameAndValueNonObservable(JSPropertyIterator* iter, JSC::JSGlobalObject* globalObject, JSC::JSObject* object, BunString* propertyName, size_t i)
{
const auto& prop = iter->properties->propertyNameVector()[i];
if (UNLIKELY(iter->isSpecialProxy)) {
return getOwnProxyObject(iter, object, prop, propertyName);
}
auto& vm = iter->vm;
auto scope = DECLARE_THROW_SCOPE(vm);

PropertySlot slot(object, PropertySlot::InternalMethodType::VMInquiry, vm.ptr());
if (!object->getNonIndexPropertySlot(globalObject, prop, slot)) {
return {};
}
RETURN_IF_EXCEPTION(scope, {});

if (slot.isAccessor() || slot.isCustom()) {
return {};
}

JSValue result = slot.getPureResult();
RETURN_IF_EXCEPTION(scope, {});

*propertyName = Bun::toString(prop.impl());
Expand Down
Loading

0 comments on commit 2b2ca32

Please sign in to comment.