Commit `6cf5305e47`

Jakub Konka <kubkon@jakubkonka.com>

2021-10-24 16:57:00

macho: remove unresolved ref in the correct place

* without this, when an included relocatable references a common symbol from another translation unit would not be correctly removed from the unresolved lookup table triggering a misleading assertion down the line * assert upon removal that we indeed removed a ref instead of silently ignoring in debug * add test case that covers this issue

master

1 parent f80fd7e

Changed files (5)

src

link

MachO.zig

test

standalone

link_common_symbols

@@ -2311,7 +2311,7 @@ fn createDsoHandleAtom(self: *MachO) !void {
         nlist.n_desc = macho.N_WEAK_DEF;
         try self.globals.append(self.base.allocator, nlist);
 
-        _ = self.unresolved.fetchSwapRemove(resolv.where_index);
+        assert(self.unresolved.swapRemove(resolv.where_index));
 
         undef.* = .{
             .n_strx = 0,
@@ -2409,7 +2409,7 @@ fn resolveSymbolsInObject(self: *MachO, object_id: u16) !void {
                     const global = &self.globals.items[resolv.where_index];
 
                     if (symbolIsTentative(global.*)) {
-                        _ = self.tentatives.fetchSwapRemove(resolv.where_index);
+                        assert(self.tentatives.swapRemove(resolv.where_index));
                     } else if (!(symbolIsWeakDef(sym) or symbolIsPext(sym)) and
                         !(symbolIsWeakDef(global.*) or symbolIsPext(global.*)))
                     {
@@ -2437,7 +2437,7 @@ fn resolveSymbolsInObject(self: *MachO, object_id: u16) !void {
                         .n_desc = 0,
                         .n_value = 0,
                     };
-                    _ = self.unresolved.fetchSwapRemove(resolv.where_index);
+                    assert(self.unresolved.swapRemove(resolv.where_index));
                 },
             }
 
@@ -2496,6 +2496,8 @@ fn resolveSymbolsInObject(self: *MachO, object_id: u16) !void {
                         .n_value = sym.n_value,
                     });
                     _ = try self.tentatives.getOrPut(self.base.allocator, global_sym_index);
+                    assert(self.unresolved.swapRemove(resolv.where_index));
+
                     resolv.* = .{
                         .where = .global,
                         .where_index = global_sym_index,
@@ -2508,7 +2510,6 @@ fn resolveSymbolsInObject(self: *MachO, object_id: u16) !void {
                         .n_desc = 0,
                         .n_value = 0,
                     };
-                    _ = self.unresolved.fetchSwapRemove(resolv.where_index);
                 },
             }
         } else {
@@ -3412,7 +3413,7 @@ pub fn updateDeclExports(
                     const sym = &self.globals.items[resolv.where_index];
 
                     if (symbolIsTentative(sym.*)) {
-                        _ = self.tentatives.fetchSwapRemove(resolv.where_index);
+                        assert(self.tentatives.swapRemove(resolv.where_index));
                     } else if (!is_weak and !(symbolIsWeakDef(sym.*) or symbolIsPext(sym.*))) {
                         _ = try module.failed_exports.put(
                             module.gpa,
@@ -3438,7 +3439,7 @@ pub fn updateDeclExports(
                     continue;
                 },
                 .undef => {
-                    _ = self.unresolved.fetchSwapRemove(resolv.where_index);
+                    assert(self.unresolved.swapRemove(resolv.where_index));
                     _ = self.symbol_resolver.remove(n_strx);
                 },
             }

@@ -1,5 +1,6 @@
 long i;
 int j = 2;
+int k;
 
 void incr_i() {
   i++;

@@ -4,7 +4,7 @@ pub fn build(b: *Builder) void {
     const mode = b.standardReleaseOptions();
 
     const lib_a = b.addStaticLibrary("a", null);
-    lib_a.addCSourceFiles(&.{ "a.c", "b.c" }, &.{"-fcommon"});
+    lib_a.addCSourceFiles(&.{ "c.c", "a.c", "b.c" }, &.{"-fcommon"});
     lib_a.setBuildMode(mode);
 
     const test_exe = b.addTest("main.zig");

@@ -0,0 +1,5 @@
+extern int k;
+
+int common_defined_externally() {
+  return k;
+}

@@ -1,9 +1,14 @@
 const std = @import("std");
 const expect = std.testing.expect;
 
+extern fn common_defined_externally() c_int;
 extern fn incr_i() void;
 extern fn add_to_i_and_j(x: c_int) c_int;
 
+test "undef shadows common symbol: issue #9937" {
+    try expect(common_defined_externally() == 0);
+}
+
 test "import C common symbols" {
     incr_i();
     const res = add_to_i_and_j(2);

Commit 6cf5305e47

Commit `6cf5305e47`