Commit d851b24180

Veikka Tuominen <git@vexu.eu>
2022-07-17 15:13:25
Sema: validate function pointer alignment
1 parent 76e7959
src/Module.zig
@@ -2348,7 +2348,72 @@ pub const SrcLoc = struct {
                     }
                 } else unreachable;
             },
-
+            .node_offset_fn_type_align => |node_off| {
+                const tree = try src_loc.file_scope.getTree(gpa);
+                const node_datas = tree.nodes.items(.data);
+                const node_tags = tree.nodes.items(.tag);
+                const node = src_loc.declRelativeToNodeIndex(node_off);
+                var params: [1]Ast.Node.Index = undefined;
+                const full = switch (node_tags[node]) {
+                    .fn_proto_simple => tree.fnProtoSimple(&params, node),
+                    .fn_proto_multi => tree.fnProtoMulti(node),
+                    .fn_proto_one => tree.fnProtoOne(&params, node),
+                    .fn_proto => tree.fnProto(node),
+                    .fn_decl => switch (node_tags[node_datas[node].lhs]) {
+                        .fn_proto_simple => tree.fnProtoSimple(&params, node_datas[node].lhs),
+                        .fn_proto_multi => tree.fnProtoMulti(node_datas[node].lhs),
+                        .fn_proto_one => tree.fnProtoOne(&params, node_datas[node].lhs),
+                        .fn_proto => tree.fnProto(node_datas[node].lhs),
+                        else => unreachable,
+                    },
+                    else => unreachable,
+                };
+                return nodeToSpan(tree, full.ast.align_expr);
+            },
+            .node_offset_fn_type_addrspace => |node_off| {
+                const tree = try src_loc.file_scope.getTree(gpa);
+                const node_datas = tree.nodes.items(.data);
+                const node_tags = tree.nodes.items(.tag);
+                const node = src_loc.declRelativeToNodeIndex(node_off);
+                var params: [1]Ast.Node.Index = undefined;
+                const full = switch (node_tags[node]) {
+                    .fn_proto_simple => tree.fnProtoSimple(&params, node),
+                    .fn_proto_multi => tree.fnProtoMulti(node),
+                    .fn_proto_one => tree.fnProtoOne(&params, node),
+                    .fn_proto => tree.fnProto(node),
+                    .fn_decl => switch (node_tags[node_datas[node].lhs]) {
+                        .fn_proto_simple => tree.fnProtoSimple(&params, node_datas[node].lhs),
+                        .fn_proto_multi => tree.fnProtoMulti(node_datas[node].lhs),
+                        .fn_proto_one => tree.fnProtoOne(&params, node_datas[node].lhs),
+                        .fn_proto => tree.fnProto(node_datas[node].lhs),
+                        else => unreachable,
+                    },
+                    else => unreachable,
+                };
+                return nodeToSpan(tree, full.ast.addrspace_expr);
+            },
+            .node_offset_fn_type_section => |node_off| {
+                const tree = try src_loc.file_scope.getTree(gpa);
+                const node_datas = tree.nodes.items(.data);
+                const node_tags = tree.nodes.items(.tag);
+                const node = src_loc.declRelativeToNodeIndex(node_off);
+                var params: [1]Ast.Node.Index = undefined;
+                const full = switch (node_tags[node]) {
+                    .fn_proto_simple => tree.fnProtoSimple(&params, node),
+                    .fn_proto_multi => tree.fnProtoMulti(node),
+                    .fn_proto_one => tree.fnProtoOne(&params, node),
+                    .fn_proto => tree.fnProto(node),
+                    .fn_decl => switch (node_tags[node_datas[node].lhs]) {
+                        .fn_proto_simple => tree.fnProtoSimple(&params, node_datas[node].lhs),
+                        .fn_proto_multi => tree.fnProtoMulti(node_datas[node].lhs),
+                        .fn_proto_one => tree.fnProtoOne(&params, node_datas[node].lhs),
+                        .fn_proto => tree.fnProto(node_datas[node].lhs),
+                        else => unreachable,
+                    },
+                    else => unreachable,
+                };
+                return nodeToSpan(tree, full.ast.section_expr);
+            },
             .node_offset_fn_type_cc => |node_off| {
                 const tree = try src_loc.file_scope.getTree(gpa);
                 const node_datas = tree.nodes.items(.data);
@@ -2778,6 +2843,24 @@ pub const LazySrcLoc = union(enum) {
     /// range nodes. The error applies to all of them.
     /// The Decl is determined contextually.
     node_offset_switch_range: i32,
+    /// The source location points to the align expr of a function type
+    /// expression, found by taking this AST node index offset from the containing
+    /// Decl AST node, which points to a function type AST node. Next, navigate to
+    /// the calling convention node.
+    /// The Decl is determined contextually.
+    node_offset_fn_type_align: i32,
+    /// The source location points to the addrspace expr of a function type
+    /// expression, found by taking this AST node index offset from the containing
+    /// Decl AST node, which points to a function type AST node. Next, navigate to
+    /// the calling convention node.
+    /// The Decl is determined contextually.
+    node_offset_fn_type_addrspace: i32,
+    /// The source location points to the linksection expr of a function type
+    /// expression, found by taking this AST node index offset from the containing
+    /// Decl AST node, which points to a function type AST node. Next, navigate to
+    /// the calling convention node.
+    /// The Decl is determined contextually.
+    node_offset_fn_type_section: i32,
     /// The source location points to the calling convention of a function type
     /// expression, found by taking this AST node index offset from the containing
     /// Decl AST node, which points to a function type AST node. Next, navigate to
@@ -2897,6 +2980,9 @@ pub const LazySrcLoc = union(enum) {
             .node_offset_switch_operand,
             .node_offset_switch_special_prong,
             .node_offset_switch_range,
+            .node_offset_fn_type_align,
+            .node_offset_fn_type_addrspace,
+            .node_offset_fn_type_section,
             .node_offset_fn_type_cc,
             .node_offset_fn_type_ret_ty,
             .node_offset_anyframe_type,
src/Sema.zig
@@ -1884,13 +1884,22 @@ fn analyzeAsAlign(
 ) !u32 {
     const alignment_big = try sema.analyzeAsInt(block, src, air_ref, align_ty, "alignment must be comptime known");
     const alignment = @intCast(u32, alignment_big); // We coerce to u16 in the prev line.
+    try sema.validateAlign(block, src, alignment);
+    return alignment;
+}
+
+fn validateAlign(
+    sema: *Sema,
+    block: *Block,
+    src: LazySrcLoc,
+    alignment: u32,
+) !void {
     if (alignment == 0) return sema.fail(block, src, "alignment must be >= 1", .{});
     if (!std.math.isPowerOfTwo(alignment)) {
         return sema.fail(block, src, "alignment value '{d}' is not a power of two", .{
             alignment,
         });
     }
-    return alignment;
 }
 
 pub fn resolveAlign(
@@ -13902,8 +13911,9 @@ fn zirPtrType(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air
                 break :blk 0;
             }
         }
-        const abi_align = (try val.getUnsignedIntAdvanced(target, sema.kit(block, align_src))).?;
-        break :blk @intCast(u32, abi_align);
+        const abi_align = @intCast(u32, (try val.getUnsignedIntAdvanced(target, sema.kit(block, align_src))).?);
+        try sema.validateAlign(block, align_src, abi_align);
+        break :blk abi_align;
     } else 0;
 
     const address_space = if (inst_data.flags.has_addrspace) blk: {
@@ -13940,6 +13950,14 @@ fn zirPtrType(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!Air
 
     if (elem_ty.zigTypeTag() == .NoReturn) {
         return sema.fail(block, elem_ty_src, "pointer to noreturn not allowed", .{});
+    } else if (elem_ty.zigTypeTag() == .Fn) {
+        if (inst_data.size != .One) {
+            return sema.fail(block, elem_ty_src, "function pointers must be single pointers", .{});
+        }
+        const fn_align = elem_ty.abiAlignment(target);
+        if (inst_data.flags.has_align and abi_align != 0 and abi_align != fn_align) {
+            return sema.fail(block, align_src, "function pointer alignment disagrees with function alignment", .{});
+        }
     } else if (inst_data.size == .Many and elem_ty.zigTypeTag() == .Opaque) {
         return sema.fail(block, elem_ty_src, "unknown-length pointer to opaque not allowed", .{});
     } else if (inst_data.size == .C) {
@@ -17709,15 +17727,14 @@ fn zirFuncFancy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!A
     defer tracy.end();
 
     const inst_data = sema.code.instructions.items(.data)[inst].pl_node;
-    const src = inst_data.src();
     const extra = sema.code.extraData(Zir.Inst.FuncFancy, inst_data.payload_index);
     const target = sema.mod.getTarget();
 
-    const align_src: LazySrcLoc = src; // TODO add a LazySrcLoc that points at align
-    const addrspace_src: LazySrcLoc = src; // TODO add a LazySrcLoc that points at addrspace
-    const section_src: LazySrcLoc = src; // TODO add a LazySrcLoc that points at section
+    const align_src: LazySrcLoc = .{ .node_offset_fn_type_align = inst_data.src_node };
+    const addrspace_src: LazySrcLoc = .{ .node_offset_fn_type_addrspace = inst_data.src_node };
+    const section_src: LazySrcLoc = .{ .node_offset_fn_type_section = inst_data.src_node };
     const cc_src: LazySrcLoc = .{ .node_offset_fn_type_cc = inst_data.src_node };
-    const ret_src: LazySrcLoc = src; // TODO add a LazySrcLoc that points at the return type
+    const ret_src: LazySrcLoc = .{ .node_offset_fn_type_ret_ty = inst_data.src_node };
 
     var extra_index: usize = extra.end;
 
@@ -17742,6 +17759,7 @@ fn zirFuncFancy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!A
             break :blk null;
         }
         const alignment = @intCast(u32, val.toUnsignedInt(target));
+        try sema.validateAlign(block, align_src, alignment);
         if (alignment == target_util.defaultFunctionAlignment(target)) {
             break :blk 0;
         } else {
@@ -17757,6 +17775,7 @@ fn zirFuncFancy(sema: *Sema, block: *Block, inst: Zir.Inst.Index) CompileError!A
             else => |e| return e,
         };
         const alignment = @intCast(u32, align_tv.val.toUnsignedInt(target));
+        try sema.validateAlign(block, align_src, alignment);
         if (alignment == target_util.defaultFunctionAlignment(target)) {
             break :blk 0;
         } else {
test/behavior/align.zig
@@ -299,7 +299,6 @@ test "implicitly decreasing fn alignment" {
     try testImplicitlyDecreaseFnAlign(alignedBig, 5678);
 }
 
-// TODO make it a compile error to put align on the fn proto instead of on the ptr
 fn testImplicitlyDecreaseFnAlign(ptr: *align(1) const fn () i32, answer: i32) !void {
     try expect(ptr() == answer);
 }
test/cases/compile_errors/align_n_expr_function_pointers_is_a_compile_error.zig
@@ -6,4 +6,4 @@ export fn foo() align(1) void {
 // backend=stage2
 // target=wasm32-freestanding-none
 //
-// :1:8: error: 'align' is not allowed on functions in wasm
+// :1:23: error: 'align' is not allowed on functions in wasm
\ No newline at end of file
test/cases/compile_errors/stage1/obj/function_alignment_non_power_of_2.zig → test/cases/compile_errors/function_alignment_non_power_of_2.zig
@@ -2,7 +2,7 @@ extern fn foo() align(3) void;
 export fn entry() void { return foo(); }
 
 // error
-// backend=stage1
+// backend=stage2
 // target=native
 //
-// tmp.zig:1:23: error: alignment value 3 is not a power of 2
+// :1:23: error: alignment value '3' is not a power of two
test/cases/compile_errors/function_ptr_alignment.zig
@@ -0,0 +1,25 @@
+comptime {
+    var a: *align(2) @TypeOf(foo) = undefined;
+    _ = a;
+}
+fn foo() void {}
+
+comptime {
+    var a: *align(1) fn () void = undefined;
+    _ = a;
+}
+comptime {
+    var a: *align(2) fn () align(2) void = undefined;
+    _ = a;
+}
+comptime {
+    var a: *align(2) fn () void = undefined;
+    _ = a;
+}
+
+// error
+// backend=stage2
+// target=native
+//
+// :2:19: error: function pointer alignment disagrees with function alignment
+// :16:19: error: function pointer alignment disagrees with function alignment