[PATCH] [compiler-rt] Symbolizer refactoring: Implement GetListOfModules on Windows

[Timur Iskhodzhanov]

Hi Kuba,

Have you seen the "Sanitizer coverage: PC vs modules are not handled efficiently?" email thread on llvm-commits I've started recently?

Basically, I'm up to refactor this very code and I think the current interface and implementation are suboptimal.
How blocked is your progress by this change?  If you can wait a week or two, I can improve the code that uses these functions first and try to simplify the interface.
Since I have to do that anyways, I'm not sure it's efficient for us to do the same thing twice in parallel :)


================
Comment at: lib/sanitizer_common/sanitizer_symbolizer.cc:164
@@ +163,3 @@
+  if (modules_ == 0 || !modules_fresh_) {
+    modules_ = (LoadedModule*)(symbolizer_allocator_.Allocate(
+        kMaxNumberOfModuleContexts * sizeof(LoadedModule)));
----------------
This line leaks a lot of memory every time a module lookup fails [e.g. a new shared object is dynamically loaded].
I understand you've just moved this code around, but I think we should come up with a better solution if we've decided to refactor this code.

================
Comment at: lib/sanitizer_common/sanitizer_win.cc:226
@@ -261,3 +225,3 @@
 
-  qsort(modules, num_modules, sizeof(ModuleInfo), CompareModulesBase);
+  qsort(modules.data(), num_modules, sizeof(LoadedModule), CompareModulesBase);
 
----------------
This line looks dangerous to me now.
LoadedModule is not guaranteed to be `qsort`-friendly.
[ I think "trivially copyable" is what matters here http://www.cplusplus.com/reference/type_traits/is_trivially_copyable/ ]

It already owns a `const char *full_name_;`, and one day somebody might decide `~LoadedModule` dtor should free up some memory, or hold some non-POD member.

http://reviews.llvm.org/D8513

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