From c6f470cac3b3b7691a2ebf98172ed43103fbb604 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Sat, 1 Nov 2025 11:31:18 -0700 Subject: [PATCH 1/7] [sanitizer-common] [Darwin] Fix overlapping dyld segment addresses 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. - The MemoryMappingLayout ranges on Darwin are not disjoint due to the fact that the LINKEDIT segments overlap for each module. This makes them 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 --- .../lib/sanitizer_common/sanitizer_procmaps.h | 6 ++ .../sanitizer_procmaps_mac.cpp | 68 ++++++++++++++++--- .../Darwin/asan-verify-module-map.cpp | 20 ++++++ 3 files changed, 86 insertions(+), 8 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.h b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h index d713ddf847dfb..8af0ad70f313f 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h @@ -104,6 +104,12 @@ class MemoryMappingLayout : public MemoryMappingLayoutBase { // Adds all mapped objects into a vector. void DumpListOfModules(InternalMmapVectorNoCtor *modules); +# if SANITIZER_APPLE + // Verify that the memory mapping is well-formed. (e.g. mappings do not + // overlap) + bool Verify(); +# endif + protected: #if SANITIZER_APPLE virtual const ImageHeader *CurrentImageHeader(); diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index a9533d6fc04ca..b8a8ef7df2c39 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -82,6 +82,7 @@ void MemoryMappedSegment::AddAddressRanges(LoadedModule *module) { MemoryMappingLayout::MemoryMappingLayout(bool cache_enabled) { Reset(); + Verify(); } MemoryMappingLayout::~MemoryMappingLayout() { @@ -187,6 +188,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)); @@ -255,16 +257,16 @@ 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) { + // The LINKEDIT sections alias, so we ignore these sections to + // ensure our mappings are disjoint. + return false; + } + 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; + base_virt_addr = (uptr)_dyld_get_image_slide(get_dyld_hdr()); + addr_mask = ~0; } else { base_virt_addr = (uptr)_dyld_get_image_vmaddr_slide(layout_data->current_image); @@ -445,6 +447,56 @@ bool MemoryMappingLayout::Next(MemoryMappedSegment *segment) { return false; } +// NOTE: Verify expects to be called immediately after Reset(), since otherwise +// it may miss some mappings. Verify will Reset() the layout after verification. +bool MemoryMappingLayout::Verify() { + InternalMmapVector module_name(kMaxPathLength); + MemoryMappedSegment segment(module_name.data(), module_name.size()); + MemoryMappedSegmentData data; + segment.data_ = &data; + + InternalMmapVector segments; + while (Next(&segment)) { + // skip the __PAGEZERO segment, its vmsize is 0 + if (segment.filename[0] == '\0' || (segment.start == segment.end)) + continue; + + segments.push_back(segment); + } + + // 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(), + [](MemoryMappedSegment& a, MemoryMappedSegment& b) { + return a.start < b.start; + }); + + // 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].start; + uptr prev_end = segments[i - 1].end; + if (cur_start < prev_end) { + well_formed = false; + VReport(2, "Overlapping mappings: cur_start = %p prev_end = %p\n", + (void*)cur_start, (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; + } + } + } + + Reset(); + return well_formed; +} + void MemoryMappingLayout::DumpListOfModules( InternalMmapVectorNoCtor *modules) { Reset(); 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..e48b2fd8cb982 --- /dev/null +++ b/compiler-rt/test/asan/TestCases/Darwin/asan-verify-module-map.cpp @@ -0,0 +1,20 @@ +// This test simply checks that the "Invalid module map" warning is not printed +// in the output of a backtrace. + +// RUN: %clangxx_asan -O0 -g %s -o %t.executable +// RUN: %env_asan_opts="print_module_map=2" not %run %t.executable 2>&1 | FileCheck %s + +// CHECK-NOT: WARN: Invalid module map + +#include + +extern "C" void foo(int *a) { *a = 5; } + +int main() { + int *a = (int *)malloc(sizeof(int)); + if (!a) + return 0; + free(a); + foo(a); + return 0; +} \ No newline at end of file From 425762e2efa7c15f852ae21b8434b190449a49f4 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Mon, 3 Nov 2025 14:29:51 -0800 Subject: [PATCH 2/7] fix test string --- .../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 e48b2fd8cb982..9e3d7c6c59690 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 @@ -1,10 +1,10 @@ -// This test simply checks that the "Invalid module map" warning is not printed +// This test simply checks that the "Invalid dyld module map" warning is not printed // in the output of a backtrace. // RUN: %clangxx_asan -O0 -g %s -o %t.executable // RUN: %env_asan_opts="print_module_map=2" not %run %t.executable 2>&1 | FileCheck %s -// CHECK-NOT: WARN: Invalid module map +// CHECK-NOT: WARN: Invalid dyld module map #include From 471137c0d10826f6ba53445d01f9b96f6bc006d6 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Mon, 3 Nov 2025 14:31:25 -0800 Subject: [PATCH 3/7] refactor to remove addr_mask --- .../lib/sanitizer_common/sanitizer_procmaps_mac.cpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index b8a8ef7df2c39..b18d3bebe48fa 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -42,7 +42,6 @@ struct MemoryMappedSegmentData { const char *current_load_cmd_addr; u32 lc_type; uptr base_virt_addr; - uptr addr_mask; }; template @@ -51,7 +50,7 @@ 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); @@ -263,17 +262,15 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, return false; } - uptr base_virt_addr, addr_mask; + uptr base_virt_addr; if (layout_data->current_image == kDyldImageIdx) { base_virt_addr = (uptr)_dyld_get_image_slide(get_dyld_hdr()); - addr_mask = ~0; } 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. @@ -283,7 +280,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)); } From 53fde9bf3ff8c1582ef4cccc1c3fc4b35d9f7ba8 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Tue, 4 Nov 2025 14:26:16 -0800 Subject: [PATCH 4/7] more robust test; nits --- .../sanitizer_procmaps_mac.cpp | 10 +++++----- .../Darwin/asan-verify-module-map.cpp | 19 ++++++++++++------- 2 files changed, 17 insertions(+), 12 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index b18d3bebe48fa..86d3f257b14d2 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -257,18 +257,18 @@ static bool NextSegmentLoad(MemoryMappedSegment *segment, if (((const load_command *)lc)->cmd == kLCSegment) { const SegmentCommand* sc = (const SegmentCommand *)lc; if (strncmp(sc->segname, "__LINKEDIT", sizeof("__LINKEDIT")) == 0) { - // The LINKEDIT sections alias, so we ignore these sections to - // ensure our mappings are disjoint. + // 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) { + if (layout_data->current_image == kDyldImageIdx) base_virt_addr = (uptr)_dyld_get_image_slide(get_dyld_hdr()); - } else { + else base_virt_addr = (uptr)_dyld_get_image_vmaddr_slide(layout_data->current_image); - } segment->start = sc->vmaddr + base_virt_addr; segment->end = segment->start + sc->vmsize; 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 9e3d7c6c59690..7660841c72877 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 @@ -1,20 +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 -O0 -g %s -o %t.executable -// RUN: %env_asan_opts="print_module_map=2" not %run %t.executable 2>&1 | FileCheck %s +// 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 -#include - +#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)); - if (!a) - return 0; free(a); foo(a); return 0; -} \ No newline at end of file +} +#endif \ No newline at end of file From 2f3f3cf2b7653a815a74891e5f4e6c6df49776c3 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Wed, 5 Nov 2025 11:28:06 -0800 Subject: [PATCH 5/7] convert to static, use DumpListOfModules --- .../sanitizer_procmaps_mac.cpp | 98 +++++++++---------- 1 file changed, 47 insertions(+), 51 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index 86d3f257b14d2..8b78bc12d9970 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -56,6 +56,52 @@ static void NextSectionLoad(LoadedModule *module, MemoryMappedSegmentData *data, 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 @@ -81,7 +127,7 @@ void MemoryMappedSegment::AddAddressRanges(LoadedModule *module) { MemoryMappingLayout::MemoryMappingLayout(bool cache_enabled) { Reset(); - Verify(); + VerifyMemoryMapping(this); } MemoryMappingLayout::~MemoryMappingLayout() { @@ -443,56 +489,6 @@ bool MemoryMappingLayout::Next(MemoryMappedSegment *segment) { return false; } -// NOTE: Verify expects to be called immediately after Reset(), since otherwise -// it may miss some mappings. Verify will Reset() the layout after verification. -bool MemoryMappingLayout::Verify() { - InternalMmapVector module_name(kMaxPathLength); - MemoryMappedSegment segment(module_name.data(), module_name.size()); - MemoryMappedSegmentData data; - segment.data_ = &data; - - InternalMmapVector segments; - while (Next(&segment)) { - // skip the __PAGEZERO segment, its vmsize is 0 - if (segment.filename[0] == '\0' || (segment.start == segment.end)) - continue; - - segments.push_back(segment); - } - - // 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(), - [](MemoryMappedSegment& a, MemoryMappedSegment& b) { - return a.start < b.start; - }); - - // 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].start; - uptr prev_end = segments[i - 1].end; - if (cur_start < prev_end) { - well_formed = false; - VReport(2, "Overlapping mappings: cur_start = %p prev_end = %p\n", - (void*)cur_start, (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; - } - } - } - - Reset(); - return well_formed; -} - void MemoryMappingLayout::DumpListOfModules( InternalMmapVectorNoCtor *modules) { Reset(); From c97b79f65d5c3cbad4acbd5eed78ede8c9a71f29 Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Wed, 5 Nov 2025 11:28:52 -0800 Subject: [PATCH 6/7] format --- .../lib/sanitizer_common/sanitizer_procmaps_mac.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp index 8b78bc12d9970..b3edcb77d13b4 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps_mac.cpp @@ -56,14 +56,14 @@ static void NextSectionLoad(LoadedModule *module, MemoryMappedSegmentData *data, sc->sectname); } -static bool VerifyMemoryMapping(MemoryMappingLayout *mapping) { +static bool VerifyMemoryMapping(MemoryMappingLayout* mapping) { InternalMmapVector modules; - modules.reserve(128); // matches DumpProcessMap + modules.reserve(128); // matches DumpProcessMap mapping->DumpListOfModules(&modules); InternalMmapVector segments; for (uptr i = 0; i < modules.size(); ++i) { - for (auto &range : modules[i].ranges()) { + for (auto& range : modules[i].ranges()) { segments.push_back(range); } } @@ -86,7 +86,8 @@ static bool VerifyMemoryMapping(MemoryMappingLayout *mapping) { 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); + 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 " @@ -101,7 +102,6 @@ static bool VerifyMemoryMapping(MemoryMappingLayout *mapping) { 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 From 10c063c51d5005bda16292bda691aad8dd9b35ca Mon Sep 17 00:00:00 2001 From: Andrew Haberlandt Date: Wed, 5 Nov 2025 11:31:09 -0800 Subject: [PATCH 7/7] remove Verify() --- compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h index 8af0ad70f313f..d713ddf847dfb 100644 --- a/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h +++ b/compiler-rt/lib/sanitizer_common/sanitizer_procmaps.h @@ -104,12 +104,6 @@ class MemoryMappingLayout : public MemoryMappingLayoutBase { // Adds all mapped objects into a vector. void DumpListOfModules(InternalMmapVectorNoCtor *modules); -# if SANITIZER_APPLE - // Verify that the memory mapping is well-formed. (e.g. mappings do not - // overlap) - bool Verify(); -# endif - protected: #if SANITIZER_APPLE virtual const ImageHeader *CurrentImageHeader();