Commit e0f5627d4a9cb1b3a1361d70d40043c8c170b2af | zig

author	Jakub Konka <kubkon@jakubkonka.com> 2022-02-25 21:53:46+01:00
committer	Jakub Konka <kubkon@jakubkonka.com> 2022-02-25 21:59:19+01:00
log	e0f5627d4a9cb1b3a1361d70d40043c8c170b2af
tree	c46515f91973faaf597ab9758743bd2190652a2e
parent	4b14384989362f93cd014628810c63b5cd9d3fff

x64+aarch64: check for pointer to zero-bit type when lowering decl

Unless the pointer is a pointer to a function, if the pointee type
has zero-bits, we need to return `MCValue.none` as the `Decl` has
not been lowered to memory, and therefore, any GOT reference will be
wrong.

6 files changed, 27 insertions(+), 4 deletions(-)

src
- arch
  - aarch64
    CodeGen.zig
  - x86_64
    CodeGen.zig
    Emit.zig
- link
  - MachO.zig
test
- behavior
  - basic.zig
  - struct.zig

src/arch/aarch64/CodeGen.zig+10

 @@ -3550,6 +3550,15 @@ fn getResolvedInstValue(self: *Self, inst: Air.Inst.Index) MCValue {
 fn lowerDeclRef(self: *Self, tv: TypedValue, decl: *Module.Decl) InnerError!MCValue {
     const ptr_bits = self.target.cpu.arch.ptrBitWidth();
     const ptr_bytes: u64 = @divExact(ptr_bits, 8);
+    // TODO this feels clunky. Perhaps we should check for it in `genTypedValue`?
+    if (tv.ty.zigTypeTag() == .Pointer) blk: {
+        if (tv.ty.castPtrToFn()) |_| break :blk;
+        if (!tv.ty.elemType2().hasRuntimeBits()) {
+            return MCValue.none;
+        }
+    }
     decl.alive = true;
     if (self.bin_file.cast(link.File.Elf)) |elf_file| {
         const got = &elf_file.program_headers.items[elf_file.phdr_got_index.?];
 @@ -3558,6 +3567,7 @@ fn lowerDeclRef(self: *Self, tv: TypedValue, decl: *Module.Decl) InnerError!MCVa
     } else if (self.bin_file.cast(link.File.MachO)) |_| {
         // Because MachO is PIE-always-on, we defer memory address resolution until
         // the linker has enough info to perform relocations.
+        assert(decl.link.macho.local_sym_index != 0);
         return MCValue{ .got_load = decl.link.macho.local_sym_index };
     } else if (self.bin_file.cast(link.File.Coff)) |coff_file| {
         const got_addr = coff_file.offset_table_virtual_address + decl.link.coff.offset_table_index * ptr_bytes;

src/arch/x86_64/CodeGen.zig+9

 @@ -5333,6 +5333,14 @@ fn lowerDeclRef(self: *Self, tv: TypedValue, decl: *Module.Decl) InnerError!MCVa
     const ptr_bits = self.target.cpu.arch.ptrBitWidth();
     const ptr_bytes: u64 = @divExact(ptr_bits, 8);
+    // TODO this feels clunky. Perhaps we should check for it in `genTypedValue`?
+    if (tv.ty.zigTypeTag() == .Pointer) blk: {
+        if (tv.ty.castPtrToFn()) |_| break :blk;
+        if (!tv.ty.elemType2().hasRuntimeBits()) {
+            return MCValue.none;
+        }
+    }
     decl.alive = true;
     if (self.bin_file.cast(link.File.Elf)) |elf_file| {
         const got = &elf_file.program_headers.items[elf_file.phdr_got_index.?];
 @@ -5341,6 +5349,7 @@ fn lowerDeclRef(self: *Self, tv: TypedValue, decl: *Module.Decl) InnerError!MCVa
     } else if (self.bin_file.cast(link.File.MachO)) |_| {
         // Because MachO is PIE-always-on, we defer memory address resolution until
         // the linker has enough info to perform relocations.
+        assert(decl.link.macho.local_sym_index != 0);
         return MCValue{ .got_load = decl.link.macho.local_sym_index };
     } else if (self.bin_file.cast(link.File.Coff)) |coff_file| {
         const got_addr = coff_file.offset_table_virtual_address + decl.link.coff.offset_table_index * ptr_bytes;

src/arch/x86_64/Emit.zig+1

...	@@ -857,6 +857,7 @@ fn mirLeaPie(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {	...	@@ -857,6 +857,7 @@ fn mirLeaPie(emit: *Emit, inst: Mir.Inst.Index) InnerError!void {
857	else => return emit.fail("TODO unused LEA PIE variants 0b10 and 0b11", .{}),	857	else => return emit.fail("TODO unused LEA PIE variants 0b10 and 0b11", .{}),
858	};	858	};
859	const atom = macho_file.atom_by_index_table.get(load_reloc.atom_index).?;	859	const atom = macho_file.atom_by_index_table.get(load_reloc.atom_index).?;
		860	log.debug("adding reloc of type {} to local @{d}", .{ reloc_type, load_reloc.sym_index });
860	try atom.relocs.append(emit.bin_file.allocator, .{	861	try atom.relocs.append(emit.bin_file.allocator, .{
861	.offset = @intCast(u32, end_offset - 4),	862	.offset = @intCast(u32, end_offset - 4),
862	.target = .{ .local = load_reloc.sym_index },	863	.target = .{ .local = load_reloc.sym_index },

src/link/MachO.zig+7

 @@ -4286,6 +4286,7 @@ pub fn deleteExport(self: *MachO, exp: Export) void {
 }
 fn freeUnnamedConsts(self: *MachO, decl: *Module.Decl) void {
+    log.debug("freeUnnamedConsts for decl {*}", .{decl});
     const unnamed_consts = self.unnamed_const_atoms.getPtr(decl) orelse return;
     for (unnamed_consts.items) |atom| {
         self.freeAtom(atom, .{
 @@ -4295,6 +4296,7 @@ fn freeUnnamedConsts(self: *MachO, decl: *Module.Decl) void {
         self.locals_free_list.append(self.base.allocator, atom.local_sym_index) catch {};
         self.locals.items[atom.local_sym_index].n_type = 0;
         _ = self.atom_by_index_table.remove(atom.local_sym_index);
+        log.debug("  adding local symbol index {d} to free list", .{atom.local_sym_index});
         atom.local_sym_index = 0;
     }
     unnamed_consts.clearAndFree(self.base.allocator);
 @@ -4319,10 +4321,15 @@ pub fn freeDecl(self: *MachO, decl: *Module.Decl) void {
             self.got_entries_free_list.append(self.base.allocator, @intCast(u32, got_index)) catch {};
             self.got_entries.items[got_index] = .{ .target = .{ .local = 0 }, .atom = undefined };
             _ = self.got_entries_table.swapRemove(.{ .local = decl.link.macho.local_sym_index });
+            log.debug("  adding GOT index {d} to free list (target local@{d})", .{
+                got_index,
+                decl.link.macho.local_sym_index,
+            });
         }
         self.locals.items[decl.link.macho.local_sym_index].n_type = 0;
         _ = self.atom_by_index_table.remove(decl.link.macho.local_sym_index);
+        log.debug("  adding local symbol index {d} to free list", .{decl.link.macho.local_sym_index});
         decl.link.macho.local_sym_index = 0;
     }
     if (self.d_sym) |*d_sym| {

test/behavior/basic.zig-2

...	@@ -399,8 +399,6 @@ fn testTakeAddressOfParameter(f: f32) !void {	...	@@ -399,8 +399,6 @@ fn testTakeAddressOfParameter(f: f32) !void {
399		399
400	test "pointer to void return type" {	400	test "pointer to void return type" {
401	if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO	401	if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO
402	if (builtin.zig_backend == .stage2_aarch64 and builtin.os.tag == .macos) return error.SkipZigTest;
403	if (builtin.zig_backend == .stage2_x86_64 and builtin.os.tag == .macos) return error.SkipZigTest;
404		402
405	try testPointerToVoidReturnType();	403	try testPointerToVoidReturnType();
406	}	404	}

test/behavior/struct.zig-2

...	@@ -370,8 +370,6 @@ test "empty struct method call" {	...	@@ -370,8 +370,6 @@ test "empty struct method call" {
370	if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO	370	if (builtin.zig_backend == .stage2_wasm) return error.SkipZigTest; // TODO
371	if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO	371	if (builtin.zig_backend == .stage2_c) return error.SkipZigTest; // TODO
372	if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO	372	if (builtin.zig_backend == .stage2_arm) return error.SkipZigTest; // TODO
373	if (builtin.zig_backend == .stage2_aarch64 and builtin.os.tag == .macos) return error.SkipZigTest; // TODO
374	if (builtin.zig_backend == .stage2_x86_64 and builtin.os.tag == .macos) return error.SkipZigTest; // TODO
375		373
376	const es = EmptyStruct{};	374	const es = EmptyStruct{};
377	try expect(es.method() == 1234);	375	try expect(es.method() == 1234);