From 1d4093bcdcdb366340c0ddab7837b68b0b351676 Mon Sep 17 00:00:00 2001
From: Maxime Coste <mawww@kakoune.org>
Date: Wed, 7 Jun 2017 19:18:15 +0100
Subject: [PATCH] Fix memory errors due to sharing the MatchResults in the
 Hooks struct

A hook execution triggered by another hook execution would change the
shared MatchResults object, which is a wrong behaviour and makes it
point to dead string data.
---
 src/hook_manager.cc | 23 +++++++++++++----------
 src/hook_manager.hh |  1 -
 2 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/src/hook_manager.cc b/src/hook_manager.cc
index 52da2d28..ef7139b3 100644
--- a/src/hook_manager.cc
+++ b/src/hook_manager.cc
@@ -16,7 +16,7 @@ namespace Kakoune
 void HookManager::add_hook(StringView hook_name, String group, Regex filter, String commands)
 {
     auto& hooks = m_hooks[hook_name];
-    hooks.emplace_back(new Hook{std::move(group), std::move(filter), {}, std::move(commands)});
+    hooks.emplace_back(new Hook{std::move(group), std::move(filter), std::move(commands)});
 }
 
 void HookManager::remove_hooks(StringView group)
@@ -82,38 +82,41 @@ void HookManager::run_hook(StringView hook_name,
     auto start_time = profile ? Clock::now() : TimePoint{};
 
     auto& disabled_hooks = context.options()["disabled_hooks"].get<Regex>();
-    Vector<Hook*> hooks_to_run; // The m_hooks_trash vector ensure hooks wont die during this method
+
+    struct ToRun { Hook* hook; MatchResults<const char*> captures; };
+    Vector<ToRun> hooks_to_run; // The m_hooks_trash vector ensure hooks wont die during this method
     for (auto& hook : hook_list->value)
     {
+        MatchResults<const char*> captures;
         if ((hook->group.empty() or disabled_hooks.empty() or
              not regex_match(hook->group.begin(), hook->group.end(), disabled_hooks))
-            and regex_match(param.begin(), param.end(), hook->captures, hook->filter))
-            hooks_to_run.push_back(hook.get());
+            and regex_match(param.begin(), param.end(), captures, hook->filter))
+            hooks_to_run.push_back({ hook.get(), std::move(captures) });
     }
 
     bool hook_error = false;
-    for (auto& hook : hooks_to_run)
+    for (auto& to_run : hooks_to_run)
     {
         try
         {
             if (debug_flags & DebugFlags::Hooks)
-                write_to_debug_buffer(format("hook {}({})/{}", hook_name, param, hook->group));
+                write_to_debug_buffer(format("hook {}({})/{}", hook_name, param, to_run.hook->group));
 
             ScopedSetBool disable_history{context.history_disabled()};
 
             EnvVarMap env_vars{ {"hook_param", param.str()} };
-            for (size_t i = 0; i < hook->captures.size(); ++i)
+            for (size_t i = 0; i < to_run.captures.size(); ++i)
                 env_vars.insert({format("hook_param_capture_{}", i),
-                                 {hook->captures[i].first, hook->captures[i].second}});
+                                 {to_run.captures[i].first, to_run.captures[i].second}});
 
-            CommandManager::instance().execute(hook->commands, context,
+            CommandManager::instance().execute(to_run.hook->commands, context,
                                                { {}, std::move(env_vars) });
         }
         catch (runtime_error& err)
         {
             hook_error = true;
             write_to_debug_buffer(format("error running hook {}({})/{}: {}",
-                               hook_name, param, hook->group, err.what()));
+                               hook_name, param, to_run.hook->group, err.what()));
         }
     }
 
diff --git a/src/hook_manager.hh b/src/hook_manager.hh
index 8614b7a5..9cfa4a9b 100644
--- a/src/hook_manager.hh
+++ b/src/hook_manager.hh
@@ -32,7 +32,6 @@ private:
     {
         String group;
         Regex filter;
-        MatchResults<const char*> captures;
         String commands;
     };