diff mbox series

[4/5] objtool: Add per-function rate limiting for unreachable warnings

Message ID b21f7791b30c54cf8c4d0f489decdc4a47a18963.1679932620.git.jpoimboe@kernel.org
State New
Headers
Series objtool: warning improvements |

Commit Message

Josh Poimboeuf March 27, 2023, 4 p.m. UTC 
  Unreachable instruction warnings are rate limited to once per object
file.  That no longer makes sense for vmlinux validation, which might
have other unreachable instructions lurking in other places.  Change it
to once per function.

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

Comments

Peter Zijlstra March 28, 2023, 8:11 a.m. UTC | #1 
On Mon, Mar 27, 2023 at 09:00:47AM -0700, Josh Poimboeuf wrote:
> Unreachable instruction warnings are rate limited to once per object
> file.  That no longer makes sense for vmlinux validation, which might
> have other unreachable instructions lurking in other places.  Change it
> to once per function.

Do we want a negative option to disable this? --no-ratelimit or such?
Miroslav Benes March 28, 2023, 9:07 a.m. UTC | #2 
On Mon, 27 Mar 2023, Josh Poimboeuf wrote:

> Unreachable instruction warnings are rate limited to once per object
> file.  That no longer makes sense for vmlinux validation, which might
> have other unreachable instructions lurking in other places.  Change it
> to once per function.
> 
> Signed-off-by: Josh Poimboeuf <jpoimboe@kernel.org>
> ---
>  tools/objtool/check.c               | 4 ++++
>  tools/objtool/include/objtool/elf.h | 1 +
>  2 files changed, 5 insertions(+)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 73dd091c0075..67a684225702 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -4557,6 +4557,10 @@ static int validate_reachable_instructions(struct objtool_file *file)
>  		if (insn->visited || ignore_unreachable_insn(file, insn))
>  			continue;
>  
> +		if (insn->sym->warned)
> +			continue;
> +		insn->sym->warned = 1;
> +

Ok

>  		WARN_FUNC("unreachable instruction", insn->sec, insn->offset);
>  		return 1;

But we still return here when an unreachable instruction is encountered 
and warned about. Or maybe I am just misunderstanding the purpose.

If not, would it be better to just collect the number of warnings per 
object as we do elsewhere?

  warnings++;

and then at the end

  return warnings;

Miroslav
Josh Poimboeuf March 28, 2023, 8:22 p.m. UTC | #3 
On Tue, Mar 28, 2023 at 10:11:05AM +0200, Peter Zijlstra wrote:
> On Mon, Mar 27, 2023 at 09:00:47AM -0700, Josh Poimboeuf wrote:
> > Unreachable instruction warnings are rate limited to once per object
> > file.  That no longer makes sense for vmlinux validation, which might
> > have other unreachable instructions lurking in other places.  Change it
> > to once per function.
> 
> Do we want a negative option to disable this? --no-ratelimit or such?

Per-function rate-limiting is almost always the right thing, personally
I don't envision needing to disable it.
Peter Zijlstra March 29, 2023, 7:26 a.m. UTC | #4 
On Tue, Mar 28, 2023 at 01:22:05PM -0700, Josh Poimboeuf wrote:
> On Tue, Mar 28, 2023 at 10:11:05AM +0200, Peter Zijlstra wrote:
> > On Mon, Mar 27, 2023 at 09:00:47AM -0700, Josh Poimboeuf wrote:
> > > Unreachable instruction warnings are rate limited to once per object
> > > file.  That no longer makes sense for vmlinux validation, which might
> > > have other unreachable instructions lurking in other places.  Change it
> > > to once per function.
> > 
> > Do we want a negative option to disable this? --no-ratelimit or such?
> 
> Per-function rate-limiting is almost always the right thing, personally
> I don't envision needing to disable it.

Ok, fair enough. I'll let you know if I ever come across the need to
disable it :-)
diff mbox series

Patch

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 73dd091c0075..67a684225702 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -4557,6 +4557,10 @@  static int validate_reachable_instructions(struct objtool_file *file)
 		if (insn->visited || ignore_unreachable_insn(file, insn))
 			continue;
 
+		if (insn->sym->warned)
+			continue;
+		insn->sym->warned = 1;
+
 		WARN_FUNC("unreachable instruction", insn->sec, insn->offset);
 		return 1;
 	}
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;
 };