Skip to content

Commit

Permalink
fix: Macro segmentation faults (#6756)
Browse files Browse the repository at this point in the history
* Fix for seg faults when using macros

* Update src/js_ast.zig to reflect review suggestions

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>

* add test for checking non-zero exitcodes under macros. regression, issue 3830

---------

Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
  • Loading branch information
I-A-S and Jarred-Sumner committed Oct 30, 2023
1 parent 2972cfa commit cbc5ca7
Show file tree
Hide file tree
Showing 3 changed files with 42 additions and 5 deletions.
15 changes: 11 additions & 4 deletions src/js_ast.zig
Original file line number Diff line number Diff line change
Expand Up @@ -7278,8 +7278,9 @@ pub const Macro = struct {

exception_holder = Zig.ZigException.Holder.init();
var js_args: []JSC.JSValue = &.{};
var js_processed_args_len: usize = 0;
defer {
for (js_args[0 .. js_args.len - @as(usize, @intFromBool(!javascript_object.isEmpty()))]) |arg| {
for (js_args[0..js_processed_args_len -| @as(usize, @intFromBool(!javascript_object.isEmpty()))]) |arg| {
arg.unprotect();
}

Expand All @@ -7292,12 +7293,18 @@ pub const Macro = struct {
.e_call => |call| {
const call_args: []Expr = call.args.slice();
js_args = try allocator.alloc(JSC.JSValue, call_args.len + @as(usize, @intFromBool(!javascript_object.isEmpty())));
js_processed_args_len = js_args.len;

for (call_args, js_args[0..call_args.len]) |in, *out| {
const value = try in.toJS(
for (0.., call_args, js_args[0..call_args.len]) |i, in, *out| {
const value = in.toJS(
allocator,
globalObject,
);
) catch |e| {
// Keeping a separate variable instead of modifying js_args.len
// due to allocator.free call in defer
js_processed_args_len = i;
return e;
};
value.protect();
out.* = value;
}
Expand Down
2 changes: 1 addition & 1 deletion src/js_parser.zig
Original file line number Diff line number Diff line change
Expand Up @@ -16389,7 +16389,7 @@ fn NewParser_(
p.log.addError(p.source, expr.loc, "macro threw exception") catch unreachable;
}
} else {
p.log.addErrorFmt(p.source, expr.loc, p.allocator, "{s} error in macro", .{@errorName(err)}) catch unreachable;
p.log.addErrorFmt(p.source, expr.loc, p.allocator, "\"{s}\" error in macro", .{@errorName(err)}) catch unreachable;
}
return expr;
};
Expand Down
30 changes: 30 additions & 0 deletions test/regression/issue/03830.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
// test/regression/issue/03830.test.ts

import { it, expect } from "bun:test";
import { bunEnv, bunExe } from "harness";
import { mkdirSync, rmSync, writeFileSync, readFileSync, mkdtempSync } from "fs";
import { tmpdir } from "os";
import { join } from "path";

it("macros should not lead to seg faults under any given input", async () => {
// this test code follows the same structure as and
// is based on the code for testing issue 4893

const testDir = mkdtempSync(join(tmpdir(), "issue3830-"));

// Clean up from prior runs if necessary
rmSync(testDir, { recursive: true, force: true });

// Create a directory with our test file
mkdirSync(testDir, { recursive: true });
writeFileSync(join(testDir, "macro.ts"), 'export function fn(str) { return str; }');
writeFileSync(join(testDir, "index.ts"), "import { fn } from './macro' assert { type: 'macro' };\nfn(`©${''}`);");

const { stdout, exitCode } = Bun.spawnSync({
cmd: [bunExe(), "build", join(testDir, "index.ts")],
env: bunEnv,
stderr: "inherit",
});

expect(exitCode).toBe(0);
});

0 comments on commit cbc5ca7

Please sign in to comment.