scripts/decode_stacktrace.sh: fix build ID and PC source parsing

[PATCH] scripts/decode_stacktrace.sh: fix build ID and PC source parsing

Posted by Carlos Llamas 1 month, 2 weeks ago

Support for parsing PC source info in stacktraces (e.g. '(P)') was
added in commit 2bff77c665ed ("scripts/decode_stacktrace.sh: fix
decoding of lines with an additional info"). However, this logic was
placed after the build ID processing. This incorrect order fails to
parse lines containing both elements, e.g.:

  drm_gem_mmap_obj+0x114/0x200 [drm 03d0564e0529947d67bb2008c3548be77279fd27] (P)

This patch fixes the problem by extracting the PC source info first and
then processing the module build ID. With this change, the line above is
now properly parsed as such:

  drm_gem_mmap_obj (./include/linux/mmap_lock.h:212 ./include/linux/mm.h:811 drivers/gpu/drm/drm_gem.c:1177) drm (P)

While here, also add a brief explanation the build ID section.

Fixes: bdf8eafbf7f5 ("arm64: stacktrace: report source of unwind data")
Fixes: 2bff77c665ed ("scripts/decode_stacktrace.sh: fix decoding of lines with an additional info")
Signed-off-by: Carlos Llamas <cmllamas@google.com>
---
 scripts/decode_stacktrace.sh | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
index c73cb802a0a3..8d01b741de62 100755
--- a/scripts/decode_stacktrace.sh
+++ b/scripts/decode_stacktrace.sh
@@ -277,12 +277,6 @@ handle_line() {
 		fi
 	done
 
-	if [[ ${words[$last]} =~ ^[0-9a-f]+\] ]]; then
-		words[$last-1]="${words[$last-1]} ${words[$last]}"
-		unset words[$last] spaces[$last]
-		last=$(( $last - 1 ))
-	fi
-
 	# Extract info after the symbol if present. E.g.:
 	# func_name+0x54/0x80 (P)
 	#                     ^^^
@@ -295,6 +289,14 @@ handle_line() {
 		last=$(( $last - 1 ))
 	fi
 
+	# Join module name with its build id if present, as these were
+	# split during tokenization (e.g. "[module" and "modbuildid]").
+	if [[ ${words[$last]} =~ ^[0-9a-f]+\] ]]; then
+		words[$last-1]="${words[$last-1]} ${words[$last]}"
+		unset words[$last] spaces[$last]
+		last=$(( $last - 1 ))
+	fi
+
 	if [[ ${words[$last]} =~ \[([^]]+)\] ]]; then
 		module=${words[$last]}
 		# some traces format is "(%pS)", which like "(foo+0x0/0x1 [bar])"
-- 
2.51.1.851.g4ebd6896fd-goog

Re: [PATCH] scripts/decode_stacktrace.sh: fix build ID and PC source parsing

Posted by Andrew Morton 1 month, 1 week ago

On Thu, 30 Oct 2025 01:03:33 +0000 Carlos Llamas <cmllamas@google.com> wrote:

> Support for parsing PC source info in stacktraces (e.g. '(P)') was
> added in commit 2bff77c665ed ("scripts/decode_stacktrace.sh: fix
> decoding of lines with an additional info"). However, this logic was
> placed after the build ID processing. This incorrect order fails to
> parse lines containing both elements, e.g.:
> 
>   drm_gem_mmap_obj+0x114/0x200 [drm 03d0564e0529947d67bb2008c3548be77279fd27] (P)
> 
> This patch fixes the problem by extracting the PC source info first and
> then processing the module build ID. With this change, the line above is
> now properly parsed as such:
> 
>   drm_gem_mmap_obj (./include/linux/mmap_lock.h:212 ./include/linux/mm.h:811 drivers/gpu/drm/drm_gem.c:1177) drm (P)
> 
> While here, also add a brief explanation the build ID section.

Thanks for fixing this.  I'll queue it in mm.git's mm-hotfixes branch
with a plan to upstream it into 6.18-rcX.

I'll add a cc:stable tag so this gets fixed in earlier kernel releases
also.

But....

> Fixes: bdf8eafbf7f5 ("arm64: stacktrace: report source of unwind data")
> Fixes: 2bff77c665ed ("scripts/decode_stacktrace.sh: fix decoding of lines with an additional info")

Which kernel releases?  We're fixing a commit from Oct 2024 and a
commit from Jan 2025.  How are downstream people to understand what
we're recommending here?

I view the Fixes: as a recommendation we make to -stable maintainers
saying "I think this should be backported to <here> and later".  As
such, giving them multiple backporting targets must make their little
heads spin.

So can we choose a single point in KernelTime here?  And reflect this
in a single Fixes: line?

Thanks.

Joe, I'd appreciate it if checkpatch weer to somehow draw people's
attention to this (fairly frequent) problem.

Re: [PATCH] scripts/decode_stacktrace.sh: fix build ID and PC source parsing

Posted by Carlos Llamas 1 month, 1 week ago

On Thu, Oct 30, 2025 at 08:57:58PM -0700, Andrew Morton wrote:
> On Thu, 30 Oct 2025 01:03:33 +0000 Carlos Llamas <cmllamas@google.com> wrote:
> 
> > Support for parsing PC source info in stacktraces (e.g. '(P)') was
> > added in commit 2bff77c665ed ("scripts/decode_stacktrace.sh: fix
> > decoding of lines with an additional info"). However, this logic was
> > placed after the build ID processing. This incorrect order fails to
> > parse lines containing both elements, e.g.:
> > 
> >   drm_gem_mmap_obj+0x114/0x200 [drm 03d0564e0529947d67bb2008c3548be77279fd27] (P)
> > 
> > This patch fixes the problem by extracting the PC source info first and
> > then processing the module build ID. With this change, the line above is
> > now properly parsed as such:
> > 
> >   drm_gem_mmap_obj (./include/linux/mmap_lock.h:212 ./include/linux/mm.h:811 drivers/gpu/drm/drm_gem.c:1177) drm (P)
> > 
> > While here, also add a brief explanation the build ID section.
> 
> Thanks for fixing this.  I'll queue it in mm.git's mm-hotfixes branch
> with a plan to upstream it into 6.18-rcX.

Thanks Andrew, and sorry for the late reply I completely missed this
email.

> 
> I'll add a cc:stable tag so this gets fixed in earlier kernel releases
> also.
> 
> But....
> 
> > Fixes: bdf8eafbf7f5 ("arm64: stacktrace: report source of unwind data")
> > Fixes: 2bff77c665ed ("scripts/decode_stacktrace.sh: fix decoding of lines with an additional info")
> 
> Which kernel releases?  We're fixing a commit from Oct 2024 and a
> commit from Jan 2025.  How are downstream people to understand what
> we're recommending here?
> 
> I view the Fixes: as a recommendation we make to -stable maintainers
> saying "I think this should be backported to <here> and later".  As
> such, giving them multiple backporting targets must make their little
> heads spin.

So I've heard conflicting arguments about backporting patches to stable
for these scripts. That's why I skipped the Cc: stable tag. IIRC, the
argument is that one should always run the ToT scripts? <shrug>

You are right about the multiple Fixes: though. I should have only used:

  Fixes: 2bff77c665ed ("scripts/decode_stacktrace.sh: fix decoding of lines with an additional info")

Thanks,
--
Carlos Llamas

Re: [PATCH] scripts/decode_stacktrace.sh: fix build ID and PC source parsing

Posted by Andrew Morton 1 month, 1 week ago

On Thu, 6 Nov 2025 00:59:55 +0000 Carlos Llamas <cmllamas@google.com> wrote:

> > I view the Fixes: as a recommendation we make to -stable maintainers
> > saying "I think this should be backported to <here> and later".  As
> > such, giving them multiple backporting targets must make their little
> > heads spin.
> 
> So I've heard conflicting arguments about backporting patches to stable
> for these scripts. That's why I skipped the Cc: stable tag. IIRC, the
> argument is that one should always run the ToT scripts? <shrug>

Seems weird.  Later scripts may use features which aren't present in
older kernels.  Moreso with selftests, of course.

> You are right about the multiple Fixes: though. I should have only used:
> 
>   Fixes: 2bff77c665ed ("scripts/decode_stacktrace.sh: fix decoding of lines with an additional info")

Updated, thanks.

Re: [PATCH] scripts/decode_stacktrace.sh: fix build ID and PC source parsing

Posted by Luca Ceresoli 1 month, 2 weeks ago

Hello,

On Thu Oct 30, 2025 at 2:03 AM CET, Carlos Llamas wrote:
> Support for parsing PC source info in stacktraces (e.g. '(P)') was
> added in commit 2bff77c665ed ("scripts/decode_stacktrace.sh: fix
> decoding of lines with an additional info"). However, this logic was
> placed after the build ID processing. This incorrect order fails to
> parse lines containing both elements, e.g.:
>
>   drm_gem_mmap_obj+0x114/0x200 [drm 03d0564e0529947d67bb2008c3548be77279fd27] (P)
>
> This patch fixes the problem by extracting the PC source info first and
> then processing the module build ID. With this change, the line above is
> now properly parsed as such:
>
>   drm_gem_mmap_obj (./include/linux/mmap_lock.h:212 ./include/linux/mm.h:811 drivers/gpu/drm/drm_gem.c:1177) drm (P)
>
> While here, also add a brief explanation the build ID section.
>
> Fixes: bdf8eafbf7f5 ("arm64: stacktrace: report source of unwind data")
> Fixes: 2bff77c665ed ("scripts/decode_stacktrace.sh: fix decoding of lines with an additional info")
> Signed-off-by: Carlos Llamas <cmllamas@google.com>

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH] scripts/decode_stacktrace.sh: fix build ID and PC source parsing

Posted by Matthieu Baerts 1 month, 2 weeks ago

Hi Carlos,

Thank you for this fix!

On 30/10/2025 02:03, Carlos Llamas wrote:
> Support for parsing PC source info in stacktraces (e.g. '(P)') was
> added in commit 2bff77c665ed ("scripts/decode_stacktrace.sh: fix
> decoding of lines with an additional info"). However, this logic was
> placed after the build ID processing. This incorrect order fails to
> parse lines containing both elements, e.g.:
> 
>   drm_gem_mmap_obj+0x114/0x200 [drm 03d0564e0529947d67bb2008c3548be77279fd27] (P)
> 
> This patch fixes the problem by extracting the PC source info first and
> then processing the module build ID. With this change, the line above is
> now properly parsed as such:
> 
>   drm_gem_mmap_obj (./include/linux/mmap_lock.h:212 ./include/linux/mm.h:811 drivers/gpu/drm/drm_gem.c:1177) drm (P)
> 
> While here, also add a brief explanation the build ID section.
> 
> Fixes: bdf8eafbf7f5 ("arm64: stacktrace: report source of unwind data")
> Fixes: 2bff77c665ed ("scripts/decode_stacktrace.sh: fix decoding of lines with an additional info")
> Signed-off-by: Carlos Llamas <cmllamas@google.com>

LGTM!

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.