From 4c972c0fbad7d81052df470caa62ac72e382de0a Mon Sep 17 00:00:00 2001 From: Nikita Smith Date: Tue, 17 Jun 2025 16:46:38 -0700 Subject: [PATCH] handle COMDAT symbols with non-zero section offset --- src/linker/lnk.c | 166 ++++++++++++------------------------------ src/linker/lnk.h | 3 +- src/torture/torture.c | 52 +++++++++++++ 3 files changed, 102 insertions(+), 119 deletions(-) diff --git a/src/linker/lnk.c b/src/linker/lnk.c index 19fb044d..e867573f 100644 --- a/src/linker/lnk.c +++ b/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"); - { - 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); + 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(); - 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); - if (sect_header->flags & COFF_SectionFlag_LnkCOMDAT) { - - - LNK_Symbol *comdat_leader = symlinks[symbol.section_number]; - COFF_ParsedSymbol comdat_leader_parsed; - LNK_SectionContrib *comdat_leader_sc; - if (comdat_leader) { - B32 is_this_leader_symbol = comdat_leader->u.defined.obj == obj && comdat_leader->u.defined.symbol_idx == symbol_idx; - 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_leader_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 = comdat_leader_sc->u.sect_idx+1; - symbol32->value = safe_cast_u32(comdat_leader_sc->u.off + symbol.value); - } else { - COFF_Symbol16 *symbol16 = symbol.raw_symbol; - symbol16->section_number = safe_cast_u16(comdat_leader_sc->u.sect_idx+1); - symbol16->value = safe_cast_u32(comdat_leader_sc->u.off + symbol.value); - } + 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); + COFF_SymbolValueInterpType interp = coff_interp_symbol(symbol.section_number, symbol.value, symbol.storage_class); + if (interp == COFF_SymbolValueInterp_Regular) { + LNK_Symbol *symlink = symlinks[symbol.section_number]; + if (symlink && symlink->u.defined.obj != obj) { + U32 section_number; + U32 value; + if (symbol.storage_class == COFF_SymStorageClass_External) { + // COMDAT leader may be at a different offset, so update this symbol with leader's offset + COFF_ParsedSymbol parsed_symlink = lnk_parsed_symbol_from_coff_symbol_idx(symlink->u.defined.obj, symlink->u.defined.symbol_idx); + section_number = symbol.section_number; + value = parsed_symlink.value; + } else { + // COMDAT section may have static symbols which are now invalid to relocate against + 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 = section_number; + symbol32->value = value; + } else { + COFF_Symbol16 *symbol16 = symbol.raw_symbol; + symbol16->section_number = (U16)section_number; + symbol16->value = value; + } } } } ProfEnd(); - scratch_end(scratch); - ProfEnd(); + + scratch_end(scratch); } 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; + 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, §ion_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(); diff --git a/src/linker/lnk.h b/src/linker/lnk.h index 84c07ce2..68d49ce8 100644 --- a/src/linker/lnk.h +++ b/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 { diff --git a/src/torture/torture.c b/src/torture/torture.c index f79bd702..41c07766 100644 --- a/src/torture/torture.c +++ b/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 },