[libunwind] 1ae8d81 - [libunwind] Fix memory leak in handling of DW_CFA_remember_state and DW_CFA_restore_state

[Jorge Gorbe Moya via cfe-commits]

Author: Jorge Gorbe Moya
Date: 2020-02-18T11:57:18-08:00
New Revision: 1ae8d81147a0724cc972054afbd72943032e4832

URL: https://github.com/llvm/llvm-project/commit/1ae8d81147a0724cc972054afbd72943032e4832
DIFF: https://github.com/llvm/llvm-project/commit/1ae8d81147a0724cc972054afbd72943032e4832.diff

LOG: [libunwind] Fix memory leak in handling of DW_CFA_remember_state and DW_CFA_restore_state

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.

Differential Revision: https://reviews.llvm.org/D66904

Added: 
    libunwind/test/remember_state_leak.pass.sh.s

Modified: 
    libunwind/src/DwarfParser.hpp

Removed: 
    


################################################################################
diff  --git a/libunwind/src/DwarfParser.hpp b/libunwind/src/DwarfParser.hpp
index df69c2a4bd23..2994bd7bb41f 100644
--- a/libunwind/src/DwarfParser.hpp
+++ b/libunwind/src/DwarfParser.hpp
@@ -360,13 +360,25 @@ bool CFI_Parser<A>::parseFDEInstructions(A &addressSpace,
   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

diff  --git a/libunwind/test/remember_state_leak.pass.sh.s b/libunwind/test/remember_state_leak.pass.sh.s
new file mode 100644
index 000000000000..821ee926eec8
--- /dev/null
+++ b/libunwind/test/remember_state_leak.pass.sh.s
@@ -0,0 +1,56 @@
+# REQUIRES: x86, linux
+# RUN: %build -target x86_64-unknown-linux-gnu
+# RUN: %run
+
+# The following assembly is a translation of this code:
+#
+#   _Unwind_Reason_Code callback(int, _Unwind_Action, long unsigned int,
+#                                _Unwind_Exception*, _Unwind_Context*, void*) {
+#     return _Unwind_Reason_Code(0);
+#   }
+#
+#   int main() {
+#     asm(".cfi_remember_state\n\t");
+#     _Unwind_Exception exc;
+#     _Unwind_ForcedUnwind(&exc, callback, 0);
+#     asm(".cfi_restore_state\n\t");
+#   }
+#
+# When unwinding, the CFI parser will stop parsing opcodes after the current PC,
+# so in this case the DW_CFA_restore_state opcode will never be processed and,
+# if the library doesn't clean up properly, the store allocated by
+# DW_CFA_remember_state will be leaked.
+#
+# This test will fail when linked with an asan-enabled libunwind if the
+# remembered state is leaked.
+
+    SIZEOF_UNWIND_EXCEPTION = 32
+
+    .text
+callback:
+    xorl    %eax, %eax
+    retq
+
+    .globl    main                    # -- Begin function main
+    .p2align    4, 0x90
+    .type    main, at function
+main:                                   # @main
+    .cfi_startproc
+    subq    $8, %rsp   # Adjust stack alignment
+    subq    $SIZEOF_UNWIND_EXCEPTION, %rsp
+    .cfi_def_cfa_offset 48
+    .cfi_remember_state
+    movq    %rsp, %rdi
+    movabsq $callback, %rsi
+    xorl    %edx, %edx
+    callq    _Unwind_ForcedUnwind
+    .cfi_restore_state
+    xorl    %eax, %eax
+    addq    $SIZEOF_UNWIND_EXCEPTION, %rsp
+    addq    $8, %rsp   # Undo stack alignment adjustment
+    .cfi_def_cfa_offset 8
+    retq
+.Lfunc_end1:
+    .size    main, .Lfunc_end1-main
+    .cfi_endproc
+                                        # -- End function