diff mbox series

[v2,3/9] objtool: Limit unreachable warnings to once per function

Message ID 81ed0b8e5218ac6ecae49af901bf5e0fb3687cef.1681325924.git.jpoimboe@kernel.org
State New
Headers
Series objtool: warning improvements |

Commit Message

Josh Poimboeuf April 12, 2023, 7:03 p.m. UTC 
  Unreachable instruction warnings are limited to once per object file.
That no longer makes sense for vmlinux validation, which might have
more unreachable instructions lurking in other places.  Change it to
once per function.

Note this affects some other (much rarer) non-fatal warnings as well.
In general I think one-warning-per-function makes sense, as related
warnings can accumulate quickly and we want to eventually get back to
failing the build with -Werror anyway.

Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
---
 tools/objtool/check.c                | 5 +++--
 tools/objtool/include/objtool/elf.h  | 1 +
 tools/objtool/include/objtool/warn.h | 5 ++++-
 3 files changed, 8 insertions(+), 3 deletions(-)

Comments

Peter Zijlstra April 13, 2023, 8:01 a.m. UTC | #1 
On Wed, Apr 12, 2023 at 12:03:18PM -0700, Josh Poimboeuf wrote:

> diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
> index b1c920dc9516..4ef9b278e5fd 100644
> --- a/tools/objtool/include/objtool/warn.h
> +++ b/tools/objtool/include/objtool/warn.h
> @@ -55,7 +55,10 @@ static inline char *offstr(struct section *sec, unsigned long offset)
>  
>  #define WARN_INSN(insn, format, ...)					\
>  ({									\
> -	WARN_FUNC(format, insn->sec, insn->offset,  ##__VA_ARGS__);	\
> +	if (!insn->sym || !insn->sym->warned)				\
> +		WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__);\
> +	if (insn->sym)							\
> +		insn->sym->warned = 1;					\
>  })

Do we want to write that like:

#define WARN_INSN(insn, format, ...)					\
({									\
	struct instruction *_insn = (insn);				\
	if (!_insn->sym || !_insn->sym->warned)				\
		WARN_FUNC(format, _insn->sec, _insn->offset, ##__VA_ARGS__);\
	if (_insn->sym)							\
		_insn->sym->warned = 1;					\
})

instead ?
Josh Poimboeuf April 13, 2023, 3:03 p.m. UTC | #2 
On Thu, Apr 13, 2023 at 10:01:12AM +0200, Peter Zijlstra wrote:
> On Wed, Apr 12, 2023 at 12:03:18PM -0700, Josh Poimboeuf wrote:
> 
> > diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
> > index b1c920dc9516..4ef9b278e5fd 100644
> > --- a/tools/objtool/include/objtool/warn.h
> > +++ b/tools/objtool/include/objtool/warn.h
> > @@ -55,7 +55,10 @@ static inline char *offstr(struct section *sec, unsigned long offset)
> >  
> >  #define WARN_INSN(insn, format, ...)					\
> >  ({									\
> > -	WARN_FUNC(format, insn->sec, insn->offset,  ##__VA_ARGS__);	\
> > +	if (!insn->sym || !insn->sym->warned)				\
> > +		WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__);\
> > +	if (insn->sym)							\
> > +		insn->sym->warned = 1;					\
> >  })
> 
> Do we want to write that like:
> 
> #define WARN_INSN(insn, format, ...)					\
> ({									\
> 	struct instruction *_insn = (insn);				\
> 	if (!_insn->sym || !_insn->sym->warned)				\
> 		WARN_FUNC(format, _insn->sec, _insn->offset, ##__VA_ARGS__);\
> 	if (_insn->sym)							\
> 		_insn->sym->warned = 1;					\
> })
> 
> instead ?

Yes indeed.
diff mbox series

Patch

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index e85440db7a5f..de0d0234527d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4509,6 +4509,7 @@  static int validate_sls(struct objtool_file *file)
 static int validate_reachable_instructions(struct objtool_file *file)
 {
 	struct instruction *insn;
+	int warnings = 0;
 
 	if (file->ignore_unreachables)
 		return 0;
@@ -4518,10 +4519,10 @@  static int validate_reachable_instructions(struct objtool_file *file)
 			continue;
 
 		WARN_INSN(insn, "unreachable instruction");
-		return 1;
+		warnings++;
 	}
 
-	return 0;
+	return warnings;
 }
 
 int check(struct objtool_file *file)
diff --git a/tools/objtool/include/objtool/elf.h b/tools/objtool/include/objtool/elf.h
index ad0024da262b..a668173a5869 100644
--- a/tools/objtool/include/objtool/elf.h
+++ b/tools/objtool/include/objtool/elf.h
@@ -61,6 +61,7 @@  struct symbol {
 	u8 return_thunk      : 1;
 	u8 fentry            : 1;
 	u8 profiling_func    : 1;
+	u8 warned	     : 1;
 	struct list_head pv_target;
 	struct list_head reloc_list;
 };
diff --git a/tools/objtool/include/objtool/warn.h b/tools/objtool/include/objtool/warn.h
index b1c920dc9516..4ef9b278e5fd 100644
--- a/tools/objtool/include/objtool/warn.h
+++ b/tools/objtool/include/objtool/warn.h
@@ -55,7 +55,10 @@  static inline char *offstr(struct section *sec, unsigned long offset)
 
 #define WARN_INSN(insn, format, ...)					\
 ({									\
-	WARN_FUNC(format, insn->sec, insn->offset,  ##__VA_ARGS__);	\
+	if (!insn->sym || !insn->sym->warned)				\
+		WARN_FUNC(format, insn->sec, insn->offset, ##__VA_ARGS__);\
+	if (insn->sym)							\
+		insn->sym->warned = 1;					\
 })
 
 #define BT_FUNC(format, insn, ...)			\