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@xxxxxxxxxxxxx> wrote:
>
> > Hi Ingo, Happy New Year!
> Happy New Year to you too! :-)
>
> > On 07/01/23 15:51, Ingo Molnar wrote:
> > > * Sathvika Vasireddy <sv@xxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxxxxx>
> > > > Signed-off-by: Sathvika Vasireddy <sv@xxxxxxxxxxxxx>
> > > 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@xxxxxxxxxxxxxxxxxxxx>
> > >           Signed-off-by: First Co-Author <first@xxxxxxxxxxxxxxxxxxxx>
> > >           Co-developed-by: Second Co-Author <second@xxxxxxxxxxxxxxxxxxxx>
> > >           Signed-off-by: Second Co-Author <second@xxxxxxxxxxxxxxxxxxxx>
> > >
> > > [ 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@xxxxxxxxxxxxxxxxxxxx>
> >          Signed-off-by: First Co-Author <first@xxxxxxxxxxxxxxxxxxxx>
> >          Co-developed-by: Second Co-Author <second@xxxxxxxxxxxxxxxxxxxx>
> >          Signed-off-by: Second Co-Author <second@xxxxxxxxxxxxxxxxxxxx>
> ... 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@xxxxxxxxx/
> 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@xxxxxxxxxxxxxxxxxx>
>    Reported-by: Sathvika Vasireddy <sv@xxxxxxxxxxxxx>
>    Acked-by: Sathvika Vasireddy <sv@xxxxxxxxxxxxx>
>
> To document & credit the efforts of your patch.

Sure, thank you!

- Sathvika