2023-03-27 16:03:18

by Josh Poimboeuf

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

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 <[email protected]>
---
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;
+
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;
};
--
2.39.2


2023-03-28 08:15:25

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 4/5] objtool: Add per-function rate limiting for unreachable warnings

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?

2023-03-28 09:10:33

by Miroslav Benes

[permalink] [raw]
Subject: Re: [PATCH 4/5] objtool: Add per-function rate limiting for unreachable warnings

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 <[email protected]>
> ---
> 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

2023-03-28 20:24:40

by Josh Poimboeuf

[permalink] [raw]
Subject: Re: [PATCH 4/5] objtool: Add per-function rate limiting for unreachable warnings

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.

--
Josh

2023-03-29 07:47:53

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 4/5] objtool: Add per-function rate limiting for unreachable warnings

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 :-)