From 85b669d497641de383070353d50a6e4fd30abd49 Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Wed, 19 Oct 2022 21:48:21 +0200 Subject: [PATCH 1/6] wasm-linker: validate feature compatibility Verifies disallowed and used/required features. After verifying, all errors will be emit to notify the user about incompatible features. When the user did not define any featureset, we infer the features from the linked objects instead. --- src/link/Wasm.zig | 107 ++++++++++++++++++++++++++++++++++++++++ src/link/Wasm/types.zig | 24 ++++----- 2 files changed, 117 insertions(+), 14 deletions(-) diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index 4c3de84e01..ed07756345 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -651,6 +651,112 @@ fn resolveSymbolsInArchives(wasm: *Wasm) !void { } } +fn validateFeatures(wasm: *const Wasm, arena: Allocator) !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. + var allowed = std.AutoHashMap(std.Target.wasm.Feature, void).init(arena); + var used = std.AutoArrayHashMap(std.Target.wasm.Feature, []const u8).init(arena); + var disallowed = std.AutoHashMap(std.Target.wasm.Feature, []const u8).init(arena); + var required = std.AutoHashMap(std.Target.wasm.Feature, []const u8).init(arena); + + // 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) { + try allowed.ensureUnusedCapacity(std.Target.wasm.all_features.len); + // std.builtin.Type.EnumField + inline for (@typeInfo(std.Target.wasm.Feature).Enum.fields) |feature_field| { + if (cpu_features.isEnabled(feature_field.value)) { + allowed.putAssumeCapacityNoClobber(@intToEnum(std.Target.wasm.Feature, feature_field.value), {}); + } + } + } + + // extract all the used, disallowed and required features from each + // linked object file so we can test them. + for (wasm.objects.items) |object| { + for (object.features) |feature| { + switch (feature.prefix) { + .used => { + const gop = try used.getOrPut(feature.tag); + if (!gop.found_existing) { + gop.value_ptr.* = object.name; + } + }, + .disallowed => { + const gop = try disallowed.getOrPut(feature.tag); + if (!gop.found_existing) { + gop.value_ptr.* = object.name; + } + }, + .required => { + const gop = try required.getOrPut(feature.tag); + if (!gop.found_existing) { + gop.value_ptr.* = object.name; + } + const used_gop = try used.getOrPut(feature.tag); + if (!used_gop.found_existing) { + used_gop.value_ptr.* = object.name; + } + }, + } + } + } + + // 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. + if (infer) try allowed.ensureUnusedCapacity(@intCast(u32, used.count())); + for (used.keys()) |used_feature, used_index| { + if (infer) { + allowed.putAssumeCapacityNoClobber(used_feature, {}); + } else if (!allowed.contains(used_feature)) { + log.err("feature '{s}' not allowed, but used by linked object", .{@tagName(used_feature)}); + log.err(" defined in '{s}'", .{used.values()[used_index]}); + 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 = std.AutoHashMap(std.Target.wasm.Feature, void).init(arena); + try object_used_features.ensureTotalCapacity(@intCast(u32, object.features.len)); + for (object.features) |feature| { + if (feature.prefix == .disallowed) continue; // already defined in 'disallowed' set. + // from here a feature is always used + if (disallowed.get(feature.tag)) |disallowed_object_name| { + log.err("feature '{s}' is disallowed, but used by linked object", .{@tagName(feature.tag)}); + log.err(" disallowed by '{s}'", .{disallowed_object_name}); + log.err(" used in '{s}'", .{object.name}); + valid_feature_set = false; + } + + object_used_features.putAssumeCapacity(feature.tag, {}); + } + + // validate the linked object file has each required feature + var required_it = required.iterator(); + while (required_it.next()) |required_feature| { + if (!object_used_features.contains(required_feature.key_ptr.*)) { + log.err("feature '{s}' is required but not used in linked object", .{@tagName(required_feature.key_ptr.*)}); + log.err(" required by '{s}'", .{required_feature.value_ptr.*}); + log.err(" missing in '{s}'", .{object.name}); + valid_feature_set = false; + } + } + } + + if (!valid_feature_set) { + return error.InvalidFeatureSet; + } +} + fn checkUndefinedSymbols(wasm: *const Wasm) !void { if (wasm.base.options.output_mode == .Obj) return; @@ -2158,6 +2264,7 @@ pub fn flushModule(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Nod try wasm.resolveSymbolsInObject(@intCast(u16, object_index)); } + try wasm.validateFeatures(arena); try wasm.resolveSymbolsInArchives(); try wasm.checkUndefinedSymbols(); diff --git a/src/link/Wasm/types.zig b/src/link/Wasm/types.zig index 2006fe1812..5e071c2f20 100644 --- a/src/link/Wasm/types.zig +++ b/src/link/Wasm/types.zig @@ -183,18 +183,7 @@ pub const Feature = struct { /// Type of the feature, must be unique in the sequence of features. tag: Tag, - pub const Tag = enum { - atomics, - bulk_memory, - exception_handling, - multivalue, - mutable_globals, - nontrapping_fptoint, - sign_ext, - simd128, - tail_call, - shared_mem, - }; + pub const Tag = std.Target.wasm.Feature; pub const Prefix = enum(u8) { used = '+', @@ -204,13 +193,18 @@ pub const Feature = struct { pub fn toString(feature: Feature) []const u8 { return switch (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", - else => @tagName(feature), }; } @@ -225,11 +219,13 @@ 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 }, - .{ "shared-mem", .shared_mem }, }); From 777bcbf96871a0250664b9cabdea5dbf51e0e64d Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Thu, 20 Oct 2022 16:58:00 +0200 Subject: [PATCH 2/6] wasm-linker: emit `target_features` section When the result is not being stripped, we emit the `target_features` section based on all the used features. This includes features inferred from linked object files. Considering we know all possible features upfront, we can use an array and therefore do not have to dynamically allocate memory. Using this trick we can also easily order all features based the same ordering as found in `std.Target.wasm` which is the same ordering used by LLVM and the like. --- src/link/Wasm.zig | 47 +++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 45 insertions(+), 2 deletions(-) diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index ed07756345..da9a878720 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -651,7 +651,12 @@ fn resolveSymbolsInArchives(wasm: *Wasm) !void { } } -fn validateFeatures(wasm: *const Wasm, arena: Allocator) !void { +fn validateFeatures( + wasm: *const Wasm, + arena: Allocator, + to_emit: *[@typeInfo(std.Target.wasm.Feature).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. var allowed = std.AutoHashMap(std.Target.wasm.Feature, void).init(arena); @@ -755,6 +760,13 @@ fn validateFeatures(wasm: *const Wasm, arena: Allocator) !void { if (!valid_feature_set) { return error.InvalidFeatureSet; } + + if (allowed.count() > 0) { + emit_features_count.* = allowed.count(); + for (to_emit) |*feature_enabled, feature_index| { + feature_enabled.* = allowed.contains(@intToEnum(std.Target.wasm.Feature, feature_index)); + } + } } fn checkUndefinedSymbols(wasm: *const Wasm) !void { @@ -2264,7 +2276,9 @@ pub fn flushModule(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Nod try wasm.resolveSymbolsInObject(@intCast(u16, object_index)); } - try wasm.validateFeatures(arena); + var emit_features_count: u32 = 0; + var enabled_features: [@typeInfo(std.Target.wasm.Feature).Enum.fields.len]bool = undefined; + try wasm.validateFeatures(arena, &enabled_features, &emit_features_count); try wasm.resolveSymbolsInArchives(); try wasm.checkUndefinedSymbols(); @@ -2710,6 +2724,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 @@ -2802,6 +2819,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.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, From b14f605dd7b808d1e6d3ee1f8d545135ebe296ee Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Sun, 23 Oct 2022 19:20:18 +0200 Subject: [PATCH 3/6] CheckObjectStep: parse and dump `target_features` When an object file or binary contains the target_features section we can now parse and then dump its contents in string format so we can use them in our linker tests to verify the features section. --- lib/std/build/CheckObjectStep.zig | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) 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 }); + } + } }; From 2f41109cc445c450029ea81c0da6873f32fa0981 Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Sun, 23 Oct 2022 20:00:27 +0200 Subject: [PATCH 4/6] test/link: add Wasm linker tests for features Adds a test for inferring features based on a different object file. Also provides a test case where cpu features are explicitly set on a library where the end result will output said target features. --- test/link.zig | 8 ++++++ test/link/wasm/basic-features/build.zig | 23 +++++++++++++++ test/link/wasm/basic-features/main.zig | 1 + test/link/wasm/infer-features/build.zig | 37 +++++++++++++++++++++++++ test/link/wasm/infer-features/foo.c | 3 ++ test/link/wasm/infer-features/main.zig | 1 + 6 files changed, 73 insertions(+) create mode 100644 test/link/wasm/basic-features/build.zig create mode 100644 test/link/wasm/basic-features/main.zig create mode 100644 test/link/wasm/infer-features/build.zig create mode 100644 test/link/wasm/infer-features/foo.c create mode 100644 test/link/wasm/infer-features/main.zig 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; From 3d1d19f3877190db42544acf9e0ed26784ba82ba Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Sun, 23 Oct 2022 20:02:25 +0200 Subject: [PATCH 5/6] wasm-linker: seperate linker -and cpu features The list of features a Wasm object/binary file can emit can differ from the list of cpu features. The reason for this is because the "target_features" section also contains linker features. An example of this is the "shared-mem" feature, which is a feature for the linker and not that of the cpu target as defined by LLVM. --- src/link/Wasm.zig | 18 +++++++++--------- src/link/Wasm/types.zig | 24 +++++++++++++++++++++++- 2 files changed, 32 insertions(+), 10 deletions(-) diff --git a/src/link/Wasm.zig b/src/link/Wasm.zig index da9a878720..4b6895c4e5 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -654,15 +654,15 @@ fn resolveSymbolsInArchives(wasm: *Wasm) !void { fn validateFeatures( wasm: *const Wasm, arena: Allocator, - to_emit: *[@typeInfo(std.Target.wasm.Feature).Enum.fields.len]bool, + 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. - var allowed = std.AutoHashMap(std.Target.wasm.Feature, void).init(arena); - var used = std.AutoArrayHashMap(std.Target.wasm.Feature, []const u8).init(arena); - var disallowed = std.AutoHashMap(std.Target.wasm.Feature, []const u8).init(arena); - var required = std.AutoHashMap(std.Target.wasm.Feature, []const u8).init(arena); + var allowed = std.AutoHashMap(types.Feature.Tag, void).init(arena); + var used = std.AutoArrayHashMap(types.Feature.Tag, []const u8).init(arena); + var disallowed = std.AutoHashMap(types.Feature.Tag, []const u8).init(arena); + var required = std.AutoHashMap(types.Feature.Tag, []const u8).init(arena); // when false, we fail linking. We only verify this after a loop to catch all invalid features. var valid_feature_set = true; @@ -674,7 +674,7 @@ fn validateFeatures( // std.builtin.Type.EnumField inline for (@typeInfo(std.Target.wasm.Feature).Enum.fields) |feature_field| { if (cpu_features.isEnabled(feature_field.value)) { - allowed.putAssumeCapacityNoClobber(@intToEnum(std.Target.wasm.Feature, feature_field.value), {}); + allowed.putAssumeCapacityNoClobber(@intToEnum(types.Feature.Tag, feature_field.value), {}); } } } @@ -730,7 +730,7 @@ fn validateFeatures( // For each linked object, validate the required and disallowed features for (wasm.objects.items) |object| { - var object_used_features = std.AutoHashMap(std.Target.wasm.Feature, void).init(arena); + var object_used_features = std.AutoHashMap(types.Feature.Tag, void).init(arena); try object_used_features.ensureTotalCapacity(@intCast(u32, object.features.len)); for (object.features) |feature| { if (feature.prefix == .disallowed) continue; // already defined in 'disallowed' set. @@ -764,7 +764,7 @@ fn validateFeatures( if (allowed.count() > 0) { emit_features_count.* = allowed.count(); for (to_emit) |*feature_enabled, feature_index| { - feature_enabled.* = allowed.contains(@intToEnum(std.Target.wasm.Feature, feature_index)); + feature_enabled.* = allowed.contains(@intToEnum(types.Feature.Tag, feature_index)); } } } @@ -2277,7 +2277,7 @@ pub fn flushModule(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Nod } var emit_features_count: u32 = 0; - var enabled_features: [@typeInfo(std.Target.wasm.Feature).Enum.fields.len]bool = undefined; + var enabled_features: [@typeInfo(types.Feature.Tag).Enum.fields.len]bool = undefined; try wasm.validateFeatures(arena, &enabled_features, &emit_features_count); try wasm.resolveSymbolsInArchives(); try wasm.checkUndefinedSymbols(); diff --git a/src/link/Wasm/types.zig b/src/link/Wasm/types.zig index 5e071c2f20..1b6df86d37 100644 --- a/src/link/Wasm/types.zig +++ b/src/link/Wasm/types.zig @@ -183,7 +183,27 @@ pub const Feature = struct { /// Type of the feature, must be unique in the sequence of features. tag: Tag, - pub const Tag = std.Target.wasm.Feature; + /// 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 const Prefix = enum(u8) { used = '+', @@ -205,6 +225,7 @@ pub const Feature = struct { .sign_ext => "sign-ext", .simd128 => "simd128", .tail_call => "tail-call", + .shared_mem => "shared-mem", }; } @@ -228,4 +249,5 @@ pub const known_features = std.ComptimeStringMap(Feature.Tag, .{ .{ "sign-ext", .sign_ext }, .{ "simd128", .simd128 }, .{ "tail-call", .tail_call }, + .{ "shared-mem", .shared_mem }, }); From c0710b0c42716bb7173b9fcc2785f9bf5175ae0f Mon Sep 17 00:00:00 2001 From: Luuk de Gram Date: Tue, 25 Oct 2022 21:16:51 +0200 Subject: [PATCH 6/6] use fixed-size arrays for feature lists Considering all possible features are known by the linker during compile-time, we can create arrays on the stack instead of dynamically allocating hash maps. We use a simple bitset to determine whether a feature is enabled or not, and from which object file it originates. This allows us to make feature validation slightly faster and use less runtime memory. In the future this could be enhanced further by having a single array instead with a more sophisticated bitset. --- src/link.zig | 1 + src/link/Wasm.zig | 85 +++++++++++++++++------------------------ src/link/Wasm/types.zig | 38 +++++++++--------- 3 files changed, 55 insertions(+), 69 deletions(-) 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 4b6895c4e5..b9f2d74bd8 100644 --- a/src/link/Wasm.zig +++ b/src/link/Wasm.zig @@ -653,16 +653,17 @@ fn resolveSymbolsInArchives(wasm: *Wasm) !void { fn validateFeatures( wasm: *const Wasm, - arena: Allocator, 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. - var allowed = std.AutoHashMap(types.Feature.Tag, void).init(arena); - var used = std.AutoArrayHashMap(types.Feature.Tag, []const u8).init(arena); - var disallowed = std.AutoHashMap(types.Feature.Tag, []const u8).init(arena); - var required = std.AutoHashMap(types.Feature.Tag, []const u8).init(arena); + 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; @@ -670,41 +671,29 @@ fn validateFeatures( // When the user has given an explicit list of features to enable, // we extract them and insert each into the 'allowed' list. if (!infer) { - try allowed.ensureUnusedCapacity(std.Target.wasm.all_features.len); - // std.builtin.Type.EnumField inline for (@typeInfo(std.Target.wasm.Feature).Enum.fields) |feature_field| { if (cpu_features.isEnabled(feature_field.value)) { - allowed.putAssumeCapacityNoClobber(@intToEnum(types.Feature.Tag, 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| { + 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 => { - const gop = try used.getOrPut(feature.tag); - if (!gop.found_existing) { - gop.value_ptr.* = object.name; - } + used[@enumToInt(feature.tag)] = value; }, .disallowed => { - const gop = try disallowed.getOrPut(feature.tag); - if (!gop.found_existing) { - gop.value_ptr.* = object.name; - } + disallowed[@enumToInt(feature.tag)] = value; }, .required => { - const gop = try required.getOrPut(feature.tag); - if (!gop.found_existing) { - gop.value_ptr.* = object.name; - } - const used_gop = try used.getOrPut(feature.tag); - if (!used_gop.found_existing) { - used_gop.value_ptr.* = object.name; - } + required[@enumToInt(feature.tag)] = value; + used[@enumToInt(feature.tag)] = value; }, } } @@ -713,13 +702,14 @@ fn validateFeatures( // 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. - if (infer) try allowed.ensureUnusedCapacity(@intCast(u32, used.count())); - for (used.keys()) |used_feature, used_index| { + for (used) |used_set, used_index| { + const is_enabled = @truncate(u1, used_set) != 0; if (infer) { - allowed.putAssumeCapacityNoClobber(used_feature, {}); - } else if (!allowed.contains(used_feature)) { - log.err("feature '{s}' not allowed, but used by linked object", .{@tagName(used_feature)}); - log.err(" defined in '{s}'", .{used.values()[used_index]}); + 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; } } @@ -730,27 +720,27 @@ fn validateFeatures( // For each linked object, validate the required and disallowed features for (wasm.objects.items) |object| { - var object_used_features = std.AutoHashMap(types.Feature.Tag, void).init(arena); - try object_used_features.ensureTotalCapacity(@intCast(u32, object.features.len)); + 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 - if (disallowed.get(feature.tag)) |disallowed_object_name| { - log.err("feature '{s}' is disallowed, but used by linked object", .{@tagName(feature.tag)}); - log.err(" disallowed by '{s}'", .{disallowed_object_name}); + 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.putAssumeCapacity(feature.tag, {}); + object_used_features[@enumToInt(feature.tag)] = true; } // validate the linked object file has each required feature - var required_it = required.iterator(); - while (required_it.next()) |required_feature| { - if (!object_used_features.contains(required_feature.key_ptr.*)) { - log.err("feature '{s}' is required but not used in linked object", .{@tagName(required_feature.key_ptr.*)}); - log.err(" required by '{s}'", .{required_feature.value_ptr.*}); + 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; } @@ -761,12 +751,7 @@ fn validateFeatures( return error.InvalidFeatureSet; } - if (allowed.count() > 0) { - emit_features_count.* = allowed.count(); - for (to_emit) |*feature_enabled, feature_index| { - feature_enabled.* = allowed.contains(@intToEnum(types.Feature.Tag, feature_index)); - } - } + to_emit.* = allowed; } fn checkUndefinedSymbols(wasm: *const Wasm) !void { @@ -2278,7 +2263,7 @@ pub fn flushModule(wasm: *Wasm, comp: *Compilation, prog_node: *std.Progress.Nod var emit_features_count: u32 = 0; var enabled_features: [@typeInfo(types.Feature.Tag).Enum.fields.len]bool = undefined; - try wasm.validateFeatures(arena, &enabled_features, &emit_features_count); + try wasm.validateFeatures(&enabled_features, &emit_features_count); try wasm.resolveSymbolsInArchives(); try wasm.checkUndefinedSymbols(); @@ -2832,7 +2817,7 @@ fn emitFeaturesSection(binary_bytes: *std.ArrayList(u8), enabled_features: []con 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.toString(); + const string = feature.tag.toString(); try leb.writeULEB128(writer, @intCast(u32, string.len)); try writer.writeAll(string); } diff --git a/src/link/Wasm/types.zig b/src/link/Wasm/types.zig index 1b6df86d37..a46fad4e53 100644 --- a/src/link/Wasm/types.zig +++ b/src/link/Wasm/types.zig @@ -203,6 +203,24 @@ pub const Feature = struct { 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) { @@ -211,28 +229,10 @@ pub const Feature = struct { required = '=', }; - pub fn toString(feature: Feature) []const u8 { - return switch (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", - .shared_mem => "shared-mem", - }; - } - 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() }); } };