* proposed patch for SW9937
@ 2010-11-01 20:34 Stan Cox
2010-11-04 18:57 ` Stan Cox
0 siblings, 1 reply; 2+ messages in thread
From: Stan Cox @ 2010-11-01 20:34 UTC (permalink / raw)
To: SystemTap List
[-- Attachment #1: Type: text/plain, Size: 1195 bytes --]
Add build id support for user modules. The build id is an offset which is
adjusted relative to dwarf_module_base for ET_EXEC or the load address from the
task manager for ET_DYN.
sym.c
Created generic build id checker: _stp_build_id_check
Modified _stp_module_check, the kernel build id checker, to use
_stp_build_id_check
Created _stp_usermodule_check, the user build id checker, which uses
_stp_build_id_check. This is called from the task manager callback
stap_uprobe_change_plus for the process found and mmap found cases.
sym.h
Added module_base for holding the base address for ET_EXEC.
For ET_DYN case the load address from task manager is used
translate.cxx
Modified dump_unwindsyms:
1. relax the check to allow user modules to set build_id_vaddr
2. save module_base for ET_EXEC. For ET_DYN we use the task finder.
3. relax the check to allow user modules to set build_id_bits.
4. add a ET_DYN case for outputting build_id_vaddr
For the above the build id offsets look like:
(via build_id_vaddr - base in dump_unwindsyms) :
.name = "libsdt_V3_uprobe.so",....build_id_offset = 0x1a0,
.name = "sdt_misc_V3_uprobe.x",....build_id_offset = 0x24c,
[-- Attachment #2: ,git.diff --]
[-- Type: text/plain, Size: 10296 bytes --]
/work/scox/systemtap/src /work/scox/systemtap/bld ~
diff --git a/runtime/sym.c b/runtime/sym.c
index 3471f63..5abe494 100644
--- a/runtime/sym.c
+++ b/runtime/sym.c
@@ -235,7 +235,64 @@ static const char *_stp_kallsyms_lookup(unsigned long addr,
return NULL;
}
-/* Validate module/kernel based on build-id if there
+static int _stp_build_id_check (struct _stp_module *m, unsigned long notes_addr, bool user_module)
+{
+ int j;
+ for (j=0; j<m->build_id_len; j++) {
+ /* Use set_fs / get_user to access
+ conceivably invalid addresses. If
+ loc2c-runtime.h were more easily usable,
+ a deref() loop could do it too. */
+ mm_segment_t oldfs = get_fs();
+ int rc1, rc2;
+ unsigned char theory, practice;
+
+#ifdef STAPCONF_PROBE_KERNEL
+ if (!user_module)
+ {
+ rc1=probe_kernel_read(&theory, (void*)&m->build_id_bits[j], 1);
+ rc2=probe_kernel_read(&practice, (void*)(notes_addr+j), 1);
+ }
+ else
+ {
+ set_fs(KERNEL_DS);
+ rc1 = get_user(theory,((unsigned char*) &m->build_id_bits[j]));
+ rc2 = get_user(practice,((unsigned char*) (void*) (notes_addr+j)));
+ set_fs(oldfs);
+ }
+#else
+ set_fs(KERNEL_DS);
+ rc1 = get_user(theory,((unsigned char*) &m->build_id_bits[j]));
+ rc2 = get_user(practice,((unsigned char*) (void*) (notes_addr+j)));
+ set_fs(oldfs);
+#endif
+
+ if (rc1 || rc2 || (theory != practice)) {
+ const char *basename;
+ basename = strrchr(m->path, '/');
+ if (basename)
+ basename++;
+ else
+ basename = m->path;
+
+#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,27)
+ _stp_error ("Build-id mismatch: \"%s\" vs. \"%s\" byte %d (0x%02x vs 0x%02x) rc %d %d\n",
+ m->name, basename, j, theory, practice, rc1, rc2);
+ return 1;
+#else
+ /* This branch is a surrogate for kernels
+ * affected by Fedora bug #465873. */
+ _stp_warn (KERN_WARNING
+ "Build-id mismatch: \"%s\" vs. \"%s\" byte %d (0x%02x vs 0x%02x) rc %d %d\n",
+ m->name, basename, j, theory, practice, rc1, rc2);
+#endif
+ break;
+ } /* end mismatch */
+ } /* end per-byte check loop */
+ return 0;
+}
+
+/* Validate module/kernel based on build-id (if present)
* The completed case is the following combination:
* Debuginfo Module Kernel
* X X
@@ -272,52 +329,48 @@ static int _stp_module_check(void)
notes_addr, base_addr);
continue;
}
- for (j=0; j<m->build_id_len; j++) {
- /* Use set_fs / get_user to access
- conceivably invalid addresses. If
- loc2c-runtime.h were more easily usable,
- a deref() loop could do it too. */
- mm_segment_t oldfs = get_fs();
- int rc1, rc2;
- unsigned char theory, practice;
-
-#ifdef STAPCONF_PROBE_KERNEL
- rc1=probe_kernel_read(&theory, (void*)&m->build_id_bits[j], 1);
- rc2=probe_kernel_read(&practice, (void*)(notes_addr+j), 1);
-#else
- set_fs(KERNEL_DS);
- rc1 = get_user(theory,((unsigned char*) &m->build_id_bits[j]));
- rc2 = get_user(practice,((unsigned char*) (void*) (notes_addr+j)));
- set_fs(oldfs);
-#endif
-
- if (rc1 || rc2 || (theory != practice)) {
- const char *basename;
- basename = strrchr(m->path, '/');
- if (basename)
- basename++;
- else
- basename = m->path;
-
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,27)
- _stp_error ("Build-id mismatch: \"%s\" vs. \"%s\" byte %d (0x%02x vs 0x%02x) rc %d %d\n",
- m->name, basename, j, theory, practice, rc1, rc2);
- return 1;
-#else
- /* This branch is a surrogate for kernels
- * affected by Fedora bug #465873. */
- _stp_warn (KERN_WARNING
- "Build-id mismatch: \"%s\" vs. \"%s\" byte %d (0x%02x vs 0x%02x) rc %d %d\n",
- m->name, basename, j, theory, practice, rc1, rc2);
-#endif
- break;
- } /* end mismatch */
- } /* end per-byte check loop */
+ _stp_build_id_check (m, notes_addr, false);
} /* end checking */
} /* end loop */
return 0;
}
+
+/* Validate user module based on build-id (if present) */
+static int _stp_usermodule_check(struct task_struct *tsk, const char *path_name, unsigned long addr)
+{
+ struct _stp_module *m = NULL;
+ unsigned long notes_addr;
+ unsigned i, j;
+ unsigned char practice_id_bits[MAXSTRINGLEN];
+ unsigned long vm_end = 0;
+
+ for (i = 0; i < _stp_num_modules; i++)
+ {
+ m = _stp_modules[i];
+ if (strcmp(path_name, _stp_modules[i]->path) != 0)
+ continue;
+ if (m->build_id_len > 0) {
+ int ret, build_id_len;
+
+ notes_addr = addr + m->build_id_offset + m->module_base;
+
+ dbug_sym(1, "build-id validation [%s] address=%#lx build_id_offset=%#lx module_base=%#lx\n",
+ m->name, addr, m->build_id_offset, m->module_base);
+
+ if (notes_addr <= addr) {
+ _stp_warn ("build-id address %lx < base %lx\n",
+ notes_addr, addr);
+ continue;
+ }
+ _stp_build_id_check (m, notes_addr, true);
+ }
+ }
+
+ return 0;
+}
+
+
/** Prints an address based on the _STP_SYM flags.
* @param address The address to lookup.
* @param task The address to lookup (if NULL lookup kernel/module address).
diff --git a/runtime/sym.h b/runtime/sym.h
index 55d02d5..bee45de 100644
--- a/runtime/sym.h
+++ b/runtime/sym.h
@@ -91,6 +91,7 @@ struct _stp_module {
unsigned long build_id_offset;
unsigned long notes_sect;
int build_id_len;
+ unsigned long module_base; /* the base address for an ET_EXEC */
};
diff --git a/runtime/uprobes-common.c b/runtime/uprobes-common.c
index d6fc6ef..4d84981 100644
--- a/runtime/uprobes-common.c
+++ b/runtime/uprobes-common.c
@@ -66,6 +66,8 @@ static int stap_uprobe_change_plus (struct task_struct *tsk, unsigned long reloc
#ifdef DEBUG_UPROBES
_stp_dbug(__FUNCTION__,__LINE__, "+uprobe spec %d idx %d process %s[%d] addr %p pp %s\n", spec_index, (slotted_p ? i : -1), tsk->comm, tsk->tgid, (void*)(relocation+sups->address), sups->probe->pp);
#endif
+ if ((rc = _stp_usermodule_check(tsk, (const char*)stf->pathname, relocation)))
+ return rc;
/* Here, slotted_p implies that `i' points to the single
stap_uprobes[] element that has been slotted in for registration
diff --git a/tapsets.cxx b/tapsets.cxx
index 78b92d5..71b9f21 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -5244,6 +5244,7 @@ sdt_query::handle_probe_entry()
results.push_back (p);
}
}
+ sess.unwindsym_modules.insert (dw.module_name);
record_semaphore(results, i);
}
diff --git a/translate.cxx b/translate.cxx
index 2e6a7de..dcb0218 100644
--- a/translate.cxx
+++ b/translate.cxx
@@ -4891,7 +4891,7 @@ dump_unwindsyms (Dwfl_Module *m,
}
#endif
- if (modname != "kernel" && modname[0] != '/') // => kernel module
+ if (modname != "kernel")
{
Dwarf_Addr reloc_vaddr = build_id_vaddr;
const char *secname;
@@ -4907,10 +4907,13 @@ dump_unwindsyms (Dwfl_Module *m,
// process("...") ones may have relocation bases like '.dynamic',
// and so we'll have to store not just a generic offset but
// the relocation section/symbol name too: just like we do
- // for probe PC addresses themselves.
- if (!secname || strcmp(secname, ".note.gnu.build-id"))
- throw semantic_error ("unexpected build-id reloc section " +
- string(secname ?: "null"));
+ // for probe PC addresses themselves. We want to set build_id_vaddr for
+ // user modules even though they will not have a secname.
+
+ if (modname[0] != '/')
+ if (!secname || strcmp(secname, ".note.gnu.build-id"))
+ throw semantic_error ("unexpected build-id reloc section " +
+ string(secname ?: "null"));
build_id_vaddr = reloc_vaddr;
}
@@ -5303,6 +5306,13 @@ dump_unwindsyms (Dwfl_Module *m,
c->output << "static struct _stp_module _stp_module_" << stpmod_idx << " = {\n";
c->output << ".name = " << lex_cast_qstring (mainname) << ", \n";
c->output << ".path = " << lex_cast_qstring (mainpath) << ",\n";
+
+ // base address for ET_EXEC; ET_DYN uses the task manager load address.
+
+ if (seclist[0].first != ".dynamic")
+ c->output << ".module_base = 0x" << hex << base << ", \n";
+ else
+ c->output << ".module_base = 0x0,\n";
c->output << ".eh_frame_addr = 0x" << hex << eh_addr << dec << ", \n";
c->output << ".unwind_hdr_addr = 0x" << hex << eh_frame_hdr_addr
<< dec << ", \n";
@@ -5359,7 +5369,7 @@ dump_unwindsyms (Dwfl_Module *m,
* See also:
* http://sources.redhat.com/ml/systemtap/2009-q4/msg00574.html
*/
- if (build_id_len > 0 && (build_id_vaddr > base + extra_offset)) {
+ if (build_id_len > 0) {
c->output << ".build_id_bits = \"" ;
for (int j=0; j<build_id_len;j++)
c->output << "\\x" << hex
@@ -5376,6 +5386,10 @@ dump_unwindsyms (Dwfl_Module *m,
if (modname == "kernel")
c->output << ".build_id_offset = 0x" << hex << build_id_vaddr - (base + extra_offset)
<< dec << ",\n";
+ // build_id_vaddr for ET_DYN; task finder will give the load address.
+ else if (seclist[0].first == ".dynamic")
+ c->output << ".build_id_offset = 0x" << hex
+ << build_id_vaddr << dec << ",\n";
else
c->output << ".build_id_offset = 0x" << hex
<< build_id_vaddr - base
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: proposed patch for SW9937
2010-11-01 20:34 proposed patch for SW9937 Stan Cox
@ 2010-11-04 18:57 ` Stan Cox
0 siblings, 0 replies; 2+ messages in thread
From: Stan Cox @ 2010-11-04 18:57 UTC (permalink / raw)
To: systemtap
Tweaked the previous to eliminate .module_base and output build_id_offset as
follows:
@@ -5376,9 +5379,10 @@ dump_unwindsyms (Dwfl_Module *m,
if (modname == "kernel")
c->output << ".build_id_offset = 0x" << hex << build_id_vaddr - (base
+ extra_offset)
<< dec << ",\n";
+ // ET_DYN: task finder gives the load address. ET_EXEC: this is absolute
address
else
c->output << ".build_id_offset = 0x" << hex
- << build_id_vaddr - base
+ << build_id_vaddr /* - base */
<< dec << ",\n";
} else
c->output << ".build_id_len = 0,\n";
for ET_EXEC we get e.g. 0x40024c
for ET_DYN we get e.g. 0x1a0
for kernel we get e.g. 0x430544
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2010-11-04 18:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-01 20:34 proposed patch for SW9937 Stan Cox
2010-11-04 18:57 ` Stan Cox
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).