2022-12-08 08:23:00

by Sathvika Vasireddy

[permalink] [raw]
Subject: [PATCH] objtool: continue if find_insn() fails in decode_instructions()

Currently, decode_instructions() is failing if it is not able to find
instruction, and this is happening since commit dbcdbdfdf137b4
("objtool: Rework instruction -> symbol mapping") because it is
expecting instruction for STT_NOTYPE symbols.

Due to this, the following objtool warnings are seen:
[1] arch/powerpc/kernel/optprobes_head.o: warning: objtool: optprobe_template_end(): can't find starting instruction
[2] arch/powerpc/kernel/kvm_emul.o: warning: objtool: kvm_template_end(): can't find starting instruction
[3] arch/powerpc/kernel/head_64.o: warning: objtool: end_first_256B(): can't find starting instruction

The warnings are thrown because find_insn() is failing for symbols that
are at the end of the file, or at the end of the section. Given how
STT_NOTYPE symbols are currently handled in decode_instructions(),
continue if the instruction is not found, instead of throwing warning
and returning.

Signed-off-by: Naveen N. Rao <[email protected]>
Signed-off-by: Sathvika Vasireddy <[email protected]>
---
tools/objtool/check.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 4350be739f4f..bce2be5ebf36 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -430,11 +430,8 @@ static int decode_instructions(struct objtool_file *file)
if (func->return_thunk || func->alias != func)
continue;

- if (!find_insn(file, sec, func->offset)) {
- WARN("%s(): can't find starting instruction",
- func->name);
- return -1;
- }
+ if (!find_insn(file, sec, func->offset))
+ continue;

sym_for_each_insn(file, func, insn) {
insn->sym = func;
--
2.31.1


2023-01-07 10:30:54

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] objtool: continue if find_insn() fails in decode_instructions()


* Sathvika Vasireddy <[email protected]> wrote:

> Currently, decode_instructions() is failing if it is not able to find
> instruction, and this is happening since commit dbcdbdfdf137b4
> ("objtool: Rework instruction -> symbol mapping") because it is
> expecting instruction for STT_NOTYPE symbols.
>
> Due to this, the following objtool warnings are seen:
> [1] arch/powerpc/kernel/optprobes_head.o: warning: objtool: optprobe_template_end(): can't find starting instruction
> [2] arch/powerpc/kernel/kvm_emul.o: warning: objtool: kvm_template_end(): can't find starting instruction
> [3] arch/powerpc/kernel/head_64.o: warning: objtool: end_first_256B(): can't find starting instruction
>
> The warnings are thrown because find_insn() is failing for symbols that
> are at the end of the file, or at the end of the section. Given how
> STT_NOTYPE symbols are currently handled in decode_instructions(),
> continue if the instruction is not found, instead of throwing warning
> and returning.
>
> Signed-off-by: Naveen N. Rao <[email protected]>
> Signed-off-by: Sathvika Vasireddy <[email protected]>

The SOB chain doesn't look valid: is Naveen N. Rao, the first SOB line, the
author of the patch? If yes then a matching From: line is needed.

Or if two people developed the patch, then Co-developed-by should be used:

Co-developed-by: First Co-Author <[email protected]>
Signed-off-by: First Co-Author <[email protected]>
Co-developed-by: Second Co-Author <[email protected]>
Signed-off-by: Second Co-Author <[email protected]>

[ In this SOB sequence "Second Co-Author" is the one who submits the patch. ]

[ Please only use Co-developed-by if actual lines of code were written by
the co-author that created copyrightable material - it's not a courtesy
tag. Reviewed-by/Acked-by/Tested-by can be used to credit non-code
contributions. ]

Thanks,

Ingo

2023-01-09 13:03:48

by Sathvika Vasireddy

[permalink] [raw]
Subject: Re: [PATCH] objtool: continue if find_insn() fails in decode_instructions()

Hi Ingo, Happy New Year!

On 07/01/23 15:51, Ingo Molnar wrote:
> * Sathvika Vasireddy <[email protected]> wrote:
>
>> Currently, decode_instructions() is failing if it is not able to find
>> instruction, and this is happening since commit dbcdbdfdf137b4
>> ("objtool: Rework instruction -> symbol mapping") because it is
>> expecting instruction for STT_NOTYPE symbols.
>>
>> Due to this, the following objtool warnings are seen:
>> [1] arch/powerpc/kernel/optprobes_head.o: warning: objtool: optprobe_template_end(): can't find starting instruction
>> [2] arch/powerpc/kernel/kvm_emul.o: warning: objtool: kvm_template_end(): can't find starting instruction
>> [3] arch/powerpc/kernel/head_64.o: warning: objtool: end_first_256B(): can't find starting instruction
>>
>> The warnings are thrown because find_insn() is failing for symbols that
>> are at the end of the file, or at the end of the section. Given how
>> STT_NOTYPE symbols are currently handled in decode_instructions(),
>> continue if the instruction is not found, instead of throwing warning
>> and returning.
>>
>> Signed-off-by: Naveen N. Rao <[email protected]>
>> Signed-off-by: Sathvika Vasireddy <[email protected]>
> The SOB chain doesn't look valid: is Naveen N. Rao, the first SOB line, the
> author of the patch? If yes then a matching From: line is needed.
>
> Or if two people developed the patch, then Co-developed-by should be used:
>
> Co-developed-by: First Co-Author <[email protected]>
> Signed-off-by: First Co-Author <[email protected]>
> Co-developed-by: Second Co-Author <[email protected]>
> Signed-off-by: Second Co-Author <[email protected]>
>
> [ In this SOB sequence "Second Co-Author" is the one who submits the patch. ]
>
> [ Please only use Co-developed-by if actual lines of code were written by
> the co-author that created copyrightable material - it's not a courtesy
> tag. Reviewed-by/Acked-by/Tested-by can be used to credit non-code
> contributions. ]
Thank you for the clarification, and for bringing these points to my
attention. I'll keep these things in mind. In this case, since both
Naveen N. Rao and I developed the patch, the below tags
are applicable.

        Co-developed-by: First Co-Author <[email protected]>
        Signed-off-by: First Co-Author <[email protected]>
        Co-developed-by: Second Co-Author <[email protected]>
        Signed-off-by: Second Co-Author <[email protected]>

However, I would be dropping this particular patch, since I think Nick's
patch [1] is better to fix the objtool issue.

[1] -
https://lore.kernel.org/linuxppc-dev/[email protected]/



Thanks for reviewing!

- Sathvika

2023-01-09 17:13:01

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] objtool: continue if find_insn() fails in decode_instructions()


* Sathvika Vasireddy <[email protected]> wrote:

> Hi Ingo, Happy New Year!

Happy New Year to you too! :-)

> On 07/01/23 15:51, Ingo Molnar wrote:
> > * Sathvika Vasireddy <[email protected]> wrote:
> >
> > > Currently, decode_instructions() is failing if it is not able to find
> > > instruction, and this is happening since commit dbcdbdfdf137b4
> > > ("objtool: Rework instruction -> symbol mapping") because it is
> > > expecting instruction for STT_NOTYPE symbols.
> > >
> > > Due to this, the following objtool warnings are seen:
> > > [1] arch/powerpc/kernel/optprobes_head.o: warning: objtool: optprobe_template_end(): can't find starting instruction
> > > [2] arch/powerpc/kernel/kvm_emul.o: warning: objtool: kvm_template_end(): can't find starting instruction
> > > [3] arch/powerpc/kernel/head_64.o: warning: objtool: end_first_256B(): can't find starting instruction
> > >
> > > The warnings are thrown because find_insn() is failing for symbols that
> > > are at the end of the file, or at the end of the section. Given how
> > > STT_NOTYPE symbols are currently handled in decode_instructions(),
> > > continue if the instruction is not found, instead of throwing warning
> > > and returning.
> > >
> > > Signed-off-by: Naveen N. Rao <[email protected]>
> > > Signed-off-by: Sathvika Vasireddy <[email protected]>
> > The SOB chain doesn't look valid: is Naveen N. Rao, the first SOB line, the
> > author of the patch? If yes then a matching From: line is needed.
> >
> > Or if two people developed the patch, then Co-developed-by should be used:
> >
> > Co-developed-by: First Co-Author <[email protected]>
> > Signed-off-by: First Co-Author <[email protected]>
> > Co-developed-by: Second Co-Author <[email protected]>
> > Signed-off-by: Second Co-Author <[email protected]>
> >
> > [ In this SOB sequence "Second Co-Author" is the one who submits the patch. ]
> >
> > [ Please only use Co-developed-by if actual lines of code were written by
> > the co-author that created copyrightable material - it's not a courtesy
> > tag. Reviewed-by/Acked-by/Tested-by can be used to credit non-code
> > contributions. ]
> Thank you for the clarification, and for bringing these points to my
> attention. I'll keep these things in mind. In this case, since both Naveen
> N. Rao and I developed the patch, the below tags
> are applicable.
>
> ??????? Co-developed-by: First Co-Author <[email protected]>
> ??????? Signed-off-by: First Co-Author <[email protected]>
> ??????? Co-developed-by: Second Co-Author <[email protected]>
> ??????? Signed-off-by: Second Co-Author <[email protected]>

... while filling in your real names & email addresses I suppose. ;-)

>
> However, I would be dropping this particular patch, since I think Nick's
> patch [1] is better to fix the objtool issue.
>
> [1] - https://lore.kernel.org/linuxppc-dev/[email protected]/

Ok, I'll pick up Nick's fix, with these tags added for the PowerPC
regression aspect and your review:

Reported-by: Naveen N. Rao <[email protected]>
Reported-by: Sathvika Vasireddy <[email protected]>
Acked-by: Sathvika Vasireddy <[email protected]>

To document & credit the efforts of your patch.

Thanks,

Ingo

2023-01-09 18:10:40

by Sathvika Vasireddy

[permalink] [raw]
Subject: Re: [PATCH] objtool: continue if find_insn() fails in decode_instructions()


On 09/01/23 22:23, Ingo Molnar wrote:
> * Sathvika Vasireddy <[email protected]> wrote:
>
>> Hi Ingo, Happy New Year!
> Happy New Year to you too! :-)
>
>> On 07/01/23 15:51, Ingo Molnar wrote:
>>> * Sathvika Vasireddy <[email protected]> wrote:
>>>
>>>> Currently, decode_instructions() is failing if it is not able to find
>>>> instruction, and this is happening since commit dbcdbdfdf137b4
>>>> ("objtool: Rework instruction -> symbol mapping") because it is
>>>> expecting instruction for STT_NOTYPE symbols.
>>>>
>>>> Due to this, the following objtool warnings are seen:
>>>> [1] arch/powerpc/kernel/optprobes_head.o: warning: objtool: optprobe_template_end(): can't find starting instruction
>>>> [2] arch/powerpc/kernel/kvm_emul.o: warning: objtool: kvm_template_end(): can't find starting instruction
>>>> [3] arch/powerpc/kernel/head_64.o: warning: objtool: end_first_256B(): can't find starting instruction
>>>>
>>>> The warnings are thrown because find_insn() is failing for symbols that
>>>> are at the end of the file, or at the end of the section. Given how
>>>> STT_NOTYPE symbols are currently handled in decode_instructions(),
>>>> continue if the instruction is not found, instead of throwing warning
>>>> and returning.
>>>>
>>>> Signed-off-by: Naveen N. Rao <[email protected]>
>>>> Signed-off-by: Sathvika Vasireddy <[email protected]>
>>> The SOB chain doesn't look valid: is Naveen N. Rao, the first SOB line, the
>>> author of the patch? If yes then a matching From: line is needed.
>>>
>>> Or if two people developed the patch, then Co-developed-by should be used:
>>>
>>> Co-developed-by: First Co-Author <[email protected]>
>>> Signed-off-by: First Co-Author <[email protected]>
>>> Co-developed-by: Second Co-Author <[email protected]>
>>> Signed-off-by: Second Co-Author <[email protected]>
>>>
>>> [ In this SOB sequence "Second Co-Author" is the one who submits the patch. ]
>>>
>>> [ Please only use Co-developed-by if actual lines of code were written by
>>> the co-author that created copyrightable material - it's not a courtesy
>>> tag. Reviewed-by/Acked-by/Tested-by can be used to credit non-code
>>> contributions. ]
>> Thank you for the clarification, and for bringing these points to my
>> attention. I'll keep these things in mind. In this case, since both Naveen
>> N. Rao and I developed the patch, the below tags
>> are applicable.
>>
>>         Co-developed-by: First Co-Author <[email protected]>
>>         Signed-off-by: First Co-Author <[email protected]>
>>         Co-developed-by: Second Co-Author <[email protected]>
>>         Signed-off-by: Second Co-Author <[email protected]>
> ... while filling in your real names & email addresses I suppose. ;-)
Indeed :-)
>
>> However, I would be dropping this particular patch, since I think Nick's
>> patch [1] is better to fix the objtool issue.
>>
>> [1] - https://lore.kernel.org/linuxppc-dev/[email protected]/
> Ok, I'll pick up Nick's fix, with these tags added for the PowerPC
> regression aspect and your review:
>
> Reported-by: Naveen N. Rao <[email protected]>
> Reported-by: Sathvika Vasireddy <[email protected]>
> Acked-by: Sathvika Vasireddy <[email protected]>
>
> To document & credit the efforts of your patch.

Sure, thank you!

- Sathvika