[PATCH] [ASan/Win] Unify handling of loaded modules between POSIX and Windows

Timur Iskhodzhanov timurrrr at google.com
Mon Apr 6 05:53:46 PDT 2015


r234150, thanks for the review!
Please take a look at the FIXME I've added to `lib/sanitizer_common/sanitizer_common.h`


================
Comment at: sanitizer_linux_libcdep.cc:439
@@ -438,4 +438,3 @@
     return 0;
-  void *mem = &data->modules[data->current_n];
-  LoadedModule *cur_module = new(mem) LoadedModule(module_name.data(),
-                                                   info->dlpi_addr);
+  LoadedModule &cur_module = data->modules[data->current_n];
+  cur_module.set(module_name.data(), info->dlpi_addr);
----------------
samsonov wrote:
> Nit: we don't usually use non-const references. Can you change it to pointer?
Umm, yeah.  Fixed

================
Comment at: sanitizer_procmaps_common.cc:145
@@ -145,3 +144,3 @@
     uptr base_address = (i ? cur_beg : 0) - cur_offset;
-    LoadedModule *cur_module = new(mem) LoadedModule(cur_name, base_address);
-    cur_module->addAddressRange(cur_beg, cur_end, prot & kProtectionExecute);
+    LoadedModule &cur_module = modules[n_modules];
+    cur_module.set(cur_name, base_address);
----------------
samsonov wrote:
> Ditto
Done

================
Comment at: sanitizer_procmaps_mac.cc:174
@@ -173,3 +173,3 @@
       continue;
     LoadedModule *cur_module = 0;
     if (n_modules > 0 &&
----------------
samsonov wrote:
> nullptr
I haven't changed that code really, but fixed anyways.

================
Comment at: sanitizer_symbolizer.h:143
@@ +142,3 @@
+  LoadedModule *FindModuleForAddress(uptr address);
+  virtual uptr PlatformGetListOfModules(LoadedModule *modules,
+                                        uptr max_modules) {
----------------
samsonov wrote:
> You don't need this function - it just calls ::GetListOfModules() in both implementations. Please delete it.
This is a little tricky, as it fails to link as `GetListOfModules` is in `libcdep` nowadays...
I've added a FIXME -- can you please take a look?  I don't really understand the logic behind `libcdep` yet.

http://reviews.llvm.org/D8805

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list