Re: [PATCH 3/4] scripts/checkstack.pl: add arm push handling for stack usage

From: Masahiro Yamada
Date: Thu May 07 2020 - 06:36:45 EST


(+CC Andi Kleen, and more mentor)


On Thu, Apr 30, 2020 at 9:50 PM Maninder Singh <maninder1.s@xxxxxxxxxxx> 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 <v.narang@xxxxxxxxxxx>
> Signed-off-by: Maninder Singh <maninder1.s@xxxxxxxxxxx>



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