Merge pull request #13287 from Luukdegram/wasm-features

wasm-linker: feature compatibility validation

10 files changed, 257 insertions, 13 deletions

diff --git a/lib/std/build/CheckObjectStep.zig b/lib/std/build/CheckObjectStep.zig
index 315bbd9b03..63b361473b 100644
--- a/lib/std/build/CheckObjectStep.zig
+++ b/lib/std/build/CheckObjectStep.zig
@@ -649,6 +649,8 @@ const WasmDumper = struct {
                     try parseDumpNames(reader, writer, data);
                 } else if (mem.eql(u8, name, "producers")) {
                     try parseDumpProducers(reader, writer, data);
+                } else if (mem.eql(u8, name, "target_features")) {
+                    try parseDumpFeatures(reader, writer, data);
                 }
                 // TODO: Implement parsing and dumping other custom sections (such as relocations)
             },
@@ -902,4 +904,19 @@ const WasmDumper = struct {
             }
         }
     }
+
+    fn parseDumpFeatures(reader: anytype, writer: anytype, data: []const u8) !void {
+        const feature_count = try std.leb.readULEB128(u32, reader);
+        try writer.print("features {d}\n", .{feature_count});
+
+        var index: u32 = 0;
+        while (index < feature_count) : (index += 1) {
+            const prefix_byte = try std.leb.readULEB128(u8, reader);
+            const name_length = try std.leb.readULEB128(u32, reader);
+            const feature_name = data[reader.context.pos..][0..name_length];
+            reader.context.pos += name_length;
+
+            try writer.print("{c} {s}\n", .{ prefix_byte, feature_name });
+        }
+    }
 };
diff --git a/src/link.zig b/src/link.zig
index 9d4ac0d55b..39f51e90ec 100644
--- a/src/link.zig
+++ b/src/link.zig
@@ -696,6 +696,7 @@ pub const File = struct {
         GlobalTypeMismatch,
         InvalidCharacter,
         InvalidEntryKind,
+        InvalidFeatureSet,
         InvalidFormat,
         InvalidIndex,
         InvalidMagicByte,
diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig
index 4c3de84e01..b9f2d74bd8 100644
--- a/src/link/Wasm.zig
+++ b/src/link/Wasm.zig
@@ -651,6 +651,109 @@ fn resolveSymbolsInArchives(wasm: *Wasm) !void {
     }
 }
 
+fn validateFeatures(
+    wasm: *const Wasm,
+    to_emit: *[@typeInfo(types.Feature.Tag).Enum.fields.len]bool,
+    emit_features_count: *u32,
+) !void {
+    const cpu_features = wasm.base.options.target.cpu.features;
+    const infer = cpu_features.isEmpty(); // when the user did not define any features, we infer them from linked objects.
+    const known_features_count = @typeInfo(types.Feature.Tag).Enum.fields.len;
+
+    var allowed = [_]bool{false} ** known_features_count;
+    var used = [_]u17{0} ** known_features_count;
+    var disallowed = [_]u17{0} ** known_features_count;
+    var required = [_]u17{0} ** known_features_count;
+
+    // when false, we fail linking. We only verify this after a loop to catch all invalid features.
+    var valid_feature_set = true;
+
+    // When the user has given an explicit list of features to enable,
+    // we extract them and insert each into the 'allowed' list.
+    if (!infer) {
+        inline for (@typeInfo(std.Target.wasm.Feature).Enum.fields) |feature_field| {
+            if (cpu_features.isEnabled(feature_field.value)) {
+                allowed[feature_field.value] = true;
+                emit_features_count.* += 1;
+            }
+        }
+    }
+
+    // extract all the used, disallowed and required features from each
+    // linked object file so we can test them.
+    for (wasm.objects.items) |object, object_index| {
+        for (object.features) |feature| {
+            const value = @intCast(u16, object_index) << 1 | @as(u1, 1);
+            switch (feature.prefix) {
+                .used => {
+                    used[@enumToInt(feature.tag)] = value;
+                },
+                .disallowed => {
+                    disallowed[@enumToInt(feature.tag)] = value;
+                },
+                .required => {
+                    required[@enumToInt(feature.tag)] = value;
+                    used[@enumToInt(feature.tag)] = value;
+                },
+            }
+        }
+    }
+
+    // when we infer the features, we allow each feature found in the 'used' set
+    // and insert it into the 'allowed' set. When features are not inferred,
+    // we validate that a used feature is allowed.
+    for (used) |used_set, used_index| {
+        const is_enabled = @truncate(u1, used_set) != 0;
+        if (infer) {
+            allowed[used_index] = is_enabled;
+            emit_features_count.* += @boolToInt(is_enabled);
+        } else if (is_enabled and !allowed[used_index]) {
+            log.err("feature '{s}' not allowed, but used by linked object", .{(@intToEnum(types.Feature.Tag, used_index)).toString()});
+            log.err("  defined in '{s}'", .{wasm.objects.items[used_set >> 1].name});
+            valid_feature_set = false;
+        }
+    }
+
+    if (!valid_feature_set) {
+        return error.InvalidFeatureSet;
+    }
+
+    // For each linked object, validate the required and disallowed features
+    for (wasm.objects.items) |object| {
+        var object_used_features = [_]bool{false} ** known_features_count;
+        for (object.features) |feature| {
+            if (feature.prefix == .disallowed) continue; // already defined in 'disallowed' set.
+            // from here a feature is always used
+            const disallowed_feature = disallowed[@enumToInt(feature.tag)];
+            if (@truncate(u1, disallowed_feature) != 0) {
+                log.err("feature '{s}' is disallowed, but used by linked object", .{feature.tag.toString()});
+                log.err("  disallowed by '{s}'", .{wasm.objects.items[disallowed_feature >> 1].name});
+                log.err("  used in '{s}'", .{object.name});
+                valid_feature_set = false;
+            }
+
+            object_used_features[@enumToInt(feature.tag)] = true;
+        }
+
+        // validate the linked object file has each required feature
+        for (required) |required_feature, feature_index| {
+            const is_required = @truncate(u1, required_feature) != 0;
+            if (is_required and !object_used_features[feature_index]) {
+                log.err("feature '{s}' is required but not used in linked object", .{(@intToEnum(types.Feature.Tag, feature_index)).toString()});
+                log.err("  required by '{s}'", .{wasm.objects.items[required_feature >> 1].name});
+                log.err("  missing in '{s}'", .{object.name});
+                valid_feature_set = false;
+            }
+        }
+    }
+
+    if (!valid_feature_set) {
+        return error.InvalidFeatureSet;
+    }
+
+    to_emit.* = allowed;
+}
+
 fn checkUndefinedSymbols(wasm: *const Wasm) !void {
     if (wasm.base.options.output_mode == .Obj) return;
 
@@ -2158,6 +2261,9 @@ pub fn flushModule(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Nod
         try wasm.resolveSymbolsInObject(@intCast(u16, object_index));
     }
 
+    var emit_features_count: u32 = 0;
+    var enabled_features: [@typeInfo(types.Feature.Tag).Enum.fields.len]bool = undefined;
+    try wasm.validateFeatures(&enabled_features, &emit_features_count);
     try wasm.resolveSymbolsInArchives();
     try wasm.checkUndefinedSymbols();
 
@@ -2603,6 +2709,9 @@ pub fn flushModule(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Nod
         }
 
         try emitProducerSection(&binary_bytes);
+        if (emit_features_count > 0) {
+            try emitFeaturesSection(&binary_bytes, &enabled_features, emit_features_count);
+        }
     }
 
     // Only when writing all sections executed properly we write the magic
@@ -2695,6 +2804,32 @@ fn emitProducerSection(binary_bytes: *std.ArrayList(u8)) !void {
     );
 }
 
+fn emitFeaturesSection(binary_bytes: *std.ArrayList(u8), enabled_features: []const bool, features_count: u32) !void {
+    const header_offset = try reserveCustomSectionHeader(binary_bytes);
+
+    const writer = binary_bytes.writer();
+    const target_features = "target_features";
+    try leb.writeULEB128(writer, @intCast(u32, target_features.len));
+    try writer.writeAll(target_features);
+
+    try leb.writeULEB128(writer, features_count);
+    for (enabled_features) |enabled, feature_index| {
+        if (enabled) {
+            const feature: types.Feature = .{ .prefix = .used, .tag = @intToEnum(types.Feature.Tag, feature_index) };
+            try leb.writeULEB128(writer, @enumToInt(feature.prefix));
+            const string = feature.tag.toString();
+            try leb.writeULEB128(writer, @intCast(u32, string.len));
+            try writer.writeAll(string);
+        }
+    }
+
+    try writeCustomSectionHeader(
+        binary_bytes.items,
+        header_offset,
+        @intCast(u32, binary_bytes.items.len - header_offset - 6),
+    );
+}
+
 fn emitNameSection(wasm: *Wasm, binary_bytes: *std.ArrayList(u8), arena: std.mem.Allocator) !void {
     const Name = struct {
         index: u32,
diff --git a/src/link/Wasm/types.zig b/src/link/Wasm/types.zig
index 2006fe1812..a46fad4e53 100644
--- a/src/link/Wasm/types.zig
+++ b/src/link/Wasm/types.zig
@@ -183,17 +183,44 @@ pub const Feature = struct {
     /// Type of the feature, must be unique in the sequence of features.
     tag: Tag,
 
+    /// Unlike `std.Target.wasm.Feature` this also contains linker-features such as shared-mem
     pub const Tag = enum {
         atomics,
         bulk_memory,
         exception_handling,
+        extended_const,
         multivalue,
         mutable_globals,
         nontrapping_fptoint,
+        reference_types,
+        relaxed_simd,
         sign_ext,
         simd128,
         tail_call,
         shared_mem,
+
+        /// From a given cpu feature, returns its linker feature
+        pub fn fromCpuFeature(feature: std.Target.wasm.Feature) Tag {
+            return @intToEnum(Tag, @enumToInt(feature));
+        }
+
+        pub fn toString(tag: Tag) []const u8 {
+            return switch (tag) {
+                .atomics => "atomics",
+                .bulk_memory => "bulk-memory",
+                .exception_handling => "exception-handling",
+                .extended_const => "extended-const",
+                .multivalue => "multivalue",
+                .mutable_globals => "mutable-globals",
+                .nontrapping_fptoint => "nontrapping-fptoint",
+                .reference_types => "reference-types",
+                .relaxed_simd => "relaxed-simd",
+                .sign_ext => "sign-ext",
+                .simd128 => "simd128",
+                .tail_call => "tail-call",
+                .shared_mem => "shared-mem",
+            };
+        }
     };
 
     pub const Prefix = enum(u8) {
@@ -202,22 +229,10 @@ pub const Feature = struct {
         required = '=',
     };
 
-    pub fn toString(feature: Feature) []const u8 {
-        return switch (feature.tag) {
-            .bulk_memory => "bulk-memory",
-            .exception_handling => "exception-handling",
-            .mutable_globals => "mutable-globals",
-            .nontrapping_fptoint => "nontrapping-fptoint",
-            .sign_ext => "sign-ext",
-            .tail_call => "tail-call",
-            else => @tagName(feature),
-        };
-    }
-
     pub fn format(feature: Feature, comptime fmt: []const u8, opt: std.fmt.FormatOptions, writer: anytype) !void {
         _ = opt;
         _ = fmt;
-        try writer.print("{c} {s}", .{ feature.prefix, feature.toString() });
+        try writer.print("{c} {s}", .{ feature.prefix, feature.tag.toString() });
     }
 };
 
@@ -225,9 +240,12 @@ pub const known_features = std.ComptimeStringMap(Feature.Tag, .{
     .{ "atomics", .atomics },
     .{ "bulk-memory", .bulk_memory },
     .{ "exception-handling", .exception_handling },
+    .{ "extended-const", .extended_const },
     .{ "multivalue", .multivalue },
     .{ "mutable-globals", .mutable_globals },
     .{ "nontrapping-fptoint", .nontrapping_fptoint },
+    .{ "reference-types", .reference_types },
+    .{ "relaxed-simd", .relaxed_simd },
     .{ "sign-ext", .sign_ext },
     .{ "simd128", .simd128 },
     .{ "tail-call", .tail_call },
diff --git a/test/link.zig b/test/link.zig
index df397cd5d2..40635b86a0 100644
--- a/test/link.zig
+++ b/test/link.zig
@@ -33,6 +33,10 @@ fn addWasmCases(cases: *tests.StandaloneContext) void {
         .requires_stage2 = true,
     });
 
+    cases.addBuildFile("test/link/wasm/basic-features/build.zig", .{
+        .requires_stage2 = true,
+    });
+
     cases.addBuildFile("test/link/wasm/bss/build.zig", .{
         .build_modes = false,
         .requires_stage2 = true,
@@ -44,6 +48,10 @@ fn addWasmCases(cases: *tests.StandaloneContext) void {
         .use_emulation = true,
     });
 
+    cases.addBuildFile("test/link/wasm/infer-features/build.zig", .{
+        .requires_stage2 = true,
+    });
+
     cases.addBuildFile("test/link/wasm/producers/build.zig", .{
         .build_modes = true,
         .requires_stage2 = true,
diff --git a/test/link/wasm/basic-features/build.zig b/test/link/wasm/basic-features/build.zig
new file mode 100644
index 0000000000..2c565f9263
--- /dev/null
+++ b/test/link/wasm/basic-features/build.zig
@@ -0,0 +1,23 @@
+const std = @import("std");
+
+pub fn build(b: *std.build.Builder) void {
+    const mode = b.standardReleaseOptions();
+
+    // Library with explicitly set cpu features
+    const lib = b.addSharedLibrary("lib", "main.zig", .unversioned);
+    lib.setTarget(.{ .cpu_arch = .wasm32, .os_tag = .freestanding });
+    lib.target.cpu_model = .{ .explicit = &std.Target.wasm.cpu.mvp };
+    lib.target.cpu_features_add.addFeature(0); // index 0 == atomics (see std.Target.wasm.Features)
+    lib.setBuildMode(mode);
+    lib.use_llvm = false;
+    lib.use_lld = false;
+
+    // Verify the result contains the features explicitly set on the target for the library.
+    const check = lib.checkObject(.wasm);
+    check.checkStart("name target_features");
+    check.checkNext("features 1");
+    check.checkNext("+ atomics");
+
+    const test_step = b.step("test", "Run linker test");
+    test_step.dependOn(&check.step);
+}
diff --git a/test/link/wasm/basic-features/main.zig b/test/link/wasm/basic-features/main.zig
new file mode 100644
index 0000000000..0e416dbf18
--- /dev/null
+++ b/test/link/wasm/basic-features/main.zig
@@ -0,0 +1 @@
+export fn foo() void {}
diff --git a/test/link/wasm/infer-features/build.zig b/test/link/wasm/infer-features/build.zig
new file mode 100644
index 0000000000..b50caf7264
--- /dev/null
+++ b/test/link/wasm/infer-features/build.zig
@@ -0,0 +1,37 @@
+const std = @import("std");
+
+pub fn build(b: *std.build.Builder) void {
+    const mode = b.standardReleaseOptions();
+
+    // Wasm Object file which we will use to infer the features from
+    const c_obj = b.addObject("c_obj", null);
+    c_obj.setTarget(.{ .cpu_arch = .wasm32, .os_tag = .freestanding });
+    c_obj.target.cpu_model = .{ .explicit = &std.Target.wasm.cpu.bleeding_edge };
+    c_obj.addCSourceFile("foo.c", &.{});
+    c_obj.setBuildMode(mode);
+
+    // Wasm library that doesn't have any features specified. This will
+    // infer its featureset from other linked object files.
+    const lib = b.addSharedLibrary("lib", "main.zig", .unversioned);
+    lib.setTarget(.{ .cpu_arch = .wasm32, .os_tag = .freestanding });
+    lib.target.cpu_model = .{ .explicit = &std.Target.wasm.cpu.mvp };
+    lib.setBuildMode(mode);
+    lib.use_llvm = false;
+    lib.use_lld = false;
+    lib.addObject(c_obj);
+
+    // Verify the result contains the features from the C Object file.
+    const check = lib.checkObject(.wasm);
+    check.checkStart("name target_features");
+    check.checkNext("features 7");
+    check.checkNext("+ atomics");
+    check.checkNext("+ bulk-memory");
+    check.checkNext("+ mutable-globals");
+    check.checkNext("+ nontrapping-fptoint");
+    check.checkNext("+ sign-ext");
+    check.checkNext("+ simd128");
+    check.checkNext("+ tail-call");
+
+    const test_step = b.step("test", "Run linker test");
+    test_step.dependOn(&check.step);
+}
diff --git a/test/link/wasm/infer-features/foo.c b/test/link/wasm/infer-features/foo.c
new file mode 100644
index 0000000000..1faba96983
--- /dev/null
+++ b/test/link/wasm/infer-features/foo.c
@@ -0,0 +1,3 @@
+int foo() {
+  return 5;
+}
diff --git a/test/link/wasm/infer-features/main.zig b/test/link/wasm/infer-features/main.zig
new file mode 100644
index 0000000000..576faf61b6
--- /dev/null
+++ b/test/link/wasm/infer-features/main.zig
@@ -0,0 +1 @@
+extern fn foo() c_int;