[PATCH] D15728: [cfi] Support for dlopen and dlclose

[Peter Collingbourne via llvm-commits]

pcc added inline comments.

================
Comment at: lib/cfi/cfi.cc:40
@@ -39,2 +39,3 @@
 static constexpr uint16_t kUncheckedShadow = 0xFFFFU;
+static constexpr int kMaxModules = 1 << 14;
 
----------------
Looks like we already have a `kMaxNumberOfModules` constant defined.

================
Comment at: lib/cfi/cfi.cc:173
@@ +172,3 @@
+
+  for (auto iter = module->ranges(); iter.hasNext();) {
+    auto *r = iter.next();
----------------
I changed this interface in r257858.

================
Comment at: lib/cfi/cfi.cc:237-263
@@ +236,29 @@
+void ShadowUpdater::update() {
+  LoadedModule *modules_after = (LoadedModule *)MmapOrDie(
+      kMaxModules * sizeof(LoadedModule), "ShadowUpdater");
+  uptr modules_after_cnt =
+      GetListOfModules(modules_after, kMaxModules, nullptr);
+
+  InternalSort(&modules_before, modules_before_cnt, LoadedModuleCompare);
+  InternalSort(&modules_after, modules_after_cnt, LoadedModuleCompare);
+
+  LoadedModule *p = modules_before;
+  LoadedModule *p_end = p + modules_before_cnt;
+  LoadedModule *q = modules_after;
+  LoadedModule *q_end = q + modules_after_cnt;
+  while (p < p_end && q < q_end) {
+    if (p->base_address() < q->base_address()) {
+      remove_module(p++);
+    } else if (p->base_address() > q->base_address()) {
+      add_module(q++);
+    } else {
+      p++;
+      q++;
+    }
+  }
+  while (p < p_end)
+    remove_module(p++);
+  while (q < q_end)
+    add_module(q++);
+
+  UnmapOrDie(modules_before, kMaxModules * sizeof(LoadedModule));
----------------
Can you avoid needing to take the set difference by rebuilding from scratch?

================
Comment at: lib/sanitizer_common/sanitizer_common.cc:341
@@ -340,1 +340,3 @@
 
+void LoadedModule::set_phdr(uptr phdr, int phnum) {
+  phdr_ = phdr;
----------------
Should there be a single `init()` function that combines `set` and `set_phdr`?

================
Comment at: lib/sanitizer_common/sanitizer_common.h:637
@@ -633,1 +636,3 @@
 
+  bool operator<(const LoadedModule &other) const {
+    return base_address_ < other.base_address_;
----------------
Is this function needed? Looks like you're passing a comparator to your sort function above.


Repository:
  rL LLVM

http://reviews.llvm.org/D15728