[PATCH] D32190: Make sure to scan mmap'd memory regions for root pointers on OS X

[Francis Ricci via Phabricator via llvm-commits]

fjricci added inline comments.


================
Comment at: lib/lsan/lsan_common_mac.cc:150-156
   while (err == KERN_SUCCESS) {
     struct vm_region_submap_info_64 info;
     err = vm_region_recurse_64(port, &address, &size, &depth,
                                (vm_region_info_t)&info, &count);
-    if (info.user_tag == VM_MEMORY_OS_ALLOC_ONCE) {
-      ScanRangeForPointers(address, address + size, frontier,
-                           "GLOBAL", kReachable);
-      return;
-    }
+    callback(frontier, address, size, info, arg);
     address += size;
   }
----------------
fjricci wrote:
> kubamracek wrote:
> > How many times is this called?  This should really be called only once per the whole scan, and it's hard to see if that's the case or not.
> > 
> > The naming is weird: `ProcessMemoryRegion` suggests that this processes some specific region, but in fact, this function *scans* for a new region.  I don't follow why this is called from ProcessPlatformSpecificRootRegion.
> This would be called once when checking for the kernel alloc once page, and once when processing the root regions. I could probably change it to do both in one loop, if that's preferable, but it would require changing the ProcessPlatformSpecificAllocations API to take root region pointers (probably not a big deal, since only mac uses that API currently).
> 
> It needs to be called from ProcessPlatformSpecificRootRegion to find root regions contained within mmap'd memory regions.
Actually that refactor would probably be even more complicated than this version, as we'd need to encapsulate the functionality of `ProcessRootRegions` (which iterates over the root regions, scanning each one). Currently we avoid duplicating this behavior by doing the platform-specific scan inside the call tree of that function.


https://reviews.llvm.org/D32190