[PATCH] recordmcount: Fix handling of elf64 big-endian objects.

[PATCH] recordmcount: Fix handling of elf64 big-endian objects.

[David Daney]

From: David Daney <david.daney@cavium.com>

In ELF64, the sh_flags field is 64-bits wide.  recordmcount was
erroneously treating it as a 32-bit wide field.  For little endian
objects this works because the flags of interest (SHF_EXECINSTR)
reside in the lower 32 bits of the word, and you get the same result
with either a 32-bit or 64-bit read.  Big endian objects on the
other hand do not work at all with this error.

The fix:  Correctly treat sh_flags as 64-bits wide in elf64 objects.

The symptom I observed was that my
__start_mcount_loc..__stop_mcount_loc was empty even though ftrace
function tracing was enabled.

Signed-off-by: David Daney <david.daney@cavium.com>
---
 scripts/recordmcount.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index f40a6af6..54e35c1 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -462,7 +462,7 @@ __has_rel_mcount(Elf_Shdr const *const relhdr,  /* is SHT_REL or SHT_RELA */
 		succeed_file();
 	}
 	if (w(txthdr->sh_type) != SHT_PROGBITS ||
-	    !(w(txthdr->sh_flags) & SHF_EXECINSTR))
+	    !(_w(txthdr->sh_flags) & SHF_EXECINSTR))
 		return NULL;
 	return txtname;
 }
-- 
1.7.2.3

Re: [PATCH] recordmcount: Fix handling of elf64 big-endian objects.

[Steven Rostedt]

On Mon, 2011-12-19 at 17:42 -0800, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> In ELF64, the sh_flags field is 64-bits wide.  recordmcount was
> erroneously treating it as a 32-bit wide field.  For little endian
> objects this works because the flags of interest (SHF_EXECINSTR)
> reside in the lower 32 bits of the word, and you get the same result
> with either a 32-bit or 64-bit read.  Big endian objects on the
> other hand do not work at all with this error.
> 
> The fix:  Correctly treat sh_flags as 64-bits wide in elf64 objects.
> 
> The symptom I observed was that my
> __start_mcount_loc..__stop_mcount_loc was empty even though ftrace
> function tracing was enabled.
> 

OUCH!

This looks like something that needs to go to stable. Let me play with
this on my PPC64 tomorrow.

Thanks!

-- Steve

> Signed-off-by: David Daney <david.daney@cavium.com>
> ---
>  scripts/recordmcount.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
> index f40a6af6..54e35c1 100644
> --- a/scripts/recordmcount.h
> +++ b/scripts/recordmcount.h
> @@ -462,7 +462,7 @@ __has_rel_mcount(Elf_Shdr const *const relhdr,  /* is SHT_REL or SHT_RELA */
>  		succeed_file();
>  	}
>  	if (w(txthdr->sh_type) != SHT_PROGBITS ||
> -	    !(w(txthdr->sh_flags) & SHF_EXECINSTR))
> +	    !(_w(txthdr->sh_flags) & SHF_EXECINSTR))
>  		return NULL;
>  	return txtname;
>  }

Re: [PATCH] recordmcount: Fix handling of elf64 big-endian objects.

[David Daney]

On 12/19/2011 07:09 PM, Steven Rostedt wrote:
> On Mon, 2011-12-19 at 17:42 -0800, David Daney wrote:
>> From: David Daney<david.daney@cavium.com>
>>
>> In ELF64, the sh_flags field is 64-bits wide.  recordmcount was
>> erroneously treating it as a 32-bit wide field.  For little endian
>> objects this works because the flags of interest (SHF_EXECINSTR)
>> reside in the lower 32 bits of the word, and you get the same result
>> with either a 32-bit or 64-bit read.  Big endian objects on the
>> other hand do not work at all with this error.
>>
>> The fix:  Correctly treat sh_flags as 64-bits wide in elf64 objects.
>>
>> The symptom I observed was that my
>> __start_mcount_loc..__stop_mcount_loc was empty even though ftrace
>> function tracing was enabled.
>>
>
> OUCH!
>
> This looks like something that needs to go to stable. Let me play with
> this on my PPC64 tomorrow.
>

Hi Steven,

Any more feedback on this patch?  Should I look for it to appear in 3.3?

Thanks,
David Daney

> Thanks!
>
> -- Steve
>
>> Signed-off-by: David Daney<david.daney@cavium.com>
>> ---
>>   scripts/recordmcount.h |    2 +-
>>   1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
>> index f40a6af6..54e35c1 100644
>> --- a/scripts/recordmcount.h
>> +++ b/scripts/recordmcount.h
>> @@ -462,7 +462,7 @@ __has_rel_mcount(Elf_Shdr const *const relhdr,  /* is SHT_REL or SHT_RELA */
>>   		succeed_file();
>>   	}
>>   	if (w(txthdr->sh_type) != SHT_PROGBITS ||
>> -	    !(w(txthdr->sh_flags)&  SHF_EXECINSTR))
>> +	    !(_w(txthdr->sh_flags)&  SHF_EXECINSTR))
>>   		return NULL;
>>   	return txtname;
>>   }
>
>

Re: [PATCH] recordmcount: Fix handling of elf64 big-endian objects.

[Steven Rostedt]

On Fri, 2012-01-06 at 12:42 -0800, David Daney wrote:

> Any more feedback on this patch?  Should I look for it to appear in 3.3?

Crap, I started testing this, and then got sidetracked on some fires I
had to put out, and then x-mas came and I got drunk and forget
everything before that ;)  (not to mention the drugs I was on for my
recent hip surgery)

Let me boot up my PPC64 box and test it out now.

Thanks for the reminder. After I get this done, then I'll push this
through my "urgent" branch, which will go into the rc releases, and I'll
probably add a "stable" tag to it as well.

-- Steve

Re: [PATCH] recordmcount: Fix handling of elf64 big-endian objects.

[Steven Rostedt]

On Fri, 2012-01-06 at 12:42 -0800, David Daney wrote:

> Any more feedback on this patch?  Should I look for it to appear in 3.3?

OK, I tested it and then I noticed that mainline doesn't have
"HAVE_C_RECORDMCOUNT" set. Is that going in the PPC tree? I added that
to arch/powerpc/Kconfig, and did see the bug you saw. After adding your
patch, I did a diff of the available_filter_functions and had this:

> .elf_core_extra_phdrs
> .elf_core_write_extra_phdrs
> .elf_core_write_extra_data
> .elf_core_extra_data_size

These 4 functions were added after doing the change. Weird.

Anyway, I'll run the change through my normal tests and then push it
out.

Thanks!

-- Steve

Re: [PATCH] recordmcount: Fix handling of elf64 big-endian objects.

[David Daney]

On 01/06/2012 03:09 PM, Steven Rostedt wrote:
> On Fri, 2012-01-06 at 12:42 -0800, David Daney wrote:
>
>> Any more feedback on this patch?  Should I look for it to appear in 3.3?
>
> OK, I tested it and then I noticed that mainline doesn't have
> "HAVE_C_RECORDMCOUNT" set. Is that going in the PPC tree?

I have no idea, I'm a MIPS guy at the moment.

> I added that
> to arch/powerpc/Kconfig, and did see the bug you saw. After adding your
> patch, I did a diff of the available_filter_functions and had this:
>
>> .elf_core_extra_phdrs
>> .elf_core_write_extra_phdrs
>> .elf_core_write_extra_data
>> .elf_core_extra_data_size
>
> These 4 functions were added after doing the change. Weird.
>
> Anyway, I'll run the change through my normal tests and then push it
> out.
>

Thanks,
David Daney

Re: [PATCH] recordmcount: Fix handling of elf64 big-endian objects.

[Steven Rostedt]

On Fri, 2012-01-06 at 15:15 -0800, David Daney wrote:
> On 01/06/2012 03:09 PM, Steven Rostedt wrote:
> > On Fri, 2012-01-06 at 12:42 -0800, David Daney wrote:
> >
> >> Any more feedback on this patch?  Should I look for it to appear in 3.3?
> >
> > OK, I tested it and then I noticed that mainline doesn't have
> > "HAVE_C_RECORDMCOUNT" set. Is that going in the PPC tree?
> 
> I have no idea, I'm a MIPS guy at the moment.
> 

Ah, I forgot about that. Sorry ;)

-- Steve

[tip:perf/core] recordmcount: Fix handling of elf64 big-endian objects.

[tip-bot for David Daney]

Commit-ID:  2e885057b7f75035f0b85e02f737891482815a81
Gitweb:     http://git.kernel.org/tip/2e885057b7f75035f0b85e02f737891482815a81
Author:     David Daney <david.daney@cavium.com>
AuthorDate: Mon, 19 Dec 2011 17:42:42 -0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Fri, 6 Jan 2012 17:06:42 -0500

recordmcount: Fix handling of elf64 big-endian objects.

In ELF64, the sh_flags field is 64-bits wide.  recordmcount was
erroneously treating it as a 32-bit wide field.  For little endian
objects this works because the flags of interest (SHF_EXECINSTR)
reside in the lower 32 bits of the word, and you get the same result
with either a 32-bit or 64-bit read.  Big endian objects on the
other hand do not work at all with this error.

The fix:  Correctly treat sh_flags as 64-bits wide in elf64 objects.

The symptom I observed was that my
__start_mcount_loc..__stop_mcount_loc was empty even though ftrace
function tracing was enabled.

Link: http://lkml.kernel.org/r/1324345362-12230-1-git-send-email-ddaney.cavm@gmail.com

Cc: stable@kernel.org # 3.0+
Signed-off-by: David Daney <david.daney@cavium.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/recordmcount.h b/scripts/recordmcount.h
index f40a6af6..54e35c1 100644
--- a/scripts/recordmcount.h
+++ b/scripts/recordmcount.h
@@ -462,7 +462,7 @@ __has_rel_mcount(Elf_Shdr const *const relhdr,  /* is SHT_REL or SHT_RELA */
 		succeed_file();
 	}
 	if (w(txthdr->sh_type) != SHT_PROGBITS ||
-	    !(w(txthdr->sh_flags) & SHF_EXECINSTR))
+	    !(_w(txthdr->sh_flags) & SHF_EXECINSTR))
 		return NULL;
 	return txtname;
 }