[PATCH 0/2] perf: tests: Fix object code reading test for riscv

[PATCH 0/2] perf: tests: Fix object code reading test for riscv

[Charlie Jenkins]

There was a breaking change to binutils objdump that causes partial
instructions to no longer be dumped. This behavior is different from
what the "Object code reading" test expects. Add a Kconfig variable that
checks the version of objdump and conditionally enables the perf test
fix for riscv objdump versions effected by this issue.

A binutils patch has been sent as well to fix this in objdump [1].

Link:
https://sourceware.org/pipermail/binutils/2024-December/138139.html [1]

To: 

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
---
Charlie Jenkins (2):
      kbuild: Check version of objdump
      tools: perf: tests: Fix code reading for riscv

 arch/riscv/Kconfig              |  5 +++
 init/Kconfig                    | 10 ++++++
 scripts/Kconfig.include         |  6 ++++
 scripts/objdump-version.sh      | 69 +++++++++++++++++++++++++++++++++++++++++
 tools/perf/tests/code-reading.c | 17 +++++++++-
 5 files changed, 106 insertions(+), 1 deletion(-)
---
base-commit: fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
change-id: 20241213-perf_fix_riscv_obj_reading-cabf02be3c85
-- 
- Charlie

[PATCH 1/2] kbuild: Check version of objdump

[Charlie Jenkins]

Similar to ld-version, add a way to check the version of objdump. This
should most of the time end up being the binutils version or the llvm
version.

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
---
 init/Kconfig               | 10 +++++++
 scripts/Kconfig.include    |  6 ++++
 scripts/objdump-version.sh | 69 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index a20e6efd3f0fbdd7f0df2448854cc30734a0ee4f..0b5d36f939e1de89c12ebdd61e4815015314d4f1 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -60,6 +60,16 @@ config LLD_VERSION
 	default $(ld-version) if LD_IS_LLD
 	default 0
 
+config OBJDUMP_IS_GNU
+	def_bool $(success,test "$(objdump-name)" = objdump)
+
+config OBJDUMP_IS_LLVM
+	def_bool $(success,test "$(objdump-name)" = llvm-objdump)
+
+config OBJDUMP_VERSION
+	int
+	default $(objdump-version)
+
 config RUSTC_VERSION
 	int
 	default $(rustc-version)
diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
index 33193ca6e8030e659d6b321acaea1acd42c387a4..cb3e2d2564fea8cce780adb3be672c9596b7ccf2 100644
--- a/scripts/Kconfig.include
+++ b/scripts/Kconfig.include
@@ -58,6 +58,12 @@ $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not supp
 ld-name := $(shell,set -- $(ld-info) && echo $1)
 ld-version := $(shell,set -- $(ld-info) && echo $2)
 
+# Get the objdump name, version, and error out if it is not supported.
+objdump-info := $(shell,$(srctree)/scripts/objdump-version.sh $(OBJDUMP))
+$(error-if,$(success,test -z "$(objdump-info)"),Sorry$(comma) this objdump is not supported.)
+objdump-name := $(shell,set -- $(objdump-info) && echo $1)
+objdump-version := $(shell,set -- $(objdump-info) && echo $2)
+
 # machine bit flags
 #  $(m32-flag): -m32 if the compiler supports it, or an empty string otherwise.
 #  $(m64-flag): -m64 if the compiler supports it, or an empty string otherwise.
diff --git a/scripts/objdump-version.sh b/scripts/objdump-version.sh
new file mode 100755
index 0000000000000000000000000000000000000000..fa24f8dc2d3c42fd1195fceb3c96b27f7127db25
--- /dev/null
+++ b/scripts/objdump-version.sh
@@ -0,0 +1,69 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+#
+# Print the objdump name and its version in a 5 or 6-digit form.
+# Also, perform the minimum version check.
+
+set -e
+
+# Convert the version string x.y.z to a canonical 5 or 6-digit form.
+get_canonical_version()
+{
+	IFS=.
+	set -- $1
+
+	# If the 2nd or 3rd field is missing, fill it with a zero.
+	#
+	# The 4th field, if present, is ignored.
+	# This occurs in development snapshots as in 2.35.1.20201116
+	echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0}))
+}
+
+orig_args="$@"
+
+# Get the first line of the --version output.
+IFS='
+'
+set -- $(LC_ALL=C "$@" --version)
+
+# Split the line on spaces.
+IFS=' '
+set -- $1
+
+min_tool_version=$(dirname $0)/min-tool-version.sh
+
+if [ "$1" = GNU -a "$2" = objdump ]; then
+	shift $(($# - 1))
+	version=$1
+	min_version=$($min_tool_version binutils)
+	disp_name="GNU objdump"
+else
+	while [ $# -gt 1 -a "$1" != "LLVM" ]; do
+		shift
+	done
+
+	if [ "$1" = LLVM ]; then
+		version=$3
+		min_version=$($min_tool_version llvm)
+		disp_name="llvm-objdump"
+	else
+		echo "$orig_args: unknown objdump" >&2
+		exit 1
+	fi
+fi
+
+version=${version%%[!0-9.]*}
+
+cversion=$(get_canonical_version $version)
+min_cversion=$(get_canonical_version $min_version)
+
+if [ "$cversion" -lt "$min_cversion" ]; then
+	echo >&2 "***"
+	echo >&2 "*** objdump is too old."
+	echo >&2 "***   Your $disp_name version:    $version"
+	echo >&2 "***   Minimum $disp_name version: $min_version"
+	echo >&2 "***"
+	exit 1
+fi
+
+echo objdump $cversion

-- 
2.34.1

[PATCH 2/2] tools: perf: tests: Fix code reading for riscv

[Charlie Jenkins]

After binutils commit e43d876 which was first included in binutils 2.41,
riscv no longer supports dumping in the middle of instructions. Increase
the objdump window by 2-bytes to ensure that any instruction that sits
on the boundary of the specified stop-address is not cut in half.

Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
---
 arch/riscv/Kconfig              |  5 +++++
 tools/perf/tests/code-reading.c | 17 ++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index d4a7ca0388c071b536df59c0eb11d55f9080c7cd..f164047471267936bc62389b7d7d9a7cbdca8f97 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -229,6 +229,11 @@ config GCC_SUPPORTS_DYNAMIC_FTRACE
 	def_bool CC_IS_GCC
 	depends on $(cc-option,-fpatchable-function-entry=8)
 
+config RISCV_OBJDUMP_SUPPORTS_SPLIT_INSTRUCTION
+	# Some versions of objdump do not support dumping partial instructions
+	def_bool y
+	depends on !(OBJDUMP_IS_GNU && OBJDUMP_VERSION > 24100)
+
 config HAVE_SHADOW_CALL_STACK
 	def_bool $(cc-option,-fsanitize=shadow-call-stack)
 	# https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/a484e843e6eeb51f0cb7b8819e50da6d2444d769
diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
index 27c82cfb7e7de42284bf5af9cf7594a3a963052e..605f4a8e1dbc00d8a572503f45053c2f30ad19e3 100644
--- a/tools/perf/tests/code-reading.c
+++ b/tools/perf/tests/code-reading.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 #include <errno.h>
+#include <linux/kconfig.h>
 #include <linux/kernel.h>
 #include <linux/types.h>
 #include <inttypes.h>
@@ -183,9 +184,23 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
 	const char *fmt;
 	FILE *f;
 	int ret;
+	u64 stop_address = addr + len;
+
+	if (IS_ENABLED(__riscv) && !IS_ENABLED(CONFIG_RISCV_OBJDUMP_SUPPORTS_SPLIT_INSTRUCTION)) {
+		/*
+		 * On some versions of riscv objdump, dumping in the middle of
+		 * instructions is not supported. riscv instructions are aligned along
+		 * 2-byte intervals and can be either 2-bytes or 4-bytes. This makes it
+		 * possible that the stop-address lands in the middle of a 4-byte
+		 * instruction. Increase the stop_address by two to ensure an
+		 * instruction is not cut in half, but leave the len as-is so only the
+		 * expected number of bytes are collected.
+		 */
+		stop_address += 2;
+	}
 
 	fmt = "%s -z -d --start-address=0x%"PRIx64" --stop-address=0x%"PRIx64" %s";
-	ret = snprintf(cmd, sizeof(cmd), fmt, test_objdump_path, addr, addr + len,
+	ret = snprintf(cmd, sizeof(cmd), fmt, test_objdump_path, addr, stop_address,
 		       filename);
 	if (ret <= 0 || (size_t)ret >= sizeof(cmd))
 		return -1;

-- 
2.34.1

Re: [PATCH 2/2] tools: perf: tests: Fix code reading for riscv

[Ian Rogers]

On Mon, Dec 16, 2024 at 3:13 PM Charlie Jenkins <charlie@rivosinc.com> wrote:
>
> After binutils commit e43d876 which was first included in binutils 2.41,
> riscv no longer supports dumping in the middle of instructions. Increase
> the objdump window by 2-bytes to ensure that any instruction that sits
> on the boundary of the specified stop-address is not cut in half.
>
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> ---
>  arch/riscv/Kconfig              |  5 +++++
>  tools/perf/tests/code-reading.c | 17 ++++++++++++++++-

Files under tools use a different Build system than the kernel. The
Kconfig value won't have an effect. Check out Makefile.config:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Makefile.config?h=perf-tools-next
which is included into the build here:
https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Makefile.perf?h=perf-tools-next#n313

>  2 files changed, 21 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index d4a7ca0388c071b536df59c0eb11d55f9080c7cd..f164047471267936bc62389b7d7d9a7cbdca8f97 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -229,6 +229,11 @@ config GCC_SUPPORTS_DYNAMIC_FTRACE
>         def_bool CC_IS_GCC
>         depends on $(cc-option,-fpatchable-function-entry=8)
>
> +config RISCV_OBJDUMP_SUPPORTS_SPLIT_INSTRUCTION
> +       # Some versions of objdump do not support dumping partial instructions
> +       def_bool y
> +       depends on !(OBJDUMP_IS_GNU && OBJDUMP_VERSION > 24100)
> +
>  config HAVE_SHADOW_CALL_STACK
>         def_bool $(cc-option,-fsanitize=shadow-call-stack)
>         # https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/a484e843e6eeb51f0cb7b8819e50da6d2444d769
> diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> index 27c82cfb7e7de42284bf5af9cf7594a3a963052e..605f4a8e1dbc00d8a572503f45053c2f30ad19e3 100644
> --- a/tools/perf/tests/code-reading.c
> +++ b/tools/perf/tests/code-reading.c
> @@ -1,5 +1,6 @@
>  // SPDX-License-Identifier: GPL-2.0
>  #include <errno.h>
> +#include <linux/kconfig.h>
>  #include <linux/kernel.h>
>  #include <linux/types.h>
>  #include <inttypes.h>
> @@ -183,9 +184,23 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
>         const char *fmt;
>         FILE *f;
>         int ret;
> +       u64 stop_address = addr + len;
> +
> +       if (IS_ENABLED(__riscv) && !IS_ENABLED(CONFIG_RISCV_OBJDUMP_SUPPORTS_SPLIT_INSTRUCTION)) {

It would be nice if this could be a runtime rather than build time detected.

Thanks,
Ian

> +               /*
> +                * On some versions of riscv objdump, dumping in the middle of
> +                * instructions is not supported. riscv instructions are aligned along
> +                * 2-byte intervals and can be either 2-bytes or 4-bytes. This makes it
> +                * possible that the stop-address lands in the middle of a 4-byte
> +                * instruction. Increase the stop_address by two to ensure an
> +                * instruction is not cut in half, but leave the len as-is so only the
> +                * expected number of bytes are collected.
> +                */
> +               stop_address += 2;
> +       }
>
>         fmt = "%s -z -d --start-address=0x%"PRIx64" --stop-address=0x%"PRIx64" %s";
> -       ret = snprintf(cmd, sizeof(cmd), fmt, test_objdump_path, addr, addr + len,
> +       ret = snprintf(cmd, sizeof(cmd), fmt, test_objdump_path, addr, stop_address,
>                        filename);
>         if (ret <= 0 || (size_t)ret >= sizeof(cmd))
>                 return -1;
>
> --
> 2.34.1
>

Re: [PATCH 2/2] tools: perf: tests: Fix code reading for riscv

[Charlie Jenkins]

On Mon, Dec 16, 2024 at 08:57:20PM -0800, Ian Rogers wrote:
> On Mon, Dec 16, 2024 at 3:13 PM Charlie Jenkins <charlie@rivosinc.com> wrote:
> >
> > After binutils commit e43d876 which was first included in binutils 2.41,
> > riscv no longer supports dumping in the middle of instructions. Increase
> > the objdump window by 2-bytes to ensure that any instruction that sits
> > on the boundary of the specified stop-address is not cut in half.
> >
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > ---
> >  arch/riscv/Kconfig              |  5 +++++
> >  tools/perf/tests/code-reading.c | 17 ++++++++++++++++-
> 
> Files under tools use a different Build system than the kernel. The
> Kconfig value won't have an effect. Check out Makefile.config:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Makefile.config?h=perf-tools-next
> which is included into the build here:
> https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/Makefile.perf?h=perf-tools-next#n313
> 

Ahh okay, thank you. It was properly enabling when I was testing, is
there some bleeding over?

> >  2 files changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index d4a7ca0388c071b536df59c0eb11d55f9080c7cd..f164047471267936bc62389b7d7d9a7cbdca8f97 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -229,6 +229,11 @@ config GCC_SUPPORTS_DYNAMIC_FTRACE
> >         def_bool CC_IS_GCC
> >         depends on $(cc-option,-fpatchable-function-entry=8)
> >
> > +config RISCV_OBJDUMP_SUPPORTS_SPLIT_INSTRUCTION
> > +       # Some versions of objdump do not support dumping partial instructions
> > +       def_bool y
> > +       depends on !(OBJDUMP_IS_GNU && OBJDUMP_VERSION > 24100)
> > +
> >  config HAVE_SHADOW_CALL_STACK
> >         def_bool $(cc-option,-fsanitize=shadow-call-stack)
> >         # https://github.com/riscv-non-isa/riscv-elf-psabi-doc/commit/a484e843e6eeb51f0cb7b8819e50da6d2444d769
> > diff --git a/tools/perf/tests/code-reading.c b/tools/perf/tests/code-reading.c
> > index 27c82cfb7e7de42284bf5af9cf7594a3a963052e..605f4a8e1dbc00d8a572503f45053c2f30ad19e3 100644
> > --- a/tools/perf/tests/code-reading.c
> > +++ b/tools/perf/tests/code-reading.c
> > @@ -1,5 +1,6 @@
> >  // SPDX-License-Identifier: GPL-2.0
> >  #include <errno.h>
> > +#include <linux/kconfig.h>
> >  #include <linux/kernel.h>
> >  #include <linux/types.h>
> >  #include <inttypes.h>
> > @@ -183,9 +184,23 @@ static int read_via_objdump(const char *filename, u64 addr, void *buf,
> >         const char *fmt;
> >         FILE *f;
> >         int ret;
> > +       u64 stop_address = addr + len;
> > +
> > +       if (IS_ENABLED(__riscv) && !IS_ENABLED(CONFIG_RISCV_OBJDUMP_SUPPORTS_SPLIT_INSTRUCTION)) {
> 
> It would be nice if this could be a runtime rather than build time detected.

Hmm that is a good point. I will change this to check the version at
runtime.

- Charlie

> 
> Thanks,
> Ian
> 
> > +               /*
> > +                * On some versions of riscv objdump, dumping in the middle of
> > +                * instructions is not supported. riscv instructions are aligned along
> > +                * 2-byte intervals and can be either 2-bytes or 4-bytes. This makes it
> > +                * possible that the stop-address lands in the middle of a 4-byte
> > +                * instruction. Increase the stop_address by two to ensure an
> > +                * instruction is not cut in half, but leave the len as-is so only the
> > +                * expected number of bytes are collected.
> > +                */
> > +               stop_address += 2;
> > +       }
> >
> >         fmt = "%s -z -d --start-address=0x%"PRIx64" --stop-address=0x%"PRIx64" %s";
> > -       ret = snprintf(cmd, sizeof(cmd), fmt, test_objdump_path, addr, addr + len,
> > +       ret = snprintf(cmd, sizeof(cmd), fmt, test_objdump_path, addr, stop_address,
> >                        filename);
> >         if (ret <= 0 || (size_t)ret >= sizeof(cmd))
> >                 return -1;
> >
> > --
> > 2.34.1
> >

Re: [PATCH 1/2] kbuild: Check version of objdump

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 4449 bytes --]

On Mon, Dec 16, 2024 at 03:12:51PM -0800, Charlie Jenkins wrote:
> Similar to ld-version, add a way to check the version of objdump. This
> should most of the time end up being the binutils version or the llvm
> version.
> 
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>

This fails for allmodconfig and rv32_defconfig with clang. 19.1.1
according to Bjorn :)

Cheers,
Conor.

> ---
>  init/Kconfig               | 10 +++++++
>  scripts/Kconfig.include    |  6 ++++
>  scripts/objdump-version.sh | 69 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 85 insertions(+)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index a20e6efd3f0fbdd7f0df2448854cc30734a0ee4f..0b5d36f939e1de89c12ebdd61e4815015314d4f1 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -60,6 +60,16 @@ config LLD_VERSION
>  	default $(ld-version) if LD_IS_LLD
>  	default 0
>  
> +config OBJDUMP_IS_GNU
> +	def_bool $(success,test "$(objdump-name)" = objdump)
> +
> +config OBJDUMP_IS_LLVM
> +	def_bool $(success,test "$(objdump-name)" = llvm-objdump)
> +
> +config OBJDUMP_VERSION
> +	int
> +	default $(objdump-version)
> +
>  config RUSTC_VERSION
>  	int
>  	default $(rustc-version)
> diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> index 33193ca6e8030e659d6b321acaea1acd42c387a4..cb3e2d2564fea8cce780adb3be672c9596b7ccf2 100644
> --- a/scripts/Kconfig.include
> +++ b/scripts/Kconfig.include
> @@ -58,6 +58,12 @@ $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not supp
>  ld-name := $(shell,set -- $(ld-info) && echo $1)
>  ld-version := $(shell,set -- $(ld-info) && echo $2)
>  
> +# Get the objdump name, version, and error out if it is not supported.
> +objdump-info := $(shell,$(srctree)/scripts/objdump-version.sh $(OBJDUMP))
> +$(error-if,$(success,test -z "$(objdump-info)"),Sorry$(comma) this objdump is not supported.)
> +objdump-name := $(shell,set -- $(objdump-info) && echo $1)
> +objdump-version := $(shell,set -- $(objdump-info) && echo $2)
> +
>  # machine bit flags
>  #  $(m32-flag): -m32 if the compiler supports it, or an empty string otherwise.
>  #  $(m64-flag): -m64 if the compiler supports it, or an empty string otherwise.
> diff --git a/scripts/objdump-version.sh b/scripts/objdump-version.sh
> new file mode 100755
> index 0000000000000000000000000000000000000000..fa24f8dc2d3c42fd1195fceb3c96b27f7127db25
> --- /dev/null
> +++ b/scripts/objdump-version.sh
> @@ -0,0 +1,69 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Print the objdump name and its version in a 5 or 6-digit form.
> +# Also, perform the minimum version check.
> +
> +set -e
> +
> +# Convert the version string x.y.z to a canonical 5 or 6-digit form.
> +get_canonical_version()
> +{
> +	IFS=.
> +	set -- $1
> +
> +	# If the 2nd or 3rd field is missing, fill it with a zero.
> +	#
> +	# The 4th field, if present, is ignored.
> +	# This occurs in development snapshots as in 2.35.1.20201116
> +	echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0}))
> +}
> +
> +orig_args="$@"
> +
> +# Get the first line of the --version output.
> +IFS='
> +'
> +set -- $(LC_ALL=C "$@" --version)
> +
> +# Split the line on spaces.
> +IFS=' '
> +set -- $1
> +
> +min_tool_version=$(dirname $0)/min-tool-version.sh
> +
> +if [ "$1" = GNU -a "$2" = objdump ]; then
> +	shift $(($# - 1))
> +	version=$1
> +	min_version=$($min_tool_version binutils)
> +	disp_name="GNU objdump"
> +else
> +	while [ $# -gt 1 -a "$1" != "LLVM" ]; do
> +		shift
> +	done
> +
> +	if [ "$1" = LLVM ]; then
> +		version=$3
> +		min_version=$($min_tool_version llvm)
> +		disp_name="llvm-objdump"
> +	else
> +		echo "$orig_args: unknown objdump" >&2
> +		exit 1
> +	fi
> +fi
> +
> +version=${version%%[!0-9.]*}
> +
> +cversion=$(get_canonical_version $version)
> +min_cversion=$(get_canonical_version $min_version)
> +
> +if [ "$cversion" -lt "$min_cversion" ]; then
> +	echo >&2 "***"
> +	echo >&2 "*** objdump is too old."
> +	echo >&2 "***   Your $disp_name version:    $version"
> +	echo >&2 "***   Minimum $disp_name version: $min_version"
> +	echo >&2 "***"
> +	exit 1
> +fi
> +
> +echo objdump $cversion
> 
> -- 
> 2.34.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] kbuild: Check version of objdump

[Conor Dooley]

[-- Attachment #1: Type: text/plain, Size: 5310 bytes --]

On Wed, Dec 18, 2024 at 03:14:46PM +0000, Conor Dooley wrote:
> On Mon, Dec 16, 2024 at 03:12:51PM -0800, Charlie Jenkins wrote:
> > Similar to ld-version, add a way to check the version of objdump. This
> > should most of the time end up being the binutils version or the llvm
> > version.
> > 
> > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> 
> This fails for allmodconfig and rv32_defconfig with clang. 19.1.1
> according to Bjorn :)

Some additional info from Bjorn:
https://paste.debian.net/1340410
and the steps to reproduce:
https://paste.debian.net/1340408

That should not be reporting 13.0.1, it should be 19.1.x, there's one
included in the toolchains we use from https://mirrors.edge.kernel.org/pub/tools/llvm/

13.0.1 looks like a host toolchain?

> 
> Cheers,
> Conor.
> 
> > ---
> >  init/Kconfig               | 10 +++++++
> >  scripts/Kconfig.include    |  6 ++++
> >  scripts/objdump-version.sh | 69 ++++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 85 insertions(+)
> > 
> > diff --git a/init/Kconfig b/init/Kconfig
> > index a20e6efd3f0fbdd7f0df2448854cc30734a0ee4f..0b5d36f939e1de89c12ebdd61e4815015314d4f1 100644
> > --- a/init/Kconfig
> > +++ b/init/Kconfig
> > @@ -60,6 +60,16 @@ config LLD_VERSION
> >  	default $(ld-version) if LD_IS_LLD
> >  	default 0
> >  
> > +config OBJDUMP_IS_GNU
> > +	def_bool $(success,test "$(objdump-name)" = objdump)
> > +
> > +config OBJDUMP_IS_LLVM
> > +	def_bool $(success,test "$(objdump-name)" = llvm-objdump)
> > +
> > +config OBJDUMP_VERSION
> > +	int
> > +	default $(objdump-version)
> > +
> >  config RUSTC_VERSION
> >  	int
> >  	default $(rustc-version)
> > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> > index 33193ca6e8030e659d6b321acaea1acd42c387a4..cb3e2d2564fea8cce780adb3be672c9596b7ccf2 100644
> > --- a/scripts/Kconfig.include
> > +++ b/scripts/Kconfig.include
> > @@ -58,6 +58,12 @@ $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not supp
> >  ld-name := $(shell,set -- $(ld-info) && echo $1)
> >  ld-version := $(shell,set -- $(ld-info) && echo $2)
> >  
> > +# Get the objdump name, version, and error out if it is not supported.
> > +objdump-info := $(shell,$(srctree)/scripts/objdump-version.sh $(OBJDUMP))
> > +$(error-if,$(success,test -z "$(objdump-info)"),Sorry$(comma) this objdump is not supported.)
> > +objdump-name := $(shell,set -- $(objdump-info) && echo $1)
> > +objdump-version := $(shell,set -- $(objdump-info) && echo $2)
> > +
> >  # machine bit flags
> >  #  $(m32-flag): -m32 if the compiler supports it, or an empty string otherwise.
> >  #  $(m64-flag): -m64 if the compiler supports it, or an empty string otherwise.
> > diff --git a/scripts/objdump-version.sh b/scripts/objdump-version.sh
> > new file mode 100755
> > index 0000000000000000000000000000000000000000..fa24f8dc2d3c42fd1195fceb3c96b27f7127db25
> > --- /dev/null
> > +++ b/scripts/objdump-version.sh
> > @@ -0,0 +1,69 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0
> > +#
> > +# Print the objdump name and its version in a 5 or 6-digit form.
> > +# Also, perform the minimum version check.
> > +
> > +set -e
> > +
> > +# Convert the version string x.y.z to a canonical 5 or 6-digit form.
> > +get_canonical_version()
> > +{
> > +	IFS=.
> > +	set -- $1
> > +
> > +	# If the 2nd or 3rd field is missing, fill it with a zero.
> > +	#
> > +	# The 4th field, if present, is ignored.
> > +	# This occurs in development snapshots as in 2.35.1.20201116
> > +	echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0}))
> > +}
> > +
> > +orig_args="$@"
> > +
> > +# Get the first line of the --version output.
> > +IFS='
> > +'
> > +set -- $(LC_ALL=C "$@" --version)
> > +
> > +# Split the line on spaces.
> > +IFS=' '
> > +set -- $1
> > +
> > +min_tool_version=$(dirname $0)/min-tool-version.sh
> > +
> > +if [ "$1" = GNU -a "$2" = objdump ]; then
> > +	shift $(($# - 1))
> > +	version=$1
> > +	min_version=$($min_tool_version binutils)
> > +	disp_name="GNU objdump"
> > +else
> > +	while [ $# -gt 1 -a "$1" != "LLVM" ]; do
> > +		shift
> > +	done
> > +
> > +	if [ "$1" = LLVM ]; then
> > +		version=$3
> > +		min_version=$($min_tool_version llvm)
> > +		disp_name="llvm-objdump"
> > +	else
> > +		echo "$orig_args: unknown objdump" >&2
> > +		exit 1
> > +	fi
> > +fi
> > +
> > +version=${version%%[!0-9.]*}
> > +
> > +cversion=$(get_canonical_version $version)
> > +min_cversion=$(get_canonical_version $min_version)
> > +
> > +if [ "$cversion" -lt "$min_cversion" ]; then
> > +	echo >&2 "***"
> > +	echo >&2 "*** objdump is too old."
> > +	echo >&2 "***   Your $disp_name version:    $version"
> > +	echo >&2 "***   Minimum $disp_name version: $min_version"
> > +	echo >&2 "***"
> > +	exit 1
> > +fi
> > +
> > +echo objdump $cversion
> > 
> > -- 
> > 2.34.1
> > 
> > 
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv



> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] kbuild: Check version of objdump

[Charlie Jenkins]

On Wed, Dec 18, 2024 at 03:40:25PM +0000, Conor Dooley wrote:
> On Wed, Dec 18, 2024 at 03:14:46PM +0000, Conor Dooley wrote:
> > On Mon, Dec 16, 2024 at 03:12:51PM -0800, Charlie Jenkins wrote:
> > > Similar to ld-version, add a way to check the version of objdump. This
> > > should most of the time end up being the binutils version or the llvm
> > > version.
> > > 
> > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>
> > 
> > This fails for allmodconfig and rv32_defconfig with clang. 19.1.1
> > according to Bjorn :)
> 
> Some additional info from Bjorn:
> https://paste.debian.net/1340410
> and the steps to reproduce:
> https://paste.debian.net/1340408
> 
> That should not be reporting 13.0.1, it should be 19.1.x, there's one
> included in the toolchains we use from https://mirrors.edge.kernel.org/pub/tools/llvm/
> 
> 13.0.1 looks like a host toolchain?

I ended up sending a v2 that dropped this in favor of detecting this at
runtime [1] so this is no longer needed.

- Charlie

Link:
https://lore.kernel.org/lkml/20241217-perf_fix_riscv_obj_reading-v2-1-58f81b7b4c7d@rivosinc.com/
[1]

> 
> > 
> > Cheers,
> > Conor.
> > 
> > > ---
> > >  init/Kconfig               | 10 +++++++
> > >  scripts/Kconfig.include    |  6 ++++
> > >  scripts/objdump-version.sh | 69 ++++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 85 insertions(+)
> > > 
> > > diff --git a/init/Kconfig b/init/Kconfig
> > > index a20e6efd3f0fbdd7f0df2448854cc30734a0ee4f..0b5d36f939e1de89c12ebdd61e4815015314d4f1 100644
> > > --- a/init/Kconfig
> > > +++ b/init/Kconfig
> > > @@ -60,6 +60,16 @@ config LLD_VERSION
> > >  	default $(ld-version) if LD_IS_LLD
> > >  	default 0
> > >  
> > > +config OBJDUMP_IS_GNU
> > > +	def_bool $(success,test "$(objdump-name)" = objdump)
> > > +
> > > +config OBJDUMP_IS_LLVM
> > > +	def_bool $(success,test "$(objdump-name)" = llvm-objdump)
> > > +
> > > +config OBJDUMP_VERSION
> > > +	int
> > > +	default $(objdump-version)
> > > +
> > >  config RUSTC_VERSION
> > >  	int
> > >  	default $(rustc-version)
> > > diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> > > index 33193ca6e8030e659d6b321acaea1acd42c387a4..cb3e2d2564fea8cce780adb3be672c9596b7ccf2 100644
> > > --- a/scripts/Kconfig.include
> > > +++ b/scripts/Kconfig.include
> > > @@ -58,6 +58,12 @@ $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not supp
> > >  ld-name := $(shell,set -- $(ld-info) && echo $1)
> > >  ld-version := $(shell,set -- $(ld-info) && echo $2)
> > >  
> > > +# Get the objdump name, version, and error out if it is not supported.
> > > +objdump-info := $(shell,$(srctree)/scripts/objdump-version.sh $(OBJDUMP))
> > > +$(error-if,$(success,test -z "$(objdump-info)"),Sorry$(comma) this objdump is not supported.)
> > > +objdump-name := $(shell,set -- $(objdump-info) && echo $1)
> > > +objdump-version := $(shell,set -- $(objdump-info) && echo $2)
> > > +
> > >  # machine bit flags
> > >  #  $(m32-flag): -m32 if the compiler supports it, or an empty string otherwise.
> > >  #  $(m64-flag): -m64 if the compiler supports it, or an empty string otherwise.
> > > diff --git a/scripts/objdump-version.sh b/scripts/objdump-version.sh
> > > new file mode 100755
> > > index 0000000000000000000000000000000000000000..fa24f8dc2d3c42fd1195fceb3c96b27f7127db25
> > > --- /dev/null
> > > +++ b/scripts/objdump-version.sh
> > > @@ -0,0 +1,69 @@
> > > +#!/bin/sh
> > > +# SPDX-License-Identifier: GPL-2.0
> > > +#
> > > +# Print the objdump name and its version in a 5 or 6-digit form.
> > > +# Also, perform the minimum version check.
> > > +
> > > +set -e
> > > +
> > > +# Convert the version string x.y.z to a canonical 5 or 6-digit form.
> > > +get_canonical_version()
> > > +{
> > > +	IFS=.
> > > +	set -- $1
> > > +
> > > +	# If the 2nd or 3rd field is missing, fill it with a zero.
> > > +	#
> > > +	# The 4th field, if present, is ignored.
> > > +	# This occurs in development snapshots as in 2.35.1.20201116
> > > +	echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0}))
> > > +}
> > > +
> > > +orig_args="$@"
> > > +
> > > +# Get the first line of the --version output.
> > > +IFS='
> > > +'
> > > +set -- $(LC_ALL=C "$@" --version)
> > > +
> > > +# Split the line on spaces.
> > > +IFS=' '
> > > +set -- $1
> > > +
> > > +min_tool_version=$(dirname $0)/min-tool-version.sh
> > > +
> > > +if [ "$1" = GNU -a "$2" = objdump ]; then
> > > +	shift $(($# - 1))
> > > +	version=$1
> > > +	min_version=$($min_tool_version binutils)
> > > +	disp_name="GNU objdump"
> > > +else
> > > +	while [ $# -gt 1 -a "$1" != "LLVM" ]; do
> > > +		shift
> > > +	done
> > > +
> > > +	if [ "$1" = LLVM ]; then
> > > +		version=$3
> > > +		min_version=$($min_tool_version llvm)
> > > +		disp_name="llvm-objdump"
> > > +	else
> > > +		echo "$orig_args: unknown objdump" >&2
> > > +		exit 1
> > > +	fi
> > > +fi
> > > +
> > > +version=${version%%[!0-9.]*}
> > > +
> > > +cversion=$(get_canonical_version $version)
> > > +min_cversion=$(get_canonical_version $min_version)
> > > +
> > > +if [ "$cversion" -lt "$min_cversion" ]; then
> > > +	echo >&2 "***"
> > > +	echo >&2 "*** objdump is too old."
> > > +	echo >&2 "***   Your $disp_name version:    $version"
> > > +	echo >&2 "***   Minimum $disp_name version: $min_version"
> > > +	echo >&2 "***"
> > > +	exit 1
> > > +fi
> > > +
> > > +echo objdump $cversion
> > > 
> > > -- 
> > > 2.34.1
> > > 
> > > 
> > > _______________________________________________
> > > linux-riscv mailing list
> > > linux-riscv@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 
> 
> 
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
> 

Re: [PATCH 1/2] kbuild: Check version of objdump

[Masahiro Yamada]

On Tue, Dec 17, 2024 at 8:13 AM Charlie Jenkins <charlie@rivosinc.com> wrote:
>
> Similar to ld-version, add a way to check the version of objdump. This
> should most of the time end up being the binutils version or the llvm
> version.
>
> Signed-off-by: Charlie Jenkins <charlie@rivosinc.com>

This patch has no point because 2/2 includes CONFIG option
from the userspace "perf".




> ---
>  init/Kconfig               | 10 +++++++
>  scripts/Kconfig.include    |  6 ++++
>  scripts/objdump-version.sh | 69 ++++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 85 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index a20e6efd3f0fbdd7f0df2448854cc30734a0ee4f..0b5d36f939e1de89c12ebdd61e4815015314d4f1 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -60,6 +60,16 @@ config LLD_VERSION
>         default $(ld-version) if LD_IS_LLD
>         default 0
>
> +config OBJDUMP_IS_GNU
> +       def_bool $(success,test "$(objdump-name)" = objdump)
> +
> +config OBJDUMP_IS_LLVM
> +       def_bool $(success,test "$(objdump-name)" = llvm-objdump)
> +
> +config OBJDUMP_VERSION
> +       int
> +       default $(objdump-version)
> +
>  config RUSTC_VERSION
>         int
>         default $(rustc-version)
> diff --git a/scripts/Kconfig.include b/scripts/Kconfig.include
> index 33193ca6e8030e659d6b321acaea1acd42c387a4..cb3e2d2564fea8cce780adb3be672c9596b7ccf2 100644
> --- a/scripts/Kconfig.include
> +++ b/scripts/Kconfig.include
> @@ -58,6 +58,12 @@ $(error-if,$(success,test -z "$(ld-info)"),Sorry$(comma) this linker is not supp
>  ld-name := $(shell,set -- $(ld-info) && echo $1)
>  ld-version := $(shell,set -- $(ld-info) && echo $2)
>
> +# Get the objdump name, version, and error out if it is not supported.
> +objdump-info := $(shell,$(srctree)/scripts/objdump-version.sh $(OBJDUMP))
> +$(error-if,$(success,test -z "$(objdump-info)"),Sorry$(comma) this objdump is not supported.)
> +objdump-name := $(shell,set -- $(objdump-info) && echo $1)
> +objdump-version := $(shell,set -- $(objdump-info) && echo $2)
> +
>  # machine bit flags
>  #  $(m32-flag): -m32 if the compiler supports it, or an empty string otherwise.
>  #  $(m64-flag): -m64 if the compiler supports it, or an empty string otherwise.
> diff --git a/scripts/objdump-version.sh b/scripts/objdump-version.sh
> new file mode 100755
> index 0000000000000000000000000000000000000000..fa24f8dc2d3c42fd1195fceb3c96b27f7127db25
> --- /dev/null
> +++ b/scripts/objdump-version.sh
> @@ -0,0 +1,69 @@
> +#!/bin/sh
> +# SPDX-License-Identifier: GPL-2.0
> +#
> +# Print the objdump name and its version in a 5 or 6-digit form.
> +# Also, perform the minimum version check.
> +
> +set -e
> +
> +# Convert the version string x.y.z to a canonical 5 or 6-digit form.
> +get_canonical_version()
> +{
> +       IFS=.
> +       set -- $1
> +
> +       # If the 2nd or 3rd field is missing, fill it with a zero.
> +       #
> +       # The 4th field, if present, is ignored.
> +       # This occurs in development snapshots as in 2.35.1.20201116
> +       echo $((10000 * $1 + 100 * ${2:-0} + ${3:-0}))
> +}
> +
> +orig_args="$@"
> +
> +# Get the first line of the --version output.
> +IFS='
> +'
> +set -- $(LC_ALL=C "$@" --version)
> +
> +# Split the line on spaces.
> +IFS=' '
> +set -- $1
> +
> +min_tool_version=$(dirname $0)/min-tool-version.sh
> +
> +if [ "$1" = GNU -a "$2" = objdump ]; then
> +       shift $(($# - 1))
> +       version=$1
> +       min_version=$($min_tool_version binutils)
> +       disp_name="GNU objdump"
> +else
> +       while [ $# -gt 1 -a "$1" != "LLVM" ]; do
> +               shift
> +       done
> +
> +       if [ "$1" = LLVM ]; then
> +               version=$3
> +               min_version=$($min_tool_version llvm)
> +               disp_name="llvm-objdump"
> +       else
> +               echo "$orig_args: unknown objdump" >&2
> +               exit 1
> +       fi
> +fi
> +
> +version=${version%%[!0-9.]*}
> +
> +cversion=$(get_canonical_version $version)
> +min_cversion=$(get_canonical_version $min_version)
> +
> +if [ "$cversion" -lt "$min_cversion" ]; then
> +       echo >&2 "***"
> +       echo >&2 "*** objdump is too old."
> +       echo >&2 "***   Your $disp_name version:    $version"
> +       echo >&2 "***   Minimum $disp_name version: $min_version"
> +       echo >&2 "***"
> +       exit 1
> +fi
> +
> +echo objdump $cversion
>
> --
> 2.34.1
>


--
Best Regards


Masahiro Yamada

Re: [PATCH 1/2] kbuild: Check version of objdump

[kernel test robot]

Hi Charlie,

kernel test robot noticed the following build errors:

[auto build test ERROR on fac04efc5c793dccbd07e2d59af9f90b7fc0dca4]

url:    https://github.com/intel-lab-lkp/linux/commits/Charlie-Jenkins/kbuild-Check-version-of-objdump/20241217-071632
base:   fac04efc5c793dccbd07e2d59af9f90b7fc0dca4
patch link:    https://lore.kernel.org/r/20241216-perf_fix_riscv_obj_reading-v1-1-b75962660a9b%40rivosinc.com
patch subject: [PATCH 1/2] kbuild: Check version of objdump
config: x86_64-randconfig-003-20241223 (attached as .config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241224/202412240044.wVXW8Eg1-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202412240044.wVXW8Eg1-lkp@intel.com/

All errors (new ones prefixed by >>):

   ***
   *** objdump is too old.
   ***   Your llvm-objdump version:    
   ***   Minimum llvm-objdump version: 13.0.1
   ***
>> scripts/Kconfig.include:63: Sorry, this objdump is not supported.
   make[3]: *** [scripts/kconfig/Makefile:85: oldconfig] Error 1 shuffle=1831414175
   make[2]: *** [Makefile:733: oldconfig] Error 2 shuffle=1831414175
   make[1]: *** [Makefile:251: __sub-make] Error 2 shuffle=1831414175
   make[1]: Target 'oldconfig' not remade because of errors.
   make: *** [Makefile:251: __sub-make] Error 2 shuffle=1831414175
   make: Target 'oldconfig' not remade because of errors.
--
   ***
   *** objdump is too old.
   ***   Your llvm-objdump version:    
   ***   Minimum llvm-objdump version: 13.0.1
   ***
>> scripts/Kconfig.include:63: Sorry, this objdump is not supported.
   make[3]: *** [scripts/kconfig/Makefile:85: olddefconfig] Error 1 shuffle=1831414175
   make[2]: *** [Makefile:733: olddefconfig] Error 2 shuffle=1831414175
   make[1]: *** [Makefile:251: __sub-make] Error 2 shuffle=1831414175
   make[1]: Target 'olddefconfig' not remade because of errors.
   make: *** [Makefile:251: __sub-make] Error 2 shuffle=1831414175
   make: Target 'olddefconfig' not remade because of errors.


vim +63 scripts/Kconfig.include

    60	
    61	# Get the objdump name, version, and error out if it is not supported.
    62	objdump-info := $(shell,$(srctree)/scripts/objdump-version.sh $(OBJDUMP))
  > 63	$(error-if,$(success,test -z "$(objdump-info)"),Sorry$(comma) this objdump is not supported.)
    64	objdump-name := $(shell,set -- $(objdump-info) && echo $1)
    65	objdump-version := $(shell,set -- $(objdump-info) && echo $2)
    66	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki