radlink: relax rules for relocations pointing into removed associative sections

src/linker/lnk.c

@@ -4392,6 +4392,11 @@ THREAD_POOL_TASK_FUNC(lnk_patch_regular_symbols_task)
       if (sc == task->null_sc) {
         section_number = lnk_obj_get_removed_section_number(obj);
         value          = max_U32;
+        
+        COFF_ComdatSelectType selection = COFF_ComdatSelect_Null;
+        if (lnk_try_comdat_props_from_section_number(obj, symbol.section_number, &selection, 0, 0, 0) && selection == COFF_ComdatSelect_Associative) {
+          value = LNK_REMOVED_ASSOCIATIVE_SYMBOL_VALUE;
+        }
       } else {
         section_number = safe_cast_u32(sc->u.sect_idx + 1);
         value          = sc->u.off + symbol.value;
@@ -4429,7 +4434,7 @@ lnk_patch_obj_symtab(LNK_SymbolTable *symtab, LNK_Obj *obj, B8 *was_symbol_patch
       U32 value;
       if (was_fixup_removed || fixup_type == COFF_SymbolValueInterp_Undefined || fixup_type == COFF_SymbolValueInterp_Weak) {
         section_number = lnk_obj_get_removed_section_number(obj);
-        value          = 0;
+        value          = was_fixup_removed && fixup_src.value == LNK_REMOVED_ASSOCIATIVE_SYMBOL_VALUE ? LNK_REMOVED_ASSOCIATIVE_SYMBOL_VALUE : 0;
       } else {
         section_number = fixup_src.section_number;
         value          = fixup_src.value;
@@ -4562,16 +4567,35 @@ THREAD_POOL_TASK_FUNC(lnk_obj_reloc_patcher)
         COFF_SymbolValueInterpType interp = coff_interp_from_parsed_symbol(symbol);
         if (interp == COFF_SymbolValueInterp_Regular) {
           if (symbol.section_number == lnk_obj_get_removed_section_number(obj)) {
-            if (~section_flags & LNK_SECTION_FLAG_DEBUG) {
-              String8 sect_name   = coff_name_from_section_header(string_table, section_header);
-              String8 symbol_name = lnk_symbol_name_from_coff_symbol_idx(obj, reloc->isymbol);
-              lnk_error_obj(LNK_Error_RelocationAgainstRemovedSection, obj, "relocating against symbol that is in a removed section (symbol: %S, reloc-section: %S 0x%llx, reloc-index: 0x%llx)", symbol_name, sect_name, it.v.section_number, reloc_idx);
+
+            // With /OPT:REF, linkers may partially discard functions. In this case, relocations referencing those symbols must be left unchanged.
+            // the rule is to leave the relocations to those symbols as they are.
+            if (section_flags & LNK_SECTION_FLAG_DEBUG) { continue; }
+
+            {
+              Temp scratch = scratch_begin(0,0);
+
+              String8 sect_name     = coff_name_from_section_header(string_table, section_header);
+              String8 symbol_name   = lnk_symbol_name_from_coff_symbol_idx(obj, reloc->isymbol);
+              String8 error_message = str8f(scratch.arena, "relocating against symbol that is in a removed section (symbol: %S, reloc-section: %S 0x%llx, reloc-index: 0x%llx)", symbol_name, sect_name, it.v.section_number, reloc_idx);
+
+              // MSVC allows relocations to reference discarded associative sections
+              if (symbol.value == LNK_REMOVED_ASSOCIATIVE_SYMBOL_VALUE) {
+                lnk_error_obj(LNK_Warning_RelocationAgainstRemovedAssociativeSection, obj, "%S", error_message);
+                goto next_reloc;
+              }
+
+              lnk_error_obj(LNK_Error_RelocationAgainstRemovedSection, obj, "%S", error_message);
+              goto next_reloc;
+
+              next_reloc:;
+                         scratch_end(scratch);
             }
-            continue;
+          } else {
+            symbol_secnum = symbol.section_number;
+            symbol_secoff = symbol.value;
+            symbol_voff   = safe_cast_u32((U64)task->image_section_table[symbol.section_number]->voff + (U64)symbol_secoff);
           }
-          symbol_secnum = symbol.section_number;
-          symbol_secoff = symbol.value;
-          symbol_voff   = safe_cast_u32((U64)task->image_section_table[symbol.section_number]->voff + (U64)symbol_secoff);
         } else if (interp == COFF_SymbolValueInterp_Abs) {
           // There aren't enough bits in COFF symbol to store full image base address,
           // so we special case __ImageBase. A better solution would be to add

src/linker/lnk.h

@@ -117,8 +117,9 @@ typedef struct LNK_LinkResult
 
 // -- Image Layout ------------------------------------------------------------
 
-#define LNK_REMOVED_SECTION_NUMBER_32 (U32)-3
-#define LNK_REMOVED_SECTION_NUMBER_16 (U16)-3
+#define LNK_REMOVED_ASSOCIATIVE_SYMBOL_VALUE ((U32)-4)
+#define LNK_REMOVED_SECTION_NUMBER_32        ((U32)-3)
+#define LNK_REMOVED_SECTION_NUMBER_16        ((U16)-3)
 
 typedef struct LNK_ImageContext
 {

src/linker/lnk_config.c

@@ -2845,6 +2845,7 @@ lnk_make_default_cmd_line(Arena *arena, LNK_CmdLine user_cmd_line)
     "/RAD_LOG:io_write",
     #else
     (char*)str8f(scratch.arena, "/RAD_IGNORE:%u", LNK_Error_InvalidTypeIndex).str,
+    (char*)str8f(scratch.arena, "/RAD_IGNORE:%u", LNK_Warning_RelocationAgainstRemovedAssociativeSection).str,
     #endif
   };
 

src/linker/lnk_log.h

@@ -119,6 +119,7 @@ typedef enum
   LNK_Warning_GHash,
   LNK_Warning_UndefinedSectionSymbol,
   LNK_Warning_CyclicSymbol,
+  LNK_Warning_RelocationAgainstRemovedAssociativeSection,
   LNK_Warning_Last,
   
   LNK_Error_Count

src/linker/tests/linker_tests.c

@@ -9841,6 +9841,105 @@ TEST(icf_cpp_multihop_functions_do_not_fold)
 
 #endif
 
+TEST(asan_discarded_associative_metadata)
+{
+  // ASan can share a filename across metadata belonging to different COMDATs.
+  // link.exe tombstones references to a discarded associative child at RVA 0;
+  // it does not redirect them to the winning owner's child or retain the loser.
+
+  U8 pointers[24] = {5};
+  pointers[8]  = 7;
+  pointers[12] = 9;
+  pointers[16] = 2;
+  pointers[20] = 11;
+
+  for EachIndex(obj_idx, 2) {
+    T_Ok(t_write_def_obj(obj_idx ? "second.obj" : "first.obj",
+                         ((T_COFF_DefObj){
+      .machine = T_COFF_DefSetMachine(X64),
+
+      .sections = (T_COFF_DefSection[]){
+        {"owner", ".owner", obj_idx ? str8_lit("OWNER_B")    : str8_lit("OWNER_A"),   .flags = "r:data@1", .raw_flags = COFF_SectionFlag_LnkCOMDAT},
+        {"meta",  ".names", obj_idx ? str8_lit("SECOND.c\0") : str8_lit("FIRST.c\0"), .flags = "r:data@1", .raw_flags = COFF_SectionFlag_LnkCOMDAT},
+        {"entry", ".text", str8_lit("\xC3"), .flags = "rx:code@1"},
+        {obj_idx ? "pointers" : 0, ".data", str8_array_fixed(pointers), .flags = "rw:data@8",
+          .relocs = (T_COFF_DefReloc[]){
+            T_COFF_DefReloc(X64_Addr64,   0,  "filename"),
+            T_COFF_DefReloc(X64_Addr32Nb, 8,  "filename"),
+            T_COFF_DefReloc(X64_SecRel,   12, "filename"),
+            T_COFF_DefReloc(X64_Section,  16, "filename"),
+            T_COFF_DefReloc(X64_Rel32,    20, "filename"),
+            {0}
+          }
+        },
+        {0}
+      },
+
+      .symbols = (T_COFF_DefSymbol[]){
+        {.type = obj_idx ? T_COFF_DefSymbol_Static : T_COFF_DefSymbol_ExternFunc, .name = "entry", .section = "entry"},
+        T_COFF_DefSymbol_Secdef("owner", COFF_ComdatSelect_Any),
+        T_COFF_DefSymbol_Extern("shared", "owner", 0),
+        T_COFF_DefSymbol_Associative("meta", "owner"),
+        T_COFF_DefSymbol_Static("filename", "meta", 3),
+        {.type = obj_idx ? T_COFF_DefSymbol_Extern : 0, .name = "pointers", .section = "pointers"},
+        {0}
+      }
+
+    })));
+  }
+
+  for EachIndex(reverse, 2) {
+    for EachIndex(ref, 2) {
+      t_invoke_linkerf("/subsystem:console /entry:entry /nodefaultlib /fixed:no /include:shared /include:pointers /out:assoc.exe /opt:%s,noicf %s",
+                       ref     ? "ref" : "noref",
+                       reverse ? "second.obj first.obj" : "first.obj second.obj");
+      T_Ok(g_last_exit_code == 0);
+
+      String8             image        = t_read_file(arena, str8_lit("assoc.exe"));
+      PE_BinInfo          bin          = pe_bin_info_from_data(arena, image);
+      COFF_SectionHeader *sections     = (COFF_SectionHeader *)(image.str + bin.section_table_range.min);
+      COFF_SectionHeader *data         = 0;
+      COFF_SectionHeader *names        = 0;
+      COFF_SectionHeader *owner        = 0;
+      COFF_SectionHeader *base_relocs  = 0;
+      U32                 names_number = 0;
+
+      for EachIndex(i, bin.section_count) {
+        if (MemoryMatch(sections[i].name, ".data", 6))  { data = &sections[i];                      }
+        if (MemoryMatch(sections[i].name, ".names", 7)) { names = &sections[i]; names_number = i+1; }
+        if (MemoryMatch(sections[i].name, ".owner", 7)) { owner = &sections[i];                     }
+        if (MemoryMatch(sections[i].name, ".reloc", 7)) { base_relocs = &sections[i];               }
+      }
+
+      T_Ok(data && names && owner && base_relocs);
+      T_Ok(MemoryMatch(image.str + owner->foff, reverse ? "OWNER_B" : "OWNER_A", 7));
+      T_Ok(MemoryMatch(image.str + names->foff, reverse ? "SECOND.c" : "FIRST.c", reverse ? 8 : 7));
+
+      U64 addr64;
+      U32 addr32nb, secrel, rel32; U16 section;
+      MemoryCopy(&addr64,   image.str + data->foff, 8);
+      MemoryCopy(&addr32nb, image.str + data->foff + 8, 4);
+      MemoryCopy(&secrel,   image.str + data->foff + 12, 4);
+      MemoryCopy(&section,  image.str + data->foff + 16, 2);
+      MemoryCopy(&rel32,    image.str + data->foff + 20, 4);
+
+      U32 target_voff = reverse ? names->voff + 3 : 0;
+      T_Ok(addr64   == bin.image_base + target_voff + 5);
+      T_Ok(addr32nb == target_voff + 7);
+      T_Ok(secrel   == (reverse ? 3 : 0) + 9);
+      T_Ok(section  == (reverse ? names_number : 0) + 2);
+      T_Ok(rel32    == (target_voff - data->voff - 24 + 11));
+
+      // Even the tombstoned ADDR64 still needs loader rebasing.
+      U32 page;
+      U16 fixup;
+      MemoryCopy(&page, image.str + base_relocs->foff, 4);
+      MemoryCopy(&fixup, image.str + base_relocs->foff + 8, 2);
+      T_Ok((fixup >> 12) == 10 && page + (fixup & 0xfff) == data->voff);
+    }
+  }
+}
+
 #if 0
 TEST(defer_imp_link)
 {