[RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern

Sathvika Vasireddy posted 1 patch 10 months ago
tools/objtool/check.c | 6 ++++++
1 file changed, 6 insertions(+)
[RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Sathvika Vasireddy 10 months ago
Architectures like PowerPC use a pattern where the compiler generates a
branch-and-link (bl) instruction that targets the very next instruction,
followed by loading the link register (mflr) later. This pattern appears
in the code like:

 bl .+4
 li r5,0
 mflr r30

Objtool currently warns about this as an "unannotated intra-function
call" because find_call_destination() fails to find any symbol at the
target offset. Add a check to skip the warning when a branch targets
the immediate next instruction in the same function.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202502180818.XnFdv8I8-lkp@intel.com/
Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
---
 tools/objtool/check.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index 753dbc4f8198..3f7cf2c917b5 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1613,6 +1613,7 @@ static struct symbol *find_call_destination(struct section *sec, unsigned long o
  */
 static int add_call_destinations(struct objtool_file *file)
 {
+	struct instruction *next_insn;
 	struct instruction *insn;
 	unsigned long dest_off;
 	struct symbol *dest;
@@ -1625,6 +1626,11 @@ static int add_call_destinations(struct objtool_file *file)
 		reloc = insn_reloc(file, insn);
 		if (!reloc) {
 			dest_off = arch_jump_destination(insn);
+
+			next_insn = next_insn_same_func(file, insn);
+			if (next_insn && dest_off == next_insn->offset)
+				continue;
+
 			dest = find_call_destination(insn->sec, dest_off);
 
 			add_call_dest(file, insn, dest, false);
-- 
2.39.3
Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Christophe Leroy 9 months, 3 weeks ago

Le 19/02/2025 à 17:20, Sathvika Vasireddy a écrit :
> Architectures like PowerPC use a pattern where the compiler generates a
> branch-and-link (bl) instruction that targets the very next instruction,
> followed by loading the link register (mflr) later. This pattern appears
> in the code like:
> 
>   bl .+4
>   li r5,0
>   mflr r30

What compiler do you use ? Is it a very old version of GCC ?

That sequence is not correct and should never be used by modern 
compilers. It should be bcl 20,31,+4 instead.

All such hand writen sequences have been removed from kernel assembly, 
see commit c974809a26a1 ("powerpc/vdso: Avoid link stack corruption in 
__get_datapage()") for details


> 
> Objtool currently warns about this as an "unannotated intra-function
> call" because find_call_destination() fails to find any symbol at the
> target offset. Add a check to skip the warning when a branch targets
> the immediate next instruction in the same function.

I think this should be done in arch_decode_instruction(), just set 
insn->type to INSN_OTHER when you see bl .+4

Something like:

diff --git a/tools/objtool/arch/powerpc/decode.c 
b/tools/objtool/arch/powerpc/decode.c
index 53b55690f320..ca264c97ee8d 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -55,7 +55,9 @@ int arch_decode_instruction(struct objtool_file *file, 
const struct section *sec

  	switch (opcode) {
  	case 18: /* b[l][a] */
-		if ((ins & 3) == 1) /* bl */
+		if (ins == 0x48000005)	/* bl .+4 */
+			typ = INSN_OTHER;
+		else if ((ins & 3) == 1) /* bl */
  			typ = INSN_CALL;

  		imm = ins & 0x3fffffc;



> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202502180818.XnFdv8I8-lkp@intel.com/
> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
> ---
>   tools/objtool/check.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 753dbc4f8198..3f7cf2c917b5 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -1613,6 +1613,7 @@ static struct symbol *find_call_destination(struct section *sec, unsigned long o
>    */
>   static int add_call_destinations(struct objtool_file *file)
>   {
> +       struct instruction *next_insn;
>          struct instruction *insn;
>          unsigned long dest_off;
>          struct symbol *dest;
> @@ -1625,6 +1626,11 @@ static int add_call_destinations(struct objtool_file *file)
>                  reloc = insn_reloc(file, insn);
>                  if (!reloc) {
>                          dest_off = arch_jump_destination(insn);
> +
> +                       next_insn = next_insn_same_func(file, insn);
> +                       if (next_insn && dest_off == next_insn->offset)
> +                               continue;
> +
>                          dest = find_call_destination(insn->sec, dest_off);
> 
>                          add_call_dest(file, insn, dest, false);
> --
> 2.39.3
> 

Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Christophe Leroy 9 months, 3 weeks ago

Le 24/02/2025 à 08:15, Christophe Leroy a écrit :
> 
> 
> Le 19/02/2025 à 17:20, Sathvika Vasireddy a écrit :
>> Architectures like PowerPC use a pattern where the compiler generates a
>> branch-and-link (bl) instruction that targets the very next instruction,
>> followed by loading the link register (mflr) later. This pattern appears
>> in the code like:
>>
>>   bl .+4
>>   li r5,0
>>   mflr r30
> 
> What compiler do you use ? Is it a very old version of GCC ?

Oh, I see that this is a report on a projet version of clang ? compiler: 
clang version 21.0.0git

Then I guess the bug needs to be fixed in Clang, not in the kernel.

> 
> That sequence is not correct and should never be used by modern 
> compilers. It should be bcl 20,31,+4 instead.
> 
> All such hand writen sequences have been removed from kernel assembly, 
> see commit c974809a26a1 ("powerpc/vdso: Avoid link stack corruption in 
> __get_datapage()") for details
> 
> 
>>
>> Objtool currently warns about this as an "unannotated intra-function
>> call" because find_call_destination() fails to find any symbol at the
>> target offset. Add a check to skip the warning when a branch targets
>> the immediate next instruction in the same function.
> 
> I think this should be done in arch_decode_instruction(), just set insn- 
>  >type to INSN_OTHER when you see bl .+4
> 
> Something like:
> 
> diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/ 
> powerpc/decode.c
> index 53b55690f320..ca264c97ee8d 100644
> --- a/tools/objtool/arch/powerpc/decode.c
> +++ b/tools/objtool/arch/powerpc/decode.c
> @@ -55,7 +55,9 @@ int arch_decode_instruction(struct objtool_file *file, 
> const struct section *sec
> 
>       switch (opcode) {
>       case 18: /* b[l][a] */
> -        if ((ins & 3) == 1) /* bl */
> +        if (ins == 0x48000005)    /* bl .+4 */
> +            typ = INSN_OTHER;
> +        else if ((ins & 3) == 1) /* bl */
>               typ = INSN_CALL;
> 
>           imm = ins & 0x3fffffc;
> 
> 
> 
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202502180818.XnFdv8I8- 
>> lkp@intel.com/
>> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
>> ---
>>   tools/objtool/check.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
>> index 753dbc4f8198..3f7cf2c917b5 100644
>> --- a/tools/objtool/check.c
>> +++ b/tools/objtool/check.c
>> @@ -1613,6 +1613,7 @@ static struct symbol 
>> *find_call_destination(struct section *sec, unsigned long o
>>    */
>>   static int add_call_destinations(struct objtool_file *file)
>>   {
>> +       struct instruction *next_insn;
>>          struct instruction *insn;
>>          unsigned long dest_off;
>>          struct symbol *dest;
>> @@ -1625,6 +1626,11 @@ static int add_call_destinations(struct 
>> objtool_file *file)
>>                  reloc = insn_reloc(file, insn);
>>                  if (!reloc) {
>>                          dest_off = arch_jump_destination(insn);
>> +
>> +                       next_insn = next_insn_same_func(file, insn);
>> +                       if (next_insn && dest_off == next_insn->offset)
>> +                               continue;
>> +
>>                          dest = find_call_destination(insn->sec, 
>> dest_off);
>>
>>                          add_call_dest(file, insn, dest, false);
>> -- 
>> 2.39.3
>>
> 

Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Christophe Leroy 9 months, 3 weeks ago

Le 24/02/2025 à 11:33, Christophe Leroy a écrit :
> 
> 
> Le 24/02/2025 à 08:15, Christophe Leroy a écrit :
>>
>>
>> Le 19/02/2025 à 17:20, Sathvika Vasireddy a écrit :
>>> Architectures like PowerPC use a pattern where the compiler generates a
>>> branch-and-link (bl) instruction that targets the very next instruction,
>>> followed by loading the link register (mflr) later. This pattern appears
>>> in the code like:
>>>
>>>   bl .+4
>>>   li r5,0
>>>   mflr r30
>>
>> What compiler do you use ? Is it a very old version of GCC ?
> 
> Oh, I see that this is a report on a projet version of clang ? compiler: 
> clang version 21.0.0git
> 
> Then I guess the bug needs to be fixed in Clang, not in the kernel.

Well, this problem already exists on clang 18 it seems:

00000004 <btext_map>:
    4:   7c 08 02 a6     mflr    r0
    8:   94 21 ff e0     stwu    r1,-32(r1)
    c:   93 c1 00 18     stw     r30,24(r1)
   10:   90 01 00 24     stw     r0,36(r1)
   14:   93 a1 00 14     stw     r29,20(r1)
   18:   48 00 00 05     bl      1c <btext_map+0x18>
   1c:   38 a0 00 00     li      r5,0
   20:   7f c8 02 a6     mflr    r30

While GCC generates:

00000418 <btext_map>:
  418:   94 21 ff e0     stwu    r1,-32(r1)
  41c:   7c 08 02 a6     mflr    r0
  420:   42 9f 00 05     bcl     20,4*cr7+so,424 <btext_map+0xc>
  424:   39 20 00 00     li      r9,0
  428:   93 c1 00 18     stw     r30,24(r1)
  42c:   7f c8 02 a6     mflr    r30

So lets make the kernel tolerate it allthough it should be fixed on 
clang at the end.

I can't find any related issue in the clang issues database 
(https://github.com/llvm/llvm-project/issues), should we open one ?

Christophe

> 
>>
>> That sequence is not correct and should never be used by modern 
>> compilers. It should be bcl 20,31,+4 instead.
>>
>> All such hand writen sequences have been removed from kernel assembly, 
>> see commit c974809a26a1 ("powerpc/vdso: Avoid link stack corruption in 
>> __get_datapage()") for details
>>
>>
>>>
>>> Objtool currently warns about this as an "unannotated intra-function
>>> call" because find_call_destination() fails to find any symbol at the
>>> target offset. Add a check to skip the warning when a branch targets
>>> the immediate next instruction in the same function.
>>
>> I think this should be done in arch_decode_instruction(), just set 
>> insn-  >type to INSN_OTHER when you see bl .+4
>>
>> Something like:
>>
>> diff --git a/tools/objtool/arch/powerpc/decode.c b/tools/objtool/arch/ 
>> powerpc/decode.c
>> index 53b55690f320..ca264c97ee8d 100644
>> --- a/tools/objtool/arch/powerpc/decode.c
>> +++ b/tools/objtool/arch/powerpc/decode.c
>> @@ -55,7 +55,9 @@ int arch_decode_instruction(struct objtool_file 
>> *file, const struct section *sec
>>
>>       switch (opcode) {
>>       case 18: /* b[l][a] */
>> -        if ((ins & 3) == 1) /* bl */
>> +        if (ins == 0x48000005)    /* bl .+4 */
>> +            typ = INSN_OTHER;
>> +        else if ((ins & 3) == 1) /* bl */
>>               typ = INSN_CALL;
>>
>>           imm = ins & 0x3fffffc;
>>
>>
>>
>>>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes: https://lore.kernel.org/oe-kbuild-all/202502180818.XnFdv8I8- 
>>> lkp@intel.com/
>>> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
>>> ---
>>>   tools/objtool/check.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>>
>>> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
>>> index 753dbc4f8198..3f7cf2c917b5 100644
>>> --- a/tools/objtool/check.c
>>> +++ b/tools/objtool/check.c
>>> @@ -1613,6 +1613,7 @@ static struct symbol 
>>> *find_call_destination(struct section *sec, unsigned long o
>>>    */
>>>   static int add_call_destinations(struct objtool_file *file)
>>>   {
>>> +       struct instruction *next_insn;
>>>          struct instruction *insn;
>>>          unsigned long dest_off;
>>>          struct symbol *dest;
>>> @@ -1625,6 +1626,11 @@ static int add_call_destinations(struct 
>>> objtool_file *file)
>>>                  reloc = insn_reloc(file, insn);
>>>                  if (!reloc) {
>>>                          dest_off = arch_jump_destination(insn);
>>> +
>>> +                       next_insn = next_insn_same_func(file, insn);
>>> +                       if (next_insn && dest_off == next_insn->offset)
>>> +                               continue;
>>> +
>>>                          dest = find_call_destination(insn->sec, 
>>> dest_off);
>>>
>>>                          add_call_dest(file, insn, dest, false);
>>> -- 
>>> 2.39.3
>>>
>>
> 

Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Nathan Chancellor 9 months, 3 weeks ago
On Mon, Feb 24, 2025 at 02:19:14PM +0100, Christophe Leroy wrote:
> Well, this problem already exists on clang 18 it seems:
> 
> 00000004 <btext_map>:
>    4:   7c 08 02 a6     mflr    r0
>    8:   94 21 ff e0     stwu    r1,-32(r1)
>    c:   93 c1 00 18     stw     r30,24(r1)
>   10:   90 01 00 24     stw     r0,36(r1)
>   14:   93 a1 00 14     stw     r29,20(r1)
>   18:   48 00 00 05     bl      1c <btext_map+0x18>
>   1c:   38 a0 00 00     li      r5,0
>   20:   7f c8 02 a6     mflr    r30
> 
> While GCC generates:
> 
> 00000418 <btext_map>:
>  418:   94 21 ff e0     stwu    r1,-32(r1)
>  41c:   7c 08 02 a6     mflr    r0
>  420:   42 9f 00 05     bcl     20,4*cr7+so,424 <btext_map+0xc>
>  424:   39 20 00 00     li      r9,0
>  428:   93 c1 00 18     stw     r30,24(r1)
>  42c:   7f c8 02 a6     mflr    r30
> 
> So lets make the kernel tolerate it allthough it should be fixed on clang at
> the end.
> 
> I can't find any related issue in the clang issues database
> (https://github.com/llvm/llvm-project/issues), should we open one ?

Yes please, especially if you happen to have a simplified reproducer
(but no worries if not). I can make sure it gets labeled for the PowerPC
backend folks to take a look at.

Cheers,
Nathan
Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Christophe Leroy 9 months, 3 weeks ago

Le 24/02/2025 à 18:09, Nathan Chancellor a écrit :
> On Mon, Feb 24, 2025 at 02:19:14PM +0100, Christophe Leroy wrote:
>> Well, this problem already exists on clang 18 it seems:
>>
>> 00000004 <btext_map>:
>>     4:   7c 08 02 a6     mflr    r0
>>     8:   94 21 ff e0     stwu    r1,-32(r1)
>>     c:   93 c1 00 18     stw     r30,24(r1)
>>    10:   90 01 00 24     stw     r0,36(r1)
>>    14:   93 a1 00 14     stw     r29,20(r1)
>>    18:   48 00 00 05     bl      1c <btext_map+0x18>
>>    1c:   38 a0 00 00     li      r5,0
>>    20:   7f c8 02 a6     mflr    r30
>>
>> While GCC generates:
>>
>> 00000418 <btext_map>:
>>   418:   94 21 ff e0     stwu    r1,-32(r1)
>>   41c:   7c 08 02 a6     mflr    r0
>>   420:   42 9f 00 05     bcl     20,4*cr7+so,424 <btext_map+0xc>
>>   424:   39 20 00 00     li      r9,0
>>   428:   93 c1 00 18     stw     r30,24(r1)
>>   42c:   7f c8 02 a6     mflr    r30
>>
>> So lets make the kernel tolerate it allthough it should be fixed on clang at
>> the end.
>>
>> I can't find any related issue in the clang issues database
>> (https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fllvm%2Fllvm-project%2Fissues&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7C3c10d37fecd94c692acb08dd54f5ff51%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638760137702082512%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=qOjbONjWUuXFKUNb42yEPXgXmvU6x%2BuwbGSg2Ep6WRk%3D&reserved=0), should we open one ?
> 
> Yes please, especially if you happen to have a simplified reproducer
> (but no worries if not). I can make sure it gets labeled for the PowerPC
> backend folks to take a look at.
> 

Done, see https://github.com/llvm/llvm-project/issues/128644
Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Josh Poimboeuf 10 months ago
On Wed, Feb 19, 2025 at 09:50:14PM +0530, Sathvika Vasireddy wrote:
> Architectures like PowerPC use a pattern where the compiler generates a
> branch-and-link (bl) instruction that targets the very next instruction,
> followed by loading the link register (mflr) later. This pattern appears
> in the code like:
> 
>  bl .+4
>  li r5,0
>  mflr r30

If I understand correctly, this is basically a fake call which is used
to get the value of the program counter?

> Objtool currently warns about this as an "unannotated intra-function
> call" because find_call_destination() fails to find any symbol at the
> target offset. Add a check to skip the warning when a branch targets
> the immediate next instruction in the same function.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202502180818.XnFdv8I8-lkp@intel.com/
> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>

This should have a Fixes tag as well.

>  static int add_call_destinations(struct objtool_file *file)
>  {
> +	struct instruction *next_insn;
>  	struct instruction *insn;
>  	unsigned long dest_off;
>  	struct symbol *dest;
> @@ -1625,6 +1626,11 @@ static int add_call_destinations(struct objtool_file *file)
>  		reloc = insn_reloc(file, insn);
>  		if (!reloc) {
>  			dest_off = arch_jump_destination(insn);
> +
> +			next_insn = next_insn_same_func(file, insn);
> +			if (next_insn && dest_off == next_insn->offset)
> +				continue;
> +

This won't work on x86, where an intra-function call is converted to a
stack-modifying JUMP.  So this should probably be checked in an
arch-specific function.

-- 
Josh
Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Sathvika Vasireddy 10 months ago
Hi Josh, Thanks for the review.

On 2/21/25 1:29 AM, Josh Poimboeuf wrote:
> On Wed, Feb 19, 2025 at 09:50:14PM +0530, Sathvika Vasireddy wrote:
>> Architectures like PowerPC use a pattern where the compiler generates a
>> branch-and-link (bl) instruction that targets the very next instruction,
>> followed by loading the link register (mflr) later. This pattern appears
>> in the code like:
>>
>>   bl .+4
>>   li r5,0
>>   mflr r30
> If I understand correctly, this is basically a fake call which is used
> to get the value of the program counter?

Yes, that's correct.

Also, just out of curiosity, how does x86 do it? Does it not use a 
branch to next instruction approach?

>> Objtool currently warns about this as an "unannotated intra-function
>> call" because find_call_destination() fails to find any symbol at the
>> target offset. Add a check to skip the warning when a branch targets
>> the immediate next instruction in the same function.
>>
>> Reported-by: kernel test robot <lkp@intel.com>
>> Closes: https://lore.kernel.org/oe-kbuild-all/202502180818.XnFdv8I8-lkp@intel.com/
>> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
> This should have a Fixes tag as well.
Thanks for catching that. I'll add the Fixes tag.
>
>>   static int add_call_destinations(struct objtool_file *file)
>>   {
>> +	struct instruction *next_insn;
>>   	struct instruction *insn;
>>   	unsigned long dest_off;
>>   	struct symbol *dest;
>> @@ -1625,6 +1626,11 @@ static int add_call_destinations(struct objtool_file *file)
>>   		reloc = insn_reloc(file, insn);
>>   		if (!reloc) {
>>   			dest_off = arch_jump_destination(insn);
>> +
>> +			next_insn = next_insn_same_func(file, insn);
>> +			if (next_insn && dest_off == next_insn->offset)
>> +				continue;
>> +
> This won't work on x86, where an intra-function call is converted to a
> stack-modifying JUMP.  So this should probably be checked in an
> arch-specific function.

Thanks for letting me know, I'll introduce arch_skip_call_warning() to 
handle architecture specific cases in the next patch I send.

Regards,
Sathvika
Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Peter Zijlstra 9 months, 3 weeks ago
On Fri, Feb 21, 2025 at 02:20:41PM +0530, Sathvika Vasireddy wrote:

> > > @@ -1625,6 +1626,11 @@ static int add_call_destinations(struct objtool_file *file)
> > >   		reloc = insn_reloc(file, insn);
> > >   		if (!reloc) {
> > >   			dest_off = arch_jump_destination(insn);
> > > +
> > > +			next_insn = next_insn_same_func(file, insn);
> > > +			if (next_insn && dest_off == next_insn->offset)
> > > +				continue;
> > > +
> > This won't work on x86, where an intra-function call is converted to a
> > stack-modifying JUMP.  So this should probably be checked in an
> > arch-specific function.
> 
> Thanks for letting me know, I'll introduce arch_skip_call_warning() to
> handle architecture specific cases in the next patch I send.

Can't you detect this pattern in decode and simpy not emit the call
instruction?
Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Christophe Leroy 9 months, 3 weeks ago

Le 24/02/2025 à 17:25, Peter Zijlstra a écrit :
> On Fri, Feb 21, 2025 at 02:20:41PM +0530, Sathvika Vasireddy wrote:
> 
>>>> @@ -1625,6 +1626,11 @@ static int add_call_destinations(struct objtool_file *file)
>>>>    		reloc = insn_reloc(file, insn);
>>>>    		if (!reloc) {
>>>>    			dest_off = arch_jump_destination(insn);
>>>> +
>>>> +			next_insn = next_insn_same_func(file, insn);
>>>> +			if (next_insn && dest_off == next_insn->offset)
>>>> +				continue;
>>>> +
>>> This won't work on x86, where an intra-function call is converted to a
>>> stack-modifying JUMP.  So this should probably be checked in an
>>> arch-specific function.
>>
>> Thanks for letting me know, I'll introduce arch_skip_call_warning() to
>> handle architecture specific cases in the next patch I send.
> 
> Can't you detect this pattern in decode and simpy not emit the call
> instruction?
> 

Yes we can, simply do:

diff --git a/tools/objtool/arch/powerpc/decode.c 
b/tools/objtool/arch/powerpc/decode.c
index 53b55690f320..4f9b1715caf1 100644
--- a/tools/objtool/arch/powerpc/decode.c
+++ b/tools/objtool/arch/powerpc/decode.c
@@ -55,7 +55,7 @@ int arch_decode_instruction(struct objtool_file *file, 
const struct section *sec

  	switch (opcode) {
  	case 18: /* b[l][a] */
-		if ((ins & 3) == 1) /* bl */
+		if ((ins & 3) == 1 && ins != 0x48000005) /* bl but not bl .+4*/
  			typ = INSN_CALL;

  		imm = ins & 0x3fffffc;

Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Peter Zijlstra 9 months, 3 weeks ago
On Fri, Feb 21, 2025 at 02:20:41PM +0530, Sathvika Vasireddy wrote:

> Also, just out of curiosity, how does x86 do it? Does it not use a branch to
> next instruction approach?

x86_64 can use LEA like:

  #define _THIS_IP_ ({ unsigned long __here; asm ("lea 0(%%rip), %0" : "=r" (__here)); __here; })

32bit needs to call a function, read the stack value and return.
Re: [RFC PATCH] objtool: Skip unannotated intra-function call warning for bl+mflr pattern
Posted by Christophe Leroy 9 months, 3 weeks ago

Le 21/02/2025 à 09:50, Sathvika Vasireddy a écrit :
> [Vous ne recevez pas souvent de courriers de sv@linux.ibm.com. Découvrez 
> pourquoi ceci est important à https://aka.ms/ 
> LearnAboutSenderIdentification ]
> 
> Hi Josh, Thanks for the review.
> 
> On 2/21/25 1:29 AM, Josh Poimboeuf wrote:
>> On Wed, Feb 19, 2025 at 09:50:14PM +0530, Sathvika Vasireddy wrote:
>>> Architectures like PowerPC use a pattern where the compiler generates a
>>> branch-and-link (bl) instruction that targets the very next instruction,
>>> followed by loading the link register (mflr) later. This pattern appears
>>> in the code like:
>>>
>>>   bl .+4
>>>   li r5,0
>>>   mflr r30
>> If I understand correctly, this is basically a fake call which is used
>> to get the value of the program counter?
> 
> Yes, that's correct.
> 
> Also, just out of curiosity, how does x86 do it? Does it not use a
> branch to next instruction approach?
> 
>>> Objtool currently warns about this as an "unannotated intra-function
>>> call" because find_call_destination() fails to find any symbol at the
>>> target offset. Add a check to skip the warning when a branch targets
>>> the immediate next instruction in the same function.
>>>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>> Closes: https://eur01.safelinks.protection.outlook.com/? 
>>> url=https%3A%2F%2Flore.kernel.org%2Foe-kbuild- 
>>> all%2F202502180818.XnFdv8I8- 
>>> lkp%40intel.com%2F&data=05%7C02%7Cchristophe.leroy%40csgroup.eu%7Cdce2affdaed147a6058008dd5254d85e%7C8b87af7d86474dc78df45f69a2011bb5%7C0%7C0%7C638757246560427230%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=dhUS9PNZKUpz%2Bc1hePG1tuTIWbiKqS46uoAJOvU76sU%3D&reserved=0
>>> Signed-off-by: Sathvika Vasireddy <sv@linux.ibm.com>
>> This should have a Fixes tag as well.
> Thanks for catching that. I'll add the Fixes tag.
>>
>>>   static int add_call_destinations(struct objtool_file *file)
>>>   {
>>> +    struct instruction *next_insn;
>>>      struct instruction *insn;
>>>      unsigned long dest_off;
>>>      struct symbol *dest;
>>> @@ -1625,6 +1626,11 @@ static int add_call_destinations(struct 
>>> objtool_file *file)
>>>              reloc = insn_reloc(file, insn);
>>>              if (!reloc) {
>>>                      dest_off = arch_jump_destination(insn);
>>> +
>>> +                    next_insn = next_insn_same_func(file, insn);
>>> +                    if (next_insn && dest_off == next_insn->offset)
>>> +                            continue;
>>> +
>> This won't work on x86, where an intra-function call is converted to a
>> stack-modifying JUMP.  So this should probably be checked in an
>> arch-specific function.
> 
> Thanks for letting me know, I'll introduce arch_skip_call_warning() to
> handle architecture specific cases in the next patch I send.

Not sure what you want to do here.

See my other response, I think it should just be handled as an 
INSN_OTHER by arch_decode_instruction()

Christophe