[PATCH v3 0/8] tracing: recordmcount.pl Bug fixes and code improvement

[PATCH v3 0/8] tracing: recordmcount.pl Bug fixes and code improvement

[Li Hong]

Hi Steven,

This a following version for last patch series on recordmcount.pl. I have made
several changes according to your suggestions. Hopefully, they are ready to be
applied into your tree.

Changes include:

  - Improved the documentation amendment patch.
  - Keep the C like notation for error paths.
  - Drop the 'exit early' patch. Yes, it's not much useful, and actually, as
  you told me, there was a bug in it :)
  - minor typos and cleanups.

Thanks very much for your elaborate review and feedback.

-Li Hong

[PATCH v3 1/8] tracing: recordmcount.pl Amend the documentation according to the implementation

[Li Hong]

>From f36a6cc55928300cc2f31f4b442f9bc21be2b059 Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 11:31:27 +0800
Subject: [PATCH] tracing: recordmcount.pl Amend the documentation according to the implementation

In documentation, we says we will use the first function in a section as a
reference. Actually, the algorithm is: choose the first global function we
meet as a reference. If there is none, choose the first local one. So let the
documentation consistent to the code.

Also add several clarifications.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
---
 scripts/recordmcount.pl |   84 +++++++++++++++++++++++++++-------------------
 1 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 090d300..9e7ceca 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -6,73 +6,89 @@
 #                   all the offsets to the calls to mcount.
 #
 #
-# What we want to end up with is a section in vmlinux called
-# __mcount_loc that contains a list of pointers to all the
-# call sites in the kernel that call mcount. Later on boot up, the kernel
-# will read this list, save the locations and turn them into nops.
-# When tracing or profiling is later enabled, these locations will then
-# be converted back to pointers to some function.
+# What we want to end up with this is that each object file will have a 
+# section called __mcount_loc that will hold the list of pointers to mcount
+# callers. After final linking, the vmlinux will have within .init.data the 
+# list of all callers to mcount between __start_mcount_loc and __stop_mcount_loc.
+# Later on boot up, the kernel will read this list, save the locations and turn
+# them into nops. When tracing or profiling is later enabled, these locations 
+# will then be converted back to pointers to some function.
 #
 # This is no easy feat. This script is called just after the original
 # object is compiled and before it is linked.
 #
-# The references to the call sites are offsets from the section of text
-# that the call site is in. Hence, all functions in a section that
-# has a call site to mcount, will have the offset from the beginning of
-# the section and not the beginning of the function.
+# When parse this object file using 'objdump', the references to the call 
+# sites are offsets from the section that the call site is in. Hence, all 
+# functions in a section that has a call site to mcount, will have the 
+# offset from the beginning of the section and not the beginning of the 
+# function.
 #
-# The trick is to find a way to record the beginning of the section.
-# The way we do this is to look at the first function in the section
-# which will also be the location of that section after final link.
+# But where this section will reside finally in vmlinx is undetermined at 
+# this point. So we can't use this kind of offsets to record the final
+# address of this call site.
+#
+# The trick is to change the call offset referring the start of a section to
+# referring a function symbol in this section. During the link step, 'ld' will
+# compute the final address according to the information we record.
+# 
 # e.g.
 #
 #  .section ".sched.text", "ax"
-#  .globl my_func
-#  my_func:
 #        [...]
-#        call mcount  (offset: 0x5)
+#  func1:
+#        [...]
+#        call mcount  (offset: 0x10)
+#        [...]
+#        ret
+#  .globl fun2
+#  func2:             (offset: 0x20)
+#        [...]
 #        [...]
 #        ret
-#  other_func:
+#  func3:
 #        [...]
-#        call mcount (offset: 0x1b)
+#        call mcount (offset: 0x30)
 #        [...]
 #
 # Both relocation offsets for the mcounts in the above example will be
-# offset from .sched.text. If we make another file called tmp.s with:
+# offset from .sched.text. If we choose global symbol func2 as a reference and
+# make another file called tmp.s with the new offsets:
 #
 #  .section __mcount_loc
-#  .quad  my_func + 0x5
-#  .quad  my_func + 0x1b
+#  .quad  func2 - 0x10
+#  .quad  func2 + 0x10
 #
-# We can then compile this tmp.s into tmp.o, and link it to the original
+# We can then compile this tmp.s into tmp.o, and link it back to the original
 # object.
 #
-# But this gets hard if my_func is not globl (a static function).
-# In such a case we have:
+# In our algorithm, we will choose the first global function we meet in this 
+# section as the reference. But this gets hard if there is no global functions 
+# in this section. In such a case we have to select a local one. E.g. func1:
 #
 #  .section ".sched.text", "ax"
-#  my_func:
+#  func1:
 #        [...]
-#        call mcount  (offset: 0x5)
+#        call mcount  (offset: 0x10)
 #        [...]
 #        ret
-#  other_func:
+#  func2:
 #        [...]
-#        call mcount (offset: 0x1b)
+#        call mcount (offset: 0x20)
 #        [...]
+#  .section "other.section"
 #
 # If we make the tmp.s the same as above, when we link together with
-# the original object, we will end up with two symbols for my_func:
+# the original object, we will end up with two symbols for func1:
 # one local, one global.  After final compile, we will end up with
-# an undefined reference to my_func.
+# an undefined reference to func1 or a wrong reference to another global
+# func1 in other files.
 #
 # Since local objects can reference local variables, we need to find
 # a way to make tmp.o reference the local objects of the original object
-# file after it is linked together. To do this, we convert the my_func
+# file after it is linked together. To do this, we convert func1
 # into a global symbol before linking tmp.o. Then after we link tmp.o
-# we will only have a single symbol for my_func that is global.
-# We can convert my_func back into a local symbol and we are done.
+# we will only have a single symbol for func1 that is global.
+# We can convert func1 back into a local symbol and we are done.
 #
 # Here are the steps we take:
 #
@@ -86,10 +102,8 @@
 # 6) Link together this new object with the list object.
 # 7) Convert the local functions back to local symbols and rename
 #    the result as the original object.
-#    End.
 # 8) Link the object with the list object.
 # 9) Move the result back to the original object.
-#    End.
 #
 
 use strict;
-- 
1.6.0.4

[tip:tracing/core] tracing: Amend documentation in recordmcount.pl to reflect implementation

[tip-bot for Li Hong]

Commit-ID:  d49f6aa76d24c60a52530474cb662e8ad9f09471
Gitweb:     http://git.kernel.org/tip/d49f6aa76d24c60a52530474cb662e8ad9f09471
Author:     Li Hong <lihong.hi@gmail.com>
AuthorDate: Wed, 28 Oct 2009 13:01:38 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 29 Oct 2009 15:11:40 -0400

tracing: Amend documentation in recordmcount.pl to reflect implementation

The documentation currently says we will use the first function in a section
as a reference. The actual algorithm is: choose the first global function we
meet as a reference. If there is none, choose the first local one.
Change the documentation to be consistent with the code.

Also add several other clarifications.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
LKML-Reference: <20091028050138.GA30758@uhli>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.pl |   84 +++++++++++++++++++++++++++-------------------
 1 files changed, 49 insertions(+), 35 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index bfb8b2c..a569be7 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -6,73 +6,89 @@
 #                   all the offsets to the calls to mcount.
 #
 #
-# What we want to end up with is a section in vmlinux called
-# __mcount_loc that contains a list of pointers to all the
-# call sites in the kernel that call mcount. Later on boot up, the kernel
-# will read this list, save the locations and turn them into nops.
-# When tracing or profiling is later enabled, these locations will then
-# be converted back to pointers to some function.
+# What we want to end up with this is that each object file will have a
+# section called __mcount_loc that will hold the list of pointers to mcount
+# callers. After final linking, the vmlinux will have within .init.data the
+# list of all callers to mcount between __start_mcount_loc and __stop_mcount_loc.
+# Later on boot up, the kernel will read this list, save the locations and turn
+# them into nops. When tracing or profiling is later enabled, these locations
+# will then be converted back to pointers to some function.
 #
 # This is no easy feat. This script is called just after the original
 # object is compiled and before it is linked.
 #
-# The references to the call sites are offsets from the section of text
-# that the call site is in. Hence, all functions in a section that
-# has a call site to mcount, will have the offset from the beginning of
-# the section and not the beginning of the function.
+# When parse this object file using 'objdump', the references to the call
+# sites are offsets from the section that the call site is in. Hence, all
+# functions in a section that has a call site to mcount, will have the
+# offset from the beginning of the section and not the beginning of the
+# function.
+#
+# But where this section will reside finally in vmlinx is undetermined at
+# this point. So we can't use this kind of offsets to record the final
+# address of this call site.
+#
+# The trick is to change the call offset referring the start of a section to
+# referring a function symbol in this section. During the link step, 'ld' will
+# compute the final address according to the information we record.
 #
-# The trick is to find a way to record the beginning of the section.
-# The way we do this is to look at the first function in the section
-# which will also be the location of that section after final link.
 # e.g.
 #
 #  .section ".sched.text", "ax"
-#  .globl my_func
-#  my_func:
 #        [...]
-#        call mcount  (offset: 0x5)
+#  func1:
+#        [...]
+#        call mcount  (offset: 0x10)
+#        [...]
+#        ret
+#  .globl fun2
+#  func2:             (offset: 0x20)
+#        [...]
 #        [...]
 #        ret
-#  other_func:
+#  func3:
 #        [...]
-#        call mcount (offset: 0x1b)
+#        call mcount (offset: 0x30)
 #        [...]
 #
 # Both relocation offsets for the mcounts in the above example will be
-# offset from .sched.text. If we make another file called tmp.s with:
+# offset from .sched.text. If we choose global symbol func2 as a reference and
+# make another file called tmp.s with the new offsets:
 #
 #  .section __mcount_loc
-#  .quad  my_func + 0x5
-#  .quad  my_func + 0x1b
+#  .quad  func2 - 0x10
+#  .quad  func2 + 0x10
 #
-# We can then compile this tmp.s into tmp.o, and link it to the original
+# We can then compile this tmp.s into tmp.o, and link it back to the original
 # object.
 #
-# But this gets hard if my_func is not globl (a static function).
-# In such a case we have:
+# In our algorithm, we will choose the first global function we meet in this
+# section as the reference. But this gets hard if there is no global functions
+# in this section. In such a case we have to select a local one. E.g. func1:
 #
 #  .section ".sched.text", "ax"
-#  my_func:
+#  func1:
 #        [...]
-#        call mcount  (offset: 0x5)
+#        call mcount  (offset: 0x10)
 #        [...]
 #        ret
-#  other_func:
+#  func2:
 #        [...]
-#        call mcount (offset: 0x1b)
+#        call mcount (offset: 0x20)
 #        [...]
+#  .section "other.section"
 #
 # If we make the tmp.s the same as above, when we link together with
-# the original object, we will end up with two symbols for my_func:
+# the original object, we will end up with two symbols for func1:
 # one local, one global.  After final compile, we will end up with
-# an undefined reference to my_func.
+# an undefined reference to func1 or a wrong reference to another global
+# func1 in other files.
 #
 # Since local objects can reference local variables, we need to find
 # a way to make tmp.o reference the local objects of the original object
-# file after it is linked together. To do this, we convert the my_func
+# file after it is linked together. To do this, we convert func1
 # into a global symbol before linking tmp.o. Then after we link tmp.o
-# we will only have a single symbol for my_func that is global.
-# We can convert my_func back into a local symbol and we are done.
+# we will only have a single symbol for func1 that is global.
+# We can convert func1 back into a local symbol and we are done.
 #
 # Here are the steps we take:
 #
@@ -86,10 +102,8 @@
 # 6) Link together this new object with the list object.
 # 7) Convert the local functions back to local symbols and rename
 #    the result as the original object.
-#    End.
 # 8) Link the object with the list object.
 # 9) Move the result back to the original object.
-#    End.
 #
 
 use strict;

[PATCH v3 2/8] tracing: recordmcount.pl Correct the check on the number of parameters

[Li Hong]

>From f39257ad204c26198beb85c9c1133f610f8b0117 Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 12:27:32 +0800
Subject: [PATCH] tracing: recordmcount.pl Correct the check on the number of parameters

Signed-off-by: Li Hong <lihong.hi@gmail.com>
---
 scripts/recordmcount.pl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 9e7ceca..9e0ec8e 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -113,7 +113,7 @@ $P =~ s@.*/@@g;
 
 my $V = '0.1';
 
-if ($#ARGV < 7) {
+if ($#ARGV != 10) {
 	print "usage: $P arch bits objdump objcopy cc ld nm rm mv is_module inputfile\n";
 	print "version: $V\n";
 	exit(1);
-- 
1.6.0.4

[PATCH v3 3/8] tracing: recordmcount.pl Support absolute path check on $inputfile

[Li Hong]

>From 6352a90805b18e4b567c3dcdd438e3828e478734 Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 12:32:18 +0800
Subject: [PATCH] tracing: recordmcount.pl Support absolute path check on $inputfile

Signed-off-by: Li Hong <lihong.hi@gmail.com>
---
 scripts/recordmcount.pl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 9e0ec8e..8ff9eab 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -123,7 +123,7 @@ my ($arch, $bits, $objdump, $objcopy, $cc,
     $ld, $nm, $rm, $mv, $is_module, $inputfile) = @ARGV;
 
 # This file refers to mcount and shouldn't be ftraced, so lets' ignore it
-if ($inputfile eq "kernel/trace/ftrace.o") {
+if ($inputfile =~ m,kernel/trace/ftrace\.o$,) {
     exit(0);
 }
 
-- 
1.6.0.4

[tip:tracing/core] tracing: Check absolute path of input file in recordmcount.pl

[tip-bot for Li Hong]

Commit-ID:  bdd3b052c63b2c19a0118937f500985c01a19956
Gitweb:     http://git.kernel.org/tip/bdd3b052c63b2c19a0118937f500985c01a19956
Author:     Li Hong <lihong.hi@gmail.com>
AuthorDate: Wed, 28 Oct 2009 13:03:32 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 29 Oct 2009 15:11:43 -0400

tracing: Check absolute path of input file in recordmcount.pl

The ftrace.c file may reference the mcount function and this may interfere
with the recordmcount.pl processing. To avoid this, the code does not
process the kernel/trace/ftrace.o. But currently the check is against
a relative path. This patch modifies the check to succeed if the path
is an absolute path.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
LKML-Reference: <20091028050332.GC30758@uhli>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.pl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index a512af1..b80e5d0 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -123,7 +123,7 @@ my ($arch, $bits, $objdump, $objcopy, $cc,
     $ld, $nm, $rm, $mv, $is_module, $inputfile) = @ARGV;
 
 # This file refers to mcount and shouldn't be ftraced, so lets' ignore it
-if ($inputfile eq "kernel/trace/ftrace.o") {
+if ($inputfile =~ m,kernel/trace/ftrace\.o$,) {
     exit(0);
 }
 

[PATCH v3 4/8] tracing: recordmcount.pl Objcopy check should disable local reference correctly

[Li Hong]

>From ceb80d8ba4fc55916c5f88d8c0a7655b869702cf Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 12:41:22 +0800
Subject: [PATCH] tracing: recordmcount.pl Objcopy check should disable local reference correctly

Use a function to do objcopy version check. Disable the local reference if
copy can't support it. Remove some unused variables.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
---
 scripts/recordmcount.pl |   55 ++++++++++++++++++++++------------------------
 1 files changed, 26 insertions(+), 29 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 8ff9eab..1de9826 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -158,6 +158,30 @@ my $function_regex;	# Find the name of a function
 my $mcount_regex;	# Find the call site to mcount (return offset)
 my $alignment;		# The .align value to use for $mcount_section
 my $section_type;	# Section header plus possible alignment command
+my $can_use_local = 0; 	# If we can use local function references
+
+##
+# check_objcopy - whether objcopy supports --globalize-symbols
+#
+#  --globalize-symbols came out in 2.17, we must test the version
+#  of objcopy, and if it is less than 2.17, then we can not
+#  record local functions.
+sub check_objcopy
+{
+    open (IN, "$objcopy --version |") or die "error running $objcopy";
+    while (<IN>) {
+        if (/objcopy.*\s(\d+)\.(\d+)/) {
+            $can_use_local = 1 if ($1 > 2 || ($1 == 2 && $2 >= 17)); 
+            last;
+        }
+    }
+    close (IN);
+
+    if (!$can_use_local) {
+        print STDERR "WARNING: could not find objcopy version or version is less than 2.17.\n" .
+	"\tLocal function references is disabled.\n"  
+    }
+}
 
 if ($arch eq "x86") {
     if ($bits == 64) {
@@ -292,34 +316,7 @@ if ($filename =~ m,^(.*)(\.\S),) {
 my $mcount_s = $dirname . "/.tmp_mc_" . $prefix . ".s";
 my $mcount_o = $dirname . "/.tmp_mc_" . $prefix . ".o";
 
-#
-# --globalize-symbols came out in 2.17, we must test the version
-# of objcopy, and if it is less than 2.17, then we can not
-# record local functions.
-my $use_locals = 01;
-my $local_warn_once = 0;
-my $found_version = 0;
-
-open (IN, "$objcopy --version |") || die "error running $objcopy";
-while (<IN>) {
-    if (/objcopy.*\s(\d+)\.(\d+)/) {
-	my $major = $1;
-	my $minor = $2;
-
-	$found_version = 1;
-	if ($major < 2 ||
-	    ($major == 2 && $minor < 17)) {
-	    $use_locals = 0;
-	}
-	last;
-    }
-}
-close (IN);
-
-if (!$found_version) {
-    print STDERR "WARNING: could not find objcopy version.\n" .
-	"\tDisabling local function references.\n";
-}
+check_objcopy();
 
 #
 # Step 1: find all the local (static functions) and weak symbols.
@@ -366,7 +363,7 @@ sub update_funcs
     if (defined $locals{$ref_func}) {
 
 	# only use locals if objcopy supports globalize-symbols
-	if (!$use_locals) {
+	if (!$can_use_local) {
 	    return;
 	}
 	$convert{$ref_func} = 1;
-- 
1.6.0.4

Re: [PATCH v3 4/8] tracing: recordmcount.pl Objcopy check should disable local reference correctly

[Steven Rostedt]

On Wed, 2009-10-28 at 13:04 +0800, Li Hong wrote:
> >From ceb80d8ba4fc55916c5f88d8c0a7655b869702cf Mon Sep 17 00:00:00 2001
> From: Li Hong <lihong.hi@gmail.com>
> Date: Tue, 27 Oct 2009 12:41:22 +0800
> Subject: [PATCH] tracing: recordmcount.pl Objcopy check should disable local reference correctly
> 
> Use a function to do objcopy version check. Disable the local reference if
> copy can't support it. Remove some unused variables.

Note, a lot of your patches have whitespace issues (white space at the
end of the line, etc). I've cleaned them up, but you want to be more
careful next time.

> 
> Signed-off-by: Li Hong <lihong.hi@gmail.com>
> ---
>  scripts/recordmcount.pl |   55 ++++++++++++++++++++++------------------------
>  1 files changed, 26 insertions(+), 29 deletions(-)
> 
> diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
> index 8ff9eab..1de9826 100755
> --- a/scripts/recordmcount.pl
> +++ b/scripts/recordmcount.pl
> @@ -158,6 +158,30 @@ my $function_regex;	# Find the name of a function
>  my $mcount_regex;	# Find the call site to mcount (return offset)
>  my $alignment;		# The .align value to use for $mcount_section
>  my $section_type;	# Section header plus possible alignment command
> +my $can_use_local = 0; 	# If we can use local function references
> +
> +##
> +# check_objcopy - whether objcopy supports --globalize-symbols
> +#
> +#  --globalize-symbols came out in 2.17, we must test the version
> +#  of objcopy, and if it is less than 2.17, then we can not
> +#  record local functions.
> +sub check_objcopy
> +{
> +    open (IN, "$objcopy --version |") or die "error running $objcopy";
> +    while (<IN>) {
> +        if (/objcopy.*\s(\d+)\.(\d+)/) {
> +            $can_use_local = 1 if ($1 > 2 || ($1 == 2 && $2 >= 17)); 
> +            last;
> +        }
> +    }
> +    close (IN);
> +
> +    if (!$can_use_local) {
> +        print STDERR "WARNING: could not find objcopy version or version is less than 2.17.\n" .
> +	"\tLocal function references is disabled.\n"  

Missing a ';' above. I've fixed this too, and broke the first line up
too, to be less than 80 characters.

Thanks,

-- Steve



> +    }
> +}
>  
>  if ($arch eq "x86") {
>      if ($bits == 64) {

[tip:tracing/core] tracing: Fix objcopy revision check in recordmcount.pl

[tip-bot for Li Hong]

Commit-ID:  7b7edc27683e20624f4daf17c76041719184201c
Gitweb:     http://git.kernel.org/tip/7b7edc27683e20624f4daf17c76041719184201c
Author:     Li Hong <lihong.hi@gmail.com>
AuthorDate: Wed, 28 Oct 2009 13:04:21 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 29 Oct 2009 15:11:44 -0400

tracing: Fix objcopy revision check in recordmcount.pl

The current logic to check objcopy's version is incorrect. This patch
fixes the algorithm and disables the use of local functions as a reference
if the objcopy version does not support static to global conversions.

Also remove some usused variables.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
LKML-Reference: <20091028050421.GD30758@uhli>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.pl |   56 ++++++++++++++++++++++------------------------
 1 files changed, 27 insertions(+), 29 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index b80e5d0..d6199fc 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -159,6 +159,31 @@ my $function_regex;	# Find the name of a function
 my $mcount_regex;	# Find the call site to mcount (return offset)
 my $alignment;		# The .align value to use for $mcount_section
 my $section_type;	# Section header plus possible alignment command
+my $can_use_local = 0; 	# If we can use local function references
+
+##
+# check_objcopy - whether objcopy supports --globalize-symbols
+#
+#  --globalize-symbols came out in 2.17, we must test the version
+#  of objcopy, and if it is less than 2.17, then we can not
+#  record local functions.
+sub check_objcopy
+{
+    open (IN, "$objcopy --version |") or die "error running $objcopy";
+    while (<IN>) {
+	if (/objcopy.*\s(\d+)\.(\d+)/) {
+	    $can_use_local = 1 if ($1 > 2 || ($1 == 2 && $2 >= 17));
+	    last;
+	}
+    }
+    close (IN);
+
+    if (!$can_use_local) {
+	print STDERR "WARNING: could not find objcopy version or version " .
+	    "is less than 2.17.\n" .
+	    "\tLocal function references is disabled.\n";
+    }
+}
 
 if ($arch eq "x86") {
     if ($bits == 64) {
@@ -293,34 +318,7 @@ if ($filename =~ m,^(.*)(\.\S),) {
 my $mcount_s = $dirname . "/.tmp_mc_" . $prefix . ".s";
 my $mcount_o = $dirname . "/.tmp_mc_" . $prefix . ".o";
 
-#
-# --globalize-symbols came out in 2.17, we must test the version
-# of objcopy, and if it is less than 2.17, then we can not
-# record local functions.
-my $use_locals = 01;
-my $local_warn_once = 0;
-my $found_version = 0;
-
-open (IN, "$objcopy --version |") || die "error running $objcopy";
-while (<IN>) {
-    if (/objcopy.*\s(\d+)\.(\d+)/) {
-	my $major = $1;
-	my $minor = $2;
-
-	$found_version = 1;
-	if ($major < 2 ||
-	    ($major == 2 && $minor < 17)) {
-	    $use_locals = 0;
-	}
-	last;
-    }
-}
-close (IN);
-
-if (!$found_version) {
-    print STDERR "WARNING: could not find objcopy version.\n" .
-	"\tDisabling local function references.\n";
-}
+check_objcopy();
 
 #
 # Step 1: find all the local (static functions) and weak symbols.
@@ -367,7 +365,7 @@ sub update_funcs
     if (defined $locals{$ref_func}) {
 
 	# only use locals if objcopy supports globalize-symbols
-	if (!$use_locals) {
+	if (!$can_use_local) {
 	    return;
 	}
 	$convert{$ref_func} = 1;

[PATCH v3 5/8] tracing: recordmcount.pl Clarify the logic on mcount section check

[Li Hong]

>From 7cff5ca3e0cdad6fcb4933ef290e5e2bf90e26fb Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 12:53:52 +0800
Subject: [PATCH] tracing: recordmcount.pl Clarify the logic on mcount section check

Move the mcount section check to the beginning of the objdump read loop.
It is clearer, because mcount section check uses headers dump part of objdump
to identify a mcount section, which goes before the section parts.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
---
 scripts/recordmcount.pl |   34 +++++++++++++++++++---------------
 1 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 1de9826..f4117d6 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -389,9 +389,27 @@ open(IN, "$objdump -hdr $inputfile|") || die "error running $objdump";
 
 my $text;
 
+
+# read headers first
 my $read_headers = 1;
 
 while (<IN>) {
+
+    if ($read_headers && /$mcount_section/) {
+	#
+	# Somehow the make process can execute this script on an
+	# object twice. If it does, we would duplicate the mcount
+	# section and it will cause the function tracer self test
+	# to fail. Check if the mcount section exists, and if it does,
+	# warn and exit.
+	#
+	print STDERR "ERROR: $mcount_section already in $inputfile\n" .
+	    "\tThis may be an indication that your build is corrupted.\n" .
+	    "\tDelete $inputfile and try again. If the same object file\n" .
+	    "\tstill causes an issue, then disable CONFIG_DYNAMIC_FTRACE.\n";
+	exit(-1);
+    }
+
     # is it a section?
     if (/$section_regex/) {
 	$read_headers = 0;
@@ -432,21 +450,7 @@ while (<IN>) {
 		$offset = hex $1;
 	    }
 	}
-    } elsif ($read_headers && /$mcount_section/) {
-	#
-	# Somehow the make process can execute this script on an
-	# object twice. If it does, we would duplicate the mcount
-	# section and it will cause the function tracer self test
-	# to fail. Check if the mcount section exists, and if it does,
-	# warn and exit.
-	#
-	print STDERR "ERROR: $mcount_section already in $inputfile\n" .
-	    "\tThis may be an indication that your build is corrupted.\n" .
-	    "\tDelete $inputfile and try again. If the same object file\n" .
-	    "\tstill causes an issue, then disable CONFIG_DYNAMIC_FTRACE.\n";
-	exit(-1);
-    }
-
+    } 
     # is this a call site to mcount? If so, record it to print later
     if ($text_found && /$mcount_regex/) {
 	$offsets[$#offsets + 1] = hex $1;
-- 
1.6.0.4

[tip:tracing/core] tracing: Move mcount section search to front of loop in recordmcount.pl

[tip-bot for Li Hong]

Commit-ID:  db24c7dcf42f78629d89b34e5d5a98ed56ea2ff5
Gitweb:     http://git.kernel.org/tip/db24c7dcf42f78629d89b34e5d5a98ed56ea2ff5
Author:     Li Hong <lihong.hi@gmail.com>
AuthorDate: Wed, 28 Oct 2009 13:05:23 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 29 Oct 2009 15:11:48 -0400

tracing: Move mcount section search to front of loop in recordmcount.pl

Move the mcount section check to the beginning of the objdump read loop.
This makes the code easier to follow since the search for the mcount
section is performed first before the mcount callers are processed.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
LKML-Reference: <20091028050523.GE30758@uhli>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.pl |   32 ++++++++++++++++++--------------
 1 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index d6199fc..02c8055 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -391,9 +391,27 @@ open(IN, "$objdump -hdr $inputfile|") || die "error running $objdump";
 
 my $text;
 
+
+# read headers first
 my $read_headers = 1;
 
 while (<IN>) {
+
+    if ($read_headers && /$mcount_section/) {
+	#
+	# Somehow the make process can execute this script on an
+	# object twice. If it does, we would duplicate the mcount
+	# section and it will cause the function tracer self test
+	# to fail. Check if the mcount section exists, and if it does,
+	# warn and exit.
+	#
+	print STDERR "ERROR: $mcount_section already in $inputfile\n" .
+	    "\tThis may be an indication that your build is corrupted.\n" .
+	    "\tDelete $inputfile and try again. If the same object file\n" .
+	    "\tstill causes an issue, then disable CONFIG_DYNAMIC_FTRACE.\n";
+	exit(-1);
+    }
+
     # is it a section?
     if (/$section_regex/) {
 	$read_headers = 0;
@@ -434,21 +452,7 @@ while (<IN>) {
 		$offset = hex $1;
 	    }
 	}
-    } elsif ($read_headers && /$mcount_section/) {
-	#
-	# Somehow the make process can execute this script on an
-	# object twice. If it does, we would duplicate the mcount
-	# section and it will cause the function tracer self test
-	# to fail. Check if the mcount section exists, and if it does,
-	# warn and exit.
-	#
-	print STDERR "ERROR: $mcount_section already in $inputfile\n" .
-	    "\tThis may be an indication that your build is corrupted.\n" .
-	    "\tDelete $inputfile and try again. If the same object file\n" .
-	    "\tstill causes an issue, then disable CONFIG_DYNAMIC_FTRACE.\n";
-	exit(-1);
     }
-
     # is this a call site to mcount? If so, record it to print later
     if ($text_found && /$mcount_regex/) {
 	$offsets[$#offsets + 1] = hex $1;

[PATCH v3 6/8] tracing: recordmcount.pl Use more friendly variables to clean up the code

[Li Hong]

>From 71a781c99a89573b57011cb8999b98db8238b3ed Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 13:13:37 +0800
Subject: [PATCH] tracing: recordmcount.pl Use more friendly variables to clean up the code

Signed-off-by: Li Hong <lihong.hi@gmail.com>
---
 scripts/recordmcount.pl |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index f4117d6..0f1107d 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -92,7 +92,7 @@
 #
 # Here are the steps we take:
 #
-# 1) Record all the local symbols by using 'nm'
+# 1) Record all the local and weak symbols by using 'nm'
 # 2) Use objdump to find all the call site offsets and sections for
 #    mcount.
 # 3) Compile the list into its own object.
@@ -151,7 +151,8 @@ my %weak;		# List of weak functions
 my %convert;		# List of local functions used that needs conversion
 
 my $type;
-my $nm_regex;		# Find the local functions (return function)
+my $local_regex;	# Match a local function (return function)
+my $weak_regex; 	# Match a weak function (return function)
 my $section_regex;	# Find the start of a section
 my $function_regex;	# Find the name of a function
 			#    (return offset and func name)
@@ -195,7 +196,8 @@ if ($arch eq "x86") {
 # We base the defaults off of i386, the other archs may
 # feel free to change them in the below if statements.
 #
-$nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
+$local_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
+$weak_regex = "^[0-9a-fA-F]+\\s+([wW])\\s+(\\S+)";
 $section_regex = "Disassembly of section\\s+(\\S+):";
 $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
 $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
@@ -244,7 +246,7 @@ if ($arch eq "x86_64") {
     $cc .= " -m32";
 
 } elsif ($arch eq "powerpc") {
-    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
+    $local_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(\\.?.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s\\.?_mcount\$";
 
@@ -320,13 +322,13 @@ check_objcopy();
 
 #
 # Step 1: find all the local (static functions) and weak symbols.
-#        't' is local, 'w/W' is weak (we never use a weak function)
+#         't' is local, 'w/W' is weak
 #
 open (IN, "$nm $inputfile|") || die "error running $nm";
 while (<IN>) {
-    if (/$nm_regex/) {
+    if (/$local_regex/) {
 	$locals{$1} = 1;
-    } elsif (/^[0-9a-fA-F]+\s+([wW])\s+(\S+)/) {
+    } elsif (/$weak_regex/) {
 	$weak{$2} = $1;
     }
 }
-- 
1.6.0.4

[tip:tracing/core] tracing: Add regex for weak functions in recordmcount.pl

[tip-bot for Li Hong]

Commit-ID:  306dcf47d28aaf9aedfafb17a602768584cfc0f2
Gitweb:     http://git.kernel.org/tip/306dcf47d28aaf9aedfafb17a602768584cfc0f2
Author:     Li Hong <lihong.hi@gmail.com>
AuthorDate: Wed, 28 Oct 2009 13:06:19 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 29 Oct 2009 15:11:52 -0400

tracing: Add regex for weak functions in recordmcount.pl

Add a variable to contain the regex needed to find weak functions
in the 'nm' output. This will allow other archs to easily override it.

Also rename the regex variable $nm_regex to $local_regex to be more
descriptive.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
LKML-Reference: <20091028050619.GF30758@uhli>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.pl |   16 +++++++++-------
 1 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 02c8055..7265a7d 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -92,7 +92,7 @@
 #
 # Here are the steps we take:
 #
-# 1) Record all the local symbols by using 'nm'
+# 1) Record all the local and weak symbols by using 'nm'
 # 2) Use objdump to find all the call site offsets and sections for
 #    mcount.
 # 3) Compile the list into its own object.
@@ -152,7 +152,8 @@ my %weak;		# List of weak functions
 my %convert;		# List of local functions used that needs conversion
 
 my $type;
-my $nm_regex;		# Find the local functions (return function)
+my $local_regex;	# Match a local function (return function)
+my $weak_regex; 	# Match a weak function (return function)
 my $section_regex;	# Find the start of a section
 my $function_regex;	# Find the name of a function
 			#    (return offset and func name)
@@ -197,7 +198,8 @@ if ($arch eq "x86") {
 # We base the defaults off of i386, the other archs may
 # feel free to change them in the below if statements.
 #
-$nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
+$local_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\S+)";
+$weak_regex = "^[0-9a-fA-F]+\\s+([wW])\\s+(\\S+)";
 $section_regex = "Disassembly of section\\s+(\\S+):";
 $function_regex = "^([0-9a-fA-F]+)\\s+<(.*?)>:";
 $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\smcount\$";
@@ -246,7 +248,7 @@ if ($arch eq "x86_64") {
     $cc .= " -m32";
 
 } elsif ($arch eq "powerpc") {
-    $nm_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
+    $local_regex = "^[0-9a-fA-F]+\\s+t\\s+(\\.?\\S+)";
     $function_regex = "^([0-9a-fA-F]+)\\s+<(\\.?.*?)>:";
     $mcount_regex = "^\\s*([0-9a-fA-F]+):.*\\s\\.?_mcount\$";
 
@@ -322,13 +324,13 @@ check_objcopy();
 
 #
 # Step 1: find all the local (static functions) and weak symbols.
-#        't' is local, 'w/W' is weak (we never use a weak function)
+#         't' is local, 'w/W' is weak
 #
 open (IN, "$nm $inputfile|") || die "error running $nm";
 while (<IN>) {
-    if (/$nm_regex/) {
+    if (/$local_regex/) {
 	$locals{$1} = 1;
-    } elsif (/^[0-9a-fA-F]+\s+([wW])\s+(\S+)/) {
+    } elsif (/$weak_regex/) {
 	$weak{$2} = $1;
     }
 }

[PATCH v3 7/8] tracing: recordmcount.pl Combine the condition validation in update_funcs

[Li Hong]

>From c40f78d26c171732e6b3546d1f2141274a48c42b Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 13:20:48 +0800
Subject: [PATCH] tracing: recordmcount.pl Combine the condition validation in update_funcs

Move all the condition validation into function update_funcs. Also
update_funcs shouldn't die if $ref_func is undefined for there may be
more than one valid sections in an object file.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
---
 scripts/recordmcount.pl |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 0f1107d..ffb1123 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -346,9 +346,7 @@ my $offset = 0;		# offset of ref_func to section beginning
 #
 sub update_funcs
 {
-    return if ($#offsets < 0);
-
-    defined($ref_func) || die "No function to reference";
+    return unless ($ref_func and @offsets);
 
     # A section only had a weak function, to represent it.
     # Unfortunately, a weak function may be overwritten by another
@@ -423,7 +421,7 @@ while (<IN>) {
 	    $read_function = 0;
 	}
 	# print out any recorded offsets
-	update_funcs() if (defined($ref_func));
+	update_funcs();
 
 	# reset all markers and arrays
 	$text_found = 0;
@@ -460,7 +458,7 @@ while (<IN>) {
 }
 
 # dump out anymore offsets that may have been found
-update_funcs() if (defined($ref_func));
+update_funcs();
 
 # If we did not find any mcount callers, we are done (do nothing).
 if (!$opened) {
-- 
1.6.0.4

[tip:tracing/core] tracing: Move conditional into update_funcs() in recordmcount.pl

[tip-bot for Li Hong]

Commit-ID:  6092858c60f168c1950f8ad73880d54271696ec5
Gitweb:     http://git.kernel.org/tip/6092858c60f168c1950f8ad73880d54271696ec5
Author:     Li Hong <lihong.hi@gmail.com>
AuthorDate: Wed, 28 Oct 2009 13:07:03 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 29 Oct 2009 15:11:54 -0400

tracing: Move conditional into update_funcs() in recordmcount.pl

Move all the condition validations into the function update_funcs().
Also update_funcs should not die if $ref_func is undefined for there may be
more than one valid section in an object file.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
LKML-Reference: <20091028050703.GG30758@uhli>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.pl |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 7265a7d..884776a 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -348,9 +348,7 @@ my $offset = 0;		# offset of ref_func to section beginning
 #
 sub update_funcs
 {
-    return if ($#offsets < 0);
-
-    defined($ref_func) || die "No function to reference";
+    return unless ($ref_func and @offsets);
 
     # A section only had a weak function, to represent it.
     # Unfortunately, a weak function may be overwritten by another
@@ -425,7 +423,7 @@ while (<IN>) {
 	    $read_function = 0;
 	}
 	# print out any recorded offsets
-	update_funcs() if (defined($ref_func));
+	update_funcs();
 
 	# reset all markers and arrays
 	$text_found = 0;
@@ -462,7 +460,7 @@ while (<IN>) {
 }
 
 # dump out anymore offsets that may have been found
-update_funcs() if (defined($ref_func));
+update_funcs();
 
 # If we did not find any mcount callers, we are done (do nothing).
 if (!$opened) {

[PATCH v3 8/8] tracing: recordmcount.pl Die if we use a weak function as reference

[Li Hong]

>From fef57ba56001830adaeea6aef479c623e3bce6c6 Mon Sep 17 00:00:00 2001
From: Li Hong <lihong.hi@gmail.com>
Date: Tue, 27 Oct 2009 13:25:39 +0800
Subject: [PATCH] tracing: recordmcount.pl Die if we use a weak function as reference

Signed-off-by: Li Hong <lihong.hi@gmail.com>
---
 scripts/recordmcount.pl |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index ffb1123..0f733c9 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -348,15 +348,11 @@ sub update_funcs
 {
     return unless ($ref_func and @offsets);
 
-    # A section only had a weak function, to represent it.
-    # Unfortunately, a weak function may be overwritten by another
-    # function of the same name, making all these offsets incorrect.
-    # To be safe, we simply print a warning and bail.
+    # Sanity check on weak function. A weak function may be overwritten by 
+    # another function of the same name, making all these offsets incorrect.
     if (defined $weak{$ref_func}) {
-	print STDERR
-	    "$inputfile: WARNING: referencing weak function" .
-	    " $ref_func for mcount\n";
-	return;
+        die "$inputfile: ERROR: referencing weak function" .
+            " $ref_func for mcount\n";
     }
 
     # is this function static? If so, note this fact.
-- 
1.6.0.4

[tip:tracing/core] tracing: Exit with error if a weak function is used in recordmcount.pl

[tip-bot for Li Hong]

Commit-ID:  c4b8ac2c1aee1398b9378b8730bac56294b3410b
Gitweb:     http://git.kernel.org/tip/c4b8ac2c1aee1398b9378b8730bac56294b3410b
Author:     Li Hong <lihong.hi@gmail.com>
AuthorDate: Wed, 28 Oct 2009 13:07:43 +0800
Committer:  Steven Rostedt <rostedt@goodmis.org>
CommitDate: Thu, 29 Oct 2009 15:11:57 -0400

tracing: Exit with error if a weak function is used in recordmcount.pl

If a weak function is used as a relocation reference for mcount callers
and that function is overridden, it will cause ftrace to fail at run time.

The current code should prevent a weak function from being used, but if
one is, the code should exit with an error to fail at compile time.

Signed-off-by: Li Hong <lihong.hi@gmail.com>
LKML-Reference: <20091028050743.GH30758@uhli>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
---
 scripts/recordmcount.pl |   10 +++-------
 1 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/scripts/recordmcount.pl b/scripts/recordmcount.pl
index 884776a..a4e2435 100755
--- a/scripts/recordmcount.pl
+++ b/scripts/recordmcount.pl
@@ -350,15 +350,11 @@ sub update_funcs
 {
     return unless ($ref_func and @offsets);
 
-    # A section only had a weak function, to represent it.
-    # Unfortunately, a weak function may be overwritten by another
-    # function of the same name, making all these offsets incorrect.
-    # To be safe, we simply print a warning and bail.
+    # Sanity check on weak function. A weak function may be overwritten by
+    # another function of the same name, making all these offsets incorrect.
     if (defined $weak{$ref_func}) {
-	print STDERR
-	    "$inputfile: WARNING: referencing weak function" .
+	die "$inputfile: ERROR: referencing weak function" .
 	    " $ref_func for mcount\n";
-	return;
     }
 
     # is this function static? If so, note this fact.

Re: [PATCH v3 0/8] tracing: recordmcount.pl Bug fixes and code improvement

[Steven Rostedt]

On Wed, 2009-10-28 at 12:57 +0800, Li Hong wrote:
> Hi Steven,
> 
> This a following version for last patch series on recordmcount.pl. I have made
> several changes according to your suggestions. Hopefully, they are ready to be
> applied into your tree.
> 
> Changes include:
> 
>   - Improved the documentation amendment patch.
>   - Keep the C like notation for error paths.
>   - Drop the 'exit early' patch. Yes, it's not much useful, and actually, as
>   you told me, there was a bug in it :)
>   - minor typos and cleanups.
> 
> Thanks very much for your elaborate review and feedback.

Thanks Li!

I'll pull them in, do some basic tests and send them off to Ingo.

I appreciate your work,

-- Steve