[PATCH RESEND v2 0/3] scripts/decode_stacktrace.sh: improve error reporting and usability

[PATCH RESEND v2 0/3] scripts/decode_stacktrace.sh: improve error reporting and usability

[Luca Ceresoli]

This small series improves usability of scripts/decode_stacktrace.sh by
improving the usage text and correctly reporting when modules are built
without debugging symbols.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v2:
- Fix a typo
- Add Stephen's review tag
- Link to v1: https://lore.kernel.org/r/20240311-decode_stacktrace-find_module-improvements-v1-0-8bea42b421aa@bootlin.com

---
Luca Ceresoli (3):
      scripts/decode_stacktrace.sh: remove find_module recursion and improve error reporting
      scripts/decode_stacktrace.sh: clarify command line
      scripts/decode_stacktrace.sh: add '-h' flag

 scripts/decode_stacktrace.sh | 49 ++++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 22 deletions(-)
---
base-commit: 45db3ab70092637967967bfd8e6144017638563c
change-id: 20240311-decode_stacktrace-find_module-improvements-a02aee28fbaf

Best regards,
-- 
Luca Ceresoli <luca.ceresoli@bootlin.com>

[PATCH RESEND v2 1/3] scripts/decode_stacktrace.sh: remove find_module recursion and improve error reporting

[Luca Ceresoli]

The find_module() function can fail for two reasons:

 * the module was not found
 * the module was found but without debugging info

In both cases the user is reported the same error:

   WARNING! Modules path isn't set, but is needed to parse this symbol

This is misleading in case the modules path is set correctly.

find_module() is currently implemented as a recursive function based on
global variables in order to check up to 4 different paths. This is not
straightforward to read and even less to modify.

Besides, the debuginfod code at the beginning of find_module() is executed
identically every time the function is entered, i.e. up to 4 times per each
module search due to recursion.

To be able to improve error reporting, first rewrite the find_module()
function to remove recursion. The new version of the function iterates over
all the same (up to 4) paths as before and for each of them does the same
checks as before. At the end of the iteration it is now able to print an
appropriate error message, so that has been moved from the caller into
find_module().

Finally, when the module is found but without debugging info, mention the
two Kconfig variables one needs to set in order to have the needed
debugging symbols.

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 scripts/decode_stacktrace.sh | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
index fa5be6f57b00..7f3fb5e82707 100755
--- a/scripts/decode_stacktrace.sh
+++ b/scripts/decode_stacktrace.sh
@@ -88,31 +88,32 @@ find_module() {
 		fi
 	fi
 
-	if [[ "$modpath" != "" ]] ; then
-		for fn in $(find "$modpath" -name "${module//_/[-_]}.ko*") ; do
-			if ${READELF} -WS "$fn" | grep -qwF .debug_line ; then
-				echo $fn
-				return
-			fi
-		done
-		return 1
-	fi
-
-	modpath=$(dirname "$vmlinux")
-	find_module && return
-
-	if [[ $release == "" ]] ; then
+	if [ -z $release ] ; then
 		release=$(gdb -ex 'print init_uts_ns.name.release' -ex 'quit' -quiet -batch "$vmlinux" 2>/dev/null | sed -n 's/\$1 = "\(.*\)".*/\1/p')
 	fi
+	if [ -n "${release}" ] ; then
+		release_dirs="/usr/lib/debug/lib/modules/$release /lib/modules/$release"
+	fi
 
-	for dn in {/usr/lib/debug,}/lib/modules/$release ; do
-		if [ -e "$dn" ] ; then
-			modpath="$dn"
-			find_module && return
+	found_without_debug_info=false
+	for dir in "$modpath" "$(dirname "$vmlinux")" ${release_dirs}; do
+		if [ -n "${dir}" ] && [ -e "${dir}" ]; then
+			for fn in $(find "$dir" -name "${module//_/[-_]}.ko*") ; do
+				if ${READELF} -WS "$fn" | grep -qwF .debug_line ; then
+					echo $fn
+					return
+				fi
+				found_without_debug_info=true
+			done
 		fi
 	done
 
-	modpath=""
+	if [[ ${found_without_debug_info} == true ]]; then
+		echo "WARNING! No debugging info in module ${module}, rebuild with DEBUG_KERNEL and DEBUG_INFO" >&2
+	else
+		echo "WARNING! Cannot find .ko for module ${module}, please pass a valid module path" >&2
+	fi
+
 	return 1
 }
 
@@ -130,7 +131,6 @@ parse_symbol() {
 	else
 		local objfile=$(find_module)
 		if [[ $objfile == "" ]] ; then
-			echo "WARNING! Modules path isn't set, but is needed to parse this symbol" >&2
 			return
 		fi
 		if [[ $aarray_support == true ]]; then

-- 
2.34.1

[PATCH RESEND v2 2/3] scripts/decode_stacktrace.sh: clarify command line

[Luca Ceresoli]

The syntax as expressed by usage() is not entirely correct: "<modules
path>" cannot be passed without "<base path>|auto". Additionally human
reading of this syntax can be subject to misunderstanding due the mixture
of '|' and '[]'.

Improve readability in various ways:
 * rewrite using two lines for the two allowed usages
 * add square brackets around "<vmlinux>" as it is optional when using
   debuginfod-find
 * move "<modules path>" to inside the square brackets of the 2nd
   positional parameter
 * use underscores instead of spaces in <...> strings

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 scripts/decode_stacktrace.sh | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
index 7f3fb5e82707..b56e79060e9f 100755
--- a/scripts/decode_stacktrace.sh
+++ b/scripts/decode_stacktrace.sh
@@ -5,7 +5,8 @@
 
 usage() {
 	echo "Usage:"
-	echo "	$0 -r <release> | <vmlinux> [<base path>|auto] [<modules path>]"
+	echo "	$0 -r <release>"
+	echo "	$0 [<vmlinux> [<base_path>|auto [<modules_path>]]]"
 }
 
 # Try to find a Rust demangler

-- 
2.34.1

[PATCH RESEND v2 3/3] scripts/decode_stacktrace.sh: add '-h' flag

[Luca Ceresoli]

When no parameters are passed, the usage instructions are presented only
when debuginfod-find is not found. This makes sense because with debuginfod
none of the positional parameters are needed. However it means that users
having debuginfod-find installed will have no chance of reading the usage
text without opening the file.

Many programs have a '-h' flag to get the usage, so add such a flag.
Invoking 'scripts/decode_stacktrace.sh -h' will now show the usage text and
exit.

Reviewed-by: Stephen Boyd <swboyd@chromium.org>
Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 scripts/decode_stacktrace.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/scripts/decode_stacktrace.sh b/scripts/decode_stacktrace.sh
index b56e79060e9f..e8c9976062d0 100755
--- a/scripts/decode_stacktrace.sh
+++ b/scripts/decode_stacktrace.sh
@@ -7,6 +7,7 @@ usage() {
 	echo "Usage:"
 	echo "	$0 -r <release>"
 	echo "	$0 [<vmlinux> [<base_path>|auto [<modules_path>]]]"
+	echo "	$0 -h"
 }
 
 # Try to find a Rust demangler
@@ -32,7 +33,10 @@ fi
 READELF=${UTIL_PREFIX}readelf${UTIL_SUFFIX}
 ADDR2LINE=${UTIL_PREFIX}addr2line${UTIL_SUFFIX}
 
-if [[ $1 == "-r" ]] ; then
+if [[ $1 == "-h" ]] ; then
+	usage
+	exit 0
+elif [[ $1 == "-r" ]] ; then
 	vmlinux=""
 	basepath="auto"
 	modpath=""

-- 
2.34.1

Re: [PATCH RESEND v2 0/3] scripts/decode_stacktrace.sh: improve error reporting and usability

[Luca Ceresoli]

Hello,

On Fri, 23 Aug 2024 10:27:41 +0200
Luca Ceresoli <luca.ceresoli@bootlin.com> wrote:

> This small series improves usability of scripts/decode_stacktrace.sh by
> improving the usage text and correctly reporting when modules are built
> without debugging symbols.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Initial submission of v2, May 2024:
https://lore.kernel.org/all/20240509-decode_stacktrace-find_module-improvements-v2-0-887fb6d5acb9@bootlin.com/

Had no replies, assuming it got forgotten.

Best regards,
Luca

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