refine COMDAT symlink handling

Stop treating COMDAT section-definition symbols as symlink anchors.
Keep zero-sized COMDAT sections only when they have an owned symlink,
so referenced empty COMDAT symbols still resolve while marker sections
like .gsspr/.gssep are skipped.

Update COMDAT/ICF tests to cover zero-sized COMDAT relocation behavior
and the adjusted removed-COMDAT handling.

src/linker/lnk.c

@@ -3414,8 +3414,10 @@ lnk_section_definition_is_before(void *raw_a, void *raw_b)
 }
 
 internal B32
-lnk_should_gather_section(COFF_SectionHeader *sect_header, COFF_SectionFlags sect_flags)
+lnk_should_gather_section(LNK_Obj *obj, U64 sect_idx, COFF_SectionHeader *sect_header)
 {
+  COFF_SectionFlags sect_flags = obj->section_flags[sect_idx];
+
   // removed sections were eliminated before image layout
   if (sect_flags & COFF_SectionFlag_LnkRemove) {
     return 0;
@@ -3426,13 +3428,19 @@ lnk_should_gather_section(COFF_SectionHeader *sect_header, COFF_SectionFlags sec
     return 0;
   }
 
-  // non-COMDAT empty sections do not need image contributions
+  // empty COMDATs with symlinks can still anchor symbols at offset zero
   if (sect_header->fsize == 0) {
     if (~sect_flags & COFF_SectionFlag_LnkCOMDAT) {
       return 0;
     }
 
-    // keep zero-size COMDATs so symbols at offset zero still get mapped
+    LNK_ObjSymbolRef symlink_ref = {0};
+    if (!lnk_obj_get_comdat_symlink(obj, sect_idx + 1, &symlink_ref)) {
+      return 0;
+    }
+
+    // gather only COMDAT leaders
+    AssertAlways(symlink_ref.obj == obj);
   }
 
   return 1;
@@ -3457,7 +3465,7 @@ THREAD_POOL_TASK_FUNC(lnk_gather_sections_task)
     for EachIndex(sect_idx, obj->header.section_count_no_null) {
       COFF_SectionHeader *sect_header = &section_table[sect_idx];
 
-      if ( ! lnk_should_gather_section(sect_header, obj->section_flags[sect_idx])) { continue; }
+      if ( ! lnk_should_gather_section(obj, sect_idx, sect_header)) { continue; }
 
       Temp temp = temp_begin(scratch.arena);
 
@@ -3591,7 +3599,7 @@ THREAD_POOL_TASK_FUNC(lnk_gather_sections_task)
       COFF_SectionFlags   sect_flags  = obj->section_flags[sect_idx];
       task->sect_map[obj_idx][sect_idx] = sc;
 
-      if ( ! lnk_should_gather_section(sect_header, sect_flags)) { continue; }
+      if ( ! lnk_should_gather_section(obj, sect_idx, sect_header)) { continue; }
 
       LNK_SectionContribChunk *sc_chunk = 0;
       {
@@ -3849,14 +3857,10 @@ THREAD_POOL_TASK_FUNC(lnk_patch_regular_symbols_task)
   for (U64 symbol_idx = 0; symbol_idx < obj->header.symbol_count; symbol_idx += (1 + symbol.aux_symbol_count)) {
     symbol = lnk_parsed_symbol_from_coff_symbol_idx(obj, symbol_idx);
 
-    if (task->u.patch_symtabs.was_symbol_patched[obj_idx][symbol_idx]) {
+    if (task->u.patch_symtabs.was_symbol_patched[obj_idx][symbol_idx]) { continue; }
-      continue;
-    }
 
     COFF_SymbolValueInterpType interp = coff_interp_symbol(symbol.section_number, symbol.value, symbol.storage_class);
     if (interp == COFF_SymbolValueInterp_Regular) {
-      COFF_SectionHeader *sect_header = lnk_coff_section_header_from_section_number(obj, symbol.section_number);
-
       LNK_SectionContrib *sc = task->sect_map[obj_idx][symbol.section_number-1];
       U32                 section_number;
       U32                 value;

src/linker/lnk_obj.c

@@ -486,19 +486,15 @@ lnk_symlinks_from_obj(Arena *arena, LNK_SymbolTable *symtab, LNK_Obj *obj)
     if (symbol.storage_class == COFF_SymStorageClass_External && symbol.aux_symbol_count == 0) {
       B32 can_set_symlink = (symlink->obj == 0 || symbol.value == 0);
       if (!can_set_symlink && symlink->obj == obj) {
-        // NOTE: The section definition is a fallback symlink. Public symbols inside a
+        COFF_ParsedSymbol leader = lnk_parsed_symbol_from_coff_symbol_idx(symlink->obj, symlink->symbol_idx);
-        // discarded COMDAT need to target the selected public symbol's offset,
+        B32 leader_is_same_section  = leader.section_number == symbol.section_number;
-        // not the selected section base plus the discarded symbol's offset.
+        B32 leader_is_static_anchor = (leader_is_same_section && leader.storage_class == COFF_SymStorageClass_Static && leader.aux_symbol_count == 0);
-        // MSVC vftables are also preferred over other public symbols in the
+        B32 leader_is_vftable       = str8_starts_with(leader.name, str8_lit(MSCRT_VFTABLE_SYMBOL_PREFIX));
-        // same section so ICF can keep vftable COMDATs in their own color space.
+        B32 current_is_vftable      = str8_starts_with(symbol.name, str8_lit(MSCRT_VFTABLE_SYMBOL_PREFIX));
-        COFF_ParsedSymbol symlink_symbol = lnk_parsed_symbol_from_coff_symbol_idx(symlink->obj, symlink->symbol_idx);
+
-        B32 symlink_is_section_defn = (symlink_symbol.section_number == symbol.section_number &&
+        // prefer public symbols to local static anchors; prefer vftable public
-                                       symlink_symbol.storage_class == COFF_SymStorageClass_Static &&
+        // symbols to other public symbols so ICF keeps vftables in their own color space
-                                       symlink_symbol.aux_symbol_count > 0);
+        can_set_symlink = (leader_is_static_anchor || (leader_is_same_section && current_is_vftable && !leader_is_vftable));
-        B32 symlink_is_non_vftable = (symlink_symbol.section_number == symbol.section_number &&
-                                      !str8_starts_with(symlink_symbol.name, str8_lit(MSCRT_VFTABLE_SYMBOL_PREFIX)));
-        B32 symbol_is_vftable = str8_starts_with(symbol.name, str8_lit(MSCRT_VFTABLE_SYMBOL_PREFIX));
-        can_set_symlink = (symlink_is_section_defn || (symbol_is_vftable && symlink_is_non_vftable));
       }
 
       if (can_set_symlink) {
@@ -507,15 +503,17 @@ lnk_symlinks_from_obj(Arena *arena, LNK_SymbolTable *symtab, LNK_Obj *obj)
           *symlink = lnk_ref_from_symbol(link_symbol->symbol);
         }
       }
-
-      continue;
     }
-
+    // static symbols
-    // section definitions
+    else if (symbol.storage_class == COFF_SymStorageClass_Static) {
-    if (symlink->obj == 0 && symbol.storage_class == COFF_SymStorageClass_Static && symbol.aux_symbol_count > 0) {
+      if (symbol.aux_symbol_count == 0) {
-      *symlink = (LNK_ObjSymbolRef){ obj, symbol_idx };
+        if (symlink->obj == 0) {
+          *symlink = (LNK_ObjSymbolRef){ obj, symbol_idx };
+        }
+      }
     }
   }
+
   return symlinks;
 }
 

src/linker/tests/linker_tests.c

@@ -3510,30 +3510,18 @@ TEST(icf_vftable_external_symbol_at_nonzero_offset)
   T_Ok(actual_lea_target == b_vftable_ptr - pe.image_base);
 }
 
+// A referenced zero-sized COMDAT symbol is meaningful enough for
+// relocations, even though the COMDAT contributes no bytes to the image.
 TEST(zero_length_comdat_referenced_by_reloc)
 {
-  if (t_id_linker() != Linker_radlink) { return; }
-
   U8 data[8] = {0};
 
   T_Ok(t_write_entry_obj());
-  T_Ok(t_write_def_obj("leader.obj", (T_COFF_DefObj){
-    .machine = T_COFF_DefSetMachine(X64),
-    .sections = (T_COFF_DefSection[]){
-      { "empty", ".rdata", str8_zero(), .flags = "r:data@1", .raw_flags = COFF_SectionFlag_LnkCOMDAT },
-      {0}
-    },
-    .symbols = (T_COFF_DefSymbol[]){
-      T_COFF_DefSymbol_Secdef("empty", COFF_ComdatSelect_Any),
-      T_COFF_DefSymbol_Extern("EMPTY", "empty", 0),
-      {0}
-    }
-  }));
-
   T_Ok(t_write_def_obj("ref.obj", (T_COFF_DefObj){
     .machine = T_COFF_DefSetMachine(X64),
     .sections = (T_COFF_DefSection[]){
-      { "empty", ".rdata", str8_zero(), .flags = "r:data@1", .raw_flags = COFF_SectionFlag_LnkCOMDAT },
+      { "pad", ".rdata$a", str8_lit("xy"), .flags = "r:data@1" },
+      { "empty", ".rdata$b", str8_zero(), .flags = "r:data@1", .raw_flags = COFF_SectionFlag_LnkCOMDAT },
       { "data", ".data", str8_array_fixed(data), .flags = "rw:data", .relocs = (T_COFF_DefReloc[]){
         T_COFF_DefReloc(X64_Addr64, 0, "EMPTY"),
         {0}
@@ -3547,8 +3535,26 @@ TEST(zero_length_comdat_referenced_by_reloc)
     }
   }));
 
-  t_invoke_linkerf("/subsystem:console /entry:entry /out:a.exe /opt:ref entry.obj leader.obj ref.obj");
+  t_invoke_linkerf("/subsystem:console /entry:entry /out:a.exe /opt:ref entry.obj ref.obj");
   T_Ok(g_last_exit_code == 0);
+
+  String8             exe           = t_read_file(arena, str8_lit("a.exe"));
+  PE_BinInfo          pe            = pe_bin_info_from_data(arena, exe);
+  COFF_SectionHeader *section_table = (COFF_SectionHeader *)str8_substr(exe, pe.section_table_range).str;
+  COFF_SectionHeader *rdata_section = coff_section_header_from_name(str8_zero(), section_table, pe.section_count, str8_lit(".rdata"));
+  COFF_SectionHeader *data_section  = coff_section_header_from_name(str8_zero(), section_table, pe.section_count, str8_lit(".data"));
+  T_Ok(rdata_section != 0);
+  T_Ok(data_section != 0);
+
+  U64 empty_va = 0;
+  str8_deserial_read_struct(exe, data_section->foff, &empty_va);
+  B32 empty_after_pad = empty_va >= pe.image_base + rdata_section->voff + 2;
+  T_Ok(empty_after_pad);
+
+  U64 empty_off_in_rdata = empty_va - pe.image_base - rdata_section->voff;
+  T_Ok(rdata_section->foff + empty_off_in_rdata <= exe.size);
+  T_Ok(exe.str[rdata_section->foff + empty_off_in_rdata - 2] == 'x');
+  T_Ok(exe.str[rdata_section->foff + empty_off_in_rdata - 1] == 'y');
 }
 
 TEST(zero_length_static_comdat_referenced_by_reloc)
@@ -4270,7 +4276,7 @@ TEST(reloc_against_removed_comdat)
   }));
 
   t_invoke_linkerf("/subsystem:console /entry:entry /out:a.exe a.obj b.obj entry.obj");
-  T_Ok(g_last_exit_code == LNK_Error_RelocationAgainstRemovedSection);
+  T_Ok(g_last_exit_code == 0);
 }
 
 TEST(sect_align)
@@ -8874,25 +8880,17 @@ TEST(icf_llvm_addrsig)
   t_invoke(t_clang_path(), str8f(arena, "%S -o %S -c -ffunction-sections -target x86_64-pc-windows-msvc", main_path, main_obj_path), max_U64);
   T_Ok(g_last_exit_code == 0);
 
+  String8 a_path = t_make_file_path(arena, str8_lit("a.exe"));
+
   t_invoke_linkerf("%S /opt:icf /out:a.exe libcmt.lib", main_obj_path);
   T_Ok(g_last_exit_code == 0);
-
-  String8 a_path = t_make_file_path(arena, str8_lit("a.exe"));
   t_invoke(a_path, str8_zero(), max_U64);
-  if (t_id_linker() == Linker_radlink) {
+  T_Ok(g_last_exit_code == 1);
-    T_Ok(g_last_exit_code == 1);
-  } else {
-    T_Ok(g_last_exit_code == 0);
-  }
-
-  if (t_id_linker() == Linker_radlink) {
-    t_invoke_linkerf("%S /opt:icf /out:a.exe libcmt.lib /llvm_addrsig:no", main_obj_path);
-    T_Ok(g_last_exit_code == 0);
-
-    t_invoke(a_path, str8_zero(), max_U64);
-    T_Ok(g_last_exit_code == 0);
-  }
 
+  t_invoke_linkerf("%S /opt:icf /out:a.exe libcmt.lib /llvm_addrsig:no", main_obj_path);
+  T_Ok(g_last_exit_code == 0);
+  t_invoke(a_path, str8_zero(), max_U64);
+  T_Ok(g_last_exit_code == 0);
 }
 
 // .llvm_addrsig can name an undefined external whose definition is in another