| Message ID | 20241216-perf_fix_riscv_obj_reading-v1-2-b75962660a9b@rivosinc.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | perf: tests: Fix object code reading test for riscv | expand |
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 >
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 > >
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;
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(-)