Only scan global sections containing data in LSan on darwin

[fjricci]

Summary

__DATA segments on Darwin contain a large number of separate sections,
many of which cannot actually contain pointers, and contain const values or
objc metadata. Not scanning sections which cannot contain pointers significantly
improves performance.

On a medium-sized (~4000 files) internal project, I saw a speedup of about 30%
in standalone LSan's execution time (30% improvement in the time spent running
LSan, not the total program time).

[fjricci]

Rebase

[alekseyshl]

lib/lsan/lsan_common_mac.cc

114 ↗

(On Diff #106724)

Maybe define an array of those and iterate over it? That will allow us to add a CHECK that literal range name length is < kMaxSegName too.

lib/sanitizer_common/sanitizer_common.h

750 ↗

(On Diff #106724)

else you need to initialize name anyway: this->name[0] = '\0';

750 ↗

(On Diff #106724)

kMaxSegName -> ARRAY_SIZE(this->name)

lib/sanitizer_common/sanitizer_procmaps_mac.cc

203 ↗

(On Diff #106724)

kMaxSegName -> ARRAY_SZIE(segment->name)

[alekseyshl]

Oh, and darin -> Darwin in the title

[fjricci]

Address comments

[fjricci]

sec_names -> kGlobalVarSecNames

[alekseyshl]

lib/sanitizer_common/sanitizer_common.h

751 ↗

(On Diff #106937)

Nit: maybe do it this way:

internal_strncpy(this->name, (name ? name : ""), ARRAY_SIZE(this->name));

[fjricci]

Dependency was reverted.

[fjricci]

Use blacklist instead of whitelist to avoid false positives

[fjricci]

This had to be reverted due to a masking error in its dependency which caused failures on 10.11 buildbots. Since it got reverted anyway, I took the opportunity to look into some false positives in swift, and found that they were caused by this patch. In order to prevent false positives, this now uses a blacklist of sections which definitely do not store pointers instead of a whitelist of sections which do (to avoid missing sections). Would appreciate a re-review.

[fjricci]

clang-format

[alekseyshl]

Makes sense!

[fjricci]

Rebase and account for new opaque data pointer

compiler-rt/trunk/lib/lsan/lsan_common_mac.cc

 AllocatorCache *GetAllocatorCache() { return &get_tls_val(true)->cache; }
 
 LoadedModule *GetLinker() { return nullptr; }
 
 // Required on Linux for initialization of TLS behavior, but should not be
 // required on Darwin.
 void InitializePlatformSpecificModules() {}
 
+// Sections which can't contain contain global pointers. This list errs on the
+// side of caution to avoid false positives, at the expense of performance.
+//
+// Other potentially safe sections include:
+// __all_image_info, __crash_info, __const, __got, __interpose, __objc_msg_break
+//
+// Sections which definitely cannot be included here are:
+// __objc_data, __objc_const, __data, __bss, __common, __thread_data,
+// __thread_bss, __thread_vars, __objc_opt_rw, __objc_opt_ptrs
+static const char *kSkippedSecNames[] = {
+    "__cfstring",       "__la_symbol_ptr",  "__mod_init_func",
+    "__mod_term_func",  "__nl_symbol_ptr",  "__objc_classlist",
+    "__objc_classrefs", "__objc_imageinfo", "__objc_nlclslist",
+    "__objc_protolist", "__objc_selrefs",   "__objc_superrefs"};
+
 // Scans global variables for heap pointers.
 void ProcessGlobalRegions(Frontier *frontier) {
+  for (auto name : kSkippedSecNames) CHECK(ARRAY_SIZE(name) < kMaxSegName);
+
   MemoryMappingLayout memory_mapping(false);
   InternalMmapVector<LoadedModule> modules(/*initial_capacity*/ 128);
   memory_mapping.DumpListOfModules(&modules);
   for (uptr i = 0; i < modules.size(); ++i) {
     // Even when global scanning is disabled, we still need to scan
     // system libraries for stashed pointers
     if (!flags()->use_globals && modules[i].instrumented()) continue;
 
     for (const __sanitizer::LoadedModule::AddressRange &range :
          modules[i].ranges()) {
       // Sections storing global variables are writable and non-executable
       if (range.executable || !range.writable) continue;
 
+      for (auto name : kSkippedSecNames) {
+        if (!internal_strcmp(range.name, name)) continue;
+      }
+
       ScanGlobalRange(range.beg, range.end, frontier);
     }
   }
 }
 
 void ProcessPlatformSpecificAllocations(Frontier *frontier) {
   mach_port_name_t port;
   if (task_for_pid(mach_task_self(), internal_getpid(), &port)

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.h

switch (arch) {
     case kModuleArchARM64:
       return "arm64";
   }
   CHECK(0 && "Invalid module arch");
   return "";
 }
 
 const uptr kModuleUUIDSize = 16;
+const uptr kMaxSegName = 16;
 
 // Represents a binary loaded into virtual memory (e.g. this can be an
 // executable or a shared object).
 class LoadedModule {
  public:
   LoadedModule()
       : full_name_(nullptr),
         base_address_(0),
         max_executable_address_(0),
         arch_(kModuleArchUnknown),
         instrumented_(false) {
     internal_memset(uuid_, 0, kModuleUUIDSize);
     ranges_.clear();
   }
   void set(const char *module_name, uptr base_address);
   void set(const char *module_name, uptr base_address, ModuleArch arch,
            u8 uuid[kModuleUUIDSize], bool instrumented);
   void clear();
-  void addAddressRange(uptr beg, uptr end, bool executable, bool writable);
+  void addAddressRange(uptr beg, uptr end, bool executable, bool writable,
+                       const char *name = nullptr);
   bool containsAddress(uptr address) const;
 
   const char *full_name() const { return full_name_; }
   uptr base_address() const { return base_address_; }
   uptr max_executable_address() const { return max_executable_address_; }
   ModuleArch arch() const { return arch_; }
   const u8 *uuid() const { return uuid_; }
   bool instrumented() const { return instrumented_; }
 
   struct AddressRange {
     AddressRange *next;
     uptr beg;
     uptr end;
     bool executable;
     bool writable;
+    char name[kMaxSegName];
 
-    AddressRange(uptr beg, uptr end, bool executable, bool writable)
+    AddressRange(uptr beg, uptr end, bool executable, bool writable,
+                 const char *name)
         : next(nullptr),
           beg(beg),
           end(end),
           executable(executable),
-          writable(writable) {}
+          writable(writable) {
+      internal_strncpy(this->name, (name ? name : ""), ARRAY_SIZE(this->name));
+    }
   };
 
   const IntrusiveList<AddressRange> &ranges() const { return ranges_; }
 
  private:
   char *full_name_;  // Owned.
   uptr base_address_;
   uptr max_executable_address_;

compiler-rt/trunk/lib/sanitizer_common/sanitizer_common.cc

void LoadedModule::clear() {
   while (!ranges_.empty()) {
     AddressRange *r = ranges_.front();
     ranges_.pop_front();
     InternalFree(r);
   }
 }
 
 void LoadedModule::addAddressRange(uptr beg, uptr end, bool executable,
-                                   bool writable) {
+                                   bool writable, const char *name) {
   void *mem = InternalAlloc(sizeof(AddressRange));
-  AddressRange *r = new(mem) AddressRange(beg, end, executable, writable);
+  AddressRange *r =
+      new(mem) AddressRange(beg, end, executable, writable, name);
   ranges_.push_back(r);
   if (executable && end > max_executable_address_)
     max_executable_address_ = end;
 }
 
 bool LoadedModule::containsAddress(uptr address) const {
   for (const AddressRange &r : ranges()) {
     if (r.beg <= address && address < r.end)

compiler-rt/trunk/lib/sanitizer_common/sanitizer_procmaps_mac.cc

 #ifndef CPU_TYPE_ARM64
 #define CPU_TYPE_ARM64        (CPU_TYPE_ARM | CPU_ARCH_ABI64)
 #endif
 
 namespace __sanitizer {
 
 // Contains information used to iterate through sections.
 struct MemoryMappedSegmentData {
+  char name[kMaxSegName];
   uptr nsects;
   char *current_load_cmd_addr;
   u32 lc_type;
   uptr base_virt_addr;
   uptr addr_mask;
 };
 
 template <typename Section>
 static void NextSectionLoad(LoadedModule *module, MemoryMappedSegmentData *data,
                             bool isWritable) {
   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_end = sec_start + sc->size;
-  module->addAddressRange(sec_start, sec_end, /*executable=*/false, isWritable);
+  module->addAddressRange(sec_start, sec_end, /*executable=*/false, isWritable,
+                          sc->sectname);
 }
 
 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
   // is executable. Avoid iterating over executable sections because
   // it will confuse libignore, and because the extra granularity
   // of information is not needed by any sanitizers.
   if (!data_ || !data_->nsects || IsExecutable()) {
-    module->addAddressRange(start, end, IsExecutable(), IsWritable());
+    module->addAddressRange(start, end, IsExecutable(), IsWritable(),
+                            data_ ? data_->name : nullptr);
     return;
   }
 
   do {
     if (data_->lc_type == LC_SEGMENT) {
       NextSectionLoad<struct section>(module, data_, IsWritable());
 #ifdef MH_MAGIC_64
     } else if (data_->lc_type == LC_SEGMENT_64) {
if (((const load_command *)lc)->cmd == kLCSegment) {
     // when required.
     if (segment->data_) {
       segment->data_->nsects = sc->nsects;
       segment->data_->current_load_cmd_addr =
           (char *)lc + sizeof(SegmentCommand);
       segment->data_->lc_type = kLCSegment;
       segment->data_->base_virt_addr = base_virt_addr;
       segment->data_->addr_mask = addr_mask;
+      internal_strncpy(segment->data_->name, sc->segname,
+                       ARRAY_SIZE(segment->data_->name));
     }
 
     // Return the initial protection.
     segment->protection = sc->initprot;
     segment->offset =
         (current_filetype_ == /*MH_EXECUTE*/ 0x2) ? sc->vmaddr : sc->fileoff;
     if (segment->filename) {
       const char *src = (current_image_ == kDyldImageIdx)