handle COMDAT symbols with non-zero section offset

src/linker/lnk.c

@@ -2404,70 +2404,6 @@ THREAD_POOL_TASK_FUNC(lnk_flag_debug_symbols_task)
 
 internal
 THREAD_POOL_TASK_FUNC(lnk_set_comdat_leaders_task)
-{
-  LNK_BuildImageTask *task    = raw_task;
-  U64                 obj_idx = task_id;
-  LNK_Obj            *obj     = task->objs[obj_idx];
-
-  ProfBeginV("Set COMDAT Leaders [%S]", obj->path);
-  COFF_ParsedSymbol symbol;
-  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);
-    COFF_SymbolValueInterpType interp = coff_interp_symbol(symbol.section_number, symbol.value, symbol.storage_class);
-    if (interp == COFF_SymbolValueInterp_Regular && symbol.storage_class == COFF_SymStorageClass_External) {
-      COFF_SectionHeader *sect_header = lnk_coff_section_header_from_section_number(obj, symbol.section_number);
-      if (sect_header->flags & COFF_SectionFlag_LnkCOMDAT) {
-        LNK_Symbol *defn = lnk_symbol_table_search(task->symtab, LNK_SymbolScope_Defined, symbol.name);
-        COFF_ParsedSymbol defn_symbol = lnk_parsed_symbol_from_coff_symbol_idx(defn->u.defined.obj, defn->u.defined.symbol_idx);
-        task->sect_map[obj_idx][symbol.section_number - 1] = task->sect_map[defn->u.defined.obj->input_idx][defn_symbol.section_number - 1];
-      }
-    }
-  }
-  ProfEnd();
-}
-
-internal
-THREAD_POOL_TASK_FUNC(lnk_patch_comdat_leaders_task)
-{
-  LNK_BuildImageTask *task    = raw_task;
-  U64                 obj_idx = task_id;
-  LNK_Obj            *obj     = task->objs[obj_idx];
-
-  ProfBeginV("Patch COMDAT Leaders [%S]", obj->path);
-  COFF_ParsedSymbol symbol;
-  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]) { continue; }
-
-    COFF_SymbolValueInterpType interp = coff_interp_symbol(symbol.section_number, symbol.value, symbol.storage_class);
-    if (interp == COFF_SymbolValueInterp_Regular && symbol.storage_class == COFF_SymStorageClass_External) {
-      COFF_SectionHeader *sect_header = lnk_coff_section_header_from_section_number(obj, symbol.section_number);
-      if (sect_header->flags & COFF_SectionFlag_LnkCOMDAT) {
-        LNK_Symbol *defn = lnk_symbol_table_search(task->symtab, LNK_SymbolScope_Defined, symbol.name);
-
-        if (defn->u.defined.obj == obj && defn->u.defined.symbol_idx == symbol_idx) {
-          LNK_SectionContrib *sc = task->sect_map[obj_idx][symbol.section_number-1];
-          if (obj->header.is_big_obj) {
-            COFF_Symbol32 *symbol32 = symbol.raw_symbol;
-            symbol32->section_number = sc->u.sect_idx+1;
-            symbol32->value = safe_cast_u32(sc->u.off + symbol.value);
-          } else {
-            COFF_Symbol16 *symbol16 = symbol.raw_symbol;
-            symbol16->section_number = safe_cast_u16(sc->u.sect_idx+1);
-            symbol16->value = safe_cast_u32(sc->u.off + symbol.value);
-          }
-
-          task->u.patch_symtabs.was_symbol_patched[obj_idx][symbol_idx] = 1;
-        }
-      }
-    }
-  }
-  ProfEnd();
-}
-
-internal
-THREAD_POOL_TASK_FUNC(lnk_patch_replaced_comdats_task)
 {
   Temp scratch = scratch_begin(&arena, 1);
 
@@ -2477,9 +2413,8 @@ THREAD_POOL_TASK_FUNC(lnk_patch_replaced_comdats_task)
 
   ProfBeginV("%S", obj->path);
 
-  LNK_Symbol **symlinks = push_array(scratch.arena, LNK_Symbol *, obj->header.section_count_no_null+1);
-
   ProfBegin("Build Symlinks");
+  LNK_Symbol **symlinks = push_array(scratch.arena, LNK_Symbol *, obj->header.section_count_no_null+1);
   {
     COFF_ParsedSymbol symbol;
     for (U64 symbol_idx = 0; symbol_idx < obj->header.symbol_count; symbol_idx += (1 + symbol.aux_symbol_count)) {
@@ -2497,54 +2432,59 @@ THREAD_POOL_TASK_FUNC(lnk_patch_replaced_comdats_task)
   }
   ProfEnd();
 
-  ProfBegin("Patch");
+  ProfBeginV("Set COMDAT Section Contribs");
-  {
+  for (U64 sect_idx = 0; sect_idx < obj->header.section_count_no_null; sect_idx += 1) {
+    COFF_SectionHeader *section_header = lnk_coff_section_header_from_section_number(obj, sect_idx+1);
+    if (section_header->flags & COFF_SectionFlag_LnkCOMDAT) {
+      LNK_Symbol *sym = symlinks[sect_idx+1];
+      if (sym) {
+        COFF_ParsedSymbol parsed_sym = lnk_parsed_symbol_from_coff_symbol_idx(sym->u.defined.obj, sym->u.defined.symbol_idx);
+        task->sect_map[obj_idx][sect_idx] = task->sect_map[sym->u.defined.obj->input_idx][parsed_sym.section_number - 1];
+      }
+    }
+  }
+  ProfEnd();
+
+  ProfBegin("Patch COMDAT Offsets");
   COFF_ParsedSymbol symbol;
   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]) { 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_Symbol *symlink = symlinks[symbol.section_number];
-        if (sect_header->flags & COFF_SectionFlag_LnkCOMDAT) {
+      if (symlink && symlink->u.defined.obj != obj) {
-
+        U32 section_number;
-
+        U32 value;
-          LNK_Symbol         *comdat_leader = symlinks[symbol.section_number];
+        if (symbol.storage_class == COFF_SymStorageClass_External) {
-          COFF_ParsedSymbol   comdat_leader_parsed;
+          // COMDAT leader may be at a different offset, so update this symbol with leader's offset
-          LNK_SectionContrib *comdat_leader_sc;
+          COFF_ParsedSymbol parsed_symlink = lnk_parsed_symbol_from_coff_symbol_idx(symlink->u.defined.obj, symlink->u.defined.symbol_idx);
-          if (comdat_leader) {
+          section_number = symbol.section_number;
-            B32 is_this_leader_symbol = comdat_leader->u.defined.obj == obj && comdat_leader->u.defined.symbol_idx == symbol_idx;
+          value          = parsed_symlink.value;
-            if (is_this_leader_symbol) continue;
-
-            comdat_leader_parsed = lnk_parsed_symbol_from_coff_symbol_idx(comdat_leader->u.defined.obj, comdat_leader->u.defined.symbol_idx);
-            comdat_leader_sc     = task->sect_map[comdat_leader->u.defined.obj->input_idx][comdat_leader_parsed.section_number-1];
         } else {
-            Assert(symbol.storage_class != COFF_SymStorageClass_External);
+          // COMDAT section may have static symbols which are now invalid to relocate against
-            comdat_leader_sc = task->sect_map[obj_idx][symbol.section_number-1];
+          section_number = LNK_REMOVED_SECTION_NUMBER_32;
+          value          = max_U32;
+          task->u.patch_symtabs.was_symbol_patched[obj_idx][symbol_idx] = 1;
         }
 
         if (obj->header.is_big_obj) {
           COFF_Symbol32 *symbol32  = symbol.raw_symbol;
-            symbol32->section_number = comdat_leader_sc->u.sect_idx+1;
+          symbol32->section_number = section_number;
-            symbol32->value          = safe_cast_u32(comdat_leader_sc->u.off + symbol.value);
+          symbol32->value          = value;
         } else {
           COFF_Symbol16 *symbol16  = symbol.raw_symbol;
-            symbol16->section_number = safe_cast_u16(comdat_leader_sc->u.sect_idx+1);
+          symbol16->section_number = (U16)section_number;
-            symbol16->value          = safe_cast_u32(comdat_leader_sc->u.off + symbol.value);
+          symbol16->value          = value;
         }
+      }
+    }
+  }
+  ProfEnd();
 
-          task->u.patch_symtabs.was_symbol_patched[obj_idx][symbol_idx] = 1;
-        }
-      }
-    }
-  }
   ProfEnd();
 
   scratch_end(scratch);
-
-  ProfEnd();
 }
 
 internal int
@@ -2638,14 +2578,12 @@ THREAD_POOL_TASK_FUNC(lnk_patch_regular_symbols_task)
     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);
-      Assert(~sect_header->flags & COFF_SectionFlag_LnkCOMDAT);
 
       LNK_SectionContrib *sc = task->sect_map[obj_idx][symbol.section_number-1];
-
       U16                 section_number;
       U32                 value;
       if (sc == &g_null_sc) {
-        section_number = LNK_REMOVED_SECTION_NUMBER;
+        section_number = LNK_REMOVED_SECTION_NUMBER_16;
         value          = max_U32;
       } else {
         section_number = safe_cast_u32(sc->u.sect_idx + 1);
@@ -3049,7 +2987,7 @@ THREAD_POOL_TASK_FUNC(lnk_obj_reloc_patcher)
         COFF_ParsedSymbol          symbol = lnk_parsed_symbol_from_coff_symbol_idx(obj, reloc->isymbol);
         COFF_SymbolValueInterpType interp = coff_interp_symbol(symbol.section_number, symbol.value, symbol.storage_class);
         if (interp == COFF_SymbolValueInterp_Regular) {
-          if (symbol.section_number == LNK_REMOVED_SECTION_NUMBER) {
+          if (symbol.section_number == LNK_REMOVED_SECTION_NUMBER_16 || symbol.section_number == LNK_REMOVED_SECTION_NUMBER_32) {
             if (!lnk_is_coff_section_debug(obj, sect_idx)) {
               String8 sect_name = coff_name_from_section_header(string_table, &section_table[sect_idx]);
               lnk_error_obj(LNK_Error_RelocationAgainstRemovedSection, obj, "relocating against symbol that is in a removed section (symbol: %S, reloc-section: %S 0x%llx, reloc: 0x%llx)", symbol.name, sect_name, sect_idx+1, reloc_idx);
@@ -4389,14 +4327,6 @@ lnk_build_image(TP_Arena *arena, TP_Context *tp, LNK_Config *config, LNK_SymbolT
       tp_for_parallel(tp, 0, objs_count, lnk_set_comdat_leaders_task, &task);
       ProfEnd();
 
-      ProfBegin("Patch replaced COMDATs");
-      tp_for_parallel(tp, 0, objs_count, lnk_patch_replaced_comdats_task, &task);
-      ProfEnd();
-
-      ProfBegin("Patch COMDAT Leaders");
-      tp_for_parallel(tp, 0, objs_count, lnk_patch_comdat_leaders_task, &task);
-      ProfEnd();
-
       ProfBegin("Patch Common Block Leaders");
       tp_for_parallel(tp, 0, tp->worker_count, lnk_patch_common_block_leaders_task, &task);
       ProfEnd();

src/linker/lnk.h

@@ -16,7 +16,8 @@ typedef struct LNK_LinkContext
 
 // -- Image --------------------------------------------------------------------
 
-#define LNK_REMOVED_SECTION_NUMBER (U16)-3
+#define LNK_REMOVED_SECTION_NUMBER_32 (U32)-3
+#define LNK_REMOVED_SECTION_NUMBER_16 (U16)-3
 
 typedef struct LNK_ImageContext
 {

src/torture/torture.c

@@ -2738,6 +2738,57 @@ exit:;
   return result;
 }
 
+internal T_Result
+t_comdat_test(void)
+{
+  Temp scratch = scratch_begin(0,0);
+  T_Result result = T_Result_Fail;
+
+  {
+    COFF_ObjWriter *obj_writer = coff_obj_writer_alloc(0, COFF_MachineType_X64);
+    U8 a[] = "1Hello, World!";
+    COFF_ObjSection *sect = coff_obj_writer_push_section(obj_writer, str8_lit(".rdata"), PE_RDATA_SECTION_FLAGS|COFF_SectionFlag_LnkCOMDAT, str8_array_fixed(a));
+    coff_obj_writer_push_symbol_secdef(obj_writer, sect, COFF_ComdatSelect_Largest);
+    coff_obj_writer_push_symbol_extern(obj_writer, str8_lit("TEST"), 1, sect);
+    String8 obj = coff_obj_writer_serialize(scratch.arena, obj_writer);
+    coff_obj_writer_release(&obj_writer);
+    if (!t_write_file(str8_lit("a.obj"), obj)) { goto exit; }
+  }
+
+  {
+    COFF_ObjWriter *obj_writer = coff_obj_writer_alloc(0, COFF_MachineType_X64);
+    U8 a[] = "Hello, World!";
+    COFF_ObjSection *sect = coff_obj_writer_push_section(obj_writer, str8_lit(".rdata"), PE_RDATA_SECTION_FLAGS|COFF_SectionFlag_LnkCOMDAT, str8_array_fixed(a));
+    coff_obj_writer_push_symbol_secdef(obj_writer, sect, COFF_ComdatSelect_Largest);
+    coff_obj_writer_push_symbol_extern(obj_writer, str8_lit("TEST"), 1, sect);
+    String8 obj = coff_obj_writer_serialize(scratch.arena, obj_writer);
+    coff_obj_writer_release(&obj_writer);
+    if (!t_write_file(str8_lit("b.obj"), obj)) { goto exit; }
+  }
+
+  {
+    COFF_ObjWriter *obj_writer = coff_obj_writer_alloc(0, COFF_MachineType_X64);
+    U8 text[] = {
+      0x48, 0xC7, 0xC0, 0x00, 0x00, 0x00, 0x00,  // mov rax, $imm
+      0xC3 // ret
+    };
+    COFF_ObjSection *sect = coff_obj_writer_push_section(obj_writer, str8_lit(".text"), PE_TEXT_SECTION_FLAGS, str8_array_fixed(text));
+    coff_obj_writer_push_symbol_extern(obj_writer, str8_lit("entry"), 0, sect);
+    COFF_ObjSymbol *symbol = coff_obj_writer_push_symbol_undef(obj_writer, str8_lit("TEST"));
+    coff_obj_writer_section_push_reloc_voff(obj_writer, sect, 3, symbol);
+    String8 obj = coff_obj_writer_serialize(scratch.arena, obj_writer);
+    coff_obj_writer_release(&obj_writer);
+    if (!t_write_file(str8_lit("entry.obj"), obj)) { goto exit; }
+  }
+
+  int linker_exit_code = t_invoke_linkerf("/subsystem:console /entry:entry /out:a.exe a.obj b.obj entry.obj");
+
+exit:;
+  result = T_Result_Pass;
+  scratch_end(scratch);
+  return result;
+}
+
 internal T_Result
 t_sect_align(void)
 {
@@ -3601,6 +3652,7 @@ entry_point(CmdLine *cmdline)
     { "comdat_associative_loop",          t_comdat_associative_loop          },
     { "comdat_associative_non_comdat",    t_comdat_associative_non_comdat    },
     { "comdat_associative_out_of_bounds", t_comdat_associative_out_of_bounds },
+    { "comdat_test",                      t_comdat_test                      },
     { "alt_name",                         t_alt_name                         },
     { "include",                          t_include                          },
     { "communal_var_vs_regular_comdat",   t_communal_var_vs_regular_comdat   },