From 74abc5ad2f4bf4f737b6c910b3048f7937345aad Mon Sep 17 00:00:00 2001 From: Joachim Henke <37883863+jo-he@users.noreply.github.com> Date: Fri, 26 Jul 2019 09:59:18 +0200 Subject: [PATCH 01/17] avoid a register copy when fetching the stack pointer in _start --- std/special/start.zig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/std/special/start.zig b/std/special/start.zig index f8a018e0a..3427ff422 100644 --- a/std/special/start.zig +++ b/std/special/start.zig @@ -35,13 +35,13 @@ nakedcc fn _start() noreturn { switch (builtin.arch) { .x86_64 => { - argc_ptr = asm ("lea (%%rsp), %[argc]" - : [argc] "=r" (-> [*]usize) + argc_ptr = asm ("" + : [argc] "={rsp}" (-> [*]usize) ); }, .i386 => { - argc_ptr = asm ("lea (%%esp), %[argc]" - : [argc] "=r" (-> [*]usize) + argc_ptr = asm ("" + : [argc] "={esp}" (-> [*]usize) ); }, .aarch64, .aarch64_be => { From 5593c63e12a8d85052f548218b2957dea314be18 Mon Sep 17 00:00:00 2001 From: emekoi Date: Fri, 26 Jul 2019 16:26:01 -0500 Subject: [PATCH 02/17] improved CMake file for MinGW --- CMakeLists.txt | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d8cf0c507..debfb9392 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -209,7 +209,7 @@ else() else() set(ZIG_LLD_COMPILE_FLAGS "-std=c++11 -fvisibility-inlines-hidden -fno-exceptions -fno-rtti -Wno-comment") if(MINGW) - set(ZIG_LLD_COMPILE_FLAGS "${ZIG_LLD_COMPILE_FLAGS} -D__STDC_FORMAT_MACROS -D__USE_MINGW_ANSI_STDIO -Wno-pedantic-ms-format") + set(ZIG_LLD_COMPILE_FLAGS "${ZIG_LLD_COMPILE_FLAGS} -D__STDC_FORMAT_MACROS -D__USE_MINGW_ANSI_STDIO") endif() endif() set_target_properties(embedded_lld_lib PROPERTIES @@ -511,19 +511,23 @@ set(OPTIMIZED_C_FLAGS "-std=c99 -O3") set(EXE_LDFLAGS " ") if(MSVC) - set(EXE_LDFLAGS "/STACK:16777216") + set(EXE_LDFLAGS "${EXE_LDFLAGS} /STACK:16777216") elseif(MINGW) set(EXE_LDFLAGS "${EXE_LDFLAGS} -Wl,--stack,16777216") endif() if(ZIG_STATIC) if(APPLE) - set(EXE_LDFLAGS "-static-libgcc -static-libstdc++") + set(EXE_LDFLAGS "${EXE_LDFLAGS} -static-libgcc -static-libstdc++") elseif(MINGW) - set(EXE_LDFLAGS "-static-libgcc -static-libstdc++ -Wl,-Bstatic,--whole-archive -lwinpthread -lz3 -lz -lgomp -Wl,--no-whole-archive") + set(EXE_LDFLAGS "${EXE_LDFLAGS} -static-libgcc -static-libstdc++ -Wl,-Bstatic, -lwinpthread -lz3 -lz -lgomp") else() - set(EXE_LDFLAGS "-static") + set(EXE_LDFLAGS "${EXE_LDFLAGS} -static") endif() +else() + if(MINGW) + set(EXE_LDFLAGS "${EXE_LDFLAGS} -lz3") + endif() endif() if(ZIG_TEST_COVERAGE) @@ -559,11 +563,6 @@ if(NOT MSVC) target_link_libraries(compiler LINK_PUBLIC ${LIBXML2}) endif() -if(MINGW) - find_library(Z3_LIBRARIES NAMES z3 z3.dll) - target_link_libraries(compiler LINK_PUBLIC ${Z3_LIBRARIES}) -endif() - if(ZIG_DIA_GUIDS_LIB) target_link_libraries(compiler LINK_PUBLIC ${ZIG_DIA_GUIDS_LIB}) endif() From 10b10177025790ffa84304f282e8323ea2b10c37 Mon Sep 17 00:00:00 2001 From: emekoi Date: Sat, 27 Jul 2019 17:50:44 -0500 Subject: [PATCH 03/17] fixed backtraces when linking libc on mingw --- std/coff.zig | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/std/coff.zig b/std/coff.zig index 9fdc36887..7c53f48f6 100644 --- a/std/coff.zig +++ b/std/coff.zig @@ -120,7 +120,15 @@ pub const Coff = struct { pub fn getPdbPath(self: *Coff, buffer: []u8) !usize { try self.loadSections(); - const header = (self.getSection(".rdata") orelse return error.MissingCoffSection).header; + const header = blk: { + if (self.getSection(".buildid")) |section| { + break :blk section.header; + } else if (self.getSection(".rdata")) |section| { + break :blk section.header; + } else { + return error.MissingCoffSection; + } + }; // The linker puts a chunk that contains the .pdb path right after the // debug_directory. From 357fb4f1436fca4b68ca5adf14ba85f3b21b5dcf Mon Sep 17 00:00:00 2001 From: emekoi Date: Sat, 27 Jul 2019 17:54:20 -0500 Subject: [PATCH 04/17] avoid passing -static to msvc when static linking --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index debfb9392..998da172f 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -521,7 +521,7 @@ if(ZIG_STATIC) set(EXE_LDFLAGS "${EXE_LDFLAGS} -static-libgcc -static-libstdc++") elseif(MINGW) set(EXE_LDFLAGS "${EXE_LDFLAGS} -static-libgcc -static-libstdc++ -Wl,-Bstatic, -lwinpthread -lz3 -lz -lgomp") - else() + elseif(NOT MSVC) set(EXE_LDFLAGS "${EXE_LDFLAGS} -static") endif() else() From a0ebfa64d9b8e98b31d37c80d118aab84f1a493e Mon Sep 17 00:00:00 2001 From: Evan Krause Date: Sun, 28 Jul 2019 18:37:35 -0700 Subject: [PATCH 05/17] support zero-sized structs in zig.fmt.format --- std/fmt.zig | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/std/fmt.zig b/std/fmt.zig index 2e9527f4c..479ea76e8 100644 --- a/std/fmt.zig +++ b/std/fmt.zig @@ -374,9 +374,10 @@ pub fn formatType( return output(context, "{ ... }"); } comptime var field_i = 0; + try output(context, "{"); inline while (field_i < @memberCount(T)) : (field_i += 1) { if (field_i == 0) { - try output(context, "{ ."); + try output(context, " ."); } else { try output(context, ", ."); } @@ -1439,6 +1440,21 @@ test "struct.self-referential" { try testFmt("S{ .a = S{ .a = S{ .a = S{ ... } } } }", "{}", inst); } +test "struct.zero-size" { + const A = struct { + fn foo() void {} + }; + const B = struct { + a: A, + c: i32, + }; + + const a = A{}; + const b = B{ .a = a, .c = 0 }; + + try testFmt("B{ .a = A{ }, .c = 0 }", "{}", b); +} + test "bytes.hex" { const some_bytes = "\xCA\xFE\xBA\xBE"; try testFmt("lowercase: cafebabe\n", "lowercase: {x}\n", some_bytes); From 7e436006be93d43a9e93206e29f9a08cbd8e109e Mon Sep 17 00:00:00 2001 From: Michael Dusan Date: Sun, 28 Jul 2019 08:38:11 -0400 Subject: [PATCH 06/17] fix std.rb.Node.getParent to return optional closes #2962 --- std/rb.zig | 3 ++- std/std.zig | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/std/rb.zig b/std/rb.zig index 0b28550a1..b5935a2ea 100644 --- a/std/rb.zig +++ b/std/rb.zig @@ -93,7 +93,8 @@ pub const Node = struct { comptime { assert(@alignOf(*Node) >= 2); } - return @intToPtr(*Node, node.parent_and_color & ~mask); + const maybe_ptr = node.parent_and_color & ~mask; + return if (maybe_ptr == 0) null else @intToPtr(*Node, maybe_ptr); } fn setColor(node: *Node, color: Color) void { diff --git a/std/std.zig b/std/std.zig index 350f0fb43..e48586d87 100644 --- a/std/std.zig +++ b/std/std.zig @@ -105,6 +105,7 @@ test "std" { _ = @import("packed_int_array.zig"); _ = @import("priority_queue.zig"); _ = @import("rand.zig"); + _ = @import("rb.zig"); _ = @import("sort.zig"); _ = @import("testing.zig"); _ = @import("thread.zig"); From 8736a5be2ac280db8fd34c86efd1b764a255f23c Mon Sep 17 00:00:00 2001 From: Nick Erdmann Date: Fri, 26 Jul 2019 15:01:49 +0200 Subject: [PATCH 07/17] std/build.zig: fix stack checking option --- std/build.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/std/build.zig b/std/build.zig index 3a0c34c8d..6be88ae7e 100644 --- a/std/build.zig +++ b/std/build.zig @@ -1802,7 +1802,7 @@ pub const LibExeObjStep = struct { try zig_args.append("--bundle-compiler-rt"); } if (self.disable_stack_probing) { - try zig_args.append("--disable-stack-probing"); + try zig_args.append("-fno-stack-check"); } switch (self.target) { From bc982e65cf77aa6551029620ceaec8f3ae49202f Mon Sep 17 00:00:00 2001 From: Michael Dusan Date: Thu, 25 Jul 2019 19:59:34 -0400 Subject: [PATCH 08/17] fix std.fmt to handle std.SegmentedList - add guards for use of prealloc_exp in SegmentedList - define prealloc_exp even when invalid because std.fmt comptime triggers lazy-init - fix std.fmt to print arrays of length 0 as style "[0]" because "@address" is n/a without address --- std/fmt.zig | 3 +++ std/segmented_list.zig | 31 +++++++++++++++++++------------ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/std/fmt.zig b/std/fmt.zig index 479ea76e8..7c08cd14e 100644 --- a/std/fmt.zig +++ b/std/fmt.zig @@ -426,6 +426,9 @@ pub fn formatType( if (info.child == u8) { return formatText(value, fmt, options, context, Errors, output); } + if (value.len == 0) { + return format(context, Errors, output, "[0]{}", @typeName(T.Child)); + } return format(context, Errors, output, "{}@{x}", @typeName(T.Child), @ptrToInt(&value)); }, .Fn => { diff --git a/std/segmented_list.zig b/std/segmented_list.zig index e0b84d5c0..3bbbde782 100644 --- a/std/segmented_list.zig +++ b/std/segmented_list.zig @@ -77,16 +77,20 @@ const Allocator = std.mem.Allocator; pub fn SegmentedList(comptime T: type, comptime prealloc_item_count: usize) type { return struct { const Self = @This(); - const prealloc_exp = blk: { - // we don't use the prealloc_exp constant when prealloc_item_count is 0. - assert(prealloc_item_count != 0); - assert(std.math.isPowerOfTwo(prealloc_item_count)); - - const value = std.math.log2_int(usize, prealloc_item_count); - break :blk @typeOf(1)(value); - }; const ShelfIndex = std.math.Log2Int(usize); + const prealloc_exp: ShelfIndex = blk: { + // we don't use the prealloc_exp constant when prealloc_item_count is 0 + // but lazy-init may still be triggered by other code so supply a value + if (prealloc_item_count == 0) { + break :blk 0; + } else { + assert(std.math.isPowerOfTwo(prealloc_item_count)); + const value = std.math.log2_int(usize, prealloc_item_count); + break :blk value; + } + }; + prealloc_segment: [prealloc_item_count]T, dynamic_segments: [][*]T, allocator: *Allocator, @@ -157,11 +161,12 @@ pub fn SegmentedList(comptime T: type, comptime prealloc_item_count: usize) type /// Grows or shrinks capacity to match usage. pub fn setCapacity(self: *Self, new_capacity: usize) !void { - if (new_capacity <= usize(1) << (prealloc_exp + self.dynamic_segments.len)) { - return self.shrinkCapacity(new_capacity); - } else { - return self.growCapacity(new_capacity); + if (prealloc_item_count != 0) { + if (new_capacity <= usize(1) << (prealloc_exp + @intCast(ShelfIndex, self.dynamic_segments.len))) { + return self.shrinkCapacity(new_capacity); + } } + return self.growCapacity(new_capacity); } /// Only grows capacity, or retains current capacity @@ -399,4 +404,6 @@ fn testSegmentedList(comptime prealloc: usize, allocator: *Allocator) !void { testing.expect(item == i); list.shrinkCapacity(list.len); } + + try list.setCapacity(0); } From d08425a0a5b15fa903d379ea5547fbb5dfecda62 Mon Sep 17 00:00:00 2001 From: Sahnvour Date: Sun, 28 Jul 2019 17:31:40 +0200 Subject: [PATCH 09/17] os: missing accessW since recent refactoring --- std/os.zig | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/std/os.zig b/std/os.zig index 190f02101..9ff2e8f87 100644 --- a/std/os.zig +++ b/std/os.zig @@ -2053,6 +2053,22 @@ pub fn accessC(path: [*]const u8, mode: u32) AccessError!void { } } +/// Call from Windows-specific code if you already have a UTF-16LE encoded, null terminated string. +/// Otherwise use `access` or `accessC`. +/// TODO currently this ignores `mode`. +pub fn accessW(path: [*]const u16, mode: u32) windows.GetFileAttributesError!void { + const ret = try windows.GetFileAttributesW(path); + if (ret != windows.INVALID_FILE_ATTRIBUTES) { + return; + } + switch (windows.kernel32.GetLastError()) { + windows.ERROR.FILE_NOT_FOUND => return error.FileNotFound, + windows.ERROR.PATH_NOT_FOUND => return error.FileNotFound, + windows.ERROR.ACCESS_DENIED => return error.PermissionDenied, + else => |err| return windows.unexpectedError(err), + } +} + pub const PipeError = error{ SystemFdQuotaExceeded, ProcessFdQuotaExceeded, From 05032c869378e7c7e3da3a2770161266058aa320 Mon Sep 17 00:00:00 2001 From: Sahnvour Date: Sun, 28 Jul 2019 19:03:36 +0200 Subject: [PATCH 10/17] coff & pdb: improved correctness of our implementation, it is now able to handle stage1's pdb and print its stack traces --- std/coff.zig | 42 +++++++++++++++++--- std/debug.zig | 13 +++++-- std/pdb.zig | 103 ++++++++++++++++++++++++++++++++------------------ 3 files changed, 113 insertions(+), 45 deletions(-) diff --git a/std/coff.zig b/std/coff.zig index 7c53f48f6..3890151d0 100644 --- a/std/coff.zig +++ b/std/coff.zig @@ -19,6 +19,7 @@ const IMAGE_NT_OPTIONAL_HDR32_MAGIC = 0x10b; const IMAGE_NT_OPTIONAL_HDR64_MAGIC = 0x20b; const IMAGE_NUMBEROF_DIRECTORY_ENTRIES = 16; +const IMAGE_DEBUG_TYPE_CODEVIEW = 2; const DEBUG_DIRECTORY = 6; pub const CoffError = error{ @@ -28,6 +29,7 @@ pub const CoffError = error{ MissingCoffSection, }; +// Official documentation of the format: https://docs.microsoft.com/en-us/windows/win32/debug/pe-format pub const Coff = struct { in_file: File, allocator: *mem.Allocator, @@ -120,6 +122,7 @@ pub const Coff = struct { pub fn getPdbPath(self: *Coff, buffer: []u8) !usize { try self.loadSections(); + const header = blk: { if (self.getSection(".buildid")) |section| { break :blk section.header; @@ -130,14 +133,32 @@ pub const Coff = struct { } }; - // The linker puts a chunk that contains the .pdb path right after the - // debug_directory. const debug_dir = &self.pe_header.data_directory[DEBUG_DIRECTORY]; const file_offset = debug_dir.virtual_address - header.virtual_address + header.pointer_to_raw_data; - try self.in_file.seekTo(file_offset + debug_dir.size); var file_stream = self.in_file.inStream(); const in = &file_stream.stream; + try self.in_file.seekTo(file_offset); + + // Find the correct DebugDirectoryEntry, and where its data is stored. + // It can be in any section. + const debug_dir_entry_count = debug_dir.size / @sizeOf(DebugDirectoryEntry); + var i: u32 = 0; + blk: while (i < debug_dir_entry_count) : (i += 1) { + const debug_dir_entry = try in.readStruct(DebugDirectoryEntry); + if (debug_dir_entry.type == IMAGE_DEBUG_TYPE_CODEVIEW) { + for (self.sections.toSlice()) |*section| { + const section_start = section.header.virtual_address; + const section_size = section.header.misc.virtual_size; + const rva = debug_dir_entry.address_of_raw_data; + const offset = rva - section_start; + if (section_start <= rva and offset < section_size and debug_dir_entry.size_of_data <= section_size - offset) { + try self.in_file.seekTo(section.header.pointer_to_raw_data + offset); + break :blk; + } + } + } + } var cv_signature: [4]u8 = undefined; // CodeView signature try in.readNoEof(cv_signature[0..]); @@ -149,7 +170,7 @@ pub const Coff = struct { // Finally read the null-terminated string. var byte = try in.readByte(); - var i: usize = 0; + i = 0; while (byte != 0 and i < buffer.len) : (i += 1) { buffer[i] = byte; byte = try in.readByte(); @@ -178,7 +199,7 @@ pub const Coff = struct { try self.sections.append(Section{ .header = SectionHeader{ .name = name, - .misc = SectionHeader.Misc{ .physical_address = try in.readIntLittle(u32) }, + .misc = SectionHeader.Misc{ .virtual_size = try in.readIntLittle(u32) }, .virtual_address = try in.readIntLittle(u32), .size_of_raw_data = try in.readIntLittle(u32), .pointer_to_raw_data = try in.readIntLittle(u32), @@ -222,6 +243,17 @@ const OptionalHeader = struct { data_directory: [IMAGE_NUMBEROF_DIRECTORY_ENTRIES]DataDirectory, }; +const DebugDirectoryEntry = packed struct { + characteristiccs: u32, + time_date_stamp: u32, + major_version: u16, + minor_version: u16, + @"type": u32, + size_of_data: u32, + address_of_raw_data: u32, + pointer_to_raw_data: u32, +}; + pub const Section = struct { header: SectionHeader, }; diff --git a/std/debug.zig b/std/debug.zig index 7d1fd6ce4..32f96d3e1 100644 --- a/std/debug.zig +++ b/std/debug.zig @@ -375,7 +375,7 @@ fn printSourceAtAddressWindows(di: *DebugInfo, out_stream: var, relocated_addres const obj_basename = fs.path.basename(mod.obj_file_name); var symbol_i: usize = 0; - const symbol_name = while (symbol_i != mod.symbols.len) { + const symbol_name = if (!mod.populated) "???" else while (symbol_i != mod.symbols.len) { const prefix = @ptrCast(*pdb.RecordPrefix, &mod.symbols[symbol_i]); if (prefix.RecordLen < 2) return error.InvalidDebugInfo; @@ -858,8 +858,10 @@ fn openSelfDebugInfoWindows(allocator: *mem.Allocator) !DebugInfo { const age = try pdb_stream.stream.readIntLittle(u32); var guid: [16]u8 = undefined; try pdb_stream.stream.readNoEof(guid[0..]); + if (version != 20000404) // VC70, only value observed by LLVM team + return error.UnknownPDBVersion; if (!mem.eql(u8, di.coff.guid, guid) or di.coff.age != age) - return error.InvalidDebugInfo; + return error.PDBMismatch; // We validated the executable and pdb match. const string_table_index = str_tab_index: { @@ -903,13 +905,18 @@ fn openSelfDebugInfoWindows(allocator: *mem.Allocator) !DebugInfo { return error.MissingDebugInfo; }; - di.pdb.string_table = di.pdb.getStreamById(string_table_index) orelse return error.InvalidDebugInfo; + di.pdb.string_table = di.pdb.getStreamById(string_table_index) orelse return error.MissingDebugInfo; di.pdb.dbi = di.pdb.getStream(pdb.StreamType.Dbi) orelse return error.MissingDebugInfo; const dbi = di.pdb.dbi; // Dbi Header const dbi_stream_header = try dbi.stream.readStruct(pdb.DbiStreamHeader); + if (dbi_stream_header.VersionHeader != 19990903) // V70, only value observed by LLVM team + return error.UnknownPDBVersion; + if (dbi_stream_header.Age != age) + return error.UnmatchingPDB; + const mod_info_size = dbi_stream_header.ModInfoSize; const section_contrib_size = dbi_stream_header.SectionContributionSize; diff --git a/std/pdb.zig b/std/pdb.zig index ffe212029..39f304afb 100644 --- a/std/pdb.zig +++ b/std/pdb.zig @@ -499,45 +499,78 @@ const Msf = struct { const superblock = try in.readStruct(SuperBlock); + // Sanity checks if (!mem.eql(u8, superblock.FileMagic, SuperBlock.file_magic)) return error.InvalidDebugInfo; - + if (superblock.FreeBlockMapBlock != 1 and superblock.FreeBlockMapBlock != 2) + return error.InvalidDebugInfo; + if (superblock.NumBlocks * superblock.BlockSize != try file.getEndPos()) + return error.InvalidDebugInfo; switch (superblock.BlockSize) { // llvm only supports 4096 but we can handle any of these values 512, 1024, 2048, 4096 => {}, else => return error.InvalidDebugInfo, } - if (superblock.NumBlocks * superblock.BlockSize != try file.getEndPos()) - return error.InvalidDebugInfo; + const dir_block_count = blockCountFromSize(superblock.NumDirectoryBytes, superblock.BlockSize); + if (dir_block_count > superblock.BlockSize / @sizeOf(u32)) + return error.UnhandledBigDirectoryStream; // cf. BlockMapAddr comment. - self.directory = try MsfStream.init( + try file.seekTo(superblock.BlockSize * superblock.BlockMapAddr); + var dir_blocks = try allocator.alloc(u32, dir_block_count); + for (dir_blocks) |*b| { + b.* = try in.readIntLittle(u32); + } + self.directory = MsfStream.init( superblock.BlockSize, - blockCountFromSize(superblock.NumDirectoryBytes, superblock.BlockSize), - superblock.BlockSize * superblock.BlockMapAddr, file, - allocator, + dir_blocks, ); + const begin = self.directory.pos; const stream_count = try self.directory.stream.readIntLittle(u32); - const stream_sizes = try allocator.alloc(u32, stream_count); - for (stream_sizes) |*s| { + defer allocator.free(stream_sizes); + + // Microsoft's implementation uses u32(-1) for inexistant streams. + // These streams are not used, but still participate in the file + // and must be taken into account when resolving stream indices. + const Nil = 0xFFFFFFFF; + for (stream_sizes) |*s, i| { const size = try self.directory.stream.readIntLittle(u32); - s.* = blockCountFromSize(size, superblock.BlockSize); + s.* = if (size == Nil) 0 else blockCountFromSize(size, superblock.BlockSize); } self.streams = try allocator.alloc(MsfStream, stream_count); for (self.streams) |*stream, i| { - stream.* = try MsfStream.init( - superblock.BlockSize, - stream_sizes[i], - // MsfStream.init expects the file to be at the part where it reads [N]u32 - try file.getPos(), - file, - allocator, - ); + const size = stream_sizes[i]; + if (size == 0) { + stream.* = MsfStream{ + .blocks = [_]u32{}, + }; + } else { + var blocks = try allocator.alloc(u32, size); + var j: u32 = 0; + while (j < size) : (j += 1) { + const block_id = try self.directory.stream.readIntLittle(u32); + const n = (block_id % superblock.BlockSize); + // 0 is for SuperBlock, 1 and 2 for FPMs. + if (block_id == 0 or n == 1 or n == 2 or block_id * superblock.BlockSize > try file.getEndPos()) + return error.InvalidBlockIndex; + blocks[j] = block_id; + } + + stream.* = MsfStream.init( + superblock.BlockSize, + file, + blocks, + ); + } } + + const end = self.directory.pos; + if (end - begin != superblock.NumDirectoryBytes) + return error.InvalidStreamDirectory; } }; @@ -574,7 +607,6 @@ const SuperBlock = packed struct { NumDirectoryBytes: u32, Unknown: u32, - /// The index of a block within the MSF file. At this block is an array of /// ulittle32_t’s listing the blocks that the stream directory resides on. /// For large MSF files, the stream directory (which describes the block @@ -584,45 +616,41 @@ const SuperBlock = packed struct { /// and the stream directory itself can be stitched together accordingly. /// The number of ulittle32_t’s in this array is given by /// ceil(NumDirectoryBytes / BlockSize). + // Note: microsoft-pdb code actually suggests this is a variable-length + // array. If the indices of blocks occupied by the Stream Directory didn't + // fit in one page, there would be other u32 following it. + // This would mean the Stream Directory is bigger than BlockSize / sizeof(u32) + // blocks. We're not even close to this with a 1GB pdb file, and LLVM didn't + // implement it so we're kind of safe making this assumption for now. BlockMapAddr: u32, }; const MsfStream = struct { - in_file: File, - pos: u64, - blocks: []u32, - block_size: u32, + in_file: File = undefined, + pos: u64 = undefined, + blocks: []u32 = undefined, + block_size: u32 = undefined, /// Implementation of InStream trait for Pdb.MsfStream - stream: Stream, + stream: Stream = undefined, pub const Error = @typeOf(read).ReturnType.ErrorSet; pub const Stream = io.InStream(Error); - fn init(block_size: u32, block_count: u32, pos: u64, file: File, allocator: *mem.Allocator) !MsfStream { - var stream = MsfStream{ + fn init(block_size: u32, file: File, blocks: []u32) MsfStream { + const stream = MsfStream{ .in_file = file, .pos = 0, - .blocks = try allocator.alloc(u32, block_count), + .blocks = blocks, .block_size = block_size, .stream = Stream{ .readFn = readFn }, }; - var file_stream = file.inStream(); - const in = &file_stream.stream; - try file.seekTo(pos); - - var i: u32 = 0; - while (i < block_count) : (i += 1) { - stream.blocks[i] = try in.readIntLittle(u32); - } - return stream; } fn readNullTermString(self: *MsfStream, allocator: *mem.Allocator) ![]u8 { var list = ArrayList(u8).init(allocator); - defer list.deinit(); while (true) { const byte = try self.stream.readByte(); if (byte == 0) { @@ -633,6 +661,7 @@ const MsfStream = struct { } fn read(self: *MsfStream, buffer: []u8) !usize { + var block_id = @intCast(usize, self.pos / self.block_size); var block = self.blocks[block_id]; var offset = self.pos % self.block_size; From c087525edae2db147c89c83513a0778f87bee30d Mon Sep 17 00:00:00 2001 From: Sahnvour Date: Sun, 28 Jul 2019 18:46:40 +0200 Subject: [PATCH 11/17] pdb: improved stream reading performance, printing stack trace from a stage1 crash is now 10x faster --- std/pdb.zig | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/std/pdb.zig b/std/pdb.zig index 39f304afb..7a2b2c6b6 100644 --- a/std/pdb.zig +++ b/std/pdb.zig @@ -661,7 +661,6 @@ const MsfStream = struct { } fn read(self: *MsfStream, buffer: []u8) !usize { - var block_id = @intCast(usize, self.pos / self.block_size); var block = self.blocks[block_id]; var offset = self.pos % self.block_size; @@ -671,11 +670,12 @@ const MsfStream = struct { const in = &file_stream.stream; var size: usize = 0; - for (buffer) |*byte| { - byte.* = try in.readByte(); - - offset += 1; - size += 1; + var rem_buffer = buffer; + while (size < buffer.len) { + const size_to_read = math.min(self.block_size - offset, rem_buffer.len); + size += try in.read(rem_buffer[0..size_to_read]); + rem_buffer = buffer[size..]; + offset += size_to_read; // If we're at the end of a block, go to the next one. if (offset == self.block_size) { @@ -686,8 +686,8 @@ const MsfStream = struct { } } - self.pos += size; - return size; + self.pos += buffer.len; + return buffer.len; } fn seekBy(self: *MsfStream, len: i64) !void { From 38b5812c4895eb0157f99348f51c40bbd17c3b94 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 1 Aug 2019 02:37:22 -0400 Subject: [PATCH 12/17] allow 128 bit cmpxchg on x86_64 --- src/ir.cpp | 7 ++-- src/target.cpp | 66 ++++++++++++++++++++++++++++++++ src/target.hpp | 1 + test/stage1/behavior/atomics.zig | 20 ++++++++++ 4 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/ir.cpp b/src/ir.cpp index be7a8e2e5..f34c84049 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -24735,10 +24735,11 @@ static ZigType *ir_resolve_atomic_operand_type(IrAnalyze *ira, IrInstruction *op operand_type->data.integral.bit_count)); return ira->codegen->builtin_types.entry_invalid; } - if (operand_type->data.integral.bit_count > ira->codegen->pointer_size_bytes * 8) { + uint32_t max_atomic_bits = target_arch_largest_atomic_bits(ira->codegen->zig_target->arch); + if (operand_type->data.integral.bit_count > max_atomic_bits) { ir_add_error(ira, op, - buf_sprintf("expected integer type pointer size or smaller, found %" PRIu32 "-bit integer type", - operand_type->data.integral.bit_count)); + buf_sprintf("expected %" PRIu32 "-bit integer type or smaller, found %" PRIu32 "-bit integer type", + max_atomic_bits, operand_type->data.integral.bit_count)); return ira->codegen->builtin_types.entry_invalid; } if (!is_power_of_2(operand_type->data.integral.bit_count)) { diff --git a/src/target.cpp b/src/target.cpp index 6a949270a..7bb248a35 100644 --- a/src/target.cpp +++ b/src/target.cpp @@ -863,6 +863,71 @@ uint32_t target_arch_pointer_bit_width(ZigLLVM_ArchType arch) { zig_unreachable(); } +uint32_t target_arch_largest_atomic_bits(ZigLLVM_ArchType arch) { + switch (arch) { + case ZigLLVM_UnknownArch: + zig_unreachable(); + + case ZigLLVM_avr: + case ZigLLVM_msp430: + return 16; + + case ZigLLVM_arc: + case ZigLLVM_arm: + case ZigLLVM_armeb: + case ZigLLVM_hexagon: + case ZigLLVM_le32: + case ZigLLVM_mips: + case ZigLLVM_mipsel: + case ZigLLVM_nvptx: + case ZigLLVM_ppc: + case ZigLLVM_r600: + case ZigLLVM_riscv32: + case ZigLLVM_sparc: + case ZigLLVM_sparcel: + case ZigLLVM_tce: + case ZigLLVM_tcele: + case ZigLLVM_thumb: + case ZigLLVM_thumbeb: + case ZigLLVM_x86: + case ZigLLVM_xcore: + case ZigLLVM_amdil: + case ZigLLVM_hsail: + case ZigLLVM_spir: + case ZigLLVM_kalimba: + case ZigLLVM_lanai: + case ZigLLVM_shave: + case ZigLLVM_wasm32: + case ZigLLVM_renderscript32: + return 32; + + case ZigLLVM_aarch64: + case ZigLLVM_aarch64_be: + case ZigLLVM_amdgcn: + case ZigLLVM_bpfel: + case ZigLLVM_bpfeb: + case ZigLLVM_le64: + case ZigLLVM_mips64: + case ZigLLVM_mips64el: + case ZigLLVM_nvptx64: + case ZigLLVM_ppc64: + case ZigLLVM_ppc64le: + case ZigLLVM_riscv64: + case ZigLLVM_sparcv9: + case ZigLLVM_systemz: + case ZigLLVM_amdil64: + case ZigLLVM_hsail64: + case ZigLLVM_spir64: + case ZigLLVM_wasm64: + case ZigLLVM_renderscript64: + return 64; + + case ZigLLVM_x86_64: + return 128; + } + zig_unreachable(); +} + uint32_t target_c_type_size_in_bits(const ZigTarget *target, CIntType id) { switch (target->os) { case OsFreestanding: @@ -1693,3 +1758,4 @@ bool target_supports_libunwind(const ZigTarget *target) { } return true; } + diff --git a/src/target.hpp b/src/target.hpp index fcda9955b..985a4c11b 100644 --- a/src/target.hpp +++ b/src/target.hpp @@ -192,6 +192,7 @@ const char *target_arch_musl_name(ZigLLVM_ArchType arch); bool target_supports_libunwind(const ZigTarget *target); uint32_t target_arch_pointer_bit_width(ZigLLVM_ArchType arch); +uint32_t target_arch_largest_atomic_bits(ZigLLVM_ArchType arch); size_t target_libc_count(void); void target_libc_enum(size_t index, ZigTarget *out_target); diff --git a/test/stage1/behavior/atomics.zig b/test/stage1/behavior/atomics.zig index daa463fd4..3d1caaaa1 100644 --- a/test/stage1/behavior/atomics.zig +++ b/test/stage1/behavior/atomics.zig @@ -69,3 +69,23 @@ test "cmpxchg with ptr" { expect(@cmpxchgStrong(*i32, &x, &data3, &data2, AtomicOrder.SeqCst, AtomicOrder.SeqCst) == null); expect(x == &data2); } + +test "128-bit cmpxchg" { + if (builtin.arch != .x86_64) { + return error.SkipZigTest; + } + var x: u128 align(16) = 1234; // TODO: https://github.com/ziglang/zig/issues/2987 + if (@cmpxchgWeak(u128, &x, 99, 5678, .SeqCst, .SeqCst)) |x1| { + expect(x1 == 1234); + } else { + @panic("cmpxchg should have failed"); + } + + while (@cmpxchgWeak(u128, &x, 1234, 5678, .SeqCst, .SeqCst)) |x1| { + expect(x1 == 1234); + } + expect(x == 5678); + + expect(@cmpxchgStrong(u128, &x, 5678, 42, .SeqCst, .SeqCst) == null); + expect(x == 42); +} From 6cb4cac5cd1c68f41c62a3c23b09513988337c8d Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 1 Aug 2019 03:36:03 -0400 Subject: [PATCH 13/17] disable behavior test for 128-bit cmpxchg once #2883 is done this can be revisited --- test/stage1/behavior/atomics.zig | 39 ++++++++++++++++---------------- 1 file changed, 20 insertions(+), 19 deletions(-) diff --git a/test/stage1/behavior/atomics.zig b/test/stage1/behavior/atomics.zig index 3d1caaaa1..a97467e41 100644 --- a/test/stage1/behavior/atomics.zig +++ b/test/stage1/behavior/atomics.zig @@ -70,22 +70,23 @@ test "cmpxchg with ptr" { expect(x == &data2); } -test "128-bit cmpxchg" { - if (builtin.arch != .x86_64) { - return error.SkipZigTest; - } - var x: u128 align(16) = 1234; // TODO: https://github.com/ziglang/zig/issues/2987 - if (@cmpxchgWeak(u128, &x, 99, 5678, .SeqCst, .SeqCst)) |x1| { - expect(x1 == 1234); - } else { - @panic("cmpxchg should have failed"); - } - - while (@cmpxchgWeak(u128, &x, 1234, 5678, .SeqCst, .SeqCst)) |x1| { - expect(x1 == 1234); - } - expect(x == 5678); - - expect(@cmpxchgStrong(u128, &x, 5678, 42, .SeqCst, .SeqCst) == null); - expect(x == 42); -} +// TODO this test is disabled until this issue is resolved: +// https://github.com/ziglang/zig/issues/2883 +// otherwise cross compiling will result in: +// lld: error: undefined symbol: __sync_val_compare_and_swap_16 +//test "128-bit cmpxchg" { +// var x: u128 align(16) = 1234; // TODO: https://github.com/ziglang/zig/issues/2987 +// if (@cmpxchgWeak(u128, &x, 99, 5678, .SeqCst, .SeqCst)) |x1| { +// expect(x1 == 1234); +// } else { +// @panic("cmpxchg should have failed"); +// } +// +// while (@cmpxchgWeak(u128, &x, 1234, 5678, .SeqCst, .SeqCst)) |x1| { +// expect(x1 == 1234); +// } +// expect(x == 5678); +// +// expect(@cmpxchgStrong(u128, &x, 5678, 42, .SeqCst, .SeqCst) == null); +// expect(x == 42); +//} From a5cb0f77d11bdcc504fe3e6afa928c88de821518 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 2 Aug 2019 13:54:58 -0400 Subject: [PATCH 14/17] assignment participates in result location fix one regression with optionals but there are more --- src/ir.cpp | 23 ++++++++++++++++------- test/stage1/behavior/eval.zig | 10 ++++++++++ 2 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/ir.cpp b/src/ir.cpp index f34c84049..8a46fec7c 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -4001,12 +4001,20 @@ static IrInstruction *ir_gen_bin_op_id(IrBuilder *irb, Scope *scope, AstNode *no static IrInstruction *ir_gen_assign(IrBuilder *irb, Scope *scope, AstNode *node) { IrInstruction *lvalue = ir_gen_node_extra(irb, node->data.bin_op_expr.op1, scope, LValPtr, nullptr); - IrInstruction *rvalue = ir_gen_node(irb, node->data.bin_op_expr.op2, scope); - - if (lvalue == irb->codegen->invalid_instruction || rvalue == irb->codegen->invalid_instruction) + if (lvalue == irb->codegen->invalid_instruction) + return irb->codegen->invalid_instruction; + + ResultLocInstruction *result_loc_inst = allocate(1); + result_loc_inst->base.id = ResultLocIdInstruction; + result_loc_inst->base.source_instruction = lvalue; + ir_ref_instruction(lvalue, irb->current_basic_block); + ir_build_reset_result(irb, scope, node, &result_loc_inst->base); + + IrInstruction *rvalue = ir_gen_node_extra(irb, node->data.bin_op_expr.op2, scope, LValNone, + &result_loc_inst->base); + if (rvalue == irb->codegen->invalid_instruction) return irb->codegen->invalid_instruction; - ir_build_store_ptr(irb, scope, node, lvalue, rvalue); return ir_build_const_void(irb, scope, node); } @@ -17477,6 +17485,7 @@ static IrInstruction *ir_analyze_instruction_elem_ptr(IrAnalyze *ira, IrInstruct return result; } else if (is_slice(array_type)) { ConstExprValue *ptr_field = &array_ptr_val->data.x_struct.fields[slice_ptr_index]; + ir_assert(ptr_field != nullptr, &elem_ptr_instruction->base); if (ptr_field->data.x_ptr.special == ConstPtrSpecialHardCodedAddr) { IrInstruction *result = ir_build_elem_ptr(&ira->new_irb, elem_ptr_instruction->base.scope, elem_ptr_instruction->base.source_node, array_ptr, casted_elem_index, false, @@ -17663,7 +17672,7 @@ static IrInstruction *ir_analyze_struct_field_ptr(IrAnalyze *ira, IrInstruction return ira->codegen->invalid_instruction; if (type_is_invalid(struct_val->type)) return ira->codegen->invalid_instruction; - if (struct_val->special == ConstValSpecialUndef && initializing) { + if (initializing && struct_val->special == ConstValSpecialUndef) { struct_val->data.x_struct.fields = create_const_vals(struct_type->data.structure.src_field_count); struct_val->special = ConstValSpecialStatic; for (size_t i = 0; i < struct_type->data.structure.src_field_count; i += 1) { @@ -18764,7 +18773,7 @@ static IrInstruction *ir_analyze_unwrap_optional_payload(IrAnalyze *ira, IrInstr if (optional_val == nullptr) return ira->codegen->invalid_instruction; - if (initializing && optional_val->special == ConstValSpecialUndef) { + if (initializing) { switch (type_has_one_possible_value(ira->codegen, child_type)) { case OnePossibleValueInvalid: return ira->codegen->invalid_instruction; @@ -23260,7 +23269,7 @@ static IrInstruction *ir_analyze_unwrap_error_payload(IrAnalyze *ira, IrInstruct ConstExprValue *err_union_val = const_ptr_pointee(ira, ira->codegen, ptr_val, source_instr->source_node); if (err_union_val == nullptr) return ira->codegen->invalid_instruction; - if (err_union_val->special == ConstValSpecialUndef && initializing) { + if (initializing && err_union_val->special == ConstValSpecialUndef) { ConstExprValue *vals = create_const_vals(2); ConstExprValue *err_set_val = &vals[0]; ConstExprValue *payload_val = &vals[1]; diff --git a/test/stage1/behavior/eval.zig b/test/stage1/behavior/eval.zig index 97d3a269c..58d662d76 100644 --- a/test/stage1/behavior/eval.zig +++ b/test/stage1/behavior/eval.zig @@ -1,5 +1,6 @@ const std = @import("std"); const expect = std.testing.expect; +const expectEqual = std.testing.expectEqual; const builtin = @import("builtin"); test "compile time recursion" { @@ -794,3 +795,12 @@ test "no undeclared identifier error in unanalyzed branches" { lol_this_doesnt_exist = nonsense; } } + +test "comptime assign int to optional int" { + comptime { + var x: ?i32 = null; + x = 2; + x.? *= 10; + expectEqual(20, x.?); + } +} From 90e64bc620edc3f3c3a2c16d01b7ca2eefc02429 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 2 Aug 2019 14:47:26 -0400 Subject: [PATCH 15/17] fix cmpxchg with discarded result --- src/codegen.cpp | 8 +++++++- test/stage1/behavior/atomics.zig | 10 ++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/codegen.cpp b/src/codegen.cpp index 0ac945e8c..ef24716df 100644 --- a/src/codegen.cpp +++ b/src/codegen.cpp @@ -4458,8 +4458,14 @@ static LLVMValueRef ir_render_cmpxchg(CodeGen *g, IrExecutable *executable, IrIn return LLVMBuildSelect(g->builder, success_bit, LLVMConstNull(get_llvm_type(g, child_type)), payload_val, ""); } + // When the cmpxchg is discarded, the result location will have no bits. + if (!type_has_bits(instruction->result_loc->value.type)) { + return nullptr; + } + LLVMValueRef result_loc = ir_llvm_value(g, instruction->result_loc); - assert(type_has_bits(child_type)); + src_assert(result_loc != nullptr, instruction->base.source_node); + src_assert(type_has_bits(child_type), instruction->base.source_node); LLVMValueRef payload_val = LLVMBuildExtractValue(g->builder, result_val, 0, ""); LLVMValueRef val_ptr = LLVMBuildStructGEP(g->builder, result_loc, maybe_child_index, ""); diff --git a/test/stage1/behavior/atomics.zig b/test/stage1/behavior/atomics.zig index a97467e41..1a941cf21 100644 --- a/test/stage1/behavior/atomics.zig +++ b/test/stage1/behavior/atomics.zig @@ -1,5 +1,6 @@ const std = @import("std"); const expect = std.testing.expect; +const expectEqual = std.testing.expectEqual; const builtin = @import("builtin"); const AtomicRmwOp = builtin.AtomicRmwOp; const AtomicOrder = builtin.AtomicOrder; @@ -90,3 +91,12 @@ test "cmpxchg with ptr" { // expect(@cmpxchgStrong(u128, &x, 5678, 42, .SeqCst, .SeqCst) == null); // expect(x == 42); //} + +test "cmpxchg with ignored result" { + var x: i32 = 1234; + var ptr = &x; + + _ = @cmpxchgStrong(i32, &x, 1234, 5678, .Monotonic, .Monotonic); + + expectEqual(i32(5678), x); +} From 9069ee957cb8c9069028b325af5b862bbf8f66af Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 2 Aug 2019 15:17:02 -0400 Subject: [PATCH 16/17] fix discarding function call results --- src/ir.cpp | 65 ++++++++++++++++++++++--------------- test/stage1/behavior/fn.zig | 19 +++++++++++ 2 files changed, 58 insertions(+), 26 deletions(-) diff --git a/src/ir.cpp b/src/ir.cpp index 8a46fec7c..de2e4e165 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -189,7 +189,8 @@ static IrInstruction *ir_analyze_bit_cast(IrAnalyze *ira, IrInstruction *source_ static IrInstruction *ir_resolve_result_raw(IrAnalyze *ira, IrInstruction *suspend_source_instr, ResultLoc *result_loc, ZigType *value_type, IrInstruction *value, bool force_runtime, bool non_null_comptime); static IrInstruction *ir_resolve_result(IrAnalyze *ira, IrInstruction *suspend_source_instr, - ResultLoc *result_loc, ZigType *value_type, IrInstruction *value, bool force_runtime, bool non_null_comptime); + ResultLoc *result_loc, ZigType *value_type, IrInstruction *value, bool force_runtime, + bool non_null_comptime, bool allow_discard); static IrInstruction *ir_analyze_unwrap_optional_payload(IrAnalyze *ira, IrInstruction *source_instr, IrInstruction *base_ptr, bool safety_check_on, bool initializing); static IrInstruction *ir_analyze_unwrap_error_payload(IrAnalyze *ira, IrInstruction *source_instr, @@ -11163,7 +11164,8 @@ static IrInstruction *ir_resolve_ptr_of_array_to_slice(IrAnalyze *ira, IrInstruc } if (result_loc == nullptr) result_loc = no_result_loc(); - IrInstruction *result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, wanted_type, nullptr, true, false); + IrInstruction *result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, wanted_type, nullptr, true, + false, true); if (type_is_invalid(result_loc_inst->value.type) || instr_is_unreachable(result_loc_inst)) { return result_loc_inst; } @@ -11623,7 +11625,7 @@ static IrInstruction *ir_analyze_optional_wrap(IrAnalyze *ira, IrInstruction *so } IrInstruction *result_loc_inst = nullptr; if (result_loc != nullptr) { - result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, wanted_type, nullptr, true, false); + result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, wanted_type, nullptr, true, false, true); if (type_is_invalid(result_loc_inst->value.type) || instr_is_unreachable(result_loc_inst)) { return result_loc_inst; } @@ -11666,7 +11668,7 @@ static IrInstruction *ir_analyze_err_wrap_payload(IrAnalyze *ira, IrInstruction IrInstruction *result_loc_inst; if (handle_is_ptr(wanted_type)) { if (result_loc == nullptr) result_loc = no_result_loc(); - result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, wanted_type, nullptr, true, false); + result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, wanted_type, nullptr, true, false, true); if (type_is_invalid(result_loc_inst->value.type) || instr_is_unreachable(result_loc_inst)) { return result_loc_inst; } @@ -11751,7 +11753,7 @@ static IrInstruction *ir_analyze_err_wrap_code(IrAnalyze *ira, IrInstruction *so IrInstruction *result_loc_inst; if (handle_is_ptr(wanted_type)) { if (result_loc == nullptr) result_loc = no_result_loc(); - result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, wanted_type, nullptr, true, false); + result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, wanted_type, nullptr, true, false, true); if (type_is_invalid(result_loc_inst->value.type) || instr_is_unreachable(result_loc_inst)) { return result_loc_inst; } @@ -11824,7 +11826,8 @@ static IrInstruction *ir_get_ref(IrAnalyze *ira, IrInstruction *source_instructi IrInstruction *result_loc; if (type_has_bits(ptr_type) && !handle_is_ptr(value->value.type)) { - result_loc = ir_resolve_result(ira, source_instruction, no_result_loc(), value->value.type, nullptr, true, false); + result_loc = ir_resolve_result(ira, source_instruction, no_result_loc(), value->value.type, nullptr, true, + false, true); } else { result_loc = nullptr; } @@ -11868,7 +11871,8 @@ static IrInstruction *ir_analyze_array_to_slice(IrAnalyze *ira, IrInstruction *s if (!array_ptr) array_ptr = ir_get_ref(ira, source_instr, array, true, false); if (result_loc == nullptr) result_loc = no_result_loc(); - IrInstruction *result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, wanted_type, nullptr, true, false); + IrInstruction *result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, wanted_type, nullptr, + true, false, true); if (type_is_invalid(result_loc_inst->value.type) || instr_is_unreachable(result_loc_inst)) { return result_loc_inst; } @@ -12524,7 +12528,8 @@ static IrInstruction *ir_analyze_vector_to_array(IrAnalyze *ira, IrInstruction * if (result_loc == nullptr) { result_loc = no_result_loc(); } - IrInstruction *result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, array_type, nullptr, true, false); + IrInstruction *result_loc_inst = ir_resolve_result(ira, source_instr, result_loc, array_type, nullptr, + true, false, true); if (type_is_invalid(result_loc_inst->value.type) || instr_is_unreachable(result_loc_inst)) { return result_loc_inst; } @@ -13105,7 +13110,8 @@ static IrInstruction *ir_get_deref(IrAnalyze *ira, IrInstruction *source_instruc IrInstruction *result_loc_inst; if (type_entry->data.pointer.host_int_bytes != 0 && handle_is_ptr(child_type)) { if (result_loc == nullptr) result_loc = no_result_loc(); - result_loc_inst = ir_resolve_result(ira, source_instruction, result_loc, child_type, nullptr, true, false); + result_loc_inst = ir_resolve_result(ira, source_instruction, result_loc, child_type, nullptr, + true, false, true); if (type_is_invalid(result_loc_inst->value.type) || instr_is_unreachable(result_loc_inst)) { return result_loc_inst; } @@ -15360,7 +15366,7 @@ static IrInstruction *ir_resolve_result_raw(IrAnalyze *ira, IrInstruction *suspe if (peer_parent->peers.length == 1) { IrInstruction *parent_result_loc = ir_resolve_result(ira, suspend_source_instr, peer_parent->parent, - value_type, value, force_runtime, non_null_comptime); + value_type, value, force_runtime, non_null_comptime, true); result_peer->suspend_pos.basic_block_index = SIZE_MAX; result_peer->suspend_pos.instruction_index = SIZE_MAX; if (parent_result_loc == nullptr || type_is_invalid(parent_result_loc->value.type) || @@ -15380,7 +15386,7 @@ static IrInstruction *ir_resolve_result_raw(IrAnalyze *ira, IrInstruction *suspe if (peer_parent->skipped) { if (non_null_comptime) { return ir_resolve_result(ira, suspend_source_instr, peer_parent->parent, - value_type, value, force_runtime, non_null_comptime); + value_type, value, force_runtime, non_null_comptime, true); } return nullptr; } @@ -15398,7 +15404,7 @@ static IrInstruction *ir_resolve_result_raw(IrAnalyze *ira, IrInstruction *suspe } IrInstruction *parent_result_loc = ir_resolve_result(ira, suspend_source_instr, peer_parent->parent, - peer_parent->resolved_type, nullptr, force_runtime, non_null_comptime); + peer_parent->resolved_type, nullptr, force_runtime, non_null_comptime, true); if (parent_result_loc == nullptr || type_is_invalid(parent_result_loc->value.type) || parent_result_loc->value.type->id == ZigTypeIdUnreachable) { @@ -15448,7 +15454,7 @@ static IrInstruction *ir_resolve_result_raw(IrAnalyze *ira, IrInstruction *suspe } IrInstruction *parent_result_loc = ir_resolve_result(ira, suspend_source_instr, result_bit_cast->parent, - dest_type, bitcasted_value, force_runtime, non_null_comptime); + dest_type, bitcasted_value, force_runtime, non_null_comptime, true); if (parent_result_loc == nullptr || type_is_invalid(parent_result_loc->value.type) || parent_result_loc->value.type->id == ZigTypeIdUnreachable) { @@ -15477,8 +15483,15 @@ static IrInstruction *ir_resolve_result_raw(IrAnalyze *ira, IrInstruction *suspe static IrInstruction *ir_resolve_result(IrAnalyze *ira, IrInstruction *suspend_source_instr, ResultLoc *result_loc_pass1, ZigType *value_type, IrInstruction *value, bool force_runtime, - bool non_null_comptime) + bool non_null_comptime, bool allow_discard) { + if (!allow_discard && result_loc_pass1->id == ResultLocIdInstruction && + instr_is_comptime(result_loc_pass1->source_instruction) && + result_loc_pass1->source_instruction->value.type->id == ZigTypeIdPointer && + result_loc_pass1->source_instruction->value.data.x_ptr.special == ConstPtrSpecialDiscard) + { + result_loc_pass1 = no_result_loc(); + } IrInstruction *result_loc = ir_resolve_result_raw(ira, suspend_source_instr, result_loc_pass1, value_type, value, force_runtime, non_null_comptime); if (result_loc == nullptr || (instr_is_unreachable(result_loc) || type_is_invalid(result_loc->value.type))) @@ -15533,7 +15546,7 @@ static IrInstruction *ir_analyze_instruction_resolve_result(IrAnalyze *ira, IrIn if (type_is_invalid(implicit_elem_type)) return ira->codegen->invalid_instruction; IrInstruction *result_loc = ir_resolve_result(ira, &instruction->base, instruction->result_loc, - implicit_elem_type, nullptr, false, true); + implicit_elem_type, nullptr, false, true, true); if (result_loc != nullptr) return result_loc; @@ -15542,7 +15555,7 @@ static IrInstruction *ir_analyze_instruction_resolve_result(IrAnalyze *ira, IrIn instruction->result_loc->id == ResultLocIdReturn) { result_loc = ir_resolve_result(ira, &instruction->base, no_result_loc(), - implicit_elem_type, nullptr, false, true); + implicit_elem_type, nullptr, false, true, true); if (result_loc != nullptr && (type_is_invalid(result_loc->value.type) || instr_is_unreachable(result_loc))) { @@ -15631,7 +15644,7 @@ static IrInstruction *ir_analyze_async_call(IrAnalyze *ira, IrInstructionCallSrc ZigType *async_return_type = get_error_union_type(ira->codegen, alloc_fn_error_set_type, promise_type); IrInstruction *result_loc = ir_resolve_result(ira, &call_instruction->base, no_result_loc(), - async_return_type, nullptr, true, true); + async_return_type, nullptr, true, true, false); if (type_is_invalid(result_loc->value.type) || instr_is_unreachable(result_loc)) { return result_loc; } @@ -16390,7 +16403,7 @@ static IrInstruction *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCallSrc *c IrInstruction *result_loc; if (handle_is_ptr(impl_fn_type_id->return_type)) { result_loc = ir_resolve_result(ira, &call_instruction->base, call_instruction->result_loc, - impl_fn_type_id->return_type, nullptr, true, true); + impl_fn_type_id->return_type, nullptr, true, true, false); if (result_loc != nullptr && (type_is_invalid(result_loc->value.type) || instr_is_unreachable(result_loc))) { @@ -16512,7 +16525,7 @@ static IrInstruction *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCallSrc *c IrInstruction *result_loc; if (handle_is_ptr(return_type)) { result_loc = ir_resolve_result(ira, &call_instruction->base, call_instruction->result_loc, - return_type, nullptr, true, true); + return_type, nullptr, true, true, false); if (result_loc != nullptr && (type_is_invalid(result_loc->value.type) || instr_is_unreachable(result_loc))) { return result_loc; } @@ -17028,7 +17041,7 @@ static IrInstruction *ir_analyze_instruction_phi(IrAnalyze *ira, IrInstructionPh // In case resolving the parent activates a suspend, do it now IrInstruction *parent_result_loc = ir_resolve_result(ira, &phi_instruction->base, peer_parent->parent, - peer_parent->resolved_type, nullptr, false, false); + peer_parent->resolved_type, nullptr, false, false, true); if (parent_result_loc != nullptr && (type_is_invalid(parent_result_loc->value.type) || instr_is_unreachable(parent_result_loc))) { @@ -21541,7 +21554,7 @@ static IrInstruction *ir_analyze_instruction_cmpxchg(IrAnalyze *ira, IrInstructi IrInstruction *result_loc; if (handle_is_ptr(result_type)) { result_loc = ir_resolve_result(ira, &instruction->base, instruction->result_loc, - result_type, nullptr, true, false); + result_type, nullptr, true, false, true); if (type_is_invalid(result_loc->value.type) || instr_is_unreachable(result_loc)) { return result_loc; } @@ -21798,7 +21811,7 @@ static IrInstruction *ir_analyze_instruction_from_bytes(IrAnalyze *ira, IrInstru } IrInstruction *result_loc = ir_resolve_result(ira, &instruction->base, instruction->result_loc, - dest_slice_type, nullptr, true, false); + dest_slice_type, nullptr, true, false, true); if (result_loc != nullptr && (type_is_invalid(result_loc->value.type) || instr_is_unreachable(result_loc))) { return result_loc; } @@ -21875,7 +21888,7 @@ static IrInstruction *ir_analyze_instruction_to_bytes(IrAnalyze *ira, IrInstruct } IrInstruction *result_loc = ir_resolve_result(ira, &instruction->base, instruction->result_loc, - dest_slice_type, nullptr, true, false); + dest_slice_type, nullptr, true, false, true); if (type_is_invalid(result_loc->value.type) || instr_is_unreachable(result_loc)) { return result_loc; } @@ -22617,7 +22630,7 @@ static IrInstruction *ir_analyze_instruction_slice(IrAnalyze *ira, IrInstruction } IrInstruction *result_loc = ir_resolve_result(ira, &instruction->base, instruction->result_loc, - return_type, nullptr, true, false); + return_type, nullptr, true, false, true); if (type_is_invalid(result_loc->value.type) || instr_is_unreachable(result_loc)) { return result_loc; } @@ -25397,7 +25410,7 @@ static IrInstruction *ir_analyze_instruction_end_expr(IrAnalyze *ira, IrInstruct bool was_written = instruction->result_loc->written; IrInstruction *result_loc = ir_resolve_result(ira, &instruction->base, instruction->result_loc, - value->value.type, value, false, false); + value->value.type, value, false, false, true); if (result_loc != nullptr) { if (type_is_invalid(result_loc->value.type)) return ira->codegen->invalid_instruction; @@ -25429,7 +25442,7 @@ static IrInstruction *ir_analyze_instruction_bit_cast_src(IrAnalyze *ira, IrInst return operand; IrInstruction *result_loc = ir_resolve_result(ira, &instruction->base, - &instruction->result_loc_bit_cast->base, operand->value.type, operand, false, false); + &instruction->result_loc_bit_cast->base, operand->value.type, operand, false, false, true); if (result_loc != nullptr && (type_is_invalid(result_loc->value.type) || instr_is_unreachable(result_loc))) return result_loc; diff --git a/test/stage1/behavior/fn.zig b/test/stage1/behavior/fn.zig index d6d670b09..6b9c8b8fe 100644 --- a/test/stage1/behavior/fn.zig +++ b/test/stage1/behavior/fn.zig @@ -228,3 +228,22 @@ test "implicit cast fn call result to optional in field result" { S.entry(); comptime S.entry(); } + +test "discard the result of a function that returns a struct" { + const S = struct { + fn entry() void { + _ = func(); + } + + fn func() Foo { + return undefined; + } + + const Foo = struct { + a: u64, + b: u64, + }; + }; + S.entry(); + comptime S.entry(); +} From d105769926fd5360a5309be3e202cc65d32ce604 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Fri, 2 Aug 2019 16:09:40 -0400 Subject: [PATCH 17/17] fix regressions regarding writing through const pointers --- src/all_types.hpp | 2 ++ src/ir.cpp | 34 ++++++++++++++++++---------------- test/compile_errors.zig | 16 ++++++++-------- 3 files changed, 28 insertions(+), 24 deletions(-) diff --git a/src/all_types.hpp b/src/all_types.hpp index a6b2bc51c..4c3aeade9 100644 --- a/src/all_types.hpp +++ b/src/all_types.hpp @@ -2543,6 +2543,7 @@ struct IrInstructionLoadPtrGen { struct IrInstructionStorePtr { IrInstruction base; + bool allow_write_through_const; IrInstruction *ptr; IrInstruction *value; }; @@ -3707,6 +3708,7 @@ enum ResultLocId { struct ResultLoc { ResultLocId id; bool written; + bool allow_write_through_const; IrInstruction *resolved_loc; // result ptr IrInstruction *source_instruction; IrInstruction *gen_instruction; // value to store to the result loc diff --git a/src/ir.cpp b/src/ir.cpp index de2e4e165..65a21a418 100644 --- a/src/ir.cpp +++ b/src/ir.cpp @@ -198,7 +198,7 @@ static IrInstruction *ir_analyze_unwrap_error_payload(IrAnalyze *ira, IrInstruct static IrInstruction *ir_analyze_unwrap_err_code(IrAnalyze *ira, IrInstruction *source_instr, IrInstruction *base_ptr, bool initializing); static IrInstruction *ir_analyze_store_ptr(IrAnalyze *ira, IrInstruction *source_instr, - IrInstruction *ptr, IrInstruction *uncasted_value); + IrInstruction *ptr, IrInstruction *uncasted_value, bool allow_write_through_const); static IrInstruction *ir_gen_union_init_expr(IrBuilder *irb, Scope *scope, AstNode *source_node, IrInstruction *union_type, IrInstruction *field_name, AstNode *expr_node, LVal lval, ResultLoc *parent_result_loc); @@ -1613,7 +1613,7 @@ static IrInstruction *ir_build_unreachable(IrBuilder *irb, Scope *scope, AstNode return &unreachable_instruction->base; } -static IrInstruction *ir_build_store_ptr(IrBuilder *irb, Scope *scope, AstNode *source_node, +static IrInstructionStorePtr *ir_build_store_ptr(IrBuilder *irb, Scope *scope, AstNode *source_node, IrInstruction *ptr, IrInstruction *value) { IrInstructionStorePtr *instruction = ir_build_instruction(irb, scope, source_node); @@ -1625,7 +1625,7 @@ static IrInstruction *ir_build_store_ptr(IrBuilder *irb, Scope *scope, AstNode * ir_ref_instruction(ptr, irb->current_basic_block); ir_ref_instruction(value, irb->current_basic_block); - return &instruction->base; + return instruction; } static IrInstruction *ir_build_var_decl_src(IrBuilder *irb, Scope *scope, AstNode *source_node, @@ -6051,6 +6051,7 @@ static IrInstruction *ir_gen_container_init_expr(IrBuilder *irb, Scope *scope, A ResultLocInstruction *result_loc_inst = allocate(1); result_loc_inst->base.id = ResultLocIdInstruction; result_loc_inst->base.source_instruction = field_ptr; + result_loc_inst->base.allow_write_through_const = true; ir_ref_instruction(field_ptr, irb->current_basic_block); ir_build_reset_result(irb, scope, expr_node, &result_loc_inst->base); @@ -6089,6 +6090,7 @@ static IrInstruction *ir_gen_container_init_expr(IrBuilder *irb, Scope *scope, A ResultLocInstruction *result_loc_inst = allocate(1); result_loc_inst->base.id = ResultLocIdInstruction; result_loc_inst->base.source_instruction = elem_ptr; + result_loc_inst->base.allow_write_through_const = true; ir_ref_instruction(elem_ptr, irb->current_basic_block); ir_build_reset_result(irb, scope, expr_node, &result_loc_inst->base); @@ -6646,7 +6648,7 @@ static IrInstruction *ir_gen_for_expr(IrBuilder *irb, Scope *parent_scope, AstNo ir_set_cursor_at_end_and_append_block(irb, continue_block); IrInstruction *new_index_val = ir_build_bin_op(irb, child_scope, node, IrBinOpAdd, index_val, one, false); - ir_mark_gen(ir_build_store_ptr(irb, child_scope, node, index_ptr, new_index_val)); + ir_build_store_ptr(irb, child_scope, node, index_ptr, new_index_val)->allow_write_through_const = true; ir_build_br(irb, child_scope, node, cond_block, is_comptime); IrInstruction *else_result = nullptr; @@ -14848,7 +14850,7 @@ static IrInstruction *ir_analyze_instruction_decl_var(IrAnalyze *ira, // instruction. assert(deref->value.special != ConstValSpecialRuntime); var_ptr->value.special = ConstValSpecialRuntime; - ir_analyze_store_ptr(ira, var_ptr, var_ptr, deref); + ir_analyze_store_ptr(ira, var_ptr, var_ptr, deref, false); } if (var_ptr->value.special == ConstValSpecialStatic && var->mem_slot_index != SIZE_MAX) { @@ -15862,7 +15864,7 @@ no_mem_slot: } static IrInstruction *ir_analyze_store_ptr(IrAnalyze *ira, IrInstruction *source_instr, - IrInstruction *ptr, IrInstruction *uncasted_value) + IrInstruction *ptr, IrInstruction *uncasted_value, bool allow_write_through_const) { assert(ptr->value.type->id == ZigTypeIdPointer); @@ -15878,7 +15880,7 @@ static IrInstruction *ir_analyze_store_ptr(IrAnalyze *ira, IrInstruction *source ZigType *child_type = ptr->value.type->data.pointer.child_type; - if (ptr->value.type->data.pointer.is_const && !source_instr->is_gen) { + if (ptr->value.type->data.pointer.is_const && !allow_write_through_const) { ir_add_error(ira, source_instr, buf_sprintf("cannot assign to constant")); return ira->codegen->invalid_instruction; } @@ -15957,10 +15959,9 @@ static IrInstruction *ir_analyze_store_ptr(IrAnalyze *ira, IrInstruction *source break; } - IrInstruction *result = ir_build_store_ptr(&ira->new_irb, source_instr->scope, source_instr->source_node, - ptr, value); - result->value.type = ira->codegen->builtin_types.entry_void; - return result; + IrInstructionStorePtr *store_ptr = ir_build_store_ptr(&ira->new_irb, source_instr->scope, + source_instr->source_node, ptr, value); + return &store_ptr->base; } static IrInstruction *ir_analyze_fn_call(IrAnalyze *ira, IrInstructionCallSrc *call_instruction, @@ -18283,7 +18284,7 @@ static IrInstruction *ir_analyze_instruction_store_ptr(IrAnalyze *ira, IrInstruc if (type_is_invalid(value->value.type)) return ira->codegen->invalid_instruction; - return ir_analyze_store_ptr(ira, &instruction->base, ptr, value); + return ir_analyze_store_ptr(ira, &instruction->base, ptr, value, instruction->allow_write_through_const); } static IrInstruction *ir_analyze_instruction_load_ptr(IrAnalyze *ira, IrInstructionLoadPtr *instruction) { @@ -19691,7 +19692,7 @@ static IrInstruction *ir_analyze_container_init_fields(IrAnalyze *ira, IrInstruc IrInstruction *field_ptr = ir_analyze_struct_field_ptr(ira, instruction, field, result_loc, container_type, true); - ir_analyze_store_ptr(ira, instruction, field_ptr, runtime_inst); + ir_analyze_store_ptr(ira, instruction, field_ptr, runtime_inst, false); if (instr_is_comptime(field_ptr) && field_ptr->value.data.x_ptr.mut != ConstPtrMutRuntimeVar) { const_ptrs.append(field_ptr); } else { @@ -19708,7 +19709,7 @@ static IrInstruction *ir_analyze_container_init_fields(IrAnalyze *ira, IrInstruc IrInstruction *field_result_loc = const_ptrs.at(i); IrInstruction *deref = ir_get_deref(ira, field_result_loc, field_result_loc, nullptr); field_result_loc->value.special = ConstValSpecialRuntime; - ir_analyze_store_ptr(ira, field_result_loc, field_result_loc, deref); + ir_analyze_store_ptr(ira, field_result_loc, field_result_loc, deref, false); } } } @@ -19835,7 +19836,7 @@ static IrInstruction *ir_analyze_instruction_container_init_list(IrAnalyze *ira, assert(elem_result_loc->value.special == ConstValSpecialStatic); IrInstruction *deref = ir_get_deref(ira, elem_result_loc, elem_result_loc, nullptr); elem_result_loc->value.special = ConstValSpecialRuntime; - ir_analyze_store_ptr(ira, elem_result_loc, elem_result_loc, deref); + ir_analyze_store_ptr(ira, elem_result_loc, elem_result_loc, deref, false); } } } @@ -25418,7 +25419,8 @@ static IrInstruction *ir_analyze_instruction_end_expr(IrAnalyze *ira, IrInstruct return result_loc; if (!was_written) { - IrInstruction *store_ptr = ir_analyze_store_ptr(ira, &instruction->base, result_loc, value); + IrInstruction *store_ptr = ir_analyze_store_ptr(ira, &instruction->base, result_loc, value, + instruction->result_loc->allow_write_through_const); if (type_is_invalid(store_ptr->value.type)) { return ira->codegen->invalid_instruction; } diff --git a/test/compile_errors.zig b/test/compile_errors.zig index 40ce8d304..a4bc2a66f 100644 --- a/test/compile_errors.zig +++ b/test/compile_errors.zig @@ -201,7 +201,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\ return error.OutOfMemory; \\} , - "tmp.zig:2:7: error: error is discarded", + "tmp.zig:2:12: error: error is discarded", ); cases.add( @@ -2740,7 +2740,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\ 3 = 3; \\} , - "tmp.zig:2:7: error: cannot assign to constant", + "tmp.zig:2:9: error: cannot assign to constant", ); cases.add( @@ -2750,7 +2750,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\ a = 4; \\} , - "tmp.zig:3:7: error: cannot assign to constant", + "tmp.zig:3:9: error: cannot assign to constant", ); cases.add( @@ -2820,7 +2820,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\} \\export fn entry() void { f(); } , - "tmp.zig:3:7: error: cannot assign to constant", + "tmp.zig:3:9: error: cannot assign to constant", ); cases.add( @@ -3883,7 +3883,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\ \\export fn entry() usize { return @sizeOf(@typeOf(a)); } , - "tmp.zig:6:24: error: unable to evaluate constant expression", + "tmp.zig:6:26: error: unable to evaluate constant expression", "tmp.zig:4:17: note: called from here", ); @@ -4133,7 +4133,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\ cstr[0] = 'W'; \\} , - "tmp.zig:3:11: error: cannot assign to constant", + "tmp.zig:3:13: error: cannot assign to constant", ); cases.add( @@ -4143,7 +4143,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\ cstr[0] = 'W'; \\} , - "tmp.zig:3:11: error: cannot assign to constant", + "tmp.zig:3:13: error: cannot assign to constant", ); cases.add( @@ -4291,7 +4291,7 @@ pub fn addCases(cases: *tests.CompileErrorContext) void { \\ f.field = 0; \\} , - "tmp.zig:6:13: error: cannot assign to constant", + "tmp.zig:6:15: error: cannot assign to constant", ); cases.add(