-
-
Save agentzh/a166220b1fefc09813db8e0859622923 to your computer and use it in GitHub Desktop.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
commit 4688a14550ee3ffaf314d52a08ebd9307c5cad0d | |
Author: Yichun Zhang (agentzh) <yichun@openresty.com> | |
Date: Sat Jun 20 13:24:51 2020 -0700 | |
bugfix: make sure we register the vma map callback for task finder before other probes like process.begin. | |
This bug was easy to reproduce on kernels with debug config options | |
enabled like fedora's kernel-debug. | |
diff --git a/elaborate.h b/elaborate.h | |
index b49a634ed..8ff452f20 100644 | |
--- a/elaborate.h | |
+++ b/elaborate.h | |
@@ -336,6 +336,8 @@ struct derived_probe_group | |
virtual bool otf_safe_context (systemtap_session&) { return false; } | |
// Whether this probe type occurs in a safe context. To be safe, we default to | |
// no, which means we'll rely on a background timer. | |
+ | |
+ virtual bool needs_task_finder (void) { return false; } | |
}; | |
diff --git a/runtime/linux/task_finder2.c b/runtime/linux/task_finder2.c | |
index 0ba614741..4a793e62c 100644 | |
--- a/runtime/linux/task_finder2.c | |
+++ b/runtime/linux/task_finder2.c | |
@@ -1294,20 +1294,23 @@ __stp_tf_quiesce_worker(struct task_work *work) | |
__stp_tf_handler_start(); | |
- /* NB make sure we run mmap callbacks before other callbacks | |
- * like 'probe process.begin' handlers so that the vma tracker | |
- * is already initialized in the latter contexts */ | |
- | |
/* If this is just a thread other than the thread group | |
* leader, don't bother inform map callback clients about its | |
* memory map, since they will simply duplicate each other. */ | |
- if (tgt->mmap_events == 1 && current->tgid == current->pid) { | |
- __stp_call_mmap_callbacks_for_task(tgt, current); | |
- } | |
+ if (tgt->mmap_events == 1) { | |
+ if (current->tgid == current->pid) { | |
+ /* NB make sure we run mmap callbacks before other callbacks | |
+ * like 'probe process.begin' handlers so that the vma tracker | |
+ * is already initialized in the latter contexts */ | |
- /* Call the callbacks. Assume that if the thread is a | |
- * thread group leader, it is a process. */ | |
- __stp_call_callbacks(tgt, current, 1, (current->pid == current->tgid)); | |
+ __stp_call_mmap_callbacks_for_task(tgt, current); | |
+ __stp_call_callbacks(tgt, current, 1, 1); | |
+ } | |
+ } else { | |
+ /* Call the callbacks. Assume that if the thread is a | |
+ * thread group leader, it is a process. */ | |
+ __stp_call_callbacks(tgt, current, 1, (current->pid == current->tgid)); | |
+ } | |
__stp_tf_handler_end(); | |
@@ -1385,21 +1388,24 @@ __stp_utrace_task_finder_target_quiesce(u32 action, | |
} | |
} | |
else { | |
- /* NB make sure we run mmap callbacks before other callbacks | |
- * like 'probe process.begin' handlers so that the vma tracker | |
- * is already initialized in the latter contexts */ | |
- | |
/* If this is just a thread other than the thread | |
group leader, don't bother inform map callback | |
clients about its memory map, since they will | |
simply duplicate each other. */ | |
- if (tgt->mmap_events == 1 && tsk->tgid == tsk->pid) { | |
- __stp_call_mmap_callbacks_for_task(tgt, tsk); | |
- } | |
+ if (tgt->mmap_events == 1) { | |
+ if (tsk->tgid == tsk->pid) { | |
+ /* NB make sure we run mmap callbacks before other callbacks | |
+ * like 'probe process.begin' handlers so that the vma tracker | |
+ * is already initialized in the latter contexts */ | |
- /* Call the callbacks. Assume that if the thread is a | |
- * thread group leader, it is a process. */ | |
- __stp_call_callbacks(tgt, tsk, 1, (tsk->pid == tsk->tgid)); | |
+ __stp_call_mmap_callbacks_for_task(tgt, tsk); | |
+ __stp_call_callbacks(tgt, tsk, 1, 1); | |
+ } | |
+ } else { | |
+ /* Call the callbacks. Assume that if the thread is a | |
+ * thread group leader, it is a process. */ | |
+ __stp_call_callbacks(tgt, tsk, 1, (tsk->pid == tsk->tgid)); | |
+ } | |
} | |
__stp_tf_handler_end(); | |
diff --git a/runtime/sym.c b/runtime/sym.c | |
index 1c11fa32e..cd6800c3a 100644 | |
--- a/runtime/sym.c | |
+++ b/runtime/sym.c | |
@@ -98,6 +98,7 @@ static unsigned long _stp_umodule_relocate(const char *path, | |
} | |
} | |
+ dbug_sym(0, "[%d] %s, %lx mapping NOT FOUND\n", tsk->pid, path, offset); | |
return 0; | |
} | |
diff --git a/tapset-dynprobe.cxx b/tapset-dynprobe.cxx | |
index fc4c1b4b4..ea243499c 100644 | |
--- a/tapset-dynprobe.cxx | |
+++ b/tapset-dynprobe.cxx | |
@@ -64,6 +64,8 @@ public: | |
void emit_module_post_init (systemtap_session& ) { } | |
void emit_module_exit (systemtap_session& ) { } | |
+ bool needs_task_finder (void) { return true; } | |
+ | |
void add(const string& path, const Dwarf_Addr offset, | |
const Dwarf_Addr semaphore_addr, const string& flags_string, | |
const string& probe_init); | |
diff --git a/tapset-itrace.cxx b/tapset-itrace.cxx | |
index fc98dcbfb..08f430ae8 100644 | |
--- a/tapset-itrace.cxx | |
+++ b/tapset-itrace.cxx | |
@@ -64,6 +64,8 @@ public: | |
void emit_module_decls (systemtap_session& s); | |
void emit_module_init (systemtap_session& s); | |
void emit_module_exit (systemtap_session& s); | |
+ | |
+ bool needs_task_finder (void) { return true; } | |
}; | |
diff --git a/tapset-utrace.cxx b/tapset-utrace.cxx | |
index 1e3d5665e..05acee26c 100644 | |
--- a/tapset-utrace.cxx | |
+++ b/tapset-utrace.cxx | |
@@ -99,6 +99,8 @@ public: | |
void emit_module_decls (systemtap_session& s); | |
void emit_module_init (systemtap_session& s); | |
void emit_module_exit (systemtap_session& s); | |
+ | |
+ bool needs_task_finder (void) { return true; } | |
}; | |
diff --git a/task_finder.cxx b/task_finder.cxx | |
index d08d44a75..330b7c201 100644 | |
--- a/task_finder.cxx | |
+++ b/task_finder.cxx | |
@@ -44,6 +44,8 @@ public: | |
void emit_module_post_init (systemtap_session& s); | |
void emit_module_exit (systemtap_session& s); | |
+ bool needs_task_finder (void) { return true; } | |
+ | |
// Whether or not to initialize the vma tracker | |
bool need_vma_tracker; | |
}; | |
@@ -53,12 +55,6 @@ void | |
task_finder_derived_probe_group::emit_module_init (systemtap_session& s) | |
{ | |
s.op->newline(); | |
- if (need_vma_tracker) | |
- { | |
- s.op->newline() << "/* ---- vma tracker ---- */"; | |
- s.op->newline() << "rc = _stp_vma_init();"; | |
- s.op->newline(); | |
- } | |
s.op->newline() << "/* ---- task finder ---- */"; | |
s.op->newline() << "if (rc == 0) {"; | |
@@ -85,13 +81,6 @@ task_finder_derived_probe_group::emit_module_exit (systemtap_session& s) | |
s.op->newline(); | |
s.op->newline() << "/* ---- task finder ---- */"; | |
s.op->newline() << "stap_stop_task_finder();"; | |
- | |
- if (need_vma_tracker) | |
- { | |
- s.op->newline(); | |
- s.op->newline() << "/* ---- vma tracker ---- */"; | |
- s.op->newline() << "_stp_vma_done();"; | |
- } | |
} | |
diff --git a/translate.cxx b/translate.cxx | |
index 7a8dc02bc..f8b214a59 100644 | |
--- a/translate.cxx | |
+++ b/translate.cxx | |
@@ -2024,8 +2024,21 @@ c_unparser::emit_module_init () | |
// Run all probe registrations. This actually runs begin probes. | |
+ bool found_first_group_needing_task_finder = false; | |
+ | |
for (unsigned i=0; i<g.size(); i++) | |
{ | |
+ if (!found_first_group_needing_task_finder && g[i]->needs_task_finder ()) | |
+ { | |
+ found_first_group_needing_task_finder = true; | |
+ if (vma_tracker_enabled (*session)) | |
+ { | |
+ o->newline(); | |
+ o->newline() << "/* ---- vma tracker ---- */"; | |
+ o->newline() << "rc = _stp_vma_init();"; | |
+ } | |
+ } | |
+ | |
g[i]->emit_module_init (*session); | |
// NB: this gives O(N**2) amount of code, but luckily there | |
// are only seven or eight derived_probe_groups, so it's ok. | |
@@ -2255,10 +2268,31 @@ c_unparser::emit_module_exit () | |
// We're processing the derived_probe_group list in reverse | |
// order. This ensures that probes get unregistered in reverse | |
// order of the way they were registered. | |
+ bool found_first_group_not_needing_task_finder = false; | |
vector<derived_probe_group*> g = all_session_groups (*session); | |
for (vector<derived_probe_group*>::reverse_iterator i = g.rbegin(); | |
i != g.rend(); i++) | |
- (*i)->emit_module_exit (*session); // NB: runs "end" probes | |
+ { | |
+ if (!found_first_group_not_needing_task_finder && !(*i)->needs_task_finder ()) | |
+ { | |
+ found_first_group_not_needing_task_finder = true; | |
+ if (vma_tracker_enabled (*session)) | |
+ { | |
+ o->newline(); | |
+ o->newline() << "/* ---- vma tracker ---- */"; | |
+ o->newline() << "_stp_vma_done();"; | |
+ } | |
+ } | |
+ (*i)->emit_module_exit (*session); // NB: runs "end" probes | |
+ } | |
+ | |
+ if (!found_first_group_not_needing_task_finder | |
+ && vma_tracker_enabled (*session)) | |
+ { | |
+ o->newline(); | |
+ o->newline() << "/* ---- vma tracker ---- */"; | |
+ o->newline() << "_stp_vma_done();"; | |
+ } | |
if (!session->runtime_usermode_p()) | |
o->newline() << "mutex_unlock(&module_refresh_mutex);"; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment