zig

commit ef42ef9ce8d36d26f91669a61f4c57fd3e4d58d7 (tree)
parent dd62f63c04fa21427869abea86bbc5fc090b6562
Author: Andrew Kelley <andrew@ziglang.org>
Date:   Tue, 26 May 2020 15:49:19 -0400

Merge pull request #5440 from kubkon/align-fn-error-wasm

Make align expr on fns a compile error in Wasm
Diffstat:
M
src/analyze.cpp
 | 
15
+++++++++++++++
M
test/compile_errors.zig
 | 
16
++++++++++++++++
M
test/stage1/behavior/align.zig
 | 
17
+++++++++++++----

3 files changed, 44 insertions(+), 4 deletions(-)
diff --git a/src/analyze.cpp b/src/analyze.cpp
@@ -1921,6 +1921,21 @@ static ZigType *analyze_fn_type(CodeGen *g, AstNode *proto_node, Scope *child_sc
     }
 
     if (fn_proto->align_expr != nullptr) {
+        if (target_is_wasm(g->zig_target)) {
+            // In Wasm, specifying alignment of function pointers makes little sense
+            // since function pointers are in fact indices to a Wasm table, therefore
+            // any alignment check on those is invalid. This can cause unexpected
+            // behaviour when checking expected alignment with `@ptrToInt(fn_ptr)`
+            // or similar. This commit proposes to make `align` expressions a
+            // compile error when compiled to Wasm architecture.
+            // 
+            // Some references:
+            // [1] [Mozilla: WebAssembly Tables](https://developer.mozilla.org/en-US/docs/WebAssembly/Understanding_the_text_format#WebAssembly_tables)
+            // [2] [Sunfishcode's Wasm Ref Manual](https://github.com/sunfishcode/wasm-reference-manual/blob/master/WebAssembly.md#indirect-call)
+            add_node_error(g, fn_proto->align_expr,
+                buf_sprintf("align(N) expr is not allowed on function prototypes in wasm32/wasm64"));
+            return g->builtin_types.entry_invalid;
+        }
         if (!analyze_const_align(g, child_scope, fn_proto->align_expr, &fn_type_id.alignment)) {
             return g->builtin_types.entry_invalid;
         }
diff --git a/test/compile_errors.zig b/test/compile_errors.zig
@@ -7445,4 +7445,20 @@ pub fn addCases(cases: *tests.CompileErrorContext) void {
     , &[_][]const u8{
         ":2:75: error: operation caused overflow",
     });
+
+    cases.addCase(x: {
+        var tc = cases.create("align(N) expr function pointers is a compile error",
+            \\export fn foo() align(1) void {
+            \\    return;
+            \\}
+        , &[_][]const u8{
+            "tmp.zig:1:23: error: align(N) expr is not allowed on function prototypes in wasm32/wasm64",
+        });
+        tc.target = std.zig.CrossTarget{
+            .cpu_arch = .wasm32,
+            .os_tag = .freestanding,
+            .abi = .none,
+        };
+        break :x tc;
+    });
 }
diff --git a/test/stage1/behavior/align.zig b/test/stage1/behavior/align.zig
@@ -25,6 +25,9 @@ fn noop1() align(1) void {}
 fn noop4() align(4) void {}
 
 test "function alignment" {
+    // function alignment is a compile error on wasm32/wasm64
+    if (builtin.arch == .wasm32 or builtin.arch == .wasm64) return error.SkipZigTest;
+
     expect(derp() == 1234);
     expect(@TypeOf(noop1) == fn () align(1) void);
     expect(@TypeOf(noop4) == fn () align(4) void);
@@ -117,6 +120,9 @@ fn sliceExpects4(slice: []align(4) u32) void {
 }
 
 test "implicitly decreasing fn alignment" {
+    // function alignment is a compile error on wasm32/wasm64
+    if (builtin.arch == .wasm32 or builtin.arch == .wasm64) return error.SkipZigTest;
+
     testImplicitlyDecreaseFnAlign(alignedSmall, 1234);
     testImplicitlyDecreaseFnAlign(alignedBig, 5678);
 }
@@ -133,8 +139,8 @@ fn alignedBig() align(16) i32 {
 }
 
 test "@alignCast functions" {
-    // TODO investigate why this fails when cross-compiled to wasm.
-    if (builtin.os.tag == .wasi) return error.SkipZigTest;
+    // function alignment is a compile error on wasm32/wasm64
+    if (builtin.arch == .wasm32 or builtin.arch == .wasm64) return error.SkipZigTest;
 
     expect(fnExpectsOnly1(simple4) == 0x19);
 }
@@ -149,6 +155,9 @@ fn simple4() align(4) i32 {
 }
 
 test "generic function with align param" {
+    // function alignment is a compile error on wasm32/wasm64
+    if (builtin.arch == .wasm32 or builtin.arch == .wasm64) return error.SkipZigTest;
+
     expect(whyWouldYouEverDoThis(1) == 0x1);
     expect(whyWouldYouEverDoThis(4) == 0x1);
     expect(whyWouldYouEverDoThis(8) == 0x1);
@@ -327,8 +336,8 @@ test "align(@alignOf(T)) T does not force resolution of T" {
 }
 
 test "align(N) on functions" {
-    // TODO investigate why this fails when cross-compiled to wasm.
-    if (builtin.os.tag == .wasi) return error.SkipZigTest;
+    // function alignment is a compile error on wasm32/wasm64
+    if (builtin.arch == .wasm32 or builtin.arch == .wasm64) return error.SkipZigTest;
 
     expect((@ptrToInt(overaligned_fn) & (0x1000 - 1)) == 0);
 }