[PATCH] Make caching of memory mappings in sanitizer_procmaps optional
Alexander Potapenko
glider at google.com
Sat Mar 23 03:51:12 PDT 2013
================
Comment at: lib/sanitizer_common/sanitizer_procmaps.h:44
@@ -43,2 +43,3 @@
public:
MemoryMappingLayout();
+ explicit MemoryMappingLayout(bool use_cache);
----------------
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?)
================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:347
@@ -337,3 +346,3 @@
}
// internal_write(2, proc_self_maps_.data, proc_self_maps_.len);
Reset();
----------------
Good time to delete this line.
================
Comment at: lib/sanitizer_common/sanitizer_linux.cc:343
@@ -334,2 +342,3 @@
if (proc_self_maps_.mmaped_size == 0) {
+ CHECK(use_cache);
LoadFromCache();
----------------
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?)
http://llvm-reviews.chandlerc.com/D569
More information about the llvm-commits
mailing list