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

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

[Sathvika Vasireddy]

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 <naveen.n.rao@linux.vnet.ibm.com>
Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
---
 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

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

[Ingo Molnar]

* Sathvika Vasireddy <sv@linux.ibm.com> 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 <naveen.n.rao@linux.vnet.ibm.com>
> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>

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 <first@coauthor.example.org>
        Signed-off-by: First Co-Author <first@coauthor.example.org>
        Co-developed-by: Second Co-Author <second@coauthor.example.org>
        Signed-off-by: Second Co-Author <second@coauthor.example.org>

[ 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

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

[Sathvika Vasireddy]

Hi Ingo, Happy New Year!

On 07/01/23 15:51, Ingo Molnar wrote:
> * Sathvika Vasireddy <sv@linux.ibm.com> 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 <naveen.n.rao@linux.vnet.ibm.com>
>> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
> 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 <first@coauthor.example.org>
>          Signed-off-by: First Co-Author <first@coauthor.example.org>
>          Co-developed-by: Second Co-Author <second@coauthor.example.org>
>          Signed-off-by: Second Co-Author <second@coauthor.example.org>
>
> [ 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 <first@coauthor.example.org>
         Signed-off-by: First Co-Author <first@coauthor.example.org>
         Co-developed-by: Second Co-Author <second@coauthor.example.org>
         Signed-off-by: Second Co-Author <second@coauthor.example.org>

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/20221220101323.3119939-1-npiggin@gmail.com/ 



Thanks for reviewing!

- Sathvika

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

[Ingo Molnar]

* Sathvika Vasireddy <sv@linux.ibm.com> wrote:

> Hi Ingo, Happy New Year!

Happy New Year to you too! :-)

> On 07/01/23 15:51, Ingo Molnar wrote:
> > * Sathvika Vasireddy <sv@linux.ibm.com> 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 <naveen.n.rao@linux.vnet.ibm.com>
> > > Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
> > 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 <first@coauthor.example.org>
> >          Signed-off-by: First Co-Author <first@coauthor.example.org>
> >          Co-developed-by: Second Co-Author <second@coauthor.example.org>
> >          Signed-off-by: Second Co-Author <second@coauthor.example.org>
> > 
> > [ 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 <first@coauthor.example.org>
>         Signed-off-by: First Co-Author <first@coauthor.example.org>
>         Co-developed-by: Second Co-Author <second@coauthor.example.org>
>         Signed-off-by: Second Co-Author <second@coauthor.example.org>

... 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/20221220101323.3119939-1-npiggin@gmail.com/

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 <naveen.n.rao@linux.vnet.ibm.com>
  Reported-by: Sathvika Vasireddy <sv@linux.ibm.com>
  Acked-by: Sathvika Vasireddy <sv@linux.ibm.com>

To document & credit the efforts of your patch.

Thanks,

	Ingo

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

[Sathvika Vasireddy]

On 09/01/23 22:23, Ingo Molnar wrote:
> * Sathvika Vasireddy <sv@linux.ibm.com> wrote:
>
>> Hi Ingo, Happy New Year!
> Happy New Year to you too! :-)
>
>> On 07/01/23 15:51, Ingo Molnar wrote:
>>> * Sathvika Vasireddy <sv@linux.ibm.com> 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 <naveen.n.rao@linux.vnet.ibm.com>
>>>> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
>>> 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 <first@coauthor.example.org>
>>>           Signed-off-by: First Co-Author <first@coauthor.example.org>
>>>           Co-developed-by: Second Co-Author <second@coauthor.example.org>
>>>           Signed-off-by: Second Co-Author <second@coauthor.example.org>
>>>
>>> [ 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 <first@coauthor.example.org>
>>          Signed-off-by: First Co-Author <first@coauthor.example.org>
>>          Co-developed-by: Second Co-Author <second@coauthor.example.org>
>>          Signed-off-by: Second Co-Author <second@coauthor.example.org>
> ... 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/20221220101323.3119939-1-npiggin@gmail.com/
> 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 <naveen.n.rao@linux.vnet.ibm.com>
>    Reported-by: Sathvika Vasireddy <sv@linux.ibm.com>
>    Acked-by: Sathvika Vasireddy <sv@linux.ibm.com>
>
> To document & credit the efforts of your patch.

Sure, thank you!

- Sathvika