[PATCH] recordmcount: Fix the wrong use of w* in arm64_is_fake_mcount()

[PATCH] recordmcount: Fix the wrong use of w* in arm64_is_fake_mcount()

[Li Huafei]

When cross-compiling the kernel, the endian of the target machine and
the local machine may not match, at this time the recordmcount tool
needs byte reversal when processing elf's variables to get the correct
value. w* callback function is used to solve this problem, w is used for
4-byte variable processing, while w8 is used for 8-byte.

arm64_is_fake_mcount() is used to filter '_mcount' relocations that are
not used by ftrace. In arm64_is_fake_mcount(), rp->info is 8 bytes in
size, but w is used. This causes arm64_is_fake_mcount() to get the wrong
type of relocation when we cross-compile the arm64_be kernel image on an
x86_le machine, and all valid '_mcount' is filtered out. The
recordmcount tool does not collect any mcount function call locations.
At kernel startup, the following ftrace log is seen:

	ftrace: No functions to be traced?

and thus ftrace cannot be used.

Using w8 to get the value of rp->r_info will fix the problem.

Fixes: ea0eada45632 ("recordmcount: only record relocation of type
R_AARCH64_CALL26 on arm64")
Signed-off-by: Li Huafei <lihuafei1@huawei.com>
---
 scripts/recordmcount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index b9c2ee7ab43f..cce12e1971d8 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -438,7 +438,7 @@ static int arm_is_fake_mcount(Elf32_Rel const *rp)
 
 static int arm64_is_fake_mcount(Elf64_Rel const *rp)
 {
-	return ELF64_R_TYPE(w(rp->r_info)) != R_AARCH64_CALL26;
+	return ELF64_R_TYPE(w8(rp->r_info)) != R_AARCH64_CALL26;
 }
 
 /* 64-bit EM_MIPS has weird ELF64_Rela.r_info.
-- 
2.17.1

Re: [PATCH] recordmcount: Fix the wrong use of w* in arm64_is_fake_mcount()

[Steven Rostedt]

This requires an acked-by from one of the ARM64 maintainers.

-- Steve


On Thu, 25 Feb 2021 22:07:47 +0800
Li Huafei <lihuafei1@huawei.com> wrote:

> When cross-compiling the kernel, the endian of the target machine and
> the local machine may not match, at this time the recordmcount tool
> needs byte reversal when processing elf's variables to get the correct
> value. w* callback function is used to solve this problem, w is used for
> 4-byte variable processing, while w8 is used for 8-byte.
> 
> arm64_is_fake_mcount() is used to filter '_mcount' relocations that are
> not used by ftrace. In arm64_is_fake_mcount(), rp->info is 8 bytes in
> size, but w is used. This causes arm64_is_fake_mcount() to get the wrong
> type of relocation when we cross-compile the arm64_be kernel image on an
> x86_le machine, and all valid '_mcount' is filtered out. The
> recordmcount tool does not collect any mcount function call locations.
> At kernel startup, the following ftrace log is seen:
> 
> 	ftrace: No functions to be traced?
> 
> and thus ftrace cannot be used.
> 
> Using w8 to get the value of rp->r_info will fix the problem.
> 
> Fixes: ea0eada45632 ("recordmcount: only record relocation of type
> R_AARCH64_CALL26 on arm64")
> Signed-off-by: Li Huafei <lihuafei1@huawei.com>
> ---
>  scripts/recordmcount.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
> index b9c2ee7ab43f..cce12e1971d8 100644
> --- a/scripts/recordmcount.c
> +++ b/scripts/recordmcount.c
> @@ -438,7 +438,7 @@ static int arm_is_fake_mcount(Elf32_Rel const *rp)
>  
>  static int arm64_is_fake_mcount(Elf64_Rel const *rp)
>  {
> -	return ELF64_R_TYPE(w(rp->r_info)) != R_AARCH64_CALL26;
> +	return ELF64_R_TYPE(w8(rp->r_info)) != R_AARCH64_CALL26;
>  }
>  
>  /* 64-bit EM_MIPS has weird ELF64_Rela.r_info.

Re: [PATCH] recordmcount: Fix the wrong use of w* in arm64_is_fake_mcount()

[Will Deacon]

On Thu, Feb 25, 2021 at 09:44:26AM -0500, Steven Rostedt wrote:
> This requires an acked-by from one of the ARM64 maintainers.
> 
> -- Steve
> 
> 
> On Thu, 25 Feb 2021 22:07:47 +0800
> Li Huafei <lihuafei1@huawei.com> wrote:
> 
> > When cross-compiling the kernel, the endian of the target machine and
> > the local machine may not match, at this time the recordmcount tool
> > needs byte reversal when processing elf's variables to get the correct
> > value. w* callback function is used to solve this problem, w is used for
> > 4-byte variable processing, while w8 is used for 8-byte.
> > 
> > arm64_is_fake_mcount() is used to filter '_mcount' relocations that are
> > not used by ftrace. In arm64_is_fake_mcount(), rp->info is 8 bytes in
> > size, but w is used. This causes arm64_is_fake_mcount() to get the wrong
> > type of relocation when we cross-compile the arm64_be kernel image on an
> > x86_le machine, and all valid '_mcount' is filtered out. The
> > recordmcount tool does not collect any mcount function call locations.
> > At kernel startup, the following ftrace log is seen:
> > 
> > 	ftrace: No functions to be traced?
> > 
> > and thus ftrace cannot be used.
> > 
> > Using w8 to get the value of rp->r_info will fix the problem.
> > 
> > Fixes: ea0eada45632 ("recordmcount: only record relocation of type
> > R_AARCH64_CALL26 on arm64")
> > Signed-off-by: Li Huafei <lihuafei1@huawei.com>
> > ---
> >  scripts/recordmcount.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
> > index b9c2ee7ab43f..cce12e1971d8 100644
> > --- a/scripts/recordmcount.c
> > +++ b/scripts/recordmcount.c
> > @@ -438,7 +438,7 @@ static int arm_is_fake_mcount(Elf32_Rel const *rp)
> >  
> >  static int arm64_is_fake_mcount(Elf64_Rel const *rp)
> >  {
> > -	return ELF64_R_TYPE(w(rp->r_info)) != R_AARCH64_CALL26;
> > +	return ELF64_R_TYPE(w8(rp->r_info)) != R_AARCH64_CALL26;

Acked-by: Will Deacon <will@kernel.org>

But you know you could avoid these sorts of problems by moving to little
endian along with everybody else? ;)

Will

Re: [PATCH] recordmcount: Fix the wrong use of w* in arm64_is_fake_mcount()

[Steven Rostedt]

On Thu, 25 Feb 2021 16:01:17 +0000
Will Deacon <will@kernel.org> wrote:

> Acked-by: Will Deacon <will@kernel.org>

Thanks!

> 
> But you know you could avoid these sorts of problems by moving to little
> endian along with everybody else? ;)

But then how do we find these bug?

-- Steve

Re: [PATCH] recordmcount: Fix the wrong use of w* in arm64_is_fake_mcount()

[Steven Rostedt]

On Thu, 25 Feb 2021 16:01:17 +0000
Will Deacon <will@kernel.org> wrote:

> On Thu, Feb 25, 2021 at 09:44:26AM -0500, Steven Rostedt wrote:
> > This requires an acked-by from one of the ARM64 maintainers.
> > 
> > -- Steve
> > 
> > 
> > On Thu, 25 Feb 2021 22:07:47 +0800
> > Li Huafei <lihuafei1@huawei.com> wrote:
> >   
> > > When cross-compiling the kernel, the endian of the target machine and
> > > the local machine may not match, at this time the recordmcount tool
> > > needs byte reversal when processing elf's variables to get the correct
> > > value. w* callback function is used to solve this problem, w is used for
> > > 4-byte variable processing, while w8 is used for 8-byte.
> > > 
> > > arm64_is_fake_mcount() is used to filter '_mcount' relocations that are
> > > not used by ftrace. In arm64_is_fake_mcount(), rp->info is 8 bytes in
> > > size, but w is used. This causes arm64_is_fake_mcount() to get the wrong
> > > type of relocation when we cross-compile the arm64_be kernel image on an
> > > x86_le machine, and all valid '_mcount' is filtered out. The
> > > recordmcount tool does not collect any mcount function call locations.
> > > At kernel startup, the following ftrace log is seen:
> > > 
> > > 	ftrace: No functions to be traced?
> > > 
> > > and thus ftrace cannot be used.
> > > 
> > > Using w8 to get the value of rp->r_info will fix the problem.
> > > 
> > > Fixes: ea0eada45632 ("recordmcount: only record relocation of type
> > > R_AARCH64_CALL26 on arm64")
> > > Signed-off-by: Li Huafei <lihuafei1@huawei.com>
> > > ---
> > >  scripts/recordmcount.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
> > > index b9c2ee7ab43f..cce12e1971d8 100644
> > > --- a/scripts/recordmcount.c
> > > +++ b/scripts/recordmcount.c
> > > @@ -438,7 +438,7 @@ static int arm_is_fake_mcount(Elf32_Rel const *rp)
> > >  
> > >  static int arm64_is_fake_mcount(Elf64_Rel const *rp)
> > >  {
> > > -	return ELF64_R_TYPE(w(rp->r_info)) != R_AARCH64_CALL26;
> > > +	return ELF64_R_TYPE(w8(rp->r_info)) != R_AARCH64_CALL26;  
> 
> Acked-by: Will Deacon <will@kernel.org>
> 
> But you know you could avoid these sorts of problems by moving to little
> endian along with everybody else? ;)
> 

I just realized that I received this patch twice, and thought it was the
same patch! Chen was three days ahead of you, so he get's the credit ;-)

 https://lore.kernel.org/r/20210222135840.56250-1-chenjun102@huawei.com

-- Steve

Re: [PATCH] recordmcount: Fix the wrong use of w* in arm64_is_fake_mcount()

[Steven Rostedt]

On Tue, 2 Mar 2021 17:30:58 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:

> I just realized that I received this patch twice, and thought it was the
> same patch! Chen was three days ahead of you, so he get's the credit ;-)
> 
>  https://lore.kernel.org/r/20210222135840.56250-1-chenjun102@huawei.com

I'm applying this patch (same one here but came earlier).

Will, you still OK with your acked-by on it?

-- Steve

From 999340d51174ce4141dd723105d4cef872b13ee9 Mon Sep 17 00:00:00 2001
From: Chen Jun <chenjun102@huawei.com>
Date: Mon, 22 Feb 2021 13:58:40 +0000
Subject: [PATCH] ftrace: Have recordmcount use w8 to read relp->r_info in
 arm64_is_fake_mcount

On little endian system, Use aarch64_be(gcc v7.3) downloaded from
linaro.org to build image with CONFIG_CPU_BIG_ENDIAN = y,
CONFIG_FTRACE = y, CONFIG_DYNAMIC_FTRACE = y.

gcc will create symbols of _mcount but recordmcount can not create
mcount_loc for *.o.
aarch64_be-linux-gnu-objdump -r fs/namei.o | grep mcount
00000000000000d0 R_AARCH64_CALL26  _mcount
...
0000000000007190 R_AARCH64_CALL26  _mcount

The reason is than funciton arm64_is_fake_mcount can not work correctly.
A symbol of _mcount in *.o compiled with big endian compiler likes:
00 00 00 2d 00 00 01 1b
w(rp->r_info) will return 0x2d instead of 0x011b. Because w() takes
uint32_t as parameter, which truncates rp->r_info.

Use w8() instead w() to read relp->r_info

Link: https://lkml.kernel.org/r/20210222135840.56250-1-chenjun102@huawei.com

Fixes: ea0eada45632 ("recordmcount: only record relocation of type R_AARCH64_CALL26 on arm64.")
Acked-by: Will Deacon <will@kernel.org>
Signed-off-by: Chen Jun <chenjun102@huawei.com>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---
 scripts/recordmcount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index b9c2ee7ab43f..cce12e1971d8 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -438,7 +438,7 @@ static int arm_is_fake_mcount(Elf32_Rel const *rp)
 
 static int arm64_is_fake_mcount(Elf64_Rel const *rp)
 {
-	return ELF64_R_TYPE(w(rp->r_info)) != R_AARCH64_CALL26;
+	return ELF64_R_TYPE(w8(rp->r_info)) != R_AARCH64_CALL26;
 }
 
 /* 64-bit EM_MIPS has weird ELF64_Rela.r_info.
-- 
2.25.4

Re: [PATCH] recordmcount: Fix the wrong use of w* in arm64_is_fake_mcount()

[Will Deacon]

On Tue, Mar 02, 2021 at 05:33:35PM -0500, Steven Rostedt wrote:
> On Tue, 2 Mar 2021 17:30:58 -0500
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > I just realized that I received this patch twice, and thought it was the
> > same patch! Chen was three days ahead of you, so he get's the credit ;-)
> > 
> >  https://lore.kernel.org/r/20210222135840.56250-1-chenjun102@huawei.com
> 
> I'm applying this patch (same one here but came earlier).
> 
> Will, you still OK with your acked-by on it?

Absolutely!

Will

Re: [PATCH] recordmcount: Fix the wrong use of w* in arm64_is_fake_mcount()

[Li Huafei]

On 2021/3/3 6:30, Steven Rostedt wrote:
> On Thu, 25 Feb 2021 16:01:17 +0000
> Will Deacon <will@kernel.org> wrote:
> 
>> On Thu, Feb 25, 2021 at 09:44:26AM -0500, Steven Rostedt wrote:
>>> This requires an acked-by from one of the ARM64 maintainers.
>>>
>>> -- Steve
>>>
>>>
>>> On Thu, 25 Feb 2021 22:07:47 +0800
>>> Li Huafei <lihuafei1@huawei.com> wrote:
>>>    
>>>> When cross-compiling the kernel, the endian of the target machine and
>>>> the local machine may not match, at this time the recordmcount tool
>>>> needs byte reversal when processing elf's variables to get the correct
>>>> value. w* callback function is used to solve this problem, w is used for
>>>> 4-byte variable processing, while w8 is used for 8-byte.
>>>>
>>>> arm64_is_fake_mcount() is used to filter '_mcount' relocations that are
>>>> not used by ftrace. In arm64_is_fake_mcount(), rp->info is 8 bytes in
>>>> size, but w is used. This causes arm64_is_fake_mcount() to get the wrong
>>>> type of relocation when we cross-compile the arm64_be kernel image on an
>>>> x86_le machine, and all valid '_mcount' is filtered out. The
>>>> recordmcount tool does not collect any mcount function call locations.
>>>> At kernel startup, the following ftrace log is seen:
>>>>
>>>> 	ftrace: No functions to be traced?
>>>>
>>>> and thus ftrace cannot be used.
>>>>
>>>> Using w8 to get the value of rp->r_info will fix the problem.
>>>>
>>>> Fixes: ea0eada45632 ("recordmcount: only record relocation of type
>>>> R_AARCH64_CALL26 on arm64")
>>>> Signed-off-by: Li Huafei <lihuafei1@huawei.com>
>>>> ---
>>>>   scripts/recordmcount.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
>>>> index b9c2ee7ab43f..cce12e1971d8 100644
>>>> --- a/scripts/recordmcount.c
>>>> +++ b/scripts/recordmcount.c
>>>> @@ -438,7 +438,7 @@ static int arm_is_fake_mcount(Elf32_Rel const *rp)
>>>>   
>>>>   static int arm64_is_fake_mcount(Elf64_Rel const *rp)
>>>>   {
>>>> -	return ELF64_R_TYPE(w(rp->r_info)) != R_AARCH64_CALL26;
>>>> +	return ELF64_R_TYPE(w8(rp->r_info)) != R_AARCH64_CALL26;
>>
>> Acked-by: Will Deacon <will@kernel.org>
>>
>> But you know you could avoid these sorts of problems by moving to little
>> endian along with everybody else? ;)
>>
> 
> I just realized that I received this patch twice, and thought it was the
> same patch! Chen was three days ahead of you, so he get's the credit ;-)
> 
>   https://lore.kernel.org/r/20210222135840.56250-1-chenjun102@huawei.com
> 
> -- Steve
> .
> 

That's fine, thanks Steve and Will!

Huafei