Received-SPF: pass (google.com: domain of linux-kernel+bounces-185705-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Date: Tue, 21 May 2024 19:48:50 -0700
From: Elliot Berman <quic_eberman@quicinc.com>
To: Xiong Nandi <xndchn@gmail.com>
CC: <akpm@linux-foundation.org>, <linux-kernel@vger.kernel.org>,
        <quic_bjorande@quicinc.com>, <cmllamas@google.com>
Subject: Re: [PATCH] scripts/decode_stacktrace.sh: better support to ARM32
 module stack trace
Message-ID: <20240521194010043-0700.eberman@hu-eberman-lv.qualcomm.com>
References: <20240521005649.12144-1-xndchn@gmail.com>
 <20240522010559.10551-1-xndchn@gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240522010559.10551-1-xndchn@gmail.com>

On Wed, May 22, 2024 at 09:05:59AM +0800, Xiong Nandi wrote:
> Sorry about the name, it is some kind of abbreviation. So I re-post here:
> ---
> Since System.map is generated by cross-compile nm tool, we should use it
> here too. Otherwise host nm may not recognize thumb2 function address well.
> 
> Beside, sometimes special characters around module name, such as ARM32
> with BACKTRACE_VERBOSE in "(%pS)" format, such as:
> [<806e4845>] (dump_stack_lvl) from [<7f806013>] (hello_init+0x13/0x1000 [test])
> 
> After stripping other characters around "[module]", it can be finally decoded:
> (dump_stack_lvl) from hello_init (/foo/test.c:10) test
> 
> Signed-off-by: Xiong Nandi <xndchn@gmail.com>
> ---
>  scripts/decode_stacktrace.sh | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
> index fa5be6f57b00..324e4a6c260a 100755
> --- a/scripts/decode_stacktrace.sh
> +++ b/scripts/decode_stacktrace.sh
> @@ -30,6 +30,7 @@ fi
>  
>  READELF=${UTIL_PREFIX}readelf${UTIL_SUFFIX}
>  ADDR2LINE=${UTIL_PREFIX}addr2line${UTIL_SUFFIX}
> +NM=${UTIL_PREFIX}nm${UTIL_SUFFIX}
>  
>  if [[ $1 == "-r" ]] ; then
>  	vmlinux=""
> @@ -158,7 +159,7 @@ parse_symbol() {
>  	if [[ $aarray_support == true && "${cache[$module,$name]+isset}" == "isset" ]]; then
>  		local base_addr=${cache[$module,$name]}
>  	else
> -		local base_addr=$(nm "$objfile" 2>/dev/null | awk '$3 == "'$name'" && ($2 == "t" || $2 == "T") {print $1; exit}')
> +		local base_addr=$(${NM} "$objfile" 2>/dev/null | awk '$3 == "'$name'" && ($2 == "t" || $2 == "T") {print $1; exit}')

The nm parts should be a separate patch.

>  		if [[ $base_addr == "" ]] ; then
>  			# address not found
>  			return
> @@ -282,8 +283,8 @@ handle_line() {
>  
>  	if [[ ${words[$last]} =~ \[([^]]+)\] ]]; then
>  		module=${words[$last]}
> -		module=${module#\[}
> -		module=${module%\]}
> +		module=${module#*\[}
> +		module=${module%\]*}

I need to get a moment to play with it. Is my understanding correct that
the problem is that the last word ($module) is:

[test])

and after the existing strip logic, $module becomes test]) whereas
expecting just "test"? Your change is to strip any leading/trailing
characters before/after the [ / ] respectively? Isn't this a problem for
$symbol as well -- it would be "(hello_init+0x13/0x1000" in the example.

- Elliot

>  		modbuildid=${module#* }
>  		module=${module% *}
>  		if [[ $modbuildid == $module ]]; then
> -- 
> 2.25.1
>