From 3a12d536a474dc0c4b1df2eb35520a6c252929de Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Thu, 6 Nov 2025 12:18:16 -0800 Subject: [PATCH 1/5] [sanitizer-common] [Darwin] Fix overlapping dyld segment addresses (#166005) This fixes two problems: - dyld itself resides within the shared cache. MemoryMappingLayout incorrectly computes the slide for dyld's segments, causing them to (appear to) overlap with other modules. This can cause symbolication issues. - The MemoryMappingLayout ranges on Darwin are not disjoint due to the fact that the LINKEDIT segments overlap for each module. We now ignore these segments to ensure the mapping is disjoint. This adds a check for disjointness, and a runtime warning if this is ever violated (as that suggests issues in the sanitizer memory mapping). There is now a test to ensure that these problems do not recur. rdar://163149325 (cherry picked from commit e33098555132439d0ee5dfdd8fef31d7177ee68c) --- .../sanitizer_procmaps_mac.cpp | 78 +++++++++++++++---- .../Darwin/asan-verify-module-map.cpp | 25 ++++++ 2 files changed, 86 insertions(+), 17 deletions(-) create mode 100644 compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index a5ec85ae16460..72f4bbf212f9a 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -45,7 +45,6 @@ struct MemoryMappedSegmentData { const char *current_load_cmd_addr; u32 lc_type; uptr base_virt_addr; - uptr addr_mask; }; template @@ -54,12 +53,58 @@ static void NextSectionLoad(LoadedModule *module, MemoryMappedSegmentData *data, const Section *sc = (const Section *)data->current_load_cmd_addr; data->current_load_cmd_addr += sizeof(Section); - uptr sec_start = (sc->addr & data->addr_mask) + data->base_virt_addr; + uptr sec_start = sc->addr + data->base_virt_addr; uptr sec_end = sec_start + sc->size; module->addAddressRange(sec_start, sec_end, /*executable=*/false, isWritable, sc->sectname); } +static bool VerifyMemoryMapping(MemoryMappingLayout* mapping) { + InternalMmapVector modules; + modules.reserve(128); // matches DumpProcessMap + mapping->DumpListOfModules(&modules); + + InternalMmapVector segments; + for (uptr i = 0; i < modules.size(); ++i) { + for (auto& range : modules[i].ranges()) { + segments.push_back(range); + } + } + + // Verify that none of the segments overlap: + // 1. Sort the segments by the start address + // 2. Check that every segment starts after the previous one ends. + Sort(segments.data(), segments.size(), + [](LoadedModule::AddressRange& a, LoadedModule::AddressRange& b) { + return a.beg < b.beg; + }); + + // To avoid spam, we only print the report message once-per-process. + static bool invalid_module_map_reported = false; + bool well_formed = true; + + for (size_t i = 1; i < segments.size(); i++) { + uptr cur_start = segments[i].beg; + uptr prev_end = segments[i - 1].end; + if (cur_start < prev_end) { + well_formed = false; + VReport(2, "Overlapping mappings: %s start = %p, %s end = %p\n", + segments[i].name, (void*)cur_start, segments[i - 1].name, + (void*)prev_end); + if (!invalid_module_map_reported) { + Report( + "WARN: Invalid dyld module map detected. This is most likely a bug " + "in the sanitizer.\n"); + Report("WARN: Backtraces may be unreliable.\n"); + invalid_module_map_reported = true; + } + } + } + + mapping->Reset(); + return well_formed; +} + void MemoryMappedSegment::AddAddressRanges(LoadedModule *module) { // Don't iterate over sections when the caller hasn't set up the // data pointer, when there are no sections, or when the segment @@ -85,6 +130,7 @@ void MemoryMappedSegment::AddAddressRanges(LoadedModule *module) { MemoryMappingLayout::MemoryMappingLayout(bool cache_enabled) { Reset(); + VerifyMemoryMapping(this); } MemoryMappingLayout::~MemoryMappingLayout() { @@ -190,6 +236,7 @@ typedef struct dyld_shared_cache_dylib_text_info extern bool _dyld_get_shared_cache_uuid(uuid_t uuid); extern const void *_dyld_get_shared_cache_range(size_t *length); +extern intptr_t _dyld_get_image_slide(const struct mach_header* mh); extern int dyld_shared_cache_iterate_text( const uuid_t cacheUuid, void (^callback)(const dyld_shared_cache_dylib_text_info *info)); @@ -258,23 +305,21 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, layout_data->current_load_cmd_count--; if (((const load_command *)lc)->cmd == kLCSegment) { const SegmentCommand* sc = (const SegmentCommand *)lc; - uptr base_virt_addr, addr_mask; - if (layout_data->current_image == kDyldImageIdx) { - base_virt_addr = (uptr)get_dyld_hdr(); - // vmaddr is masked with 0xfffff because on macOS versions < 10.12, - // it contains an absolute address rather than an offset for dyld. - // To make matters even more complicated, this absolute address - // isn't actually the absolute segment address, but the offset portion - // of the address is accurate when combined with the dyld base address, - // and the mask will give just this offset. - addr_mask = 0xfffff; - } else { + if (strncmp(sc->segname, "__LINKEDIT", sizeof("__LINKEDIT")) == 0) { + // The LINKEDIT sections are for internal linker use, and may alias + // with the LINKEDIT section for other modules. (If we included them, + // our memory map would contain overlappping sections.) + return false; + } + + uptr base_virt_addr; + if (layout_data->current_image == kDyldImageIdx) + base_virt_addr = (uptr)_dyld_get_image_slide(get_dyld_hdr()); + else base_virt_addr = (uptr)_dyld_get_image_vmaddr_slide(layout_data->current_image); - addr_mask = ~0; - } - segment->start = (sc->vmaddr & addr_mask) + base_virt_addr; + segment->start = sc->vmaddr + base_virt_addr; segment->end = segment->start + sc->vmsize; // Most callers don't need section information, so only fill this struct // when required. @@ -284,7 +329,6 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, (const char *)lc + sizeof(SegmentCommand); seg_data->lc_type = kLCSegment; seg_data->base_virt_addr = base_virt_addr; - seg_data->addr_mask = addr_mask; internal_strncpy(seg_data->name, sc->segname, ARRAY_SIZE(seg_data->name)); } diff --git a/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp b/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp new file mode 100644 index 0000000000000..7660841c72877 --- /dev/null +++ b/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp @@ -0,0 +1,25 @@ +// This test simply checks that the "Invalid dyld module map" warning is not printed +// in the output of a backtrace. + +// RUN: %clangxx_asan -DSHARED_LIB -g %s -dynamiclib -o %t.dylib +// RUN: %clangxx_asan -O0 -g %s %t.dylib -o %t.executable +// RUN: %env_asan_opts="print_module_map=2" not %run %t.executable 2>&1 | FileCheck %s -DDYLIB=%t.dylib + +// CHECK-NOT: WARN: Invalid dyld module map +// CHECK-DAG: 0x{{.*}}-0x{{.*}} [[DYLIB]] +// CHECK-DAG: 0x{{.*}}-0x{{.*}} {{.*}}libsystem + +#ifdef SHARED_LIB +extern "C" void foo(int *a) { *a = 5; } +#else +# include + +extern "C" void foo(int *a); + +int main() { + int *a = (int *)malloc(sizeof(int)); + free(a); + foo(a); + return 0; +} +#endif \ No newline at end of file From c17c3bc9fc1876f65527617b7f709d2052e29041 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Wed, 12 Nov 2025 16:47:30 -0800 Subject: [PATCH 2/5] Ensure string termination, fix LoadedModule leak --- compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index 72f4bbf212f9a..fb480e42b127e 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -101,6 +101,8 @@ static bool VerifyMemoryMapping(MemoryMappingLayout* mapping) { } } + for (auto& m : modules) m.clear(); + mapping->Reset(); return well_formed; } @@ -331,6 +333,7 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, seg_data->base_virt_addr = base_virt_addr; internal_strncpy(seg_data->name, sc->segname, ARRAY_SIZE(seg_data->name)); + seg_data->name[ARRAY_SIZE(seg_data->name) - 1] = 0; } // Return the initial protection. @@ -344,6 +347,7 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, ? kDyldPath : _dyld_get_image_name(layout_data->current_image); internal_strncpy(segment->filename, src, segment->filename_size); + segment->filename[segment->filename_size - 1] = 0; } segment->arch = layout_data->current_arch; internal_memcpy(segment->uuid, layout_data->current_uuid, kModuleUUIDSize); From 6d14b1958a8a26d9b4db5441d29b5a2b7496bf22 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Wed, 12 Nov 2025 16:51:03 -0800 Subject: [PATCH 3/5] Make test more robust --- .../test/asan/TestCases/Darwin/asan-verify-module-map.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp b/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp index 7660841c72877..15be1cd6754c3 100644 --- a/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp +++ b/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp @@ -3,10 +3,10 @@ // RUN: %clangxx_asan -DSHARED_LIB -g %s -dynamiclib -o %t.dylib // RUN: %clangxx_asan -O0 -g %s %t.dylib -o %t.executable -// RUN: %env_asan_opts="print_module_map=2" not %run %t.executable 2>&1 | FileCheck %s -DDYLIB=%t.dylib +// RUN: %env_asan_opts="print_module_map=2" not %run %t.executable 2>&1 | FileCheck %s -DDYLIB=%{t:stem}.tmp.dylib // CHECK-NOT: WARN: Invalid dyld module map -// CHECK-DAG: 0x{{.*}}-0x{{.*}} [[DYLIB]] +// CHECK-DAG: 0x{{.*}}-0x{{.*}} {{.*}}[[DYLIB]] // CHECK-DAG: 0x{{.*}}-0x{{.*}} {{.*}}libsystem #ifdef SHARED_LIB From 6d9a10f0eade6dd18afe154713203dff7dc115ab Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Wed, 12 Nov 2025 16:53:52 -0800 Subject: [PATCH 4/5] strncmp => internal_strncmp --- compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index fb480e42b127e..122f11ba523ca 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -307,7 +307,8 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, layout_data->current_load_cmd_count--; if (((const load_command *)lc)->cmd == kLCSegment) { const SegmentCommand* sc = (const SegmentCommand *)lc; - if (strncmp(sc->segname, "__LINKEDIT", sizeof("__LINKEDIT")) == 0) { + if (internal_strncmp(sc->segname, "__LINKEDIT", sizeof("__LINKEDIT")) == + 0) { // The LINKEDIT sections are for internal linker use, and may alias // with the LINKEDIT section for other modules. (If we included them, // our memory map would contain overlappping sections.) From 05f2dd7c576af9f08eaa4cfdce3ec19d786728db Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Thu, 13 Nov 2025 13:31:48 -0800 Subject: [PATCH 5/5] nits --- compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index 122f11ba523ca..f40fba6bf7151 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -307,8 +307,7 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, layout_data->current_load_cmd_count--; if (((const load_command *)lc)->cmd == kLCSegment) { const SegmentCommand* sc = (const SegmentCommand *)lc; - if (internal_strncmp(sc->segname, "__LINKEDIT", sizeof("__LINKEDIT")) == - 0) { + if (internal_strcmp(sc->segname, "__LINKEDIT") == 0) { // The LINKEDIT sections are for internal linker use, and may alias // with the LINKEDIT section for other modules. (If we included them, // our memory map would contain overlappping sections.)