Skip to content

Instantly share code, notes, and snippets.

@agentzh
Created June 20, 2020 20:26
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save agentzh/a166220b1fefc09813db8e0859622923 to your computer and use it in GitHub Desktop.
Save agentzh/a166220b1fefc09813db8e0859622923 to your computer and use it in GitHub Desktop.
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