[libcxx-commits] [PATCH] D66904: [libunwind] Fix memory leak in handling of DW_CFA_remember_state and DW_CFA_restore_state

[Jorge Gorbe Moya via Phabricator via libcxx-commits]

jgorbe created this revision.
Herald added subscribers: libcxx-commits, christof, aprantl.
Herald added a project: libc++.

parseInstructions() doesn't always process the whole set of DWARF instructions for a frame. It will stop once the target PC is reached, or if malformed instructions are found. So, for example, if we have an instruction sequence like this:

  <start>
  ...
  DW_CFA_remember_state
  ...
  DW_CFA_advance_loc past the location we're unwinding at (pcoffset in parseInstructions() main loop)
  ...
  DW_CFA_restore_state
  <end>

... the saved state will never be freed, even though the `DW_CFA_remember_state` opcode has a matching `DW_CFA_restore_state` later in the sequence.

This change adds code to free whatever is left on rememberStack after parsing the CIE and the FDE instructions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66904

Files:
  libunwind/src/DwarfParser.hpp


Index: libunwind/src/DwarfParser.hpp
===================================================================
--- libunwind/src/DwarfParser.hpp
+++ libunwind/src/DwarfParser.hpp
@@ -360,13 +360,25 @@
   PrologInfoStackEntry *rememberStack = NULL;
 
   // parse CIE then FDE instructions
-  return parseInstructions(addressSpace, cieInfo.cieInstructions,
-                           cieInfo.cieStart + cieInfo.cieLength, cieInfo,
-                           (pint_t)(-1), rememberStack, arch, results) &&
-         parseInstructions(addressSpace, fdeInfo.fdeInstructions,
-                           fdeInfo.fdeStart + fdeInfo.fdeLength, cieInfo,
-                           upToPC - fdeInfo.pcStart, rememberStack, arch,
-                           results);
+  bool returnValue =
+      parseInstructions(addressSpace, cieInfo.cieInstructions,
+                        cieInfo.cieStart + cieInfo.cieLength, cieInfo,
+                        (pint_t)(-1), rememberStack, arch, results) &&
+      parseInstructions(addressSpace, fdeInfo.fdeInstructions,
+                        fdeInfo.fdeStart + fdeInfo.fdeLength, cieInfo,
+                        upToPC - fdeInfo.pcStart, rememberStack, arch, results);
+
+  // Clean up rememberStack. Even in the case where every DW_CFA_remember_state
+  // is paired with a DW_CFA_restore_state, parseInstructions can skip restore
+  // opcodes if it reaches the target PC and stops interpreting, so we have to
+  // make sure we don't leak memory.
+  while (rememberStack) {
+    PrologInfoStackEntry *next = rememberStack->next;
+    free(rememberStack);
+    rememberStack = next;
+  }
+
+  return returnValue;
 }
 
 /// "run" the DWARF instructions


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D66904.217717.patch
Type: text/x-patch
Size: 1687 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20190828/63577f07/attachment-0001.bin>