From 6cf5305e47dd8382508f867b04067be615448b41 Mon Sep 17 00:00:00 2001
From: Jakub Konka <kubkon@jakubkonka.com>
Date: Sun, 24 Oct 2021 16:57:00 +0200
Subject: [PATCH] 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
---
 src/link/MachO.zig                            | 13 +++++++------
 test/standalone/link_common_symbols/b.c       |  1 +
 test/standalone/link_common_symbols/build.zig |  2 +-
 test/standalone/link_common_symbols/c.c       |  5 +++++
 test/standalone/link_common_symbols/main.zig  |  5 +++++
 5 files changed, 19 insertions(+), 7 deletions(-)
 create mode 100644 test/standalone/link_common_symbols/c.c

diff --git a/src/link/MachO.zig b/src/link/MachO.zig
index 923811af362e3607cbad629fcc6fd943d1fd6d71..2490ec9124fe995e77b4a6e765829b448cc3b934 100644
--- a/src/link/MachO.zig
+++ b/src/link/MachO.zig
@@ -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);
                 },
             }
diff --git a/test/standalone/link_common_symbols/b.c b/test/standalone/link_common_symbols/b.c
index d3789c0fdf3eb9dff7a0c183004f219cac06a6f4..18e8a8c23babf363a3da7c405cba5b6f4b8ea370 100644
--- a/test/standalone/link_common_symbols/b.c
+++ b/test/standalone/link_common_symbols/b.c
@@ -1,5 +1,6 @@
 long i;
 int j = 2;
+int k;
 
 void incr_i() {
   i++;
diff --git a/test/standalone/link_common_symbols/build.zig b/test/standalone/link_common_symbols/build.zig
index 43bb41fe32610d247503263bf6bf05690b34499e..2f9f892e86c9f69bbd12df6712194bacd724ec50 100644
--- a/test/standalone/link_common_symbols/build.zig
+++ b/test/standalone/link_common_symbols/build.zig
@@ -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");
diff --git a/test/standalone/link_common_symbols/c.c b/test/standalone/link_common_symbols/c.c
new file mode 100644
index 0000000000000000000000000000000000000000..fdf60b9ca84495d78d27e642eba94e64a3f64a54
--- /dev/null
+++ b/test/standalone/link_common_symbols/c.c
@@ -0,0 +1,5 @@
+extern int k;
+
+int common_defined_externally() {
+  return k;
+}
diff --git a/test/standalone/link_common_symbols/main.zig b/test/standalone/link_common_symbols/main.zig
index 9d00d0d4fb94cda9921185f034b2b92917a39b24..255b5aa6215612f1a169c3169f9064d0e3fe3516 100644
--- a/test/standalone/link_common_symbols/main.zig
+++ b/test/standalone/link_common_symbols/main.zig
@@ -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);
-- 
2.54.0