Avoid endless .discard.foo sections for each annotation, create a
single .discard.annotate section that takes an annotation type along
with the instruction.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
include/linux/objtool.h | 18 ++++++++++++++++++
tools/objtool/check.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 64 insertions(+)
--- a/include/linux/objtool.h
+++ b/include/linux/objtool.h
@@ -57,6 +57,13 @@
".long 998b\n\t" \
".popsection\n\t"
+#define ASM_ANNOTATE(x) \
+ "911:\n\t" \
+ ".pushsection .discard.annotate,\"M\",@progbits,8\n\t" \
+ ".long 911b - .\n\t" \
+ ".long " __stringify(x) "\n\t" \
+ ".popsection\n\t"
+
#else /* __ASSEMBLY__ */
/*
@@ -146,6 +153,14 @@
.popsection
.endm
+.macro ANNOTATE type:req
+.Lhere_\@:
+ .pushsection .discard.annotate,"M",@progbits,8
+ .long .Lhere_\@ - .
+ .long \type
+ .popsection
+.endm
+
#endif /* __ASSEMBLY__ */
#else /* !CONFIG_OBJTOOL */
@@ -155,6 +170,7 @@
#define UNWIND_HINT(type, sp_reg, sp_offset, signal) "\n\t"
#define STACK_FRAME_NON_STANDARD(func)
#define STACK_FRAME_NON_STANDARD_FP(func)
+#define ASM_ANNOTATE(x)
#define ANNOTATE_NOENDBR
#define ASM_REACHABLE
#else
@@ -167,6 +183,8 @@
.endm
.macro REACHABLE
.endm
+.macro ANNOTATE type:req
+.endm
#endif
#endif /* CONFIG_OBJTOOL */
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2373,6 +2373,50 @@ static int read_unwind_hints(struct objt
return 0;
}
+static int read_annotate(struct objtool_file *file, void (*func)(int type, struct instruction *insn))
+{
+ struct section *rsec, *sec;
+ struct instruction *insn;
+ struct reloc *reloc;
+ int type;
+
+ rsec = find_section_by_name(file->elf, ".rela.discard.annotate");
+ if (!rsec)
+ return 0;
+
+ sec = find_section_by_name(file->elf, ".discard.annotate");
+ if (!sec)
+ return 0;
+
+ if (sec->sh.sh_entsize != 8) {
+ static bool warn = false;
+ if (!warn) {
+ WARN("%s: dodgy linker, sh_entsize != 8", sec->name);
+ warn = true;
+ }
+ sec->sh.sh_entsize = 8;
+ }
+
+ for_each_reloc(rsec, reloc) {
+ insn = find_insn(file, reloc->sym->sec,
+ reloc->sym->offset + reloc_addend(reloc));
+ if (!insn) {
+ WARN("bad .discard.annotate entry: %d", reloc_idx(reloc));
+ return -1;
+ }
+
+ type = *(u32 *)(sec->data->d_buf + (reloc_idx(reloc) * sec->sh.sh_entsize) + 4);
+
+ func(type, insn);
+ }
+
+ return 0;
+}
+
+static void __annotate_nop(int type, struct instruction *insn)
+{
+}
+
static int read_noendbr_hints(struct objtool_file *file)
{
struct instruction *insn;
@@ -2670,6 +2714,8 @@ static int decode_sections(struct objtoo
if (ret)
return ret;
+ ret = read_annotate(file, __annotate_nop);
+
/*
* Must be before read_unwind_hints() since that needs insn->noendbr.
*/On Mon, Nov 11, 2024 at 12:59:36PM +0100, Peter Zijlstra wrote:
> +#define ASM_ANNOTATE(x) \
> + "911:\n\t" \
> + ".pushsection .discard.annotate,\"M\",@progbits,8\n\t" \
> + ".long 911b - .\n\t" \
> + ".long " __stringify(x) "\n\t" \
> + ".popsection\n\t"
Why mergeable and progbits?
> +static int read_annotate(struct objtool_file *file, void (*func)(int type, struct instruction *insn))
> +{
> + struct section *rsec, *sec;
> + struct instruction *insn;
> + struct reloc *reloc;
> + int type;
> +
> + rsec = find_section_by_name(file->elf, ".rela.discard.annotate");
> + if (!rsec)
> + return 0;
> +
> + sec = find_section_by_name(file->elf, ".discard.annotate");
> + if (!sec)
> + return 0;
Instead of looking for .rela.discard.annotate you can just get it from
sec->rsec.
> +
> + if (sec->sh.sh_entsize != 8) {
> + static bool warn = false;
"warned" ?
> + if (!warn) {
> + WARN("%s: dodgy linker, sh_entsize != 8", sec->name);
> + warn = true;
> + }
Any reason not to make this a fatal error?
> + sec->sh.sh_entsize = 8;
> + }
> +
> + for_each_reloc(rsec, reloc) {
> + insn = find_insn(file, reloc->sym->sec,
> + reloc->sym->offset + reloc_addend(reloc));
> + if (!insn) {
> + WARN("bad .discard.annotate entry: %d", reloc_idx(reloc));
> + return -1;
> + }
Would be nice to print the type here as well.
> @@ -2670,6 +2714,8 @@ static int decode_sections(struct objtoo
> if (ret)
> return ret;
>
> + ret = read_annotate(file, __annotate_nop);
> +
'ret' is ignored here (not that it matters much as this goes away in the
next patch)
--
Josh
On Fri, Nov 15, 2024 at 10:38:28AM -0800, Josh Poimboeuf wrote:
> On Mon, Nov 11, 2024 at 12:59:36PM +0100, Peter Zijlstra wrote:
> > +#define ASM_ANNOTATE(x) \
> > + "911:\n\t" \
> > + ".pushsection .discard.annotate,\"M\",@progbits,8\n\t" \
> > + ".long 911b - .\n\t" \
> > + ".long " __stringify(x) "\n\t" \
> > + ".popsection\n\t"
>
> Why mergeable and progbits?
In order to get sh_entsize ?
> > +static int read_annotate(struct objtool_file *file, void (*func)(int type, struct instruction *insn))
> > +{
> > + struct section *rsec, *sec;
> > + struct instruction *insn;
> > + struct reloc *reloc;
> > + int type;
> > +
> > + rsec = find_section_by_name(file->elf, ".rela.discard.annotate");
> > + if (!rsec)
> > + return 0;
> > +
> > + sec = find_section_by_name(file->elf, ".discard.annotate");
> > + if (!sec)
> > + return 0;
>
> Instead of looking for .rela.discard.annotate you can just get it from
> sec->rsec.
Oh, indeed.
> > +
> > + if (sec->sh.sh_entsize != 8) {
> > + static bool warn = false;
>
> "warned" ?
Sure.
> > + if (!warn) {
> > + WARN("%s: dodgy linker, sh_entsize != 8", sec->name);
> > + warn = true;
> > + }
>
> Any reason not to make this a fatal error?
lld is currently suffering from this, it would get us build failures on
llvm builds. Once that's fixed, then yes, this should become fatal.
https://github.com/ClangBuiltLinux/linux/issues/2057
> > + sec->sh.sh_entsize = 8;
> > + }
> > +
> > + for_each_reloc(rsec, reloc) {
> > + insn = find_insn(file, reloc->sym->sec,
> > + reloc->sym->offset + reloc_addend(reloc));
> > + if (!insn) {
> > + WARN("bad .discard.annotate entry: %d", reloc_idx(reloc));
> > + return -1;
> > + }
>
> Would be nice to print the type here as well.
Sure.
> > @@ -2670,6 +2714,8 @@ static int decode_sections(struct objtoo
> > if (ret)
> > return ret;
> >
> > + ret = read_annotate(file, __annotate_nop);
> > +
>
> 'ret' is ignored here (not that it matters much as this goes away in the
> next patch)
Right..
On Sat, Nov 16, 2024 at 10:33:31AM +0100, Peter Zijlstra wrote: > On Fri, Nov 15, 2024 at 10:38:28AM -0800, Josh Poimboeuf wrote: > > On Mon, Nov 11, 2024 at 12:59:36PM +0100, Peter Zijlstra wrote: > > > +#define ASM_ANNOTATE(x) \ > > > + "911:\n\t" \ > > > + ".pushsection .discard.annotate,\"M\",@progbits,8\n\t" \ > > > + ".long 911b - .\n\t" \ > > > + ".long " __stringify(x) "\n\t" \ > > > + ".popsection\n\t" > > > > Why mergeable and progbits? > > In order to get sh_entsize ? Is that a guess? If so, it's not very convincing as I don't see what entsize would have to do with it. -- Josh
On Tue, Nov 19, 2024 at 04:31:25PM -0800, Josh Poimboeuf wrote: > On Sat, Nov 16, 2024 at 10:33:31AM +0100, Peter Zijlstra wrote: > > On Fri, Nov 15, 2024 at 10:38:28AM -0800, Josh Poimboeuf wrote: > > > On Mon, Nov 11, 2024 at 12:59:36PM +0100, Peter Zijlstra wrote: > > > > +#define ASM_ANNOTATE(x) \ > > > > + "911:\n\t" \ > > > > + ".pushsection .discard.annotate,\"M\",@progbits,8\n\t" \ > > > > + ".long 911b - .\n\t" \ > > > > + ".long " __stringify(x) "\n\t" \ > > > > + ".popsection\n\t" > > > > > > Why mergeable and progbits? > > > > In order to get sh_entsize ? > > Is that a guess? If so, it's not very convincing as I don't see what > entsize would have to do with it. Oh, nevermind... I see it's a gas syntax issue. -- Josh
On Tue, Nov 19, 2024 at 05:04:24PM -0800, Josh Poimboeuf wrote: > On Tue, Nov 19, 2024 at 04:31:25PM -0800, Josh Poimboeuf wrote: > > On Sat, Nov 16, 2024 at 10:33:31AM +0100, Peter Zijlstra wrote: > > > On Fri, Nov 15, 2024 at 10:38:28AM -0800, Josh Poimboeuf wrote: > > > > On Mon, Nov 11, 2024 at 12:59:36PM +0100, Peter Zijlstra wrote: > > > > > +#define ASM_ANNOTATE(x) \ > > > > > + "911:\n\t" \ > > > > > + ".pushsection .discard.annotate,\"M\",@progbits,8\n\t" \ > > > > > + ".long 911b - .\n\t" \ > > > > > + ".long " __stringify(x) "\n\t" \ > > > > > + ".popsection\n\t" > > > > > > > > Why mergeable and progbits? > > > > > > In order to get sh_entsize ? > > > > Is that a guess? If so, it's not very convincing as I don't see what > > entsize would have to do with it. > > Oh, nevermind... I see it's a gas syntax issue. Not a guess, only mergable gets entsize, and progbits is a required argument per the syntax in order to specify entsize.
On Wed, Nov 20, 2024 at 09:52:54AM +0100, Peter Zijlstra wrote: > On Tue, Nov 19, 2024 at 05:04:24PM -0800, Josh Poimboeuf wrote: > > On Tue, Nov 19, 2024 at 04:31:25PM -0800, Josh Poimboeuf wrote: > > > On Sat, Nov 16, 2024 at 10:33:31AM +0100, Peter Zijlstra wrote: > > > > On Fri, Nov 15, 2024 at 10:38:28AM -0800, Josh Poimboeuf wrote: > > > > > On Mon, Nov 11, 2024 at 12:59:36PM +0100, Peter Zijlstra wrote: > > > > > > +#define ASM_ANNOTATE(x) \ > > > > > > + "911:\n\t" \ > > > > > > + ".pushsection .discard.annotate,\"M\",@progbits,8\n\t" \ > > > > > > + ".long 911b - .\n\t" \ > > > > > > + ".long " __stringify(x) "\n\t" \ > > > > > > + ".popsection\n\t" > > > > > > > > > > Why mergeable and progbits? > > > > > > > > In order to get sh_entsize ? > > > > > > Is that a guess? If so, it's not very convincing as I don't see what > > > entsize would have to do with it. > > > > Oh, nevermind... I see it's a gas syntax issue. > > Not a guess, only mergable gets entsize, and progbits is a required > argument per the syntax in order to specify entsize. If you look at "readelf -WS vmlinux" there are plenty of non-mergeable sections with entsize. -- Josh
On Wed, Nov 20, 2024 at 08:03:10AM -0800, Josh Poimboeuf wrote: > On Wed, Nov 20, 2024 at 09:52:54AM +0100, Peter Zijlstra wrote: > > On Tue, Nov 19, 2024 at 05:04:24PM -0800, Josh Poimboeuf wrote: > > > On Tue, Nov 19, 2024 at 04:31:25PM -0800, Josh Poimboeuf wrote: > > > > On Sat, Nov 16, 2024 at 10:33:31AM +0100, Peter Zijlstra wrote: > > > > > On Fri, Nov 15, 2024 at 10:38:28AM -0800, Josh Poimboeuf wrote: > > > > > > On Mon, Nov 11, 2024 at 12:59:36PM +0100, Peter Zijlstra wrote: > > > > > > > +#define ASM_ANNOTATE(x) \ > > > > > > > + "911:\n\t" \ > > > > > > > + ".pushsection .discard.annotate,\"M\",@progbits,8\n\t" \ > > > > > > > + ".long 911b - .\n\t" \ > > > > > > > + ".long " __stringify(x) "\n\t" \ > > > > > > > + ".popsection\n\t" > > > > > > > > > > > > Why mergeable and progbits? > > > > > > > > > > In order to get sh_entsize ? > > > > > > > > Is that a guess? If so, it's not very convincing as I don't see what > > > > entsize would have to do with it. > > > > > > Oh, nevermind... I see it's a gas syntax issue. > > > > Not a guess, only mergable gets entsize, and progbits is a required > > argument per the syntax in order to specify entsize. > > If you look at "readelf -WS vmlinux" there are plenty of non-mergeable > sections with entsize. Er, vmlinux.o -- Josh
On Wed, Nov 20, 2024 at 08:03:24AM -0800, Josh Poimboeuf wrote: > > If you look at "readelf -WS vmlinux" there are plenty of non-mergeable > > sections with entsize. > > Er, vmlinux.o Well yes, but how do you set entsize from as? The manual only mentions entsize in relation to M(ergable) with the .section command.
© 2016 - 2024 Red Hat, Inc.