From 60fb50ee5a4a06687bf2f7b8774cc46f73a5b07e Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Sun, 16 Aug 2020 02:21:20 +0200 Subject: [PATCH] stage2/wasm: write exports on flush, cleanup Exports now have a dirty flag and are rewritten on flush if this flag has been set. A couple other minor changes have been made based on Andrew's review. --- src-self-hosted/Module.zig | 2 +- src-self-hosted/codegen/wasm.zig | 2 +- src-self-hosted/link.zig | 16 +++++----- src-self-hosted/link/MachO.zig | 2 +- src-self-hosted/link/Wasm.zig | 55 ++++++++++++++++++-------------- 5 files changed, 42 insertions(+), 35 deletions(-) diff --git a/src-self-hosted/Module.zig b/src-self-hosted/Module.zig index 5e22058acf..6e33101e76 100644 --- a/src-self-hosted/Module.zig +++ b/src-self-hosted/Module.zig @@ -974,7 +974,7 @@ pub fn update(self: *Module) !void { } // This is needed before reading the error flags. - try self.bin_file.flush(); + try self.bin_file.flush(self); self.link_error_flags = self.bin_file.errorFlags(); diff --git a/src-self-hosted/codegen/wasm.zig b/src-self-hosted/codegen/wasm.zig index 8e794ff934..57eb002e82 100644 --- a/src-self-hosted/codegen/wasm.zig +++ b/src-self-hosted/codegen/wasm.zig @@ -52,7 +52,7 @@ pub fn genCode(buf: *ArrayList(u8), decl: *Decl) !void { const writer = buf.writer(); // Reserve space to write the size after generating the code - try writer.writeAll(&([1]u8{undefined} ** 5)); + try buf.resize(5); // Write the size of the locals vec // TODO: implement locals diff --git a/src-self-hosted/link.zig b/src-self-hosted/link.zig index 6a51138785..6ed76ce561 100644 --- a/src-self-hosted/link.zig +++ b/src-self-hosted/link.zig @@ -168,16 +168,15 @@ pub const File = struct { } } - /// Commit pending changes and write headers. - pub fn flush(base: *File) !void { + pub fn flush(base: *File, module: *Module) !void { const tracy = trace(@src()); defer tracy.end(); try switch (base.tag) { - .elf => @fieldParentPtr(Elf, "base", base).flush(), - .macho => @fieldParentPtr(MachO, "base", base).flush(), - .c => @fieldParentPtr(C, "base", base).flush(), - .wasm => @fieldParentPtr(Wasm, "base", base).flush(), + .elf => @fieldParentPtr(Elf, "base", base).flush(module), + .macho => @fieldParentPtr(MachO, "base", base).flush(module), + .c => @fieldParentPtr(C, "base", base).flush(module), + .wasm => @fieldParentPtr(Wasm, "base", base).flush(module), }; } @@ -285,7 +284,7 @@ pub const File = struct { }; } - pub fn flush(self: *File.C) !void { + pub fn flush(self: *File.C, module: *Module) !void { const writer = self.base.file.?.writer(); try writer.writeAll(@embedFile("cbe.h")); var includes = false; @@ -1038,7 +1037,8 @@ pub const File = struct { pub const abbrev_pad1 = 5; pub const abbrev_parameter = 6; - pub fn flush(self: *Elf) !void { + /// Commit pending changes and write headers. + pub fn flush(self: *Elf, module: *Module) !void { const target_endian = self.base.options.target.cpu.arch.endian(); const foreign_endian = target_endian != std.Target.current.cpu.arch.endian(); const ptr_width_bytes: u8 = self.ptrWidthBytes(); diff --git a/src-self-hosted/link/MachO.zig b/src-self-hosted/link/MachO.zig index 1b6c395ea0..49e365d203 100644 --- a/src-self-hosted/link/MachO.zig +++ b/src-self-hosted/link/MachO.zig @@ -73,7 +73,7 @@ fn createFile(allocator: *Allocator, file: fs.File, options: link.Options) !Mach } } -pub fn flush(self: *MachO) !void {} +pub fn flush(self: *MachO, module: *Module) !void {} pub fn deinit(self: *MachO) void {} diff --git a/src-self-hosted/link/Wasm.zig b/src-self-hosted/link/Wasm.zig index a62cff15ee..cf1a8c21c2 100644 --- a/src-self-hosted/link/Wasm.zig +++ b/src-self-hosted/link/Wasm.zig @@ -59,34 +59,37 @@ pub fn openPath(allocator: *Allocator, dir: fs.Dir, sub_path: []const u8, option try file.writeAll(&(spec.magic ++ spec.version)); - wasm.base = .{ - .tag = .wasm, - .options = options, - .file = file, - .allocator = allocator, - }; - // TODO: this should vary depending on the section and be less arbitrary const size = 1024; const offset = @sizeOf(@TypeOf(spec.magic ++ spec.version)); - wasm.types = try Types.init(file, offset, size); - wasm.funcs = try Funcs.init(file, offset + size, size, offset + 3 * size, size); - wasm.exports = try Exports.init(file, offset + 2 * size, size); - try file.setEndPos(offset + 4 * size); + wasm.* = .{ + .base = .{ + .tag = .wasm, + .options = options, + .file = file, + .allocator = allocator, + }, - wasm.sections = [_]*Section{ - &wasm.types.typesec.section, - &wasm.funcs.funcsec, - &wasm.exports.exportsec, - &wasm.funcs.codesec.section, + .types = try Types.init(file, offset, size), + .funcs = try Funcs.init(file, offset + size, size, offset + 3 * size, size), + .exports = try Exports.init(file, offset + 2 * size, size), + + // These must be ordered as they will appear in the output file + .sections = [_]*Section{ + &wasm.types.typesec.section, + &wasm.funcs.funcsec, + &wasm.exports.exportsec, + &wasm.funcs.codesec.section, + }, }; + try file.setEndPos(offset + 4 * size); + return &wasm.base; } pub fn deinit(self: *Wasm) void { - if (self.base.file) |f| f.close(); self.types.deinit(); self.funcs.deinit(); } @@ -112,7 +115,8 @@ pub fn updateDecl(self: *Wasm, module: *Module, decl: *Module.Decl) !void { decl.fn_link.wasm = .{ .typeidx = typeidx, .funcidx = funcidx }; - try self.exports.writeAll(module); + // TODO: we should be more smart and set this only when needed + self.exports.dirty = true; } pub fn updateDeclExports( @@ -121,11 +125,7 @@ pub fn updateDeclExports( decl: *const Module.Decl, exports: []const *Module.Export, ) !void { - // TODO: updateDeclExports() may currently be called before updateDecl, - // presumably due to a bug. For now just rely on the following call - // being made in updateDecl(). - - //try self.exports.writeAll(module); + self.exports.dirty = true; } pub fn freeDecl(self: *Wasm, decl: *Module.Decl) void { @@ -138,7 +138,9 @@ pub fn freeDecl(self: *Wasm, decl: *Module.Decl) void { } } -pub fn flush(self: *Wasm) !void {} +pub fn flush(self: *Wasm, module: *Module) !void { + if (self.exports.dirty) try self.exports.writeAll(module); +} /// This struct describes the location of a named section + custom section /// padding in the output file. This is all the data we need to allow for @@ -373,11 +375,14 @@ const Exports = struct { /// Size in bytes of the contents of the section. Does not include /// the "header" containing the section id and this value. contents_size: u32, + /// If this is true, then exports will be rewritten on flush() + dirty: bool, fn init(file: fs.File, offset: u64, initial_size: u64) !Exports { return Exports{ .exportsec = (try VecSection.init(spec.exports_id, file, offset, initial_size)).section, .contents_size = 5, + .dirty = false, }; } @@ -410,6 +415,8 @@ const Exports = struct { for (module.decl_exports.entries.items) |entry| for (entry.value) |e| try writeExport(writer, e); + + self.dirty = false; } /// Return the total number of bytes an export will take.