[PATCH] D22757: Use RAII for ensuring that mprotect calls are undone

[Dean Michael Berris via llvm-commits]

dberris created this revision.
dberris added reviewers: rSerge, echristo.
dberris added a subscriber: llvm-commits.
Herald added a subscriber: mehdi_amini.

This fixes an mprotect leak identified in D21612.

https://reviews.llvm.org/D22757

Files:
  lib/xray/xray_interface.cc

Index: lib/xray/xray_interface.cc
===================================================================
--- lib/xray/xray_interface.cc
+++ lib/xray/xray_interface.cc
@@ -26,6 +26,31 @@
 // This is the function to call when we encounter the entry or exit sleds.
 std::atomic<void (*)(int32_t, XRayEntryType)> XRayPatchedFunction{nullptr};
 
+class MProtectHelper {
+  void *PageAlignedAddr;
+  std::size_t MProtectLen;
+  bool MustCleanup;
+
+public:
+  explicit MProtectHelper(void *PageAlignedAddr, std::size_t MProtectLen)
+      : PageAlignedAddr(PageAlignedAddr), MProtectLen(MProtectLen),
+        MustCleanup(false) {}
+
+  int MakeWriteable() {
+    auto R = mprotect(PageAlignedAddr, MProtectLen,
+                      PROT_READ | PROT_WRITE | PROT_EXEC);
+    if (R != -1)
+      MustCleanup = true;
+    return R;
+  }
+
+  ~MProtectHelper() {
+    if (MustCleanup) {
+      mprotect(PageAlignedAddr, MProtectLen, PROT_READ | PROT_EXEC);
+    }
+  }
+};
+
 } // namespace __xray
 
 extern "C" {
@@ -48,6 +73,8 @@
 
 std::atomic<bool> XRayPatching{false};
 
+using namespace __xray;
+
 XRayPatchingStatus __xray_patch() {
   // FIXME: Make this happen asynchronously. For now just do this sequentially.
   if (!XRayInitialized.load(std::memory_order_acquire))
@@ -62,7 +89,7 @@
 
   // Step 1: Compute the function id, as a unique identifier per function in the
   // instrumentation map.
-  __xray::XRaySledMap InstrMap = XRayInstrMap.load(std::memory_order_acquire);
+  XRaySledMap InstrMap = XRayInstrMap.load(std::memory_order_acquire);
   if (InstrMap.Entries == 0)
     return XRayPatchingStatus::NOT_INITIALIZED;
 
@@ -87,8 +114,8 @@
         reinterpret_cast<void *>(Sled.Address & ~((2 << 16) - 1));
     std::size_t MProtectLen =
         (Sled.Address + 12) - reinterpret_cast<uint64_t>(PageAlignedAddr);
-    if (mprotect(PageAlignedAddr, MProtectLen,
-                 PROT_READ | PROT_WRITE | PROT_EXEC) == -1) {
+    MProtectHelper Protector(PageAlignedAddr, MProtectLen);
+    if (Protector.MakeWriteable() == -1) {
       printf("Failed mprotect: %d\n", errno);
       return XRayPatchingStatus::FAILED;
     }
@@ -168,11 +195,6 @@
           reinterpret_cast<std::atomic<uint16_t> *>(Sled.Address), MovR10Seq,
           std::memory_order_release);
     }
-
-    if (mprotect(PageAlignedAddr, MProtectLen, PROT_READ | PROT_EXEC) == -1) {
-      printf("Failed mprotect: %d\n", errno);
-      return XRayPatchingStatus::FAILED;
-    }
   }
   XRayPatching.store(false, std::memory_order_release);
   return XRayPatchingStatus::NOTIFIED;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D22757.65346.patch
Type: text/x-patch
Size: 2562 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160725/595ee9d1/attachment.bin>