[PATCH 0/3] recordmcount: bugfix and amend

[PATCH 0/3] recordmcount: bugfix and amend

[Li Bin]

Li Bin (3):
  recordmcount: fix endianness handling bug for nop_mcount
  recordmcount: x86: assign a meaningful value to rel_type_nop
  recordmcount: arm64: replace the ignored mcount call into nop

 scripts/recordmcount.c | 26 +++++++++++++++++++++++++-
 scripts/recordmcount.h |  5 +++--
 2 files changed, 28 insertions(+), 3 deletions(-)

-- 
1.7.12.4

[PATCH 1/3] recordmcount: fix endianness handling bug for nop_mcount

[Li Bin]

In nop_mcount, shdr->sh_offset and welp->r_offset should handle
endianness properly, otherwise it will trigger Segmentation fault
if the recordmcount main and file.o have different endianness.

Cc: <stable@vger.kernel.org> # 3.0+
Signed-off-by: Li Bin <huawei.libin@huawei.com>
---
 scripts/recordmcount.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 49b582a..dda9dba 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -376,8 +376,9 @@ static void nop_mcount(Elf_Shdr const *const relhdr,
 			mcountsym = get_mcountsym(sym0, relp, str0);
 
 		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
-			if (make_nop)
-				ret = make_nop((void *)ehdr, shdr->sh_offset + relp->r_offset);
+			if (make_nop) {
+				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
+			}
 			if (warn_on_notrace_sect && !once) {
 				printf("Section %s has mcount callers being ignored\n",
 				       txtname);
-- 
1.7.12.4

[PATCH 2/3] recordmcount: x86: assign a meaningful value to rel_type_nop

[Li Bin]

Although, the default value of rel_type_nop is zero, and the value
of R_386_NONE/R_X86_64_NONE is zero too, but it should be assigned
a meaningful value explicitly, otherwise it looks confused.

Assign R_386_NONE to rel_type_nop for 386, assign R_X86_64_NONE
to rel_type_nop for x86_64.

Signed-off-by: Li Bin <huawei.libin@huawei.com>
---
 scripts/recordmcount.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 3d1984e..8cc020b 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -345,6 +345,7 @@ do_file(char const *const fname)
 		break;
 	case EM_386:
 		reltype = R_386_32;
+		rel_type_nop = R_386_NONE;
 		make_nop = make_nop_x86;
 		ideal_nop = ideal_nop5_x86_32;
 		mcount_adjust_32 = -1;
@@ -371,6 +372,7 @@ do_file(char const *const fname)
 		make_nop = make_nop_x86;
 		ideal_nop = ideal_nop5_x86_64;
 		reltype = R_X86_64_64;
+		rel_type_nop = R_X86_64_NONE;
 		mcount_adjust_64 = -1;
 		break;
 	}  /* end switch */
-- 
1.7.12.4

[PATCH 3/3] recordmcount: arm64: replace the ignored mcount call into nop

[Li Bin]

By now, the recordmcount only records the function that in
following sections:
.text/.ref.text/.sched.text/.spinlock.text/.irqentry.text/
.kprobes.text/.text.unlikely

For the function that not in these sections, the call mcount
will be in place and not be replaced when kernel boot up. And
it will bring performance overhead, such as do_mem_abort (in
.exception.text section). This patch make the call mcount to
nop for this case in recordmcount.

Cc: <stable@vger.kernel.org> # 3.18+
Signed-off-by: Li Bin <huawei.libin@huawei.com>
---
 scripts/recordmcount.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
index 8cc020b..698768b 100644
--- a/scripts/recordmcount.c
+++ b/scripts/recordmcount.c
@@ -42,6 +42,7 @@
 
 #ifndef EM_AARCH64
 #define EM_AARCH64	183
+#define R_AARCH64_NONE		0
 #define R_AARCH64_ABS64	257
 #endif
 
@@ -160,6 +161,22 @@ static int make_nop_x86(void *map, size_t const offset)
 	return 0;
 }
 
+static unsigned char ideal_nop4_arm64[4] = {0x1f, 0x20, 0x03, 0xd5};
+static int make_nop_arm64(void *map, size_t const offset)
+{
+	uint32_t *ptr;
+
+	ptr = map + offset;
+	/* bl <_mcount> is 0x94000000 before relocation */
+	if (*ptr != 0x94000000)
+		return -1;
+
+	/* Convert to nop */
+	ulseek(fd_map, offset, SEEK_SET);
+	uwrite(fd_map, ideal_nop, 4);
+	return 0;
+}
+
 /*
  * Get the whole file as a programming convenience in order to avoid
  * malloc+lseek+read+free of many pieces.  If successful, then mmap
@@ -354,7 +371,12 @@ do_file(char const *const fname)
 			 altmcount = "__gnu_mcount_nc";
 			 break;
 	case EM_AARCH64:
-			 reltype = R_AARCH64_ABS64; gpfx = '_'; break;
+			reltype = R_AARCH64_ABS64;
+			make_nop = make_nop_arm64;
+			rel_type_nop = R_AARCH64_NONE;
+			ideal_nop = ideal_nop4_arm64;
+			gpfx = '_';
+			break;
 	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
 	case EM_METAG:	 reltype = R_METAG_ADDR32;
 			 altmcount = "_mcount_wrapper";
-- 
1.7.12.4

Re: [PATCH 3/3] recordmcount: arm64: replace the ignored mcount call into nop

[libin]

on 2015/10/30 16:31, Li Bin wrote:
> By now, the recordmcount only records the function that in
> following sections:
> .text/.ref.text/.sched.text/.spinlock.text/.irqentry.text/
> .kprobes.text/.text.unlikely
> 
> For the function that not in these sections, the call mcount
> will be in place and not be replaced when kernel boot up. And
> it will bring performance overhead, such as do_mem_abort (in
> .exception.text section). This patch make the call mcount to
> nop for this case in recordmcount.
> 
> Cc: <stable@vger.kernel.org> # 3.18+
> Signed-off-by: Li Bin <huawei.libin@huawei.com>

Acked-by: Will Deacon <will.deacon@arm.com>
https://lkml.org/lkml/2015/10/28/301

> ---
>  scripts/recordmcount.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/scripts/recordmcount.c b/scripts/recordmcount.c
> index 8cc020b..698768b 100644
> --- a/scripts/recordmcount.c
> +++ b/scripts/recordmcount.c
> @@ -42,6 +42,7 @@
>  
>  #ifndef EM_AARCH64
>  #define EM_AARCH64	183
> +#define R_AARCH64_NONE		0
>  #define R_AARCH64_ABS64	257
>  #endif
>  
> @@ -160,6 +161,22 @@ static int make_nop_x86(void *map, size_t const offset)
>  	return 0;
>  }
>  
> +static unsigned char ideal_nop4_arm64[4] = {0x1f, 0x20, 0x03, 0xd5};
> +static int make_nop_arm64(void *map, size_t const offset)
> +{
> +	uint32_t *ptr;
> +
> +	ptr = map + offset;
> +	/* bl <_mcount> is 0x94000000 before relocation */
> +	if (*ptr != 0x94000000)
> +		return -1;
> +
> +	/* Convert to nop */
> +	ulseek(fd_map, offset, SEEK_SET);
> +	uwrite(fd_map, ideal_nop, 4);
> +	return 0;
> +}
> +
>  /*
>   * Get the whole file as a programming convenience in order to avoid
>   * malloc+lseek+read+free of many pieces.  If successful, then mmap
> @@ -354,7 +371,12 @@ do_file(char const *const fname)
>  			 altmcount = "__gnu_mcount_nc";
>  			 break;
>  	case EM_AARCH64:
> -			 reltype = R_AARCH64_ABS64; gpfx = '_'; break;
> +			reltype = R_AARCH64_ABS64;
> +			make_nop = make_nop_arm64;
> +			rel_type_nop = R_AARCH64_NONE;
> +			ideal_nop = ideal_nop4_arm64;
> +			gpfx = '_';
> +			break;
>  	case EM_IA_64:	 reltype = R_IA64_IMM64;   gpfx = '_'; break;
>  	case EM_METAG:	 reltype = R_METAG_ADDR32;
>  			 altmcount = "_mcount_wrapper";
> 

Re: [PATCH 1/3] recordmcount: fix endianness handling bug for nop_mcount

[libin]

on 2015/10/30 16:31, Li Bin wrote:
> In nop_mcount, shdr->sh_offset and welp->r_offset should handle
> endianness properly, otherwise it will trigger Segmentation fault
> if the recordmcount main and file.o have different endianness.
> 

For more information, please refer to
	https://lkml.org/lkml/2015/10/29/258

> Cc: <stable@vger.kernel.org> # 3.0+
> Signed-off-by: Li Bin <huawei.libin@huawei.com>
> ---
>  scripts/recordmcount.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> index 49b582a..dda9dba 100644
> --- a/scripts/recordmcount.h
> +++ b/scripts/recordmcount.h
> @@ -376,8 +376,9 @@ static void nop_mcount(Elf_Shdr const *const relhdr,
>  			mcountsym = get_mcountsym(sym0, relp, str0);
>  
>  		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
> -			if (make_nop)
> -				ret = make_nop((void *)ehdr, shdr->sh_offset + relp->r_offset);
> +			if (make_nop) {
> +				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
> +			}
>  			if (warn_on_notrace_sect && !once) {
>  				printf("Section %s has mcount callers being ignored\n",
>  				       txtname);
> 

Re: [PATCH 1/3] recordmcount: fix endianness handling bug for nop_mcount

[Sergei Shtylyov]

Hello.

On 10/30/2015 11:31 AM, Li Bin wrote:

> In nop_mcount, shdr->sh_offset and welp->r_offset should handle
> endianness properly, otherwise it will trigger Segmentation fault
> if the recordmcount main and file.o have different endianness.
>
> Cc: <stable@vger.kernel.org> # 3.0+
> Signed-off-by: Li Bin <huawei.libin@huawei.com>
> ---
>   scripts/recordmcount.h | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> index 49b582a..dda9dba 100644
> --- a/scripts/recordmcount.h
> +++ b/scripts/recordmcount.h
> @@ -376,8 +376,9 @@ static void nop_mcount(Elf_Shdr const *const relhdr,
>   			mcountsym = get_mcountsym(sym0, relp, str0);
>
>   		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
> -			if (make_nop)
> -				ret = make_nop((void *)ehdr, shdr->sh_offset + relp->r_offset);
> +			if (make_nop) {
> +				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
> +			}

    {} not needed here.

MBR, Sergei

Re: [PATCH 1/3] recordmcount: fix endianness handling bug for nop_mcount

[libin]

on 2015/10/30 22:35, Sergei Shtylyov wrote:
> Hello.
> 
> On 10/30/2015 11:31 AM, Li Bin wrote:
> 
>> In nop_mcount, shdr->sh_offset and welp->r_offset should handle
>> endianness properly, otherwise it will trigger Segmentation fault
>> if the recordmcount main and file.o have different endianness.
>>
>> Cc: <stable@vger.kernel.org> # 3.0+
>> Signed-off-by: Li Bin <huawei.libin@huawei.com>
>> ---
>>   scripts/recordmcount.h | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
>> index 49b582a..dda9dba 100644
>> --- a/scripts/recordmcount.h
>> +++ b/scripts/recordmcount.h
>> @@ -376,8 +376,9 @@ static void nop_mcount(Elf_Shdr const *const relhdr,
>>               mcountsym = get_mcountsym(sym0, relp, str0);
>>
>>           if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
>> -            if (make_nop)
>> -                ret = make_nop((void *)ehdr, shdr->sh_offset + relp->r_offset);
>> +            if (make_nop) {
>> +                ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
>> +            }
> 
>    {} not needed here.

Oh, yes. When I cleaned the debugging code, forgot delete the {}.

Thanks,
Li Bin

> 
> MBR, Sergei
> 
> 
> .
> 

Re: [PATCH 1/3] recordmcount: fix endianness handling bug for nop_mcount

[Steven Rostedt]

On Sat, 31 Oct 2015 10:07:38 +0800
libin <huawei.libin@huawei.com> wrote:

> 
> 
> on 2015/10/30 22:35, Sergei Shtylyov wrote:
> > Hello.
> > 
> > On 10/30/2015 11:31 AM, Li Bin wrote:
> > 
> >> In nop_mcount, shdr->sh_offset and welp->r_offset should handle
> >> endianness properly, otherwise it will trigger Segmentation fault
> >> if the recordmcount main and file.o have different endianness.
> >>
> >> Cc: <stable@vger.kernel.org> # 3.0+
> >> Signed-off-by: Li Bin <huawei.libin@huawei.com>
> >> ---
> >>   scripts/recordmcount.h | 5 +++--
> >>   1 file changed, 3 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> >> index 49b582a..dda9dba 100644
> >> --- a/scripts/recordmcount.h
> >> +++ b/scripts/recordmcount.h
> >> @@ -376,8 +376,9 @@ static void nop_mcount(Elf_Shdr const *const relhdr,
> >>               mcountsym = get_mcountsym(sym0, relp, str0);
> >>
> >>           if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
> >> -            if (make_nop)
> >> -                ret = make_nop((void *)ehdr, shdr->sh_offset + relp->r_offset);
> >> +            if (make_nop) {
> >> +                ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
> >> +            }
> > 
> >    {} not needed here.
> 
> Oh, yes. When I cleaned the debugging code, forgot delete the {}.
> 
>

Is there a new version of this patch, or should I just take this and
remove the '{}' myself?

I already added the x86 update.

-- Steve

[PATCH] recordmcount: fix endianness handling bug for nop_mcount

[libin]

In nop_mcount, shdr->sh_offset and welp->r_offset should handle
endianness properly, otherwise it will trigger Segmentation fault
if the recordmcount main and file.o have different endianness.

Cc: <stable@vger.kernel.org> # 3.0+
Signed-off-by: Li Bin <huawei.libin@huawei.com>
---
 scripts/recordmcount.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index 49b582a..b9897e2 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -377,7 +377,7 @@ static void nop_mcount(Elf_Shdr const *const relhdr,

 		if (mcountsym == Elf_r_sym(relp) && !is_fake_mcount(relp)) {
 			if (make_nop)
-				ret = make_nop((void *)ehdr, shdr->sh_offset + relp->r_offset);
+				ret = make_nop((void *)ehdr, _w(shdr->sh_offset) + _w(relp->r_offset));
 			if (warn_on_notrace_sect && !once) {
 				printf("Section %s has mcount callers being ignored\n",
 				       txtname);
-- 
1.7.1