Re: Kernel build with gcc 8 a lot of warnings

[damian <linux_dti@icloud.com>]

Hi Josh,

i tested the patch on my maschine it works fine. 

Best regards
Damian

On Tue, 08. May 22:46, Josh Poimboeuf wrote:
> On Tue, May 08, 2018 at 09:32:41AM -0500, Josh Poimboeuf wrote:
> > On Tue, May 08, 2018 at 04:25:15PM +0200, Greg KH wrote:
> > > Much nicer, thanks for the patch, seems to build things "quieter" now.
> > > For the other warnings, they are "safe" to ignore, right?  It's just
> > > objtool checking to see if all is ok, but the result should be fine no
> > > matter what, right?
> > 
> > Yeah, from what I can tell, there's nothing catastrophic in the rest of
> > the warnings.  Worst case, the ORC unwinder might get confused in a few
> > places.
> 
> FWIW, here's the latest version of the patch.  This fixes all the
> warnings on my config at least.  I'll be sending it to -tip soon if it
> survives 0-day testing.
> 
> From: Josh Poimboeuf <jpoimboe@redhat.com>
> Subject: [PATCH] objtool: Detect GCC 8 cold subfunctions
> 
> GCC 8 moves a lot of unlikely code out of line to "cold" subfunctions in
> .text.unlikely.  Properly detect the new subfunctions and treat them as
> extensions of the original functions.
> 
> This fixes a bunch of warnings like:
> 
>   kernel/cgroup/cgroup.o: warning: objtool: parse_cgroup_root_flags()+0x33: sibling call from callable instruction with modified stack frame
>   kernel/cgroup/cgroup.o: warning: objtool: cgroup_addrm_files()+0x290: sibling call from callable instruction with modified stack frame
>   kernel/cgroup/cgroup.o: warning: objtool: cgroup_apply_control_enable()+0x25b: sibling call from callable instruction with modified stack frame
>   kernel/cgroup/cgroup.o: warning: objtool: rebind_subsystems()+0x325: sibling call from callable instruction with modified stack frame
> 
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  tools/objtool/check.c | 104 +++++++++++++++++++++++++-----------------
>  tools/objtool/elf.c   |  42 ++++++++++++++++-
>  tools/objtool/elf.h   |   2 +
>  3 files changed, 104 insertions(+), 44 deletions(-)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 264522d4e4af..ee72958d3464 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -59,6 +59,31 @@ static struct instruction *next_insn_same_sec(struct objtool_file *file,
>  	return next;
>  }
>  
> +static struct instruction *next_insn_same_func(struct objtool_file *file,
> +					       struct instruction *insn)
> +{
> +	struct instruction *next = list_next_entry(insn, list);
> +	struct symbol *func = insn->func;
> +
> +	if (!func)
> +		return NULL;
> +
> +	if (&next->list != &file->insn_list && next->func == func)
> +		return next;
> +
> +	/* Check if we're already in the subfunction: */
> +	if (func == func->cfunc)
> +		return NULL;
> +
> +	/* Move to the subfunction: */
> +	return find_insn(file, func->cfunc->sec, func->cfunc->offset);
> +}
> +
> +#define func_for_each_insn_all(file, func, insn)			\
> +	for (insn = find_insn(file, func->sec, func->offset);		\
> +	     insn;							\
> +	     insn = next_insn_same_func(file, insn))
> +
>  #define func_for_each_insn(file, func, insn)				\
>  	for (insn = find_insn(file, func->sec, func->offset);		\
>  	     insn && &insn->list != &file->insn_list &&			\
> @@ -149,10 +174,14 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func,
>  			if (!strcmp(func->name, global_noreturns[i]))
>  				return 1;
>  
> -	if (!func->sec)
> +	if (!func->len)
>  		return 0;
>  
> -	func_for_each_insn(file, func, insn) {
> +	insn = find_insn(file, func->sec, func->offset);
> +	if (!insn->func)
> +		return 0;
> +
> +	func_for_each_insn_all(file, func, insn) {
>  		empty = false;
>  
>  		if (insn->type == INSN_RETURN)
> @@ -167,28 +196,17 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func,
>  	 * case, the function's dead-end status depends on whether the target
>  	 * of the sibling call returns.
>  	 */
> -	func_for_each_insn(file, func, insn) {
> -		if (insn->sec != func->sec ||
> -		    insn->offset >= func->offset + func->len)
> -			break;
> -
> +	func_for_each_insn_all(file, func, insn) {
>  		if (insn->type == INSN_JUMP_UNCONDITIONAL) {
>  			struct instruction *dest = insn->jump_dest;
> -			struct symbol *dest_func;
>  
>  			if (!dest)
>  				/* sibling call to another file */
>  				return 0;
>  
> -			if (dest->sec != func->sec ||
> -			    dest->offset < func->offset ||
> -			    dest->offset >= func->offset + func->len) {
> -				/* local sibling call */
> -				dest_func = find_symbol_by_offset(dest->sec,
> -								  dest->offset);
> -				if (!dest_func)
> -					continue;
> +			if (dest->func && dest->func->pfunc != insn->func->pfunc) {
>  
> +				/* local sibling call */
>  				if (recursion == 5) {
>  					/*
>  					 * Infinite recursion: two functions
> @@ -199,7 +217,7 @@ static int __dead_end_function(struct objtool_file *file, struct symbol *func,
>  					return 0;
>  				}
>  
> -				return __dead_end_function(file, dest_func,
> +				return __dead_end_function(file, dest->func,
>  							   recursion + 1);
>  			}
>  		}
> @@ -426,7 +444,7 @@ static void add_ignores(struct objtool_file *file)
>  			if (!ignore_func(file, func))
>  				continue;
>  
> -			func_for_each_insn(file, func, insn)
> +			func_for_each_insn_all(file, func, insn)
>  				insn->ignore = true;
>  		}
>  	}
> @@ -786,30 +804,28 @@ static int add_special_section_alts(struct objtool_file *file)
>  	return ret;
>  }
>  
> -static int add_switch_table(struct objtool_file *file, struct symbol *func,
> -			    struct instruction *insn, struct rela *table,
> -			    struct rela *next_table)
> +static int add_switch_table(struct objtool_file *file, struct instruction *insn,
> +			    struct rela *table, struct rela *next_table)
>  {
>  	struct rela *rela = table;
>  	struct instruction *alt_insn;
>  	struct alternative *alt;
> +	struct symbol *pfunc = insn->func->pfunc;
> +	unsigned int prev_offset = 0;
>  
>  	list_for_each_entry_from(rela, &file->rodata->rela->rela_list, list) {
>  		if (rela == next_table)
>  			break;
>  
> -		if (rela->sym->sec != insn->sec ||
> -		    rela->addend <= func->offset ||
> -		    rela->addend >= func->offset + func->len)
> +		if (prev_offset && rela->offset != prev_offset + sizeof(long))
>  			break;
>  
> -		alt_insn = find_insn(file, insn->sec, rela->addend);
> -		if (!alt_insn) {
> -			WARN("%s: can't find instruction at %s+0x%x",
> -			     file->rodata->rela->name, insn->sec->name,
> -			     rela->addend);
> -			return -1;
> -		}
> +		alt_insn = find_insn(file, rela->sym->sec, rela->addend);
> +		if (!alt_insn)
> +			break;
> +
> +		if (alt_insn->func->pfunc != pfunc)
> +			break;
>  
>  		alt = malloc(sizeof(*alt));
>  		if (!alt) {
> @@ -819,6 +835,13 @@ static int add_switch_table(struct objtool_file *file, struct symbol *func,
>  
>  		alt->insn = alt_insn;
>  		list_add_tail(&alt->list, &insn->alts);
> +		prev_offset = rela->offset;
> +	}
> +
> +	if (!prev_offset) {
> +		WARN_FUNC("can't find switch jump table",
> +			  insn->sec, insn->offset);
> +		return -1;
>  	}
>  
>  	return 0;
> @@ -947,7 +970,7 @@ static int add_func_switch_tables(struct objtool_file *file,
>  	struct rela *rela, *prev_rela = NULL;
>  	int ret;
>  
> -	func_for_each_insn(file, func, insn) {
> +	func_for_each_insn_all(file, func, insn) {
>  		if (!last)
>  			last = insn;
>  
> @@ -978,8 +1001,7 @@ static int add_func_switch_tables(struct objtool_file *file,
>  		 * the beginning of another switch table in the same function.
>  		 */
>  		if (prev_jump) {
> -			ret = add_switch_table(file, func, prev_jump, prev_rela,
> -					       rela);
> +			ret = add_switch_table(file, prev_jump, prev_rela, rela);
>  			if (ret)
>  				return ret;
>  		}
> @@ -989,7 +1011,7 @@ static int add_func_switch_tables(struct objtool_file *file,
>  	}
>  
>  	if (prev_jump) {
> -		ret = add_switch_table(file, func, prev_jump, prev_rela, NULL);
> +		ret = add_switch_table(file, prev_jump, prev_rela, NULL);
>  		if (ret)
>  			return ret;
>  	}
> @@ -1753,15 +1775,13 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
>  	while (1) {
>  		next_insn = next_insn_same_sec(file, insn);
>  
> -
> -		if (file->c_file && func && insn->func && func != insn->func) {
> +		if (file->c_file && func && insn->func && func != insn->func->pfunc) {
>  			WARN("%s() falls through to next function %s()",
>  			     func->name, insn->func->name);
>  			return 1;
>  		}
>  
> -		if (insn->func)
> -			func = insn->func;
> +		func = insn->func ? insn->func->pfunc : NULL;
>  
>  		if (func && insn->ignore) {
>  			WARN_FUNC("BUG: why am I validating an ignored function?",
> @@ -1782,7 +1802,7 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
>  
>  				i = insn;
>  				save_insn = NULL;
> -				func_for_each_insn_continue_reverse(file, func, i) {
> +				func_for_each_insn_continue_reverse(file, insn->func, i) {
>  					if (i->save) {
>  						save_insn = i;
>  						break;
> @@ -1869,7 +1889,7 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
>  		case INSN_JUMP_UNCONDITIONAL:
>  			if (insn->jump_dest &&
>  			    (!func || !insn->jump_dest->func ||
> -			     func == insn->jump_dest->func)) {
> +			     insn->jump_dest->func->pfunc == func)) {
>  				ret = validate_branch(file, insn->jump_dest,
>  						      state);
>  				if (ret)
> @@ -2064,7 +2084,7 @@ static int validate_functions(struct objtool_file *file)
>  
>  	for_each_sec(file, sec) {
>  		list_for_each_entry(func, &sec->symbol_list, list) {
> -			if (func->type != STT_FUNC)
> +			if (func->type != STT_FUNC || func->pfunc != func)
>  				continue;
>  
>  			insn = find_insn(file, sec, func->offset);
> diff --git a/tools/objtool/elf.c b/tools/objtool/elf.c
> index c1c338661699..4e60e105583e 100644
> --- a/tools/objtool/elf.c
> +++ b/tools/objtool/elf.c
> @@ -79,6 +79,19 @@ struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset)
>  	return NULL;
>  }
>  
> +struct symbol *find_symbol_by_name(struct elf *elf, const char *name)
> +{
> +	struct section *sec;
> +	struct symbol *sym;
> +
> +	list_for_each_entry(sec, &elf->sections, list)
> +		list_for_each_entry(sym, &sec->symbol_list, list)
> +			if (!strcmp(sym->name, name))
> +				return sym;
> +
> +	return NULL;
> +}
> +
>  struct symbol *find_symbol_containing(struct section *sec, unsigned long offset)
>  {
>  	struct symbol *sym;
> @@ -203,10 +216,11 @@ static int read_sections(struct elf *elf)
>  
>  static int read_symbols(struct elf *elf)
>  {
> -	struct section *symtab;
> -	struct symbol *sym;
> +	struct section *symtab, *sec;
> +	struct symbol *sym, *pfunc;
>  	struct list_head *entry, *tmp;
>  	int symbols_nr, i;
> +	char *coldstr;
>  
>  	symtab = find_section_by_name(elf, ".symtab");
>  	if (!symtab) {
> @@ -281,6 +295,30 @@ static int read_symbols(struct elf *elf)
>  		hash_add(sym->sec->symbol_hash, &sym->hash, sym->idx);
>  	}
>  
> +	/* Create parent/child links for any cold subfunctions */
> +	list_for_each_entry(sec, &elf->sections, list) {
> +		list_for_each_entry(sym, &sec->symbol_list, list) {
> +			if (sym->type != STT_FUNC)
> +				continue;
> +			sym->pfunc = sym->cfunc = sym;
> +			coldstr = strstr(sym->name, ".cold.");
> +			if (coldstr) {
> +				coldstr[0] = '\0';
> +				pfunc = find_symbol_by_name(elf, sym->name);
> +				coldstr[0] = '.';
> +
> +				if (!pfunc) {
> +					WARN("%s(): can't find parent function",
> +					     sym->name);
> +					goto err;
> +				}
> +
> +				sym->pfunc = pfunc;
> +				pfunc->cfunc = sym;
> +			}
> +		}
> +	}
> +
>  	return 0;
>  
>  err:
> diff --git a/tools/objtool/elf.h b/tools/objtool/elf.h
> index d86e2ff14466..de5cd2ddded9 100644
> --- a/tools/objtool/elf.h
> +++ b/tools/objtool/elf.h
> @@ -61,6 +61,7 @@ struct symbol {
>  	unsigned char bind, type;
>  	unsigned long offset;
>  	unsigned int len;
> +	struct symbol *pfunc, *cfunc;
>  };
>  
>  struct rela {
> @@ -86,6 +87,7 @@ struct elf {
>  struct elf *elf_open(const char *name, int flags);
>  struct section *find_section_by_name(struct elf *elf, const char *name);
>  struct symbol *find_symbol_by_offset(struct section *sec, unsigned long offset);
> +struct symbol *find_symbol_by_name(struct elf *elf, const char *name);
>  struct symbol *find_symbol_containing(struct section *sec, unsigned long offset);
>  struct rela *find_rela_by_dest(struct section *sec, unsigned long offset);
>  struct rela *find_rela_by_dest_range(struct section *sec, unsigned long offset,
> -- 
> 2.17.0
>