[Courgette] Fix memory leak in AssemblyProgram::TrimLabels().

The leaky code is only used by EXE_ELF_32_ARM flow.

In AssemblyProgram::TrimLabels(), originally we (A) remove underused
Labels from std::map<RVA, Label*> that stores it, (B) convert all
instructions that use underused Labels to a non-Label instruction.
Problem is (A) does not deallocate Label* (owned by the std::map),
and memory leak ensues.

Naively deallocating Labels in (A) would lead to use-after free.
So we do (B) first, then do (A) while deallocating Label*.

Review URL: https://codereview.chromium.org/1570173003

Cr-Commit-Position: refs/heads/master@{#370045}

1 files changed, 13 insertions, 16 deletions

diff --git a/courgette/assembly_program.cc b/courgette/assembly_program.cc
index 0767892..28a348e 100644
--- a/courgette/assembly_program.cc
+++ b/courgette/assembly_program.cc
@@ -502,7 +502,7 @@ Instruction* AssemblyProgram::GetByteInstruction(uint8_t byte) {
 const int AssemblyProgram::kLabelLowerLimit = 5;
 
 CheckBool AssemblyProgram::TrimLabels() {
-  // For now only trim for ARM binaries
+  // For now only trim for ARM binaries.
   if (kind() != EXE_ELF_32_ARM)
     return true;
 
@@ -510,22 +510,8 @@ CheckBool AssemblyProgram::TrimLabels() {
 
   VLOG(1) << "TrimLabels: threshold " << lower_limit;
 
-  // Remove underused labels from the list of labels
-  RVAToLabel::iterator it = rel32_labels_.begin();
-  while (it != rel32_labels_.end()) {
-    if (it->second->count_ <= lower_limit) {
-      // Note: it appears to me (grt) that this leaks the Label instances. I
-      // *think* the right thing would be to add it->second to a collection for
-      // which all elements are freed via UncheckedDelete after the instruction
-      // fixup loop below.
-      rel32_labels_.erase(it++);
-    } else {
-      ++it;
-    }
-  }
-
   // Walk through the list of instructions, replacing trimmed labels
-  // with the original machine instruction
+  // with the original machine instruction.
   for (size_t i = 0; i < instructions_.size(); ++i) {
     Instruction* instruction = instructions_[i];
     switch (instruction->op()) {
@@ -552,6 +538,17 @@ CheckBool AssemblyProgram::TrimLabels() {
     }
   }
 
+  // Remove and deallocate underused Labels.
+  RVAToLabel::iterator it = rel32_labels_.begin();
+  while (it != rel32_labels_.end()) {
+    if (it->second->count_ <= lower_limit) {
+      UncheckedDelete(it->second);
+      rel32_labels_.erase(it++);
+    } else {
+      ++it;
+    }
+  }
+
   return true;
 }