Hi Ken'ichi,
On 2011-07-29 16:21:04 Fri, Ken'ichi Ohmichi wrote:
>
> Hi Mahesh,
>
> On Thu, 28 Jul 2011 16:08:15 +0530
> Mahesh Jagannath Salgaonkar <mahesh at linux.vnet.ibm.com> wrote:
> > >
> > > If adding the searching method to the blow position and removing the code
> > > from init_dwarf_info(), I guess it makes the code simple.
> > >
> > > @ process_config_file()
> > > 9402 if (!set_dwarf_debuginfo(config->module_name,
> > > 9403 NULL, -1)) {
> > > 9404 ERRMSG("Skipping to next Module section\n");
> > > 9405 skip_section = 1;
> > > 9406 free_config(config);
> > > 9407 continue;
> > > 9408 }
> > > 9409 << HERE >>
> >
> > This may not be the correct place to call search method. We may end up
> > calling search method multiple times for same kernel module. I think
> > moving the search method inside set_dwarf_debuginfo() routine at below
> > position is a better place:
> >
> > @set_dwarf_debuginfo()
> > ......
> > ......
> > if (!strcmp(dwarf_info.module_name, "vmlinux") ||
> > !strcmp(dwarf_info.module_name, "xen-syms"))
> > return TRUE;
> > + << HERE >>
> > - /* check to see whether module debuginfo is available */
> > - if (!init_dwarf_info())
> > - return FALSE;
> > - else
> > - clean_dwfl_info();
> > return TRUE;
> > }
> >
> > And then we can remove search routine from init_dwarf_info(). What do
> > you think?
>
> I think the above change will be good, thanks in advance.
Please find the inline patch below that separates the search method from
init_dwarf_info() function. Please review it and let know your comments.
Thanks,
-Mahesh.
----------
makedumpfile: Move debuginfo search to set_dwarf_debuginfo() routine.
From: Mahesh Salgaonkar <mahesh@xxxxxxxxxxxxxxxxxx>
Move the debuginfo search logic out of init_dwarf_info() function to make
this function more simpler. Now we call search method in set_dwarf_debuginfo
when switch to different module section.
This patch also fixes a BUG where dwarf_info.module_name becomes a stale
pointer after free_config() invocation. This patch now uses strdup() to get
duplicate string and assigns it to dwarf_info.module_name.
Signed-off-by: Mahesh Salgaonkar <mahesh at linux.vnet.ibm.com>
---
makedumpfile.c | 126 ++++++++++++++++++++++++++++++++++++++------------------
1 files changed, 86 insertions(+), 40 deletions(-)
diff --git a/makedumpfile.c b/makedumpfile.c
index 35cbed9..fe8e8b0 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -1186,7 +1186,60 @@ clean_dwfl_info(void)
}
/*
- * Intitialize the dwarf info.
+ * Search module debuginfo.
+ * This function searches for module debuginfo in default debuginfo path for
+ * a given module in dwarf_info.module_name.
+ *
+ * On success, dwarf_info.name_debuginfo is set to absolute path of
+ * module debuginfo.
+ */
+static int
+search_module_debuginfo(void)
+{
+ Dwfl *dwfl = NULL;
+ static char *debuginfo_path = DEFAULT_DEBUGINFO_PATH;
+ static const Dwfl_Callbacks callbacks = {
+ .section_address = dwfl_offline_section_address,
+ .find_debuginfo = dwfl_standard_find_debuginfo,
+ .debuginfo_path = &debuginfo_path,
+ };
+
+ /*
+ * Check if We already have debuginfo file name with us. If yes,
+ * then we don't need to proceed with search method.
+ */
+ if (dwarf_info.name_debuginfo)
+ return TRUE;
+
+ if ((dwfl = dwfl_begin(&callbacks)) == NULL) {
+ ERRMSG("Can't create a handle for a new dwfl session.\n");
+ return FALSE;
+ }
+
+ /* Search for module debuginfo file. */
+ if (dwfl_linux_kernel_report_offline(dwfl,
+ info->system_utsname.release,
+ &dwfl_report_module_p)) {
+ ERRMSG("Can't get Module debuginfo for module '%s'\n",
+ dwarf_info.module_name);
+ dwfl_end(dwfl);
+ return FALSE;
+ }
+ dwfl_report_end(dwfl, NULL, NULL);
+ dwfl_getmodules(dwfl, &process_module, NULL, 0);
+
+ dwfl_end(dwfl);
+ clean_dwfl_info();
+
+ /* Return success if module debuginfo is found. */
+ if (dwarf_info.name_debuginfo)
+ return TRUE;
+
+ return FALSE;
+}
+
+/*
+ * Initialize the dwarf info.
* Linux kernel module debuginfo are of ET_REL (relocatable) type.
* This function uses dwfl API's to apply relocation before reading the
* dwarf information from module debuginfo.
@@ -1198,51 +1251,45 @@ init_dwarf_info(void)
{
Dwfl *dwfl = NULL;
int dwfl_fd = -1;
- static char *debuginfo_path = DEFAULT_DEBUGINFO_PATH;
static const Dwfl_Callbacks callbacks = {
.section_address = dwfl_offline_section_address,
- .find_debuginfo = dwfl_standard_find_debuginfo,
- .debuginfo_path = &debuginfo_path,
};
dwarf_info.elfd = NULL;
dwarf_info.dwarfd = NULL;
+ /*
+ * We already know the absolute path of debuginfo file. Fail if we
+ * still don't have one. Ideally we should never be in this situation.
+ */
+ if (!dwarf_info.name_debuginfo) {
+ ERRMSG("Can't find absolute path to debuginfo file.\n");
+ return FALSE;
+ }
+
if ((dwfl = dwfl_begin(&callbacks)) == NULL) {
ERRMSG("Can't create a handle for a new dwfl session.\n");
return FALSE;
}
- if (dwarf_info.name_debuginfo) {
- /* We have absolute path for debuginfo file, use it directly
- * instead of searching it through
- * dwfl_linux_kernel_report_offline() call.
- *
- * Open the debuginfo file if it is not already open.
- */
- if (dwarf_info.fd_debuginfo < 0)
- dwarf_info.fd_debuginfo =
- open(dwarf_info.name_debuginfo, O_RDONLY);
-
- dwfl_fd = dup(dwarf_info.fd_debuginfo);
- if (dwfl_fd < 0) {
- ERRMSG("Failed to get a duplicate handle for"
- " debuginfo.\n");
- goto err_out;
- }
- if (dwfl_report_offline(dwfl, dwarf_info.module_name,
- dwarf_info.name_debuginfo, dwfl_fd) == NULL) {
- ERRMSG("Failed reading %s: %s\n",
- dwarf_info.name_debuginfo, dwfl_errmsg (-1));
- /* dwfl_fd is consumed on success, not on failure */
- close(dwfl_fd);
- goto err_out;
- }
- } else if (dwfl_linux_kernel_report_offline(dwfl,
- info->system_utsname.release,
- &dwfl_report_module_p)) {
- ERRMSG("Can't get Module debuginfo for module '%s'\n",
- dwarf_info.module_name);
+ /* Open the debuginfo file if it is not already open. */
+ if (dwarf_info.fd_debuginfo < 0)
+ dwarf_info.fd_debuginfo =
+ open(dwarf_info.name_debuginfo, O_RDONLY);
+
+ dwfl_fd = dup(dwarf_info.fd_debuginfo);
+ if (dwfl_fd < 0) {
+ ERRMSG("Failed to get a duplicate handle for"
+ " debuginfo.\n");
+ goto err_out;
+ }
+ /* Apply relocations. */
+ if (dwfl_report_offline(dwfl, dwarf_info.module_name,
+ dwarf_info.name_debuginfo, dwfl_fd) == NULL) {
+ ERRMSG("Failed reading %s: %s\n",
+ dwarf_info.name_debuginfo, dwfl_errmsg (-1));
+ /* dwfl_fd is consumed on success, not on failure */
+ close(dwfl_fd);
goto err_out;
}
dwfl_report_end(dwfl, NULL, NULL);
@@ -2522,20 +2569,19 @@ set_dwarf_debuginfo(char *mod_name, char *name_debuginfo, int fd_debuginfo)
if (dwarf_info.name_debuginfo)
free(dwarf_info.name_debuginfo);
}
+ if (dwarf_info.module_name)
+ free(dwarf_info.module_name);
+
dwarf_info.fd_debuginfo = fd_debuginfo;
dwarf_info.name_debuginfo = name_debuginfo;
- dwarf_info.module_name = mod_name;
+ dwarf_info.module_name = strdup(mod_name);
if (!strcmp(dwarf_info.module_name, "vmlinux") ||
!strcmp(dwarf_info.module_name, "xen-syms"))
return TRUE;
/* check to see whether module debuginfo is available */
- if (!init_dwarf_info())
- return FALSE;
- else
- clean_dwfl_info();
- return TRUE;
+ return search_module_debuginfo();
}
int
