motiejus/zig

commit 015c899297c9fdc8ef9d0d22af29a7a0d0bdbc5c (tree)
parent 4b8077ea8e888382fc73dd8a19c6e9160f2cef46
Author: Jakub Konka <kubkon@jakubkonka.com>
Date:   Tue, 26 May 2020 17:16:25 +0200

Make align expr on fns a compile error in Wasm

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)

Diffstat:
M
src/analyze.cpp
 | 
15
+++++++++++++++
M
test/stage1/behavior/align.zig
 | 
17
+++++++++++++----

2 files changed, 28 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/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);
 }