currnetly script prints stack usage for functions
in two ways:($re and $dre)
dre breaks sorting mechanism.
0xffffa00011f26f88 sunxi_mux_clk_setup.isra.0 [vmlinux]:Dynamic (0x140)
..
0xffffa00011f27210 sunxi_divs_clk_setup [vmlinux]: Dynamic (0x1d0)
so we can print it in decimal only.
Also address before function name is changed to function
start address rather than stack consumption address.
Because in next patch, arm has two ways to use stack
which can be clubbed and printed in one function only.
All symbols whose stack by adding(re and dre) is greater than
100, will be printed.
0xffffa00011f2720c0 sunxi_divs_clk_setup [vmlinux]: 464
...
0xffffa00011f26f840 sunxi_mux_clk_setup.isra.0 [vmlinux]:320
Signed-off-by: Vaneet Narang <[email protected]>
Signed-off-by: Maninder Singh <[email protected]>
---
scripts/checkstack.pl | 52 +++++++++++++++++++++++++--------------------------
1 file changed, 25 insertions(+), 27 deletions(-)
diff --git a/scripts/checkstack.pl b/scripts/checkstack.pl
index 371bd17..412c459 100755
--- a/scripts/checkstack.pl
+++ b/scripts/checkstack.pl
@@ -109,11 +109,28 @@ my (@stack, $re, $dre, $x, $xs, $funcre);
#
# main()
#
-my ($func, $file, $lastslash);
+my ($func, $file, $lastslash, $total_size, $addr, $intro);
while (my $line = <STDIN>) {
if ($line =~ m/$funcre/) {
+ if ($total_size > 100) {
+ push @stack, "$intro$total_size\n";
+ }
+
$func = $1;
+ next if $line !~ m/^($xs*)/;
+ $addr = $1;
+ $addr =~ s/ /0/g;
+ $addr = "0x$addr";
+
+ $intro = "$addr $func [$file]:";
+ my $padlen = 56 - length($intro);
+ while ($padlen > 0) {
+ $intro .= ' ';
+ $padlen -= 8;
+ }
+
+ $total_size = 0;
}
elsif ($line =~ m/(.*):\s*file format/) {
$file = $1;
@@ -134,37 +151,18 @@ while (my $line = <STDIN>) {
}
next if ($size > 0x10000000);
- next if $line !~ m/^($xs*)/;
- my $addr = $1;
- $addr =~ s/ /0/g;
- $addr = "0x$addr";
-
- my $intro = "$addr $func [$file]:";
- my $padlen = 56 - length($intro);
- while ($padlen > 0) {
- $intro .= ' ';
- $padlen -= 8;
- }
- next if ($size < 100);
- push @stack, "$intro$size\n";
+ $total_size = $total_size + $size
}
elsif (defined $dre && $line =~ m/$dre/) {
- my $size = "Dynamic ($1)";
-
- next if $line !~ m/^($xs*)/;
- my $addr = $1;
- $addr =~ s/ /0/g;
- $addr = "0x$addr";
+ my $size = $1;
- my $intro = "$addr $func [$file]:";
- my $padlen = 56 - length($intro);
- while ($padlen > 0) {
- $intro .= ' ';
- $padlen -= 8;
- }
- push @stack, "$intro$size\n";
+ $size = hex($size) if ($size =~ /^0x/);
+ $total_size = $total_size + $size
}
}
+if ($total_size > 100) {
+ push @stack, "$intro$total_size\n";
+}
# Sort output by size (last field)
print sort { ($b =~ /:\t*(\d+)$/)[0] <=> ($a =~ /:\t*(\d+)$/)[0] } @stack;
--
1.9.1
if objdump has below entries;
c01ed608 <X>:
c01ed614: e24ddff7 sub sp, sp, #120 ; 0x78
c01f0d50 <Y>:
c01f0d50: e24dd094 sub sp, sp, #140 ; 0x8c
scripts fails to read stack usage.
so making regex $re for ARM similar to aarch64
Signed-off-by: Vaneet Narang <[email protected]>
Signed-off-by: Maninder Singh <[email protected]>
---
scripts/checkstack.pl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/checkstack.pl b/scripts/checkstack.pl
index b292ef4..e80de70 100755
--- a/scripts/checkstack.pl
+++ b/scripts/checkstack.pl
@@ -60,7 +60,7 @@ my (@stack, $re, $dre, $sub, $x, $xs, $funcre, $min_stack);
$dre = qr/^.*sub.*sp, sp, #(0x$x{1,8})/o;
} elsif ($arch eq 'arm') {
#c0008ffc: e24dd064 sub sp, sp, #100 ; 0x64
- $re = qr/.*sub.*sp, sp, #(([0-9]{2}|[3-9])[0-9]{2})/o;
+ $re = qr/.*sub.*sp, sp, #([0-9]{1,4})/o;
$sub = \&arm_push_handling;
} elsif ($arch =~ /^x86(_64)?$/ || $arch =~ /^i[3456]86$/) {
#c0105234: 81 ec ac 05 00 00 sub $0x5ac,%esp
--
1.9.1
Add arguments support to print stacks which are greater than
argument value only.
Signed-off-by: Vaneet Narang <[email protected]>
Signed-off-by: Maninder Singh <[email protected]>
---
scripts/checkstack.pl | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/scripts/checkstack.pl b/scripts/checkstack.pl
index 412c459..8e5ef98 100755
--- a/scripts/checkstack.pl
+++ b/scripts/checkstack.pl
@@ -35,7 +35,7 @@ use strict;
# $1 (first bracket) matches the dynamic amount of the stack growth
#
# use anything else and feel the pain ;)
-my (@stack, $re, $dre, $x, $xs, $funcre);
+my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
{
my $arch = shift;
if ($arch eq "") {
@@ -43,6 +43,11 @@ my (@stack, $re, $dre, $x, $xs, $funcre);
chomp($arch);
}
+ $min_stack = shift;
+ if ($min_stack eq "" || $min_stack !~ /^\d+$/) {
+ $min_stack = 100;
+ }
+
$x = "[0-9a-f]"; # hex character
$xs = "[0-9a-f ]"; # hex character or space
$funcre = qr/^$x* <(.*)>:$/;
@@ -113,7 +118,7 @@ my ($func, $file, $lastslash, $total_size, $addr, $intro);
while (my $line = <STDIN>) {
if ($line =~ m/$funcre/) {
- if ($total_size > 100) {
+ if ($total_size > $min_stack) {
push @stack, "$intro$total_size\n";
}
@@ -150,7 +155,6 @@ while (my $line = <STDIN>) {
$size += 0x80000000;
}
next if ($size > 0x10000000);
-
$total_size = $total_size + $size
}
elsif (defined $dre && $line =~ m/$dre/) {
@@ -160,7 +164,7 @@ while (my $line = <STDIN>) {
$total_size = $total_size + $size
}
}
-if ($total_size > 100) {
+if ($total_size > $min_stack) {
push @stack, "$intro$total_size\n";
}
--
1.9.1
To count stack usage of push {*, fp, ip, lr, pc} instruction in ARM,
if FRAME POINTER is enabled.
e.g. c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
c01f0d50 <Y>:
c01f0d44: e1a0c00d mov ip, sp
c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
c01f0d4c: e24cb004 sub fp, ip, #4
c01f0d50: e24dd094 sub sp, sp, #448 ; 0x1C0
$ cat dump | scripts/checkstack.pl arm
0xc01f0d50 Y []: 448
added subroutine frame work for this.
After change:
0xc01f0d500 Y []: 492
Signed-off-by: Vaneet Narang <[email protected]>
Signed-off-by: Maninder Singh <[email protected]>
---
scripts/checkstack.pl | 27 ++++++++++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/scripts/checkstack.pl b/scripts/checkstack.pl
index 8e5ef98..b292ef4 100755
--- a/scripts/checkstack.pl
+++ b/scripts/checkstack.pl
@@ -34,8 +34,10 @@ use strict;
# $& (whole re) matches the complete objdump line with the stack growth
# $1 (first bracket) matches the dynamic amount of the stack growth
#
+# $sub: subroutine for special handling to check stack usage.
+#
# use anything else and feel the pain ;)
-my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
+my (@stack, $re, $dre, $sub, $x, $xs, $funcre, $min_stack);
{
my $arch = shift;
if ($arch eq "") {
@@ -59,6 +61,7 @@ my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
} elsif ($arch eq 'arm') {
#c0008ffc: e24dd064 sub sp, sp, #100 ; 0x64
$re = qr/.*sub.*sp, sp, #(([0-9]{2}|[3-9])[0-9]{2})/o;
+ $sub = \&arm_push_handling;
} elsif ($arch =~ /^x86(_64)?$/ || $arch =~ /^i[3456]86$/) {
#c0105234: 81 ec ac 05 00 00 sub $0x5ac,%esp
# or
@@ -112,6 +115,24 @@ my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
}
#
+# To count stack usage of push {*, fp, ip, lr, pc} instruction in ARM,
+# if FRAME POINTER is enabled.
+# e.g. c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
+#
+sub arm_push_handling {
+ my $regex = qr/.*push.*fp, ip, lr, pc}/o;
+ my $size = 0;
+ my $line_arg = shift;
+
+ if ($line_arg =~ m/$regex/) {
+ $size = $line_arg =~ tr/,//;
+ $size = ($size + 1) * 4;
+ }
+
+ return $size;
+}
+
+#
# main()
#
my ($func, $file, $lastslash, $total_size, $addr, $intro);
@@ -163,6 +184,10 @@ while (my $line = <STDIN>) {
$size = hex($size) if ($size =~ /^0x/);
$total_size = $total_size + $size
}
+ elsif (defined $sub) {
+ my $size = &$sub($line);
+ $total_size = $total_size + $size;
+ }
}
if ($total_size > $min_stack) {
push @stack, "$intro$total_size\n";
--
1.9.1
On Thu, Apr 30, 2020 at 9:50 PM Maninder Singh <[email protected]> wrote:
>
> currnetly script prints stack usage for functions
> in two ways:($re and $dre)
>
> dre breaks sorting mechanism.
> 0xffffa00011f26f88 sunxi_mux_clk_setup.isra.0 [vmlinux]:Dynamic (0x140)
> ..
> 0xffffa00011f27210 sunxi_divs_clk_setup [vmlinux]: Dynamic (0x1d0)
>
> so we can print it in decimal only.
>
> Also address before function name is changed to function
> start address rather than stack consumption address.
> Because in next patch, arm has two ways to use stack
> which can be clubbed and printed in one function only.
>
> All symbols whose stack by adding(re and dre) is greater than
> 100, will be printed.
>
> 0xffffa00011f2720c0 sunxi_divs_clk_setup [vmlinux]: 464
> ...
> 0xffffa00011f26f840 sunxi_mux_clk_setup.isra.0 [vmlinux]:320
>
> Signed-off-by: Vaneet Narang <[email protected]>
> Signed-off-by: Maninder Singh <[email protected]>
> ---
> scripts/checkstack.pl | 52 +++++++++++++++++++++++++--------------------------
> 1 file changed, 25 insertions(+), 27 deletions(-)
>
> diff --git a/scripts/checkstack.pl b/scripts/checkstack.pl
> index 371bd17..412c459 100755
> --- a/scripts/checkstack.pl
> +++ b/scripts/checkstack.pl
> @@ -109,11 +109,28 @@ my (@stack, $re, $dre, $x, $xs, $funcre);
> #
> # main()
> #
> -my ($func, $file, $lastslash);
> +my ($func, $file, $lastslash, $total_size, $addr, $intro);
$total_size is undefined for the first function.
I think 0 is implied, but is it clearer to initialize it here?
$total_size = 0;
> while (my $line = <STDIN>) {
> if ($line =~ m/$funcre/) {
> + if ($total_size > 100) {
> + push @stack, "$intro$total_size\n";
> + }
> +
> $func = $1;
> + next if $line !~ m/^($xs*)/;
Hmm, I think this 'next' is unlikely to happen.
But, it happened, the same line would be pushed twice.
Maybe, is it better to move 'next it' above
the 'if ($total_size > 100)' check?
> + $addr = $1;
> + $addr =~ s/ /0/g;
> + $addr = "0x$addr";
> +
> + $intro = "$addr $func [$file]:";
> + my $padlen = 56 - length($intro);
> + while ($padlen > 0) {
> + $intro .= ' ';
> + $padlen -= 8;
> + }
> +
> + $total_size = 0;
> }
> elsif ($line =~ m/(.*):\s*file format/) {
> $file = $1;
> @@ -134,37 +151,18 @@ while (my $line = <STDIN>) {
> }
> next if ($size > 0x10000000);
>
> - next if $line !~ m/^($xs*)/;
> - my $addr = $1;
> - $addr =~ s/ /0/g;
> - $addr = "0x$addr";
> -
> - my $intro = "$addr $func [$file]:";
> - my $padlen = 56 - length($intro);
> - while ($padlen > 0) {
> - $intro .= ' ';
> - $padlen -= 8;
> - }
> - next if ($size < 100);
> - push @stack, "$intro$size\n";
> + $total_size = $total_size + $size
For consistency, I personally prefer adding ';'
to every statement even for the last one in the block...
Is this simpler ?
$total_size += $size;
> }
> elsif (defined $dre && $line =~ m/$dre/) {
> - my $size = "Dynamic ($1)";
> -
> - next if $line !~ m/^($xs*)/;
> - my $addr = $1;
> - $addr =~ s/ /0/g;
> - $addr = "0x$addr";
> + my $size = $1;
>
> - my $intro = "$addr $func [$file]:";
> - my $padlen = 56 - length($intro);
> - while ($padlen > 0) {
> - $intro .= ' ';
> - $padlen -= 8;
> - }
> - push @stack, "$intro$size\n";
> + $size = hex($size) if ($size =~ /^0x/);
> + $total_size = $total_size + $size
Ditto. How about this?
$total_size += $size;
> }
> }
> +if ($total_size > 100) {
> + push @stack, "$intro$total_size\n";
> +}
>
> # Sort output by size (last field)
> print sort { ($b =~ /:\t*(\d+)$/)[0] <=> ($a =~ /:\t*(\d+)$/)[0] } @stack;
> --
> 1.9.1
>
--
Best Regards
Masahiro Yamada
On Thu, Apr 30, 2020 at 9:50 PM Maninder Singh <[email protected]> wrote:
>
> Add arguments support to print stacks which are greater than
> argument value only.
>
> Signed-off-by: Vaneet Narang <[email protected]>
> Signed-off-by: Maninder Singh <[email protected]>
> ---
> scripts/checkstack.pl | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/checkstack.pl b/scripts/checkstack.pl
> index 412c459..8e5ef98 100755
> --- a/scripts/checkstack.pl
> +++ b/scripts/checkstack.pl
> @@ -35,7 +35,7 @@ use strict;
> # $1 (first bracket) matches the dynamic amount of the stack growth
> #
> # use anything else and feel the pain ;)
> -my (@stack, $re, $dre, $x, $xs, $funcre);
> +my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
> {
> my $arch = shift;
> if ($arch eq "") {
> @@ -43,6 +43,11 @@ my (@stack, $re, $dre, $x, $xs, $funcre);
> chomp($arch);
> }
>
> + $min_stack = shift;
> + if ($min_stack eq "" || $min_stack !~ /^\d+$/) {
> + $min_stack = 100;
> + }
> +
> $x = "[0-9a-f]"; # hex character
> $xs = "[0-9a-f ]"; # hex character or space
> $funcre = qr/^$x* <(.*)>:$/;
> @@ -113,7 +118,7 @@ my ($func, $file, $lastslash, $total_size, $addr, $intro);
>
> while (my $line = <STDIN>) {
> if ($line =~ m/$funcre/) {
> - if ($total_size > 100) {
> + if ($total_size > $min_stack) {
> push @stack, "$intro$total_size\n";
> }
>
> @@ -150,7 +155,6 @@ while (my $line = <STDIN>) {
> $size += 0x80000000;
> }
> next if ($size > 0x10000000);
> -
This is a noise change.
You can do this in 1/4 if you want to.
> $total_size = $total_size + $size
> }
> elsif (defined $dre && $line =~ m/$dre/) {
> @@ -160,7 +164,7 @@ while (my $line = <STDIN>) {
> $total_size = $total_size + $size
> }
> }
> -if ($total_size > 100) {
> +if ($total_size > $min_stack) {
> push @stack, "$intro$total_size\n";
> }
>
> --
> 1.9.1
>
--
Best Regards
Masahiro Yamada
(+CC Andi Kleen, and more mentor)
On Thu, Apr 30, 2020 at 9:50 PM Maninder Singh <[email protected]> wrote:
>
> To count stack usage of push {*, fp, ip, lr, pc} instruction in ARM,
> if FRAME POINTER is enabled.
> e.g. c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
>
> c01f0d50 <Y>:
> c01f0d44: e1a0c00d mov ip, sp
> c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
> c01f0d4c: e24cb004 sub fp, ip, #4
> c01f0d50: e24dd094 sub sp, sp, #448 ; 0x1C0
>
> $ cat dump | scripts/checkstack.pl arm
> 0xc01f0d50 Y []: 448
>
> added subroutine frame work for this.
> After change:
> 0xc01f0d500 Y []: 492
>
> Signed-off-by: Vaneet Narang <[email protected]>
> Signed-off-by: Maninder Singh <[email protected]>
I do not mean to block this patch, but
some people still invest efforts to improve this script.
So, please let me ask this question.
Do you know CONFIG_FRAME_WARN?
Commit 35bb5b1e0e84cfa1a8906f7e6a77f391ff315791
mentioned -Wframe-larger-than should obsolete
make checkstack.
Now, -Wframe-larger-than is supported by
both minimal version of GCC and Clang.
I know checkstack.pl dumps the stack size
of functions, which is different from what
-Wframe-larger-than does, but the goal is
quite similar, I think.
I just wondered if we need both.
> ---
> scripts/checkstack.pl | 27 ++++++++++++++++++++++++++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/checkstack.pl b/scripts/checkstack.pl
> index 8e5ef98..b292ef4 100755
> --- a/scripts/checkstack.pl
> +++ b/scripts/checkstack.pl
> @@ -34,8 +34,10 @@ use strict;
> # $& (whole re) matches the complete objdump line with the stack growth
> # $1 (first bracket) matches the dynamic amount of the stack growth
> #
> +# $sub: subroutine for special handling to check stack usage.
> +#
> # use anything else and feel the pain ;)
> -my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
> +my (@stack, $re, $dre, $sub, $x, $xs, $funcre, $min_stack);
> {
> my $arch = shift;
> if ($arch eq "") {
> @@ -59,6 +61,7 @@ my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
> } elsif ($arch eq 'arm') {
> #c0008ffc: e24dd064 sub sp, sp, #100 ; 0x64
> $re = qr/.*sub.*sp, sp, #(([0-9]{2}|[3-9])[0-9]{2})/o;
> + $sub = \&arm_push_handling;
> } elsif ($arch =~ /^x86(_64)?$/ || $arch =~ /^i[3456]86$/) {
> #c0105234: 81 ec ac 05 00 00 sub $0x5ac,%esp
> # or
> @@ -112,6 +115,24 @@ my (@stack, $re, $dre, $x, $xs, $funcre, $min_stack);
> }
>
> #
> +# To count stack usage of push {*, fp, ip, lr, pc} instruction in ARM,
> +# if FRAME POINTER is enabled.
> +# e.g. c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
> +#
> +sub arm_push_handling {
> + my $regex = qr/.*push.*fp, ip, lr, pc}/o;
> + my $size = 0;
> + my $line_arg = shift;
> +
> + if ($line_arg =~ m/$regex/) {
> + $size = $line_arg =~ tr/,//;
> + $size = ($size + 1) * 4;
> + }
> +
> + return $size;
> +}
> +
> +#
> # main()
> #
> my ($func, $file, $lastslash, $total_size, $addr, $intro);
> @@ -163,6 +184,10 @@ while (my $line = <STDIN>) {
> $size = hex($size) if ($size =~ /^0x/);
> $total_size = $total_size + $size
> }
> + elsif (defined $sub) {
> + my $size = &$sub($line);
> + $total_size = $total_size + $size;
$total_size += $size;
Or
$total_size += &$sub($line);
then, delete $size.
> + }
> }
> if ($total_size > $min_stack) {
> push @stack, "$intro$total_size\n";
> --
> 1.9.1
>
--
Best Regards
Masahiro Yamada
On Thu, Apr 30, 2020 at 9:50 PM Maninder Singh <[email protected]> wrote:
>
> if objdump has below entries;
> c01ed608 <X>:
> c01ed614: e24ddff7 sub sp, sp, #120 ; 0x78
>
> c01f0d50 <Y>:
> c01f0d50: e24dd094 sub sp, sp, #140 ; 0x8c
>
> scripts fails to read stack usage.
> so making regex $re for ARM similar to aarch64
>
> Signed-off-by: Vaneet Narang <[email protected]>
> Signed-off-by: Maninder Singh <[email protected]>
This looks good to me, and it is a bug fix.
Just a question about the SOB.
Maninder Singh is the author and also the submitter, right?
What does "Signed-off-by: Vaneet Narang <[email protected]>" mean?
Co-developed-by or something else?
> ---
> scripts/checkstack.pl | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/checkstack.pl b/scripts/checkstack.pl
> index b292ef4..e80de70 100755
> --- a/scripts/checkstack.pl
> +++ b/scripts/checkstack.pl
> @@ -60,7 +60,7 @@ my (@stack, $re, $dre, $sub, $x, $xs, $funcre, $min_stack);
> $dre = qr/^.*sub.*sp, sp, #(0x$x{1,8})/o;
> } elsif ($arch eq 'arm') {
> #c0008ffc: e24dd064 sub sp, sp, #100 ; 0x64
> - $re = qr/.*sub.*sp, sp, #(([0-9]{2}|[3-9])[0-9]{2})/o;
> + $re = qr/.*sub.*sp, sp, #([0-9]{1,4})/o;
> $sub = \&arm_push_handling;
> } elsif ($arch =~ /^x86(_64)?$/ || $arch =~ /^i[3456]86$/) {
> #c0105234: 81 ec ac 05 00 00 sub $0x5ac,%esp
> --
> 1.9.1
>
--
Best Regards
Masahiro Yamada
Hi Masahiro,
Thanks for review.
We will integrate your review comements and send v2.
>>
>> so making regex $re for ARM similar to aarch64
>>
>> Signed-off-by: Vaneet Narang <[email protected]>
>> Signed-off-by: Maninder Singh <[email protected]>
>
>
>This looks good to me, and it is a bug fix.
>
>Just a question about the SOB.
>
>
>Maninder Singh is the author and also the submitter, right?
>
>What does "Signed-off-by: Vaneet Narang <[email protected]>" mean?
>
>Co-developed-by or something else?
Yes All 4 patches are co-developed by Vaneet and me.
we were checking stack usage for arm and AARCH64 and found
some changes in scrips which can help others also.
Thanks,
Maninder Singh
On Thu, May 7, 2020 at 8:43 PM Maninder Singh <[email protected]> wrote:
>
> Hi Masahiro,
>
> Thanks for review.
> We will integrate your review comements and send v2.
>
> >>
>
> >> so making regex $re for ARM similar to aarch64
> >>
> >> Signed-off-by: Vaneet Narang <[email protected]>
> >> Signed-off-by: Maninder Singh <[email protected]>
> >
> >
> >This looks good to me, and it is a bug fix.
> >
> >Just a question about the SOB.
> >
> >
> >Maninder Singh is the author and also the submitter, right?
> >
> >What does "Signed-off-by: Vaneet Narang <[email protected]>" mean?
> >
> >Co-developed-by or something else?
>
> Yes All 4 patches are co-developed by Vaneet and me.
> we were checking stack usage for arm and AARCH64 and found
> some changes in scrips which can help others also.
>
OK. Then,
Co-developed-by: Vaneet Narang <[email protected]>
is the correct tag.
Signed-off-by is added when the patch is forwarded by
somebody else. This is not the case.
--
Best Regards
Masahiro Yamada
Hi Masahiro,
>> To count stack usage of push {*, fp, ip, lr, pc} instruction in ARM,
>> if FRAME POINTER is enabled.
>> e.g. c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
>>
>> c01f0d50 <Y>:
>> c01f0d44: e1a0c00d mov ip, sp
>> c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
>> c01f0d4c: e24cb004 sub fp, ip, #4
>> c01f0d50: e24dd094 sub sp, sp, #448 ; 0x1C0
>>
>> $ cat dump | scripts/checkstack.pl arm
>> 0xc01f0d50 Y []: 448
>>
>> added subroutine frame work for this.
>> After change:
>> 0xc01f0d500 Y []: 492
> Do you know CONFIG_FRAME_WARN?
Yes we know this and we use it to get compilation error if some function is using more stack.
This config will report issue at compilation.
>I know checkstack.pl dumps the stack size
>of functions, which is different from what
>-Wframe-larger-than does, but the goal is
>quite similar, I think.
>
>I just wondered if we need both.
We feel purpose of this patch is different from CONFIG_FRAME_WARN.
This patch is specific to ARM and fixes bug in stack usage calculation.
We were comparing stack usage of ARM with ARM64 and found big gap.
We realised ARM is not calculating stack usage properly.
It only considers stack used by local variables but it doesn't consider
stack used to store register context at the start of functions.
This is not the case with ARM64. It seems ARM64 considers both.
We found even stack variables are of same size on both target but
arm64 stack usage is high.
Considering below assembly, Actual stack usage is 492 but current script reports 448.
push instruction uses 44 bytes of stack to take backup of registers as per ARM calling
convention.
c01f0d44: e1a0c00d mov ip, sp
c01f0d48: e92ddff0 push {r4, r5, r6, r7, r8, r9, sl, fp, ip, lr, pc}
c01f0d4c: e24cb004 sub fp, ip, #4
c01f0d50: e24dd094 sub sp, sp, #448 ; 0x1C0
Thanks & Regards,
Vaneet Narang