[Crash-utility] [PATCH] Also search module init section for symbols

Dave Anderson anderson at redhat.com
Thu Sep 30 14:54:34 UTC 2010


----- "Hu Tao" <hutao at cn.fujitsu.com> wrote:

> Hi Dave,
> 
>    When I was investigating the backtrace problem(crash doesn't show
>    some functions from backtrace output), I found the reason was that
>    there was a dead-looping(or anything that would block module init
>    function) in module init function, in which case kernel had no chance
>    to update mod->symtab from mod->core_symtab, and mod->symtab was
>    still referring into the module init section which was not freed
>    until the end of module init function.
> 
>    Since crash never searches module init function for symbols, in the
>    case we can't see any symbol from module from the backtrace output.
> 
>    Following patch makes crash search the module init section for
>    symbols too if the section is not null.

This is a pretty adventurous patch, but it looks pretty good.

Given that your new module init code is pretty much segregated based
upon the setting of lm->mod_init_size, it shouldn't break the handling
of normal "post-init" modules.

A few minor issues:

# touch symbols.c
# make warn
cc -c -g -DX86_64   symbols.c -DGDB_7_0 -I./gdb-7.0/bfd -I./gdb-7.0/include -Wall -O2 -Wstrict-prototypes -Wmissing-prototypes -fstack-protector -Wp,-D_FORTIFY_SOURCE=2 
symbols.c:3651: warning: no previous prototype for ‘value_search_module’
symbols.c: In function ‘value_search’:
symbols.c:3734: warning: unused variable ‘lm’
symbols.c:3733: warning: unused variable ‘splast’
symbols.c:3733: warning: unused variable ‘sp_end’
symbols.c:3732: warning: unused variable ‘i’
#

Also, I'm wondering why you felt it necessary to make two
calls to the new value_search_module() function here in
value_search():

check_modules:
        sp = value_search_module(value, offset, 0);
        if (!sp)
                sp = value_search_module(value, offset, 1);

What I would suggest is this:

(1) If during initialization it is seen that a module's init symbols are
    still in place, then set a new "MOD_INIT" flag in lm->mod_flags.
(2) Then, if value_search_module() cannot find a symbol in the lm->mod_symtable,
    it could check lm->mod_flags for the MOD_INIT flag, and then go back and
    retry the search in the lm->mod_init_symtable.

If it were done that way, I can envision making value_search_module()
exportable, and it would not require the caller to make the "0/1" argument
decision.

Dave


 

> 
> -- 
> Thanks,
> Hu Tao
> 
> diff --git a/defs.h b/defs.h
> index d431d6e..0fe9d48 100755
> --- a/defs.h
> +++ b/defs.h
> @@ -1515,6 +1515,7 @@ struct offset_table {                    /*
> stash of commonly-used offsets */
>  	long mm_struct_rss_stat;
>  	long mm_rss_stat_count;
>  	long module_module_init;
> +	long module_init_size;
>  	long module_init_text_size;
>  	long cpu_context_save_fp;
>  	long cpu_context_save_sp;
> @@ -2038,6 +2039,8 @@ struct load_module {
>          ulong mod_flags;
>  	struct syment *mod_symtable;
>  	struct syment *mod_symend;
> +	struct syment *mod_init_symtable;
> +	struct syment *mod_init_symend;
>          long mod_ext_symcnt;
>  	struct syment *mod_ext_symtable;
>  	struct syment *mod_ext_symend;
> @@ -2054,6 +2057,7 @@ struct load_module {
>  	ulong mod_bss_start;
>  	int mod_sections;
>  	struct mod_section_data *mod_section_data;
> +        ulong mod_init_size;
>          ulong mod_init_text_size;
>          ulong mod_init_module_ptr;
>  };
> @@ -2061,6 +2065,9 @@ struct load_module {
>  #define IN_MODULE(A,L) \
>   (((ulong)(A) >= (L)->mod_base) && ((ulong)(A) <
> ((L)->mod_base+(L)->mod_size)))
>  
> +#define IN_MODULE_INIT(A,L) \
> + (((ulong)(A) >= (L)->mod_init_module_ptr) && ((ulong)(A) <
> ((L)->mod_init_module_ptr+(L)->mod_init_size)))
> +
>  #ifndef GDB_COMMON
>  
>  #define KVADDR             (0x1)
> diff --git a/kernel.c b/kernel.c
> index 5dc2c45..a50882a 100755
> --- a/kernel.c
> +++ b/kernel.c
> @@ -2691,6 +2691,7 @@ module_init(void)
>          	MEMBER_OFFSET_INIT(module_core_text_size, "module", 
>  			"core_text_size");
>  		MEMBER_OFFSET_INIT(module_module_init, "module", "module_init");
> +		MEMBER_OFFSET_INIT(module_init_size, "module", "init_size");
>  		MEMBER_OFFSET_INIT(module_init_text_size, "module", 
>  			"init_text_size");
>  
> diff --git a/symbols.c b/symbols.c
> index c373668..f1c308e 100755
> --- a/symbols.c
> +++ b/symbols.c
> @@ -884,10 +884,13 @@ symname_hash_search(char *name)
>   */
>  
>  #define MODULE_PSEUDO_SYMBOL(sp) \
> -    (STRNEQ((sp)->name, "_MODULE_START_") || STRNEQ((sp)->name,
> "_MODULE_END_"))
> +    ((STRNEQ((sp)->name, "_MODULE_START_") || STRNEQ((sp)->name,
> "_MODULE_END_")) || \
> +    (STRNEQ((sp)->name, "_MODULE_INIT_START_") || STRNEQ((sp)->name,
> "_MODULE_INIT_END_")))
>  
>  #define MODULE_START(sp) (STRNEQ((sp)->name, "_MODULE_START_"))
>  #define MODULE_END(sp)   (STRNEQ((sp)->name, "_MODULE_END_"))
> +#define MODULE_INIT_START(sp) (STRNEQ((sp)->name,
> "_MODULE_INIT_START_"))
> +#define MODULE_INIT_END(sp)   (STRNEQ((sp)->name,
> "_MODULE_INIT_END_"))
>  
>  static void
>  symbol_dump(ulong flags, char *module)
> @@ -927,6 +930,26 @@ symbol_dump(ulong flags, char *module)
>  			} else
>  				show_symbol(sp, 0, SHOW_RADIX());
>                  }
> +
> +		if (lm->mod_init_symtable) {
> +			sp = lm->mod_init_symtable;
> +			sp_end = lm->mod_init_symend;
> +
> +			for ( ; sp <= sp_end; sp++) {
> +				if (MODULE_PSEUDO_SYMBOL(sp)) {
> +					if (MODULE_INIT_START(sp)) {
> +						p1 = "MODULE INIT START";
> +						p2 = sp->name+strlen("_MODULE_INIT_START_");
> +					} else {
> +						p1 = "MODULE INIT END";
> +						p2 = sp->name+strlen("_MODULE_INIT_END_");
> +					}
> +					fprintf(fp, "%lx %s: %s\n", sp->value, p1, p2);
> +				} else
> +					show_symbol(sp, 0, SHOW_RADIX());
> +			}
> +		}
> +
>  	}
>  }
>  
> @@ -1244,6 +1267,8 @@ store_module_symbols_v2(ulong total, int
> mods_installed)
>  	struct load_module *lm;
>  	char buf1[BUFSIZE];
>  	char buf2[BUFSIZE];
> +	char buf3[BUFSIZE];
> +	char buf4[BUFSIZE];
>  	char *strbuf, *modbuf, *modsymbuf;
>  	struct syment *sp;
>  	ulong first, last;
> @@ -1326,11 +1351,15 @@ store_module_symbols_v2(ulong total, int
> mods_installed)
>  		if (THIS_KERNEL_VERSION >= LINUX(2,6,27)) {
>  			lm->mod_etext_guess = lm->mod_base +
>  				UINT(modbuf + OFFSET(module_core_text_size));
> +			lm->mod_init_size =
> +				UINT(modbuf + OFFSET(module_init_size));
>  			lm->mod_init_text_size = 
>  				UINT(modbuf + OFFSET(module_init_text_size));
>  		} else {
>  			lm->mod_etext_guess = lm->mod_base +
>  				ULONG(modbuf + OFFSET(module_core_text_size));
> +			lm->mod_init_size =
> +				ULONG(modbuf + OFFSET(module_init_size));
>  			lm->mod_init_text_size = 
>  				ULONG(modbuf + OFFSET(module_init_text_size));
>  		}
> @@ -1344,6 +1373,18 @@ store_module_symbols_v2(ulong total, int
> mods_installed)
>  		lm_mcnt = mcnt;
>  		mcnt++;
>  
> +		if (lm->mod_init_size > 0) {
> +			st->ext_module_symtable[mcnt].value = lm->mod_init_module_ptr;
> +			st->ext_module_symtable[mcnt].type = 'm';
> +			sprintf(buf3, "%s%s", "_MODULE_INIT_START_", mod_name);
> +			namespace_ctl(NAMESPACE_INSTALL, 
> +					&st->ext_module_namespace,
> +					&st->ext_module_symtable[mcnt], buf3);
> +			lm_mcnt = mcnt;
> +			mcnt++;
> +		}
> +
> +
>  		if (nsyms && !IN_MODULE(syms, lm)) {
>  			error(WARNING, 
>  			    "[%s] module.syms outside of module "
> @@ -1520,6 +1561,16 @@ store_module_symbols_v2(ulong total, int
> mods_installed)
>                          &st->ext_module_symtable[mcnt], buf2);
>  		mcnt++;
>  
> +		if (lm->mod_init_size > 0) {
> +			st->ext_module_symtable[mcnt].value = lm->mod_init_module_ptr +
> lm->mod_init_size;
> +			st->ext_module_symtable[mcnt].type = 'm';
> +			sprintf(buf4, "%s%s", "_MODULE_INIT_END_", mod_name);
> +			namespace_ctl(NAMESPACE_INSTALL, 
> +					&st->ext_module_namespace,
> +					&st->ext_module_symtable[mcnt], buf4);
> +			mcnt++;
> +		}
> +
>  		lm->mod_ext_symcnt = mcnt - lm->mod_ext_symcnt;
>  
>  		if (!lm->mod_etext_guess)
> @@ -1545,6 +1596,8 @@ store_module_symbols_v2(ulong total, int
> mods_installed)
>                  lm = &st->load_modules[m];
>  		sprintf(buf1, "_MODULE_START_%s", lm->mod_name);
>  		sprintf(buf2, "_MODULE_END_%s", lm->mod_name);
> +		sprintf(buf3, "_MODULE_INIT_START_%s", lm->mod_name);
> +		sprintf(buf4, "_MODULE_INIT_END_%s", lm->mod_name);
>  
>          	for (sp = st->ext_module_symtable; 
>  		     sp < st->ext_module_symend; sp++) {
> @@ -1556,6 +1609,12 @@ store_module_symbols_v2(ulong total, int
> mods_installed)
>  				lm->mod_ext_symend = sp;
>  				lm->mod_symend = sp;
>  			}
> +			if (STREQ(sp->name, buf3)) {
> +				lm->mod_init_symtable = sp;
> +			}
> +			if (STREQ(sp->name, buf4)) {
> +				lm->mod_init_symend = sp;
> +			}
>  		}
>  	}
>  
> @@ -1750,6 +1809,7 @@ store_module_kallsyms_v2(struct load_module *lm,
> int start, int curr,
>  	struct namespace *ns;
>          int mcnt;
>          int mcnt_idx;
> +	char *module_buf_init = NULL;
>  
>  	if (!(kt->flags & KALLSYMS_V2))
>  		return 0;
> @@ -1772,30 +1832,50 @@ store_module_kallsyms_v2(struct load_module
> *lm, int start, int curr,
>                  return 0;
>          }
>  
> +	if (lm->mod_init_size > 0) {
> +		module_buf_init = GETBUF(lm->mod_init_size);
> +
> +		if (!readmem(lm->mod_init_module_ptr, KVADDR, module_buf_init,
> lm->mod_init_size,
> +					"module init (kallsyms)", RETURN_ON_ERROR|QUIET)) {
> +			error(WARNING,"cannot access module init kallsyms\n");
> +			FREEBUF(module_buf_init);
> +		}
> +	}
> +
>  	if (THIS_KERNEL_VERSION >= LINUX(2,6,27))
>  		nksyms = UINT(modbuf + OFFSET(module_num_symtab));
>  	else
>  		nksyms = ULONG(modbuf + OFFSET(module_num_symtab));
>  
>  	ksymtab = ULONG(modbuf + OFFSET(module_symtab));
> -	if (!IN_MODULE(ksymtab, lm)) {
> +	if (!IN_MODULE(ksymtab, lm) && !IN_MODULE_INIT(ksymtab, lm)) {
>  		error(WARNING,
>  		    "%s: module.symtab outside of module address space\n",
>  			lm->mod_name);
>  		FREEBUF(module_buf);
> +		if (module_buf_init)
> +			FREEBUF(module_buf_init);
>  		return 0;
>  	} 
> -	locsymtab = module_buf + (ksymtab - lm->mod_base);
> +	if (IN_MODULE(ksymtab, lm))
> +		locsymtab = module_buf + (ksymtab - lm->mod_base);
> +	else
> +		locsymtab = module_buf_init + (ksymtab - lm->mod_init_module_ptr);
>  
>  	kstrtab = ULONG(modbuf + OFFSET(module_strtab));
> -	if (!IN_MODULE(kstrtab, lm)) {
> +	if (!IN_MODULE(kstrtab, lm) && !IN_MODULE_INIT(kstrtab, lm)) {
>  		error(WARNING, 
>  		    "%s: module.strtab outside of module address space\n",
>  			lm->mod_name);
>  		FREEBUF(module_buf);
> +		if (module_buf_init)
> +			FREEBUF(module_buf_init);
>  		return 0;
>  	}
> -	locstrtab = module_buf + (kstrtab - lm->mod_base);
> +	if (IN_MODULE(kstrtab, lm))
> +		locstrtab = module_buf + (kstrtab - lm->mod_base);
> +	else
> +		locstrtab = module_buf_init + (kstrtab - lm->mod_init_module_ptr);
>  
>  	for (i = 1; i < nksyms; i++) {  /* ELF starts real symbols at 1 */
>  		switch (BITS())
> @@ -1810,14 +1890,16 @@ store_module_kallsyms_v2(struct load_module
> *lm, int start, int curr,
>  			break;
>  		}
>  
> -		if ((ec->st_value < lm->mod_base) || 
> -		    (ec->st_value >  (lm->mod_base + lm->mod_size))) 
> +		if (((ec->st_value < lm->mod_base) ||
> +		    (ec->st_value >  (lm->mod_base + lm->mod_size)))
> +		    && (ec->st_value < lm->mod_init_module_ptr ||
> +			ec->st_value > (lm->mod_init_module_ptr + lm->mod_init_size)))
>  				continue;
>  
>  		if (ec->st_shndx == SHN_UNDEF)
>                          continue;
>  
> -		if (!IN_MODULE(kstrtab + ec->st_name, lm)) {
> +		if (!IN_MODULE(kstrtab + ec->st_name, lm) &&
> !IN_MODULE_INIT(kstrtab + ec->st_name, lm)) {
>  			if (CRASHDEBUG(3)) {
>  				error(WARNING, 
>  				   "%s: bad st_name index: %lx -> %lx\n        "
> @@ -1869,6 +1951,8 @@ store_module_kallsyms_v2(struct load_module *lm,
> int start, int curr,
>  
>          lm->mod_flags |= MOD_KALLSYMS;
>          FREEBUF(module_buf);
> +	if (module_buf_init)
> +		FREEBUF(module_buf_init);
>  
>          return mcnt;
>  }
> @@ -2211,7 +2295,7 @@ is_kernel_text(ulong value)
>          for (i = 0; i < st->mods_installed; i++) {
>                  lm = &st->load_modules[i];
>  
> -		if (!IN_MODULE(value, lm))
> +		if (!IN_MODULE(value, lm) && !IN_MODULE_INIT(value, lm))
>  			continue;
>  
>  		if (lm->mod_flags & MOD_LOAD_SYMS) {
> @@ -2233,10 +2317,15 @@ is_kernel_text(ulong value)
>  				start = lm->mod_base + lm->mod_size_of_struct;
>  				break;
>  			case KMOD_V2:
> -				start = lm->mod_base;
> +				if (IN_MODULE(value, lm))
> +					start = lm->mod_base;
> +				else
> +					start = lm->mod_init_module_ptr;
>  				break;
>  			}
>  			end = lm->mod_etext_guess;
> +			if (IN_MODULE_INIT(value, lm) && end < lm->mod_init_module_ptr +
> lm->mod_init_size)
> +				end = lm->mod_init_module_ptr + lm->mod_init_size;
>  
>  	        	if ((value >= start) && (value < end)) 
>  	               		return TRUE;
> @@ -2524,6 +2613,8 @@ dump_symbol_table(void)
>                          lm->mod_bss_start,
>                          lm->mod_bss_start ?
>                          lm->mod_bss_start - lm->mod_base : 0);
> +		fprintf(fp, "    mod_init_size: %ld\n",
> +			lm->mod_init_size);
>  		fprintf(fp, "    mod_init_text_size: %ld\n",
>  			lm->mod_init_text_size);
>  		fprintf(fp, "   mod_init_module_ptr: %lx\n",
> @@ -3493,6 +3584,7 @@ module_symbol(ulong value,
>  	long mcnt;
>  	char buf[BUFSIZE];
>  	ulong offs, offset;
> +	ulong base, end;
>  
>  	if (NO_MODULES())
>  		return FALSE;
> @@ -3506,13 +3598,20 @@ module_symbol(ulong value,
>  	for (i = 0; i < st->mods_installed; i++) {
>  		lm = &st->load_modules[i];
>  
> -		if ((value >= lm->mod_base) && 
> -		    (value < (lm->mod_base + lm->mod_size))) {
> +		if (IN_MODULE(value, lm)) {
> +			base = lm->mod_base;
> +			end = lm->mod_base + lm->mod_size;
> +		} else {
> +			base = lm->mod_init_module_ptr;
> +			end = lm->mod_init_module_ptr + lm->mod_init_size;
> +		}
> +
> +		if ((value >= base) && (value < end)) {
>  			if (lmp) 
>  				*lmp = lm;
>  
>  			if (name) {
> -				offs = value - lm->mod_base;
> +				offs = value - base;
>          			if ((sp = value_search(value, &offset))) {
>                  			if (offset)
>                          			sprintf(buf, radix == 16 ? 
> @@ -3541,54 +3640,26 @@ module_symbol(ulong value,
>  	return FALSE;
>  }
>  
> -/*
> - *  Return the syment of the symbol closest to the value, along with
> - *  the offset from the symbol value if requested.
> - */
>  struct syment *
> -value_search(ulong value, ulong *offset)
> +value_search_module(ulong value, ulong *offset, int
> search_init_section)
>  {
>  	int i;
>          struct syment *sp, *sp_end, *spnext, *splast;
>  	struct load_module *lm;
>  
> -        if (!in_ksymbol_range(value))
> -                return((struct syment *)NULL);
> -
> -	if ((sp = machdep->value_to_symbol(value, offset)))
> -		return sp;
> -
> -	if (IS_VMALLOC_ADDR(value)) 
> -		goto check_modules;
> -
> -	if ((sp = symval_hash_search(value)) == NULL)
> -		sp = st->symtable;
> - 
> -        for ( ; sp < st->symend; sp++) {
> -                if (value == sp->value) {
> -#ifdef GDB_7_0
> -			if (STRNEQ(sp->name, ".text.")) {
> -				spnext = sp+1;
> -				if (spnext->value == value)
> -					sp = spnext;
> -			}
> -#endif
> -                        if (offset) 
> -				*offset = 0;
> -                        return((struct syment *)sp);
> -                }
> -                if (sp->value > value) {
> -			if (offset)
> -                        	*offset = value - ((sp-1)->value);
> -                        return((struct syment *)(sp-1));
> -                }
> -        }
> -
> -check_modules:
>          for (i = 0; i < st->mods_installed; i++) {
>                  lm = &st->load_modules[i];
> -		sp = lm->mod_symtable;
> -                sp_end = lm->mod_symend;
> +
> +		if (!search_init_section) {
> +			sp = lm->mod_symtable;
> +			sp_end = lm->mod_symend;
> +		} else {
> +			if (lm->mod_init_symtable) {
> +				sp = lm->mod_init_symtable;
> +				sp_end = lm->mod_init_symend;
> +			} else
> +				continue;
> +		}
>  
>  		if (sp->value > value)  /* invalid -- between modules */
>  			break;
> @@ -3599,7 +3670,7 @@ check_modules:
>                  *  when they have unique values.
>  		*/
>  		splast = NULL;
> -                for ( ; sp < sp_end; sp++) {
> +                for ( ; sp <= sp_end; sp++) {
>                  	if (value == sp->value) {
>  				if (MODULE_START(sp)) {
>  					spnext = sp+1;
> @@ -3644,6 +3715,57 @@ check_modules:
>          return((struct syment *)NULL);
>  }
>  
> +/*
> + *  Return the syment of the symbol closest to the value, along with
> + *  the offset from the symbol value if requested.
> + */
> +struct syment *
> +value_search(ulong value, ulong *offset)
> +{
> +	int i;
> +        struct syment *sp, *sp_end, *spnext, *splast;
> +	struct load_module *lm;
> +
> +        if (!in_ksymbol_range(value))
> +                return((struct syment *)NULL);
> +
> +	if ((sp = machdep->value_to_symbol(value, offset)))
> +		return sp;
> +
> +	if (IS_VMALLOC_ADDR(value)) 
> +		goto check_modules;
> +
> +	if ((sp = symval_hash_search(value)) == NULL)
> +		sp = st->symtable;
> + 
> +        for ( ; sp < st->symend; sp++) {
> +                if (value == sp->value) {
> +#ifdef GDB_7_0
> +			if (STRNEQ(sp->name, ".text.")) {
> +				spnext = sp+1;
> +				if (spnext->value == value)
> +					sp = spnext;
> +			}
> +#endif
> +                        if (offset) 
> +				*offset = 0;
> +                        return((struct syment *)sp);
> +                }
> +                if (sp->value > value) {
> +			if (offset)
> +                        	*offset = value - ((sp-1)->value);
> +                        return((struct syment *)(sp-1));
> +                }
> +        }
> +
> +check_modules:
> +	sp = value_search_module(value, offset, 0);
> +	if (!sp)
> +		sp = value_search_module(value, offset, 1);
> +
> +	return sp;
> +}
> +
>  ulong
>  highest_bss_symbol(void)
>  {
> @@ -6536,6 +6658,8 @@ dump_offset_table(char *spec, ulong makestruct)
>  		OFFSET(module_core_size));
>  	fprintf(fp, "         module_core_text_size: %ld\n",
>  		OFFSET(module_core_text_size));
> +	fprintf(fp, "         module_init_size: %ld\n",
> +		OFFSET(module_init_size));
>  	fprintf(fp, "         module_init_text_size: %ld\n",
>  		OFFSET(module_init_text_size));
>  	fprintf(fp, "            module_module_init: %ld\n",




More information about the Crash-utility mailing list