[PATCH] Make caching of memory mappings in sanitizer_procmaps optional

[Sergey Matveev]

================
Comment at: lib/sanitizer_common/sanitizer_procmaps.h:44
@@ -43,2 +43,3 @@
  public:
   MemoryMappingLayout();
+  explicit MemoryMappingLayout(bool use_cache);
----------------
Alexander Potapenko wrote:
> Pls add a comment that the default ctor makes MemoryMappingLayout use the cache.
> OTOH I don't think we really need the default constructor - maybe it's better to always state whether we use the cache or no (Kostya, Alexey, WDYT?)
I personally think it's better to make it explicit. I left the default constructor in because I thought there were too many instances too fix across the codebase, but that's actually not the case, so I'm going to do that if no one objects.

================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:343
@@ -334,2 +342,3 @@
   if (proc_self_maps_.mmaped_size == 0) {
+    CHECK(use_cache);
     LoadFromCache();
----------------
Alexander Potapenko wrote:
> I think the actual invariant we want to check for is that proc_self_maps_.mmaped_size is greater than zero in the case use_cache is false.
> Otherwise if ReadFileToBuffer fails you'll get a strange error message that use_cache is false (which should be correct if you're running LSan)
> I suggest the following:
> 
> if use_cache is true {
>   if mmaped_size == 0 (i.e. ReadFileToBuffer failed) {
>     LoadFromCache()
>     CHECK_GT(...)
>   }
> } else {
>   CHECK(mmaped_size > 0)
> }
> 
> (uh, this sucks - why can't I use indentation in comments?)
Agreed.


http://llvm-reviews.chandlerc.com/D569