[PATCH] makedumpfile: close the dwfl handle before next invocation of init_dwarf_info()

Atsushi Kumagai kumagai-atsushi at mxc.nes.nec.co.jp
Wed Apr 17 20:46:50 EDT 2013


Hello Mahesh,

On Mon, 15 Apr 2013 15:50:52 +0530
Mahesh J Salgaonkar <mahesh at linux.vnet.ibm.com> wrote:

> From: Mahesh Salgaonkar <mahesh at linux.vnet.ibm.com>
> 
> The init_dwarf_info() function initializes "dwarf_info.dwfl" handle everytime
> when it is called. The "dwarf_info.dwfl" handle should be freed once we are
> done with searching of desired debuginfo and before next invocation of
> init_dwarf_info().
> 
> Most of the functions (e.g get_debug_info()) that invokes init_dwarf_info()
> also invokes clean_dwfl_info() to free dwfl handle. But the function
> get_die_from_offset() that invokes init_dwarf_info(), does not free the dwfl
> handle and hence it keeps re-reading debuginfo ELF section into memory every
> time it is invoked until it starts failing with following errors:
> 
> init_dwarf_info: Can't get first elf header of /usr/lib/debug/boot/vmlinux-x.x.x
> get_die_attr_type: Can't find the DIE.
> init_dwarf_info: Can't get first elf header of /usr/lib/debug/boot/vmlinux-x.x.x
> init_dwarf_info: Can't get first elf header of /usr/lib/debug/boot/vmlinux-x.x.x
> File eppic_macro/key.c, line 32, Error: Oops! Var ref1.[keyring_name_hash]
> 
> This patch makes sure that clean_dwfl_info() is called once we are done with
> the debuginfo search.
> 
> Signed-off-by: Mahesh Salgaonkar <mahesh at linux.vnet.ibm.com>

It looks good to me.
I'll merge this fix into v1.5.4.


Thanks
Atsushi Kumagai

> ---
>  dwarf_info.c      |   58 +++++++++++++++++++++++++++++++++++++++++++++--------
>  extension_eppic.c |   23 +++++++++++++++------
>  2 files changed, 65 insertions(+), 16 deletions(-)
> 
> diff --git a/dwarf_info.c b/dwarf_info.c
> index a11b6ad..478d637 100644
> --- a/dwarf_info.c
> +++ b/dwarf_info.c
> @@ -1347,11 +1347,14 @@ get_die_nfields(unsigned long long die_off)
>  	tag = dwarf_tag(die);
>  	if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type) {
>  		ERRMSG("DIE is not of structure or union type.\n");
> +		clean_dwfl_info();
>  		return -1;
>  	}
>  
> -	if (dwarf_child(die, &child) != 0)
> +	if (dwarf_child(die, &child) != 0) {
> +		clean_dwfl_info();
>  		return -1;
> +	}
>  
>  	/* Find the number of fields in the structure */
>  	die = &child;
> @@ -1363,6 +1366,7 @@ get_die_nfields(unsigned long long die_off)
>  			continue;
>  	} while (!dwarf_siblingof(die, die));
>  
> +	clean_dwfl_info();
>  	return nfields;
>  }
>  
> @@ -1388,11 +1392,14 @@ get_die_member(unsigned long long die_off, int index, long *offset,
>  	tag = dwarf_tag(die);
>  	if (tag != DW_TAG_structure_type && tag != DW_TAG_union_type) {
>  		ERRMSG("DIE is not of structure or union type.\n");
> +		clean_dwfl_info();
>  		return -1;
>  	}
>  
> -	if (dwarf_child(die, &child) != 0)
> +	if (dwarf_child(die, &child) != 0) {
> +		clean_dwfl_info();
>  		return -1;
> +	}
>  
>  	/* Find the correct field in the structure */
>  	die = &child;
> @@ -1408,6 +1415,7 @@ get_die_member(unsigned long long die_off, int index, long *offset,
>  
>  	if (nfields != index) {
>  		ERRMSG("No member found at index %d.\n", index);
> +		clean_dwfl_info();
>  		return -1;
>  	}
>  
> @@ -1416,6 +1424,13 @@ get_die_member(unsigned long long die_off, int index, long *offset,
>  		*offset = 0;
>  
>  	*name = dwarf_diename(die);
> +	/*
> +	 * Duplicate the string before we pass it to eppic layer. The
> +	 * original string returned by dwarf layer will become invalid
> +	 * after clean_dwfl_info() call.
> +	 */
> +	if (*name)
> +		*name = strdup(*name);
>  	*m_die = dwarf_dieoffset(die);
>  
>  	get_die_type(die, &die_base);
> @@ -1432,6 +1447,7 @@ get_die_member(unsigned long long die_off, int index, long *offset,
>  	 */
>  	*nbits = *fbits = 0;
>  
> +	clean_dwfl_info();
>  	if (size < 0)
>  		return 0;
>  	else
> @@ -1455,21 +1471,35 @@ get_die_attr_type(unsigned long long die_off, int *type_flag,
>  		return FALSE;
>  	}
>  
> -	if (!get_die_type(&result, &result))
> +	if (!get_die_type(&result, &result)) {
> +		clean_dwfl_info();
>  		return FALSE;
> +	}
>  
>  	*die_attr_off = dwarf_dieoffset(&result);
>  	*type_flag = dwarf_tag(&result);
> +	clean_dwfl_info();
>  	return TRUE;
>  }
>  
>  /*
> - * Get name attribute given the die offset
> + * Get name attribute given the die offset This function is called by eppic
> + * layer directly as one of the callback functions.
> + *
> + * This function returns a pointer to newly allocated string which is a
> + * duplicate of original string returned from dwarf APIs. The reason for doing
> + * this is because the original string returned by dwarf layer will become
> + * invalid (freed) as soon as we close the dwarf handle through
> + * clean_dwfl_info(). This avoids the segfault when caller (eppic layer) of
> + * this function tries to access the string pointer.
> + *
> + * NOTE: It is callers responsibility to free the memory of new string.
>   */
>  char *
>  get_die_name(unsigned long long die_off)
>  {
>  	Dwarf_Die result;
> +	char *name = NULL;
>  
>  	if (!die_off)
>  		return NULL;
> @@ -1479,7 +1509,11 @@ get_die_name(unsigned long long die_off)
>  		return NULL;
>  	}
>  
> -	return dwarf_diename(&result);
> +	name = dwarf_diename(&result);
> +	if (name)
> +		name = strdup(name);
> +	clean_dwfl_info();
> +	return name;
>  }
>  
>  /*
> @@ -1510,6 +1544,7 @@ int
>  get_die_length(unsigned long long die_off, int flag)
>  {
>  	Dwarf_Die result, die_base;
> +	int size = 0;
>  
>  	if (!die_off)
>  		return FALSE;
> @@ -1519,17 +1554,22 @@ get_die_length(unsigned long long die_off, int flag)
>  		return FALSE;
>  	}
>  
> -	if (flag)
> -		return dwarf_bytesize(&result);
> +	if (flag) {
> +		size = dwarf_bytesize(&result);
> +		goto out;
> +	}
>  
>  	get_die_type(&result, &die_base);
>  	if (dwarf_tag(&die_base) == DW_TAG_array_type) {
>  		dwarf_info.array_length = 0;
>  		get_data_array_length(&result);
> -		return dwarf_info.array_length;
> +		size = dwarf_info.array_length;
>  	} else {
> -		return dwarf_bytesize(&die_base);
> +		size = dwarf_bytesize(&die_base);
>  	}
> +out:
> +	clean_dwfl_info();
> +	return size;
>  }
>  
>  /*
> diff --git a/extension_eppic.c b/extension_eppic.c
> index a1981eb..d5e5ad0 100644
> --- a/extension_eppic.c
> +++ b/extension_eppic.c
> @@ -121,7 +121,7 @@ drilldown(ull offset, type_t *t)
>  	int type_flag, len = 0, t_len = 0, nidx = 0;
>  	int fctflg = 0, ref = 0, *idxlst = 0;
>  	ull die_off = offset, t_die_off;
> -	char *tstr = NULL;
> +	char *tstr = NULL, *tstr_dup = NULL;
>  
>  	while (GET_DIE_ATTR_TYPE(die_off, &type_flag, &t_die_off)) {
>  		switch (type_flag) {
> @@ -201,9 +201,10 @@ out:
>  	if (fctflg)
>  		eppic_type_setfct(t, 1);
>  	eppic_pushref(t, ref + (nidx ? 1 : 0));
> -	if (tstr)
> -		return eppic_strdup(tstr);
> -	return eppic_strdup("");
> +	tstr_dup = (tstr) ? eppic_strdup(tstr) : eppic_strdup("");
> +	/* Free the memory allocated by makedumpfile. */
> +	free(tstr);
> +	return tstr_dup;
>  }
>  
>  /*
> @@ -216,7 +217,7 @@ apimember(char *mname, ull idx, type_t *tm, member_t *m, ull *last_index)
>  	int nbits = 0, fbits = 0;
>  	long offset;
>  	ull m_die, die_off = idx;
> -	char *name;
> +	char *name = NULL;
>  
>  	nfields = GET_DIE_NFIELDS(die_off);
>  	/*
> @@ -243,8 +244,13 @@ apimember(char *mname, ull idx, type_t *tm, member_t *m, ull *last_index)
>  
>  		if (!mname || !mname[0] || !strcmp(mname, name)) {
>  			eppic_member_ssize(m, size);
> -			if (name)
> +			if (name) {
>  				eppic_member_sname(m, name);
> +				/*
> +				 * Free the memory allocated by makedumpfile.
> +				 */
> +				free(name);
> +			}
>  			else
>  				eppic_member_sname(m, "");
>  			eppic_member_soffset(m, offset);
> @@ -334,8 +340,11 @@ apigetval(char *name, ull *val, VALUE_S *value)
>  
>  	if (!eppic_typeislocal(stype) && eppic_type_getidx(stype) > 100) {
>  		char *tname = GET_DIE_NAME(eppic_type_getidx(stype));
> -		if (tname)
> +		if (tname) {
>  			eppic_chktype(stype, tname);
> +			/* Free the memory allocated by makedumpfile. */
> +			free(tname);
> +		}
>  	}
>  	return 1;
>  }
>



More information about the kexec mailing list