[sanitizer] Track architecture and UUID of modules in LoadedModule

[kubamracek]

Summary

When we enumerate loaded modules, we only track the module name and base address, which then has several problems on macOS. Dylibs and executables often have several architecture slices and not storing which architecture/UUID is actually loaded creates problems with symbolication: A file path + offset isn't enough to correctly symbolicate, since the offset can be valid in multiple slices. This is especially common for Haswell+ X86_64 machines, where x86_64h slices are preferred, but if one is not available, a regular x86_64 is loaded instead. But the same issue exists for i386 vs. x86_64 as well.

This patch adds tracking of arch and UUID for each LoadedModule. At this point, this information isn't used in reports, but this is the first step. The goal is to correctly identify which slice is loaded in symbolication, and also to output this information in reports so that we can tell which exact slices were loaded in post-mortem analysis.

[zaks.anna]

Could you add a test at least for the architecture or will you be testing this in subsequent patches?

lib/sanitizer_common/sanitizer_procmaps.h

40 ↗

(On Diff #77867)

Maybe use default arguments here?

lib/sanitizer_common/sanitizer_procmaps_freebsd.cc

53 ↗

(On Diff #77867)

It would be good to add asserts saying that support for passing non-null arch and uuid is not implemented for freebsd and linux.

[kubamracek]

Addressing review comments. Adding a unit test.

[filcab]

lib/sanitizer_common/sanitizer_common.h

686 ↗

(On Diff #78823)

Why not just return uuid_?

lib/sanitizer_common/sanitizer_procmaps_mac.cc

139 ↗

(On Diff #78823)

How about an outer switch/case and then ternary ops/more ifs/another switch if needed?

156 ↗

(On Diff #78823)

Should we assert that we always find a UUID?

lib/sanitizer_common/tests/sanitizer_procmaps_test.cc

66 ↗

(On Diff #78823)

Are the unit tests only run on X86(-64)? Never on ARM? Interesting.
Can you add a comment, please?

[kubamracek]

Updating patch.

Should we assert that we always find a UUID?

No, I don't think so. The linker emits a LC_UUID load command by default, but you can suppress this with -no_uuid. Not sure why you'd ever want to do that, but a Mach-O is valid even without a UUID.

Are the unit tests only run on X86(-64)? Never on ARM?

That's currently correct: On Darwin we can only run unit tests locally on the machine that builds compiler-rt, and I'm not sure but I think the same holds for Linux as well.

[filcab]

Almost LGTM, but I have some questions:
How are you planning on using the ModuleArch part? Would it be better (see my inline comment) to just save the cputype + cpusubtype?
IIUC, you can only have a slice per arch (type+subtype?).
Are you planning on always relying on ModuleArch (or something similar) all the time, or only as a fallback, if no UUID is available?

lib/sanitizer_common/sanitizer_procmaps_mac.cc

122 ↗

(On Diff #78920)

I wonder if we want a CHECK(0); here (or the opposite, a check that the subtype is one of those two). Same for the ARM ones.

[kubamracek]

How are you planning on using the ModuleArch part? Would it be better (see my inline comment) to just save the cputype + cpusubtype?

An immediate plan is to use ModuleArch for symbolication. When you just have module name + address, that's not enough to symbolicate, because the address might be valid in multiple slices in the module file on disk. Both atos and llvm-symbolizer suffer from this and it's especially painful on Haswell+ Intel machines, where plenty of modules (including the ASan and TSan dylibs) have both the x86_64 and x86_64h slices and basically each pointer is usually valid in both slices. I'm simply planning to pass the arch (as a string most likely) to atos and to llvm-symbolizer and to produce this in reports (again as a string). I don't think the CPU type+subtype is useful for this. And the set of archs that Darwin supports is very small. I think that type+subtype is too Mach-O specific to store it in LoadedModule.

IIUC, you can only have a slice per arch (type+subtype?).

That's right.

Are you planning on always relying on ModuleArch (or something similar) all the time, or only as a fallback, if no UUID is available?

I'm planning to use the ModuleArch all the time to correctly select the slice when symbolicating.

The plan for UUIDs is related, but slightly different. We have a few requests from people who use ASan and TSan in CI and/or process the text reports in various ways. They want to be able to pass ASan/TSan reports to symbolication servers. All of the existing debug symbols server support on macOS relies on module UUIDs. My plan is to have an option (maybe on by default, maybe not) that includes UUIDs of modules involved in the text report.

I'll send patches for review for this and the previously mentioned symbolication change soon.

[zaks.anna]

LGTM

[filcab]

LGTM too

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

while (last > first) {
     if (comp(v[mid], val))
       first = mid + 1;
     else
       last = mid;
   }
   return first;
 }
 
+enum ModuleArch {
+  kModuleArchUnknown,
+  kModuleArchI386,
+  kModuleArchX86_64,
+  kModuleArchX86_64H,
+  kModuleArchARMV6,
+  kModuleArchARMV7,
+  kModuleArchARMV7S,
+  kModuleArchARMV7K,
+  kModuleArchARM64
+};
+
+const uptr kModuleUUIDSize = 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) { ranges_.clear(); }
+  LoadedModule()
+      : full_name_(nullptr), base_address_(0), arch_(kModuleArchUnknown) {
+    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]);
   void clear();
   void addAddressRange(uptr beg, uptr end, bool executable);
   bool containsAddress(uptr address) const;
 
   const char *full_name() const { return full_name_; }
   uptr base_address() const { return base_address_; }
+  ModuleArch arch() const { return arch_; }
+  const u8 *uuid() const { return uuid_; }
 
   struct AddressRange {
     AddressRange *next;
     uptr beg;
     uptr end;
     bool executable;
 
     AddressRange(uptr beg, uptr end, bool executable)
         : next(nullptr), beg(beg), end(end), executable(executable) {}
   };
 
   const IntrusiveList<AddressRange> &ranges() const { return ranges_; }
 
  private:
   char *full_name_;  // Owned.
   uptr base_address_;
+  ModuleArch arch_;
+  u8 uuid_[kModuleUUIDSize];
   IntrusiveList<AddressRange> ranges_;
 };
 
 // List of LoadedModules. OS-dependent implementation is responsible for
 // filling this information.
 class ListOfModules {
  public:
   ListOfModules() : modules_(kInitialCapacity) {}

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

 }
 
 void LoadedModule::set(const char *module_name, uptr base_address) {
   clear();
   full_name_ = internal_strdup(module_name);
   base_address_ = base_address;
 }
 
+void LoadedModule::set(const char *module_name, uptr base_address,
+                       ModuleArch arch, u8 uuid[kModuleUUIDSize]) {
+  set(module_name, base_address);
+  arch_ = arch;
+  internal_memcpy(uuid_, uuid, sizeof(uuid_));
+}
+
 void LoadedModule::clear() {
   InternalFree(full_name_);
   full_name_ = nullptr;
+  arch_ = kModuleArchUnknown;
+  internal_memset(uuid_, 0, kModuleUUIDSize);
   while (!ranges_.empty()) {
     AddressRange *r = ranges_.front();
     ranges_.pop_front();
     InternalFree(r);
   }
 }
 
 void LoadedModule::addAddressRange(uptr beg, uptr end, bool executable) {

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

 // Reads process memory map in an OS-specific way.
 void ReadProcMaps(ProcSelfMapsBuff *proc_maps);
 #endif  // SANITIZER_FREEBSD || SANITIZER_LINUX
 
 class MemoryMappingLayout {
  public:
   explicit MemoryMappingLayout(bool cache_enabled);
   ~MemoryMappingLayout();
-  bool Next(uptr *start, uptr *end, uptr *offset,
-            char filename[], uptr filename_size, uptr *protection);
+  bool Next(uptr *start, uptr *end, uptr *offset, char filename[],
+            uptr filename_size, uptr *protection, ModuleArch *arch = nullptr,
+            u8 *uuid = nullptr);
   void Reset();
   // In some cases, e.g. when running under a sandbox on Linux, ASan is unable
   // to obtain the memory mappings. It should fall back to pre-cached data
   // instead of aborting.
   static void CacheMemoryMappings();
 
   // Adds all mapped objects into a vector.
   void DumpListOfModules(InternalMmapVector<LoadedModule> *modules);

 # if SANITIZER_FREEBSD || SANITIZER_LINUX
   ProcSelfMapsBuff proc_self_maps_;
   const char *current_;
 
   // Static mappings cache.
   static ProcSelfMapsBuff cached_proc_self_maps_;
   static StaticSpinMutex cache_lock_;  // protects cached_proc_self_maps_.
 # elif SANITIZER_MAC
-  template<u32 kLCSegment, typename SegmentCommand>
-  bool NextSegmentLoad(uptr *start, uptr *end, uptr *offset,
-                       char filename[], uptr filename_size,
+  template <u32 kLCSegment, typename SegmentCommand>
+  bool NextSegmentLoad(uptr *start, uptr *end, uptr *offset, char filename[],
+                       uptr filename_size, ModuleArch *arch, u8 *uuid,
                        uptr *protection);
   int current_image_;
   u32 current_magic_;
   u32 current_filetype_;
+  ModuleArch current_arch_;
+  u8 current_uuid_[kModuleUUIDSize];
   int current_load_cmd_count_;
   char *current_load_cmd_addr_;
 # endif
 };
 
 typedef void (*fill_profile_f)(uptr start, uptr rss, bool file,
                                /*out*/uptr *stats, uptr stats_size);
 

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

void ReadProcMaps(ProcSelfMapsBuff *proc_maps) {
 
   proc_maps->data = (char*)VmMap;
   proc_maps->mmaped_size = MmapedSize;
   proc_maps->len = Size;
 }
 
 bool MemoryMappingLayout::Next(uptr *start, uptr *end, uptr *offset,
                                char filename[], uptr filename_size,
-                               uptr *protection) {
+                               uptr *protection, ModuleArch *arch, u8 *uuid) {
+  CHECK(!arch && "not implemented");
+  CHECK(!uuid && "not implemented");
   char *last = proc_self_maps_.data + proc_self_maps_.len;
   if (current_ >= last) return false;
   uptr dummy;
   if (!start) start = &dummy;
   if (!end) end = &dummy;
   if (!offset) offset = &dummy;
   if (!protection) protection = &dummy;
   struct kinfo_vmentry *VmEntry = (struct kinfo_vmentry*)current_;

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

 }
 
 static bool IsOneOf(char c, char c1, char c2) {
   return c == c1 || c == c2;
 }
 
 bool MemoryMappingLayout::Next(uptr *start, uptr *end, uptr *offset,
                                char filename[], uptr filename_size,
-                               uptr *protection) {
+                               uptr *protection, ModuleArch *arch, u8 *uuid) {
+  CHECK(!arch && "not implemented");
+  CHECK(!uuid && "not implemented");
   char *last = proc_self_maps_.data + proc_self_maps_.len;
   if (current_ >= last) return false;
   uptr dummy;
   if (!start) start = &dummy;
   if (!end) end = &dummy;
   if (!offset) offset = &dummy;
   if (!protection) protection = &dummy;
   char *next_line = (char*)internal_memchr(current_, '\n', last - current_);

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

void MemoryMappingLayout::Reset() {
   // _dyld_image_count is thread-unsafe. We need to register callbacks for
   // adding and removing images which will invalidate the MemoryMappingLayout
   // state.
   current_image_ = _dyld_image_count();
   current_load_cmd_count_ = -1;
   current_load_cmd_addr_ = 0;
   current_magic_ = 0;
   current_filetype_ = 0;
+  current_arch_ = kModuleArchUnknown;
+  internal_memset(current_uuid_, 0, kModuleUUIDSize);
 }
 
 // static
 void MemoryMappingLayout::CacheMemoryMappings() {
   // No-op on Mac for now.
 }
 
 void MemoryMappingLayout::LoadFromCache() {
   // No-op on Mac for now.
 }
 
 // Next and NextSegmentLoad were inspired by base/sysinfo.cc in
 // Google Perftools, https://github.com/gperftools/gperftools.
 
 // NextSegmentLoad scans the current image for the next segment load command
 // and returns the start and end addresses and file offset of the corresponding
 // segment.
 // Note that the segment addresses are not necessarily sorted.
-template<u32 kLCSegment, typename SegmentCommand>
-bool MemoryMappingLayout::NextSegmentLoad(
-    uptr *start, uptr *end, uptr *offset,
-    char filename[], uptr filename_size, uptr *protection) {
-  const char* lc = current_load_cmd_addr_;
+template <u32 kLCSegment, typename SegmentCommand>
+bool MemoryMappingLayout::NextSegmentLoad(uptr *start, uptr *end, uptr *offset,
+                                          char filename[], uptr filename_size,
+                                          ModuleArch *arch, u8 *uuid,
+                                          uptr *protection) {
+  const char *lc = current_load_cmd_addr_;
   current_load_cmd_addr_ += ((const load_command *)lc)->cmdsize;
   if (((const load_command *)lc)->cmd == kLCSegment) {
     const sptr dlloff = _dyld_get_image_vmaddr_slide(current_image_);
     const SegmentCommand* sc = (const SegmentCommand *)lc;
     if (start) *start = sc->vmaddr + dlloff;
     if (protection) {
       // Return the initial protection.
       *protection = sc->initprot;
     }
     if (end) *end = sc->vmaddr + sc->vmsize + dlloff;
     if (offset) {
       if (current_filetype_ == /*MH_EXECUTE*/ 0x2) {
         *offset = sc->vmaddr;
       } else {
         *offset = sc->fileoff;
       }
     }
     if (filename) {
       internal_strncpy(filename, _dyld_get_image_name(current_image_),
                        filename_size);
     }
+    if (arch) {
+      *arch = current_arch_;
+    }
+    if (uuid) {
+      internal_memcpy(uuid, current_uuid_, kModuleUUIDSize);
+    }
     return true;
   }
   return false;
 }
 
+ModuleArch ModuleArchFromCpuType(cpu_type_t cputype, cpu_subtype_t cpusubtype) {
+  cpusubtype = cpusubtype & ~CPU_SUBTYPE_MASK;
+  switch (cputype) {
+    case CPU_TYPE_I386:
+      return kModuleArchI386;
+    case CPU_TYPE_X86_64:
+      if (cpusubtype == CPU_SUBTYPE_X86_64_ALL) return kModuleArchX86_64;
+      if (cpusubtype == CPU_SUBTYPE_X86_64_H) return kModuleArchX86_64H;
+      CHECK(0 && "Invalid subtype of x86_64");
+      return kModuleArchUnknown;
+    case CPU_TYPE_ARM:
+      if (cpusubtype == CPU_SUBTYPE_ARM_V6) return kModuleArchARMV6;
+      if (cpusubtype == CPU_SUBTYPE_ARM_V7) return kModuleArchARMV7;
+      if (cpusubtype == CPU_SUBTYPE_ARM_V7S) return kModuleArchARMV7S;
+      if (cpusubtype == CPU_SUBTYPE_ARM_V7K) return kModuleArchARMV7K;
+      CHECK(0 && "Invalid subtype of ARM");
+      return kModuleArchUnknown;
+    case CPU_TYPE_ARM64:
+      return kModuleArchARM64;
+    default:
+      CHECK(0 && "Invalid CPU type");
+      return kModuleArchUnknown;
+  }
+}
+
+static void FindUUID(const load_command *first_lc, u8 *uuid_output) {
+  const load_command *current_lc = first_lc;
+  while (1) {
+    if (current_lc->cmd == 0) return;
+    if (current_lc->cmd == LC_UUID) {
+      const uuid_command *uuid_lc = (const uuid_command *)current_lc;
+      const uint8_t *uuid = &uuid_lc->uuid[0];
+      internal_memcpy(uuid_output, uuid, kModuleUUIDSize);
+      return;
+    }
+
+    current_lc =
+        (const load_command *)(((char *)current_lc) + current_lc->cmdsize);
+  }
+}
+
 bool MemoryMappingLayout::Next(uptr *start, uptr *end, uptr *offset,
                                char filename[], uptr filename_size,
-                               uptr *protection) {
+                               uptr *protection, ModuleArch *arch, u8 *uuid) {
   for (; current_image_ >= 0; current_image_--) {
     const mach_header* hdr = _dyld_get_image_header(current_image_);
     if (!hdr) continue;
     if (current_load_cmd_count_ < 0) {
       // Set up for this image;
       current_load_cmd_count_ = hdr->ncmds;
       current_magic_ = hdr->magic;
       current_filetype_ = hdr->filetype;
+      current_arch_ = ModuleArchFromCpuType(hdr->cputype, hdr->cpusubtype);
       switch (current_magic_) {
 #ifdef MH_MAGIC_64
         case MH_MAGIC_64: {
           current_load_cmd_addr_ = (char*)hdr + sizeof(mach_header_64);
           break;
         }
 #endif
         case MH_MAGIC: {
           current_load_cmd_addr_ = (char*)hdr + sizeof(mach_header);
           break;
         }
         default: {
           continue;
         }
       }
     }
 
+    FindUUID((const load_command *)current_load_cmd_addr_, &current_uuid_[0]);
+
     for (; current_load_cmd_count_ >= 0; current_load_cmd_count_--) {
       switch (current_magic_) {
         // current_magic_ may be only one of MH_MAGIC, MH_MAGIC_64.
 #ifdef MH_MAGIC_64
         case MH_MAGIC_64: {
           if (NextSegmentLoad<LC_SEGMENT_64, struct segment_command_64>(
-                  start, end, offset, filename, filename_size, protection))
+                  start, end, offset, filename, filename_size, arch, uuid,
+                  protection))
             return true;
           break;
         }
 #endif
         case MH_MAGIC: {
           if (NextSegmentLoad<LC_SEGMENT, struct segment_command>(
-                  start, end, offset, filename, filename_size, protection))
+                  start, end, offset, filename, filename_size, arch, uuid,
+                  protection))
             return true;
           break;
         }
       }
     }
     // If we get here, no more load_cmd's in this image talk about
     // segments.  Go on to the next image.
   }
   return false;
 }
 
 void MemoryMappingLayout::DumpListOfModules(
     InternalMmapVector<LoadedModule> *modules) {
   Reset();
   uptr cur_beg, cur_end, prot;
+  ModuleArch cur_arch;
+  u8 cur_uuid[kModuleUUIDSize];
   InternalScopedString module_name(kMaxPathLength);
   for (uptr i = 0; Next(&cur_beg, &cur_end, 0, module_name.data(),
-                        module_name.size(), &prot);
+                        module_name.size(), &prot, &cur_arch, &cur_uuid[0]);
        i++) {
     const char *cur_name = module_name.data();
     if (cur_name[0] == '\0')
       continue;
     LoadedModule *cur_module = nullptr;
     if (!modules->empty() &&
         0 == internal_strcmp(cur_name, modules->back().full_name())) {
       cur_module = &modules->back();
     } else {
       modules->push_back(LoadedModule());
       cur_module = &modules->back();
-      cur_module->set(cur_name, cur_beg);
+      cur_module->set(cur_name, cur_beg, cur_arch, cur_uuid);
     }
     cur_module->addAddressRange(cur_beg, cur_end, prot & kProtectionExecute);
   }
 }
 
 }  // namespace __sanitizer
 
 #endif  // SANITIZER_MAC

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

if (modules[i].containsAddress((uptr)&noop)) {
       if (strstr(modules[i].full_name(), binary_name) != 0)
         found = true;
     }
     modules[i].clear();
   }
   EXPECT_TRUE(found);
 }
 
+TEST(MemoryMapping, LoadedModuleArchAndUUID) {
+  if (SANITIZER_MAC) {
+    MemoryMappingLayout memory_mapping(false);
+    const uptr kMaxModules = 100;
+    InternalMmapVector<LoadedModule> modules(kMaxModules);
+    memory_mapping.DumpListOfModules(&modules);
+    for (uptr i = 0; i < modules.size(); ++i) {
+      ModuleArch arch = modules[i].arch();
+      // Darwin unit tests are only run on i386/x86_64/x86_64h.
+      if (SANITIZER_WORDSIZE == 32) {
+        EXPECT_EQ(arch, kModuleArchI386);
+      } else if (SANITIZER_WORDSIZE == 64) {
+        EXPECT_TRUE(arch == kModuleArchX86_64 || arch == kModuleArchX86_64H);
+      }
+      const u8 *uuid = modules[i].uuid();
+      u8 null_uuid[kModuleUUIDSize] = {0};
+      EXPECT_NE(memcmp(null_uuid, uuid, kModuleUUIDSize), 0);
+    }
+  }
+}
+
 }  // namespace __sanitizer
 #endif  // !defined(_WIN32)