From 2ca503ec059c96d864034d2e1b09305eea7722cd Mon Sep 17 00:00:00 2001 From: Tau Date: Mon, 10 Oct 2022 18:17:52 +0200 Subject: [PATCH 1/4] translate-c: Fix #12263 --- src/translate_c/ast.zig | 2 +- test/run_translated_c.zig | 69 +++++++++++++++++++-------------------- 2 files changed, 34 insertions(+), 37 deletions(-) diff --git a/src/translate_c/ast.zig b/src/translate_c/ast.zig index 55f197c644..fb71e1c325 100644 --- a/src/translate_c/ast.zig +++ b/src/translate_c/ast.zig @@ -910,7 +910,7 @@ fn renderNode(c: *Context, node: Node) Allocator.Error!NodeIndex { }, .call => { const payload = node.castTag(.call).?.data; - const lhs = try renderNode(c, payload.lhs); + const lhs = try renderNodeGrouped(c, payload.lhs); return renderCall(c, lhs, payload.args); }, .null_literal => return c.addNode(.{ diff --git a/test/run_translated_c.zig b/test/run_translated_c.zig index 71654afba4..a64a3eb7a4 100644 --- a/test/run_translated_c.zig +++ b/test/run_translated_c.zig @@ -891,42 +891,39 @@ pub fn addCases(cases: *tests.RunTranslatedCContext) void { \\} , ""); - if (@import("builtin").zig_backend == .stage1) { - // https://github.com/ziglang/zig/issues/12263 - cases.add("Obscure ways of calling functions; issue #4124", - \\#include - \\static int add(int a, int b) { - \\ return a + b; - \\} - \\typedef int (*adder)(int, int); - \\typedef void (*funcptr)(void); - \\int main() { - \\ if ((add)(1, 2) != 3) abort(); - \\ if ((&add)(1, 2) != 3) abort(); - \\ if (add(3, 1) != 4) abort(); - \\ if ((*add)(2, 3) != 5) abort(); - \\ if ((**add)(7, -1) != 6) abort(); - \\ if ((***add)(-2, 9) != 7) abort(); - \\ - \\ int (*ptr)(int a, int b); - \\ ptr = add; - \\ - \\ if (ptr(1, 2) != 3) abort(); - \\ if ((*ptr)(3, 1) != 4) abort(); - \\ if ((**ptr)(2, 3) != 5) abort(); - \\ if ((***ptr)(7, -1) != 6) abort(); - \\ if ((****ptr)(-2, 9) != 7) abort(); - \\ - \\ funcptr addr1 = (funcptr)(add); - \\ funcptr addr2 = (funcptr)(&add); - \\ - \\ if (addr1 != addr2) abort(); - \\ if (((int(*)(int, int))addr1)(1, 2) != 3) abort(); - \\ if (((adder)addr2)(1, 2) != 3) abort(); - \\ return 0; - \\} - , ""); - } + cases.add("Obscure ways of calling functions; issue #4124", + \\#include + \\static int add(int a, int b) { + \\ return a + b; + \\} + \\typedef int (*adder)(int, int); + \\typedef void (*funcptr)(void); + \\int main() { + \\ if ((add)(1, 2) != 3) abort(); + \\ if ((&add)(1, 2) != 3) abort(); + \\ if (add(3, 1) != 4) abort(); + \\ if ((*add)(2, 3) != 5) abort(); + \\ if ((**add)(7, -1) != 6) abort(); + \\ if ((***add)(-2, 9) != 7) abort(); + \\ + \\ int (*ptr)(int a, int b); + \\ ptr = add; + \\ + \\ if (ptr(1, 2) != 3) abort(); + \\ if ((*ptr)(3, 1) != 4) abort(); + \\ if ((**ptr)(2, 3) != 5) abort(); + \\ if ((***ptr)(7, -1) != 6) abort(); + \\ if ((****ptr)(-2, 9) != 7) abort(); + \\ + \\ funcptr addr1 = (funcptr)(add); + \\ funcptr addr2 = (funcptr)(&add); + \\ + \\ if (addr1 != addr2) abort(); + \\ if (((int(*)(int, int))addr1)(1, 2) != 3) abort(); + \\ if (((adder)addr2)(1, 2) != 3) abort(); + \\ return 0; + \\} + , ""); cases.add("Return boolean expression as int; issue #6215", \\#include From 6be16eeae92120fab81c51dbeaf9960375dc0ab1 Mon Sep 17 00:00:00 2001 From: Tau Date: Mon, 10 Oct 2022 22:30:33 +0200 Subject: [PATCH 2/4] translate-c: fix the remaining function pointer issues --- src/translate_c.zig | 18 +++------- src/translate_c/ast.zig | 76 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 73 insertions(+), 21 deletions(-) diff --git a/src/translate_c.zig b/src/translate_c.zig index e24c78f052..aaf288934e 100644 --- a/src/translate_c.zig +++ b/src/translate_c.zig @@ -436,7 +436,7 @@ pub fn translate( } } - return ast.render(gpa, context.global_scope.nodes.items); + return ast.render(gpa, zig_is_stage1, context.global_scope.nodes.items); } /// Determines whether macro is of the form: `#define FOO FOO` (Possibly with trailing tokens) @@ -2072,10 +2072,7 @@ fn transImplicitCastExpr( }, .PointerToBoolean => { // @ptrToInt(val) != 0 - var ptr_node = try transExpr(c, scope, sub_expr, .used); - if (ptr_node.tag() == .fn_identifier) { - ptr_node = try Tag.address_of.create(c.arena, ptr_node); - } + const ptr_node = try transExpr(c, scope, sub_expr, .used); const ptr_to_int = try Tag.ptr_to_int.create(c.arena, ptr_node); const ne = try Tag.not_equal.create(c.arena, .{ .lhs = ptr_to_int, .rhs = Tag.zero_literal.init() }); @@ -2524,10 +2521,7 @@ fn transCCast( } if (cIsInteger(dst_type) and qualTypeIsPtr(src_type)) { // @intCast(dest_type, @ptrToInt(val)) - const ptr_to_int = if (expr.tag() == .fn_identifier) - try Tag.ptr_to_int.create(c.arena, try Tag.address_of.create(c.arena, expr)) - else - try Tag.ptr_to_int.create(c.arena, expr); + const ptr_to_int = try Tag.ptr_to_int.create(c.arena, expr); return Tag.int_cast.create(c.arena, .{ .lhs = dst_node, .rhs = ptr_to_int }); } if (cIsInteger(src_type) and qualTypeIsPtr(dst_type)) { @@ -3566,7 +3560,8 @@ fn transArrayAccess(c: *Context, scope: *Scope, stmt: *const clang.ArraySubscrip // Special case: actual pointer (not decayed array) and signed integer subscript // See discussion at https://github.com/ziglang/zig/pull/8589 - if (is_signed and (base_stmt == unwrapped_base) and !is_vector and !is_nonnegative_int_literal) return transSignedArrayAccess(c, scope, base_stmt, subscr_expr, result_used); + if (is_signed and (base_stmt == unwrapped_base) and !is_vector and !is_nonnegative_int_literal) + return transSignedArrayAccess(c, scope, base_stmt, subscr_expr, result_used); const container_node = try transExpr(c, scope, unwrapped_base, .used); const rhs = if (is_longlong or is_signed) blk: { @@ -3761,9 +3756,6 @@ fn transUnaryOperator(c: *Context, scope: *Scope, stmt: *const clang.UnaryOperat else return transCreatePreCrement(c, scope, stmt, .sub_assign, used), .AddrOf => { - if (c.zig_is_stage1 and cIsFunctionDeclRef(op_expr)) { - return transExpr(c, scope, op_expr, used); - } return Tag.address_of.create(c.arena, try transExpr(c, scope, op_expr, used)); }, .Deref => { diff --git a/src/translate_c/ast.zig b/src/translate_c/ast.zig index fb71e1c325..4dcdbc4250 100644 --- a/src/translate_c/ast.zig +++ b/src/translate_c/ast.zig @@ -717,10 +717,11 @@ pub const Payload = struct { /// Converts the nodes into a Zig Ast. /// Caller must free the source slice. -pub fn render(gpa: Allocator, nodes: []const Node) !std.zig.Ast { +pub fn render(gpa: Allocator, zig_is_stage1: bool, nodes: []const Node) !std.zig.Ast { var ctx = Context{ .gpa = gpa, .buf = std.ArrayList(u8).init(gpa), + .zig_is_stage1 = zig_is_stage1, }; defer ctx.buf.deinit(); defer ctx.nodes.deinit(gpa); @@ -789,6 +790,11 @@ const Context = struct { extra_data: std.ArrayListUnmanaged(std.zig.Ast.Node.Index) = .{}, tokens: std.zig.Ast.TokenList = .{}, + /// This is used to emit different code depending on whether + /// the output zig source code is intended to be compiled with stage1 or stage2. + /// Refer to the Context in translate_c.zig. + zig_is_stage1: bool, + fn addTokenFmt(c: *Context, tag: TokenTag, comptime format: []const u8, args: anytype) Allocator.Error!TokenIndex { const start_index = c.buf.items.len; try c.buf.writer().print(format ++ " ", args); @@ -910,7 +916,15 @@ fn renderNode(c: *Context, node: Node) Allocator.Error!NodeIndex { }, .call => { const payload = node.castTag(.call).?.data; - const lhs = try renderNodeGrouped(c, payload.lhs); + // Cosmetic: avoids an unnecesary address_of on most function calls. + const lhs = if (!c.zig_is_stage1 and payload.lhs.tag() == .fn_identifier) + try c.addNode(.{ + .tag = .identifier, + .main_token = try c.addIdentifier(payload.lhs.castTag(.fn_identifier).?.data), + .data = undefined, + }) + else + try renderNodeGrouped(c, payload.lhs); return renderCall(c, lhs, payload.args); }, .null_literal => return c.addNode(.{ @@ -1064,12 +1078,32 @@ fn renderNode(c: *Context, node: Node) Allocator.Error!NodeIndex { }); }, .fn_identifier => { + // C semantics are that a function identifier has address + // value (implicit in stage1, explicit in stage2), except in + // the context of an address_of, which is handled there. const payload = node.castTag(.fn_identifier).?.data; - return c.addNode(.{ - .tag = .identifier, - .main_token = try c.addIdentifier(payload), - .data = undefined, - }); + if (c.zig_is_stage1) { + return try c.addNode(.{ + .tag = .identifier, + .main_token = try c.addIdentifier(payload), + .data = undefined, + }); + } else { + const tok = try c.addToken(.ampersand, "&"); + const arg = try c.addNode(.{ + .tag = .identifier, + .main_token = try c.addIdentifier(payload), + .data = undefined, + }); + return c.addNode(.{ + .tag = .address_of, + .main_token = tok, + .data = .{ + .lhs = arg, + .rhs = undefined, + }, + }); + } }, .float_literal => { const payload = node.castTag(.float_literal).?.data; @@ -1391,7 +1425,33 @@ fn renderNode(c: *Context, node: Node) Allocator.Error!NodeIndex { .bit_not => return renderPrefixOp(c, node, .bit_not, .tilde, "~"), .not => return renderPrefixOp(c, node, .bool_not, .bang, "!"), .optional_type => return renderPrefixOp(c, node, .optional_type, .question_mark, "?"), - .address_of => return renderPrefixOp(c, node, .address_of, .ampersand, "&"), + .address_of => { + const payload = node.castTag(.address_of).?.data; + if (c.zig_is_stage1 and payload.tag() == .fn_identifier) + return try c.addNode(.{ + .tag = .identifier, + .main_token = try c.addIdentifier(payload.castTag(.fn_identifier).?.data), + .data = undefined, + }); + + const ampersand = try c.addToken(.ampersand, "&"); + const base = if (payload.tag() == .fn_identifier) + try c.addNode(.{ + .tag = .identifier, + .main_token = try c.addIdentifier(payload.castTag(.fn_identifier).?.data), + .data = undefined, + }) + else + try renderNodeGrouped(c, payload); + return c.addNode(.{ + .tag = .address_of, + .main_token = ampersand, + .data = .{ + .lhs = base, + .rhs = undefined, + }, + }); + }, .deref => { const payload = node.castTag(.deref).?.data; const operand = try renderNodeGrouped(c, payload); From 770a1aa967d22c51e7bf8a5999c289bdf959ecba Mon Sep 17 00:00:00 2001 From: Tau Date: Mon, 10 Oct 2022 22:50:22 +0200 Subject: [PATCH 3/4] translate-c: fix #5305 --- src/translate_c.zig | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/translate_c.zig b/src/translate_c.zig index aaf288934e..d71e5f30e2 100644 --- a/src/translate_c.zig +++ b/src/translate_c.zig @@ -6496,7 +6496,11 @@ fn parseCPostfixExpr(c: *Context, m: *MacroCtx, scope: *Scope, type_name: ?Node) node = try Tag.field_access.create(c.arena, .{ .lhs = deref, .field_name = m.slice() }); }, .LBracket => { - const index = try macroBoolToInt(c, try parseCExpr(c, m, scope)); + const index_val = try macroBoolToInt(c, try parseCExpr(c, m, scope)); + const index = try Tag.int_cast.create(c.arena, .{ + .lhs = try Tag.type.create(c.arena, "usize"), + .rhs = index_val, + }); node = try Tag.array_access.create(c.arena, .{ .lhs = node, .rhs = index }); try m.skip(c, .RBracket); }, From 85b105d4f9970e81fef130978cae0e0ef7268571 Mon Sep 17 00:00:00 2001 From: Tau Date: Tue, 11 Oct 2022 00:43:53 +0200 Subject: [PATCH 4/4] Update translate-c tests --- test/translate_c.zig | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/translate_c.zig b/test/translate_c.zig index 9854e783d4..d6b6bcbbba 100644 --- a/test/translate_c.zig +++ b/test/translate_c.zig @@ -907,7 +907,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { , &[_][]const u8{ \\pub extern fn foo() void; \\pub export fn bar() void { - \\ var func_ptr: ?*anyopaque = @ptrCast(?*anyopaque, foo); + \\ var func_ptr: ?*anyopaque = @ptrCast(?*anyopaque, &foo); \\ var typed_func_ptr: ?*const fn () callconv(.C) void = @intToPtr(?*const fn () callconv(.C) void, @intCast(c_ulong, @ptrToInt(func_ptr))); \\ _ = @TypeOf(typed_func_ptr); \\} @@ -2726,7 +2726,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ return array[@intCast(c_uint, index)]; \\} , - \\pub const ACCESS = array[@as(c_int, 2)]; + \\pub const ACCESS = array[@intCast(usize, @as(c_int, 2))]; }); cases.add("cast signed array index to unsigned", @@ -2956,8 +2956,8 @@ pub fn addCases(cases: *tests.TranslateCContext) void { \\ return 0; \\} \\pub export fn bar() void { - \\ var f: ?*const fn () callconv(.C) void = foo; - \\ var b: ?*const fn () callconv(.C) c_int = baz; + \\ var f: ?*const fn () callconv(.C) void = &foo; + \\ var b: ?*const fn () callconv(.C) c_int = &baz; \\ f.?(); \\ f.?(); \\ foo(); @@ -3780,7 +3780,7 @@ pub fn addCases(cases: *tests.TranslateCContext) void { cases.add("Demote function that dereference types that contain opaque type", \\struct inner { - \\ _Atomic int a; + \\ _Atomic int a; \\}; \\struct outer { \\ int thing;