* [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation
@ 2025-11-14 9:29 Namhyung Kim
2025-11-14 9:29 ` [PATCH 2/2] perf test: Add python JIT dump test Namhyung Kim
2025-11-14 17:33 ` [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation Ian Rogers
0 siblings, 2 replies; 11+ messages in thread
From: Namhyung Kim @ 2025-11-14 9:29 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark
Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML,
linux-perf-users, Eric Biggers, Pablo Galindo
It was reported that python backtrace with JIT dump was broken after the
change to built-in SHA-1 implementation. It seems python generates the
same JIT code for each function. They will become separate DSOs but the
contents are the same. Only difference is in the symbol name.
But this caused a problem that every JIT'ed DSOs will have the same
build-ID which makes perf confused. And it resulted in no python
symbols (from JIT) in the output.
Looking back at the original code before the conversion, it used the
load_addr as well as the code section to distinguish each DSO. I think
we should do the same or use symbol table as an additional input for
SHA-1.
This patch is a quick-and-dirty fix just to add each byte of the
load_addr to the first 8 bytes of SHA-1 result. Probably we need to add
sha1_update() or similar to update the existing hash value and use it
here. I'd like something that can be backported to the stable trees
easily.
Fixes: e3f612c1d8f3945b ("perf genelf: Remove libcrypto dependency and use built-in sha1()")
Cc: Eric Biggers <ebiggers@kernel.org>
Cc: Pablo Galindo <pablogsal@gmail.com>
Link: https://github.com/python/cpython/issues/139544
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
tools/perf/util/genelf.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
index 591548b10e34ef6a..a412e6faf70e37f3 100644
--- a/tools/perf/util/genelf.c
+++ b/tools/perf/util/genelf.c
@@ -395,6 +395,15 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym,
* build-id generation
*/
sha1(code, csize, bnote.build_id);
+ /* FIXME: update the SHA-1 hash using additional contents */
+ bnote.build_id[0] += (load_addr >> 0) & 0xff;
+ bnote.build_id[1] += (load_addr >> 8) & 0xff;
+ bnote.build_id[2] += (load_addr >> 16) & 0xff;
+ bnote.build_id[3] += (load_addr >> 24) & 0xff;
+ bnote.build_id[4] += (load_addr >> 32) & 0xff;
+ bnote.build_id[5] += (load_addr >> 40) & 0xff;
+ bnote.build_id[6] += (load_addr >> 48) & 0xff;
+ bnote.build_id[7] += (load_addr >> 56) & 0xff;
bnote.desc.namesz = sizeof(bnote.name); /* must include 0 termination */
bnote.desc.descsz = sizeof(bnote.build_id);
bnote.desc.type = NT_GNU_BUILD_ID;
--
2.52.0.rc1.455.g30608eb744-goog
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 2/2] perf test: Add python JIT dump test 2025-11-14 9:29 [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation Namhyung Kim @ 2025-11-14 9:29 ` Namhyung Kim 2025-11-14 17:44 ` Ian Rogers 2025-11-14 17:33 ` [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation Ian Rogers 1 sibling, 1 reply; 11+ messages in thread From: Namhyung Kim @ 2025-11-14 9:29 UTC (permalink / raw) To: Arnaldo Carvalho de Melo, Ian Rogers, James Clark Cc: Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Pablo Galindo Add a test case for the python interpreter like below so that we can make sure it won't break again. To validate the effect of build-ID generation, it adds and removes the JIT'ed DSOs to/from the build-ID cache for the test. $ perf test -vv jitdump 84: python profiling with jitdump: --- start --- test child forked, pid 214316 Run python with -Xperf_jit [ perf record: Woken up 5 times to write data ] [ perf record: Captured and wrote 1.180 MB /tmp/__perf_test.perf.data.XbqZNm (140 samples) ] Generate JIT-ed DSOs using perf inject Add JIT-ed DSOs to the build-ID cache Check the symbol containing the script name Found 108 matching lines Remove JIT-ed DSOs from the build-ID cache ---- end(0) ---- 84: python profiling with jitdump : Ok Cc: Pablo Galindo <pablogsal@gmail.com> Link: https://docs.python.org/3/howto/perf_profiling.html#how-to-work-without-frame-pointers Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/tests/shell/jitdump-python.sh | 41 ++++++++++++++++++++++++ tools/perf/tests/shell/jitdump-test.py | 14 ++++++++ 2 files changed, 55 insertions(+) create mode 100755 tools/perf/tests/shell/jitdump-python.sh create mode 100644 tools/perf/tests/shell/jitdump-test.py diff --git a/tools/perf/tests/shell/jitdump-python.sh b/tools/perf/tests/shell/jitdump-python.sh new file mode 100755 index 0000000000000000..101c15e65da1a0c0 --- /dev/null +++ b/tools/perf/tests/shell/jitdump-python.sh @@ -0,0 +1,41 @@ +#!/bin/bash +# python profiling with jitdump +# SPDX-License-Identifier: GPL-2.0 + +if ! command -v python > /dev/null; then + echo "Skip: no python found" + exit 2 +fi + +SHELLDIR=$(dirname $0) +PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXXX) + +echo "Run python with -Xperf_jit" +perf record -k 1 -g --call-graph dwarf -o "${PERF_DATA}" -- python -Xperf_jit ${SHELLDIR}/jitdump-test.py + +_PID=$(perf report -i "${PERF_DATA}" -F pid -q -g none | cut -d: -f1 -s) +PID=$(echo -n $_PID) # remove newlines + +echo "Generate JIT-ed DSOs using perf inject" +perf inject -i "${PERF_DATA}" -j -o "${PERF_DATA}.jit" + +echo "Add JIT-ed DSOs to the build-ID cache" +for F in /tmp/jitted-${PID}-*.so; do + perf buildid-cache -a "${F}" +done + +echo "Check the symbol containing the script name" +NUM=$(perf report -i "${PERF_DATA}.jit" -s sym | grep -c jitdump-test.py) + +echo "Found ${NUM} matching lines" + +echo "Remove JIT-ed DSOs from the build-ID cache" +for F in /tmp/jitted-${PID}-*.so; do + perf buildid-cache -r "${F}" +done + +rm -f ${PERF_DATA} ${PERF_DATA}.jit /tmp/jit-${PID}.dump /tmp/jitted-${PID}-*.so + +if [ "${NUM}" -eq 0 ]; then + exit 1 +fi diff --git a/tools/perf/tests/shell/jitdump-test.py b/tools/perf/tests/shell/jitdump-test.py new file mode 100644 index 0000000000000000..b427363ae4956db6 --- /dev/null +++ b/tools/perf/tests/shell/jitdump-test.py @@ -0,0 +1,14 @@ +def foo(n): + result = 0 + for _ in range(n): + result += 1 + return result + +def bar(n): + foo(n) + +def baz(n): + bar(n) + +if __name__ == "__main__": + baz(1000000) -- 2.52.0.rc1.455.g30608eb744-goog ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf test: Add python JIT dump test 2025-11-14 9:29 ` [PATCH 2/2] perf test: Add python JIT dump test Namhyung Kim @ 2025-11-14 17:44 ` Ian Rogers 2025-11-14 19:03 ` Namhyung Kim 0 siblings, 1 reply; 11+ messages in thread From: Ian Rogers @ 2025-11-14 17:44 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Pablo Galindo On Fri, Nov 14, 2025 at 1:29 AM Namhyung Kim <namhyung@kernel.org> wrote: > > Add a test case for the python interpreter like below so that we can > make sure it won't break again. To validate the effect of build-ID > generation, it adds and removes the JIT'ed DSOs to/from the build-ID > cache for the test. > > $ perf test -vv jitdump > 84: python profiling with jitdump: > --- start --- > test child forked, pid 214316 > Run python with -Xperf_jit > [ perf record: Woken up 5 times to write data ] > [ perf record: Captured and wrote 1.180 MB /tmp/__perf_test.perf.data.XbqZNm (140 samples) ] > Generate JIT-ed DSOs using perf inject > Add JIT-ed DSOs to the build-ID cache > Check the symbol containing the script name > Found 108 matching lines > Remove JIT-ed DSOs from the build-ID cache > ---- end(0) ---- > 84: python profiling with jitdump : Ok > > Cc: Pablo Galindo <pablogsal@gmail.com> > Link: https://docs.python.org/3/howto/perf_profiling.html#how-to-work-without-frame-pointers > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/tests/shell/jitdump-python.sh | 41 ++++++++++++++++++++++++ > tools/perf/tests/shell/jitdump-test.py | 14 ++++++++ > 2 files changed, 55 insertions(+) > create mode 100755 tools/perf/tests/shell/jitdump-python.sh > create mode 100644 tools/perf/tests/shell/jitdump-test.py > > diff --git a/tools/perf/tests/shell/jitdump-python.sh b/tools/perf/tests/shell/jitdump-python.sh > new file mode 100755 > index 0000000000000000..101c15e65da1a0c0 > --- /dev/null > +++ b/tools/perf/tests/shell/jitdump-python.sh > @@ -0,0 +1,41 @@ > +#!/bin/bash > +# python profiling with jitdump > +# SPDX-License-Identifier: GPL-2.0 > + > +if ! command -v python > /dev/null; then > + echo "Skip: no python found" > + exit 2 > +fi Can this just reuse the python set up logic in: https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/lib/setup_python.sh?h=perf-tools-next > + > +SHELLDIR=$(dirname $0) > +PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXXX) > + > +echo "Run python with -Xperf_jit" > +perf record -k 1 -g --call-graph dwarf -o "${PERF_DATA}" -- python -Xperf_jit ${SHELLDIR}/jitdump-test.py We can avoid an additional file by using the REPL, this is why the java test is using jshell: https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/test_java_symbol.sh?h=perf-tools-next#n41 > + > +_PID=$(perf report -i "${PERF_DATA}" -F pid -q -g none | cut -d: -f1 -s) > +PID=$(echo -n $_PID) # remove newlines > + > +echo "Generate JIT-ed DSOs using perf inject" > +perf inject -i "${PERF_DATA}" -j -o "${PERF_DATA}.jit" Thomas Richter did a workaround in the Java version to avoid debuginfod queries: https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/test_java_symbol.sh?h=perf-tools-next#n59 > + > +echo "Add JIT-ed DSOs to the build-ID cache" > +for F in /tmp/jitted-${PID}-*.so; do > + perf buildid-cache -a "${F}" > +done > + > +echo "Check the symbol containing the script name" > +NUM=$(perf report -i "${PERF_DATA}.jit" -s sym | grep -c jitdump-test.py) > + > +echo "Found ${NUM} matching lines" > + > +echo "Remove JIT-ed DSOs from the build-ID cache" > +for F in /tmp/jitted-${PID}-*.so; do > + perf buildid-cache -r "${F}" > +done > + > +rm -f ${PERF_DATA} ${PERF_DATA}.jit /tmp/jit-${PID}.dump /tmp/jitted-${PID}-*.so It'd be nice to have this in a function that also gets run if there is a trap. It looks like the Java test is missing a call to cleanup_files on the success path. Thanks, Ian > + > +if [ "${NUM}" -eq 0 ]; then > + exit 1 > +fi > diff --git a/tools/perf/tests/shell/jitdump-test.py b/tools/perf/tests/shell/jitdump-test.py > new file mode 100644 > index 0000000000000000..b427363ae4956db6 > --- /dev/null > +++ b/tools/perf/tests/shell/jitdump-test.py > @@ -0,0 +1,14 @@ > +def foo(n): > + result = 0 > + for _ in range(n): > + result += 1 > + return result > + > +def bar(n): > + foo(n) > + > +def baz(n): > + bar(n) > + > +if __name__ == "__main__": > + baz(1000000) > -- > 2.52.0.rc1.455.g30608eb744-goog > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/2] perf test: Add python JIT dump test 2025-11-14 17:44 ` Ian Rogers @ 2025-11-14 19:03 ` Namhyung Kim 0 siblings, 0 replies; 11+ messages in thread From: Namhyung Kim @ 2025-11-14 19:03 UTC (permalink / raw) To: Ian Rogers Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Pablo Galindo On Fri, Nov 14, 2025 at 09:44:21AM -0800, Ian Rogers wrote: > On Fri, Nov 14, 2025 at 1:29 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > Add a test case for the python interpreter like below so that we can > > make sure it won't break again. To validate the effect of build-ID > > generation, it adds and removes the JIT'ed DSOs to/from the build-ID > > cache for the test. > > > > $ perf test -vv jitdump > > 84: python profiling with jitdump: > > --- start --- > > test child forked, pid 214316 > > Run python with -Xperf_jit > > [ perf record: Woken up 5 times to write data ] > > [ perf record: Captured and wrote 1.180 MB /tmp/__perf_test.perf.data.XbqZNm (140 samples) ] > > Generate JIT-ed DSOs using perf inject > > Add JIT-ed DSOs to the build-ID cache > > Check the symbol containing the script name > > Found 108 matching lines > > Remove JIT-ed DSOs from the build-ID cache > > ---- end(0) ---- > > 84: python profiling with jitdump : Ok > > > > Cc: Pablo Galindo <pablogsal@gmail.com> > > Link: https://docs.python.org/3/howto/perf_profiling.html#how-to-work-without-frame-pointers > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > tools/perf/tests/shell/jitdump-python.sh | 41 ++++++++++++++++++++++++ > > tools/perf/tests/shell/jitdump-test.py | 14 ++++++++ > > 2 files changed, 55 insertions(+) > > create mode 100755 tools/perf/tests/shell/jitdump-python.sh > > create mode 100644 tools/perf/tests/shell/jitdump-test.py > > > > diff --git a/tools/perf/tests/shell/jitdump-python.sh b/tools/perf/tests/shell/jitdump-python.sh > > new file mode 100755 > > index 0000000000000000..101c15e65da1a0c0 > > --- /dev/null > > +++ b/tools/perf/tests/shell/jitdump-python.sh > > @@ -0,0 +1,41 @@ > > +#!/bin/bash > > +# python profiling with jitdump > > +# SPDX-License-Identifier: GPL-2.0 > > + > > +if ! command -v python > /dev/null; then > > + echo "Skip: no python found" > > + exit 2 > > +fi > > Can this just reuse the python set up logic in: > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/lib/setup_python.sh?h=perf-tools-next Will do! > > > + > > +SHELLDIR=$(dirname $0) > > +PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXXX) > > + > > +echo "Run python with -Xperf_jit" > > +perf record -k 1 -g --call-graph dwarf -o "${PERF_DATA}" -- python -Xperf_jit ${SHELLDIR}/jitdump-test.py > > We can avoid an additional file by using the REPL, this is why the > java test is using jshell: > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/test_java_symbol.sh?h=perf-tools-next#n41 I thought that too, but I wanted a filename to check the result. Hmm.. probably I can use a different pattern without it. > > > + > > +_PID=$(perf report -i "${PERF_DATA}" -F pid -q -g none | cut -d: -f1 -s) > > +PID=$(echo -n $_PID) # remove newlines > > + > > +echo "Generate JIT-ed DSOs using perf inject" > > +perf inject -i "${PERF_DATA}" -j -o "${PERF_DATA}.jit" > > Thomas Richter did a workaround in the Java version to avoid debuginfod queries: > https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/tests/shell/test_java_symbol.sh?h=perf-tools-next#n59 Yep, will add. > > > + > > +echo "Add JIT-ed DSOs to the build-ID cache" > > +for F in /tmp/jitted-${PID}-*.so; do > > + perf buildid-cache -a "${F}" > > +done > > + > > +echo "Check the symbol containing the script name" > > +NUM=$(perf report -i "${PERF_DATA}.jit" -s sym | grep -c jitdump-test.py) > > + > > +echo "Found ${NUM} matching lines" > > + > > +echo "Remove JIT-ed DSOs from the build-ID cache" > > +for F in /tmp/jitted-${PID}-*.so; do > > + perf buildid-cache -r "${F}" > > +done > > + > > +rm -f ${PERF_DATA} ${PERF_DATA}.jit /tmp/jit-${PID}.dump /tmp/jitted-${PID}-*.so > > It'd be nice to have this in a function that also gets run if there is > a trap. It looks like the Java test is missing a call to cleanup_files > on the success path. I thought that it will go the end of the script regardless of any command failures but we should protect it from Ctrl-C as well.. ok. Thanks for your review! Namhyung > > > + > > +if [ "${NUM}" -eq 0 ]; then > > + exit 1 > > +fi > > diff --git a/tools/perf/tests/shell/jitdump-test.py b/tools/perf/tests/shell/jitdump-test.py > > new file mode 100644 > > index 0000000000000000..b427363ae4956db6 > > --- /dev/null > > +++ b/tools/perf/tests/shell/jitdump-test.py > > @@ -0,0 +1,14 @@ > > +def foo(n): > > + result = 0 > > + for _ in range(n): > > + result += 1 > > + return result > > + > > +def bar(n): > > + foo(n) > > + > > +def baz(n): > > + bar(n) > > + > > +if __name__ == "__main__": > > + baz(1000000) > > -- > > 2.52.0.rc1.455.g30608eb744-goog > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation 2025-11-14 9:29 [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation Namhyung Kim 2025-11-14 9:29 ` [PATCH 2/2] perf test: Add python JIT dump test Namhyung Kim @ 2025-11-14 17:33 ` Ian Rogers 2025-11-14 18:57 ` Namhyung Kim 1 sibling, 1 reply; 11+ messages in thread From: Ian Rogers @ 2025-11-14 17:33 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Eric Biggers, Pablo Galindo On Fri, Nov 14, 2025 at 1:29 AM Namhyung Kim <namhyung@kernel.org> wrote: > > It was reported that python backtrace with JIT dump was broken after the > change to built-in SHA-1 implementation. It seems python generates the > same JIT code for each function. They will become separate DSOs but the > contents are the same. Only difference is in the symbol name. > > But this caused a problem that every JIT'ed DSOs will have the same > build-ID which makes perf confused. And it resulted in no python > symbols (from JIT) in the output. The lookup of a DSO involves the build ID and the filename. I'm confused as to why things weren't deduplicated and why no symbols rather than repeatedly the same symbol? > Looking back at the original code before the conversion, it used the > load_addr as well as the code section to distinguish each DSO. I think > we should do the same or use symbol table as an additional input for > SHA-1. Hmm.. the build ID for the contents of the code should be a constant. As the build ID is a note for the entire ELF file then something is wrong with the filename handling it seems. Thanks, Ian > This patch is a quick-and-dirty fix just to add each byte of the > load_addr to the first 8 bytes of SHA-1 result. Probably we need to add > sha1_update() or similar to update the existing hash value and use it > here. I'd like something that can be backported to the stable trees > easily. > > Fixes: e3f612c1d8f3945b ("perf genelf: Remove libcrypto dependency and use built-in sha1()") > Cc: Eric Biggers <ebiggers@kernel.org> > Cc: Pablo Galindo <pablogsal@gmail.com> > Link: https://github.com/python/cpython/issues/139544 > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/util/genelf.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c > index 591548b10e34ef6a..a412e6faf70e37f3 100644 > --- a/tools/perf/util/genelf.c > +++ b/tools/perf/util/genelf.c > @@ -395,6 +395,15 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym, > * build-id generation > */ > sha1(code, csize, bnote.build_id); > + /* FIXME: update the SHA-1 hash using additional contents */ > + bnote.build_id[0] += (load_addr >> 0) & 0xff; > + bnote.build_id[1] += (load_addr >> 8) & 0xff; > + bnote.build_id[2] += (load_addr >> 16) & 0xff; > + bnote.build_id[3] += (load_addr >> 24) & 0xff; > + bnote.build_id[4] += (load_addr >> 32) & 0xff; > + bnote.build_id[5] += (load_addr >> 40) & 0xff; > + bnote.build_id[6] += (load_addr >> 48) & 0xff; > + bnote.build_id[7] += (load_addr >> 56) & 0xff; > bnote.desc.namesz = sizeof(bnote.name); /* must include 0 termination */ > bnote.desc.descsz = sizeof(bnote.build_id); > bnote.desc.type = NT_GNU_BUILD_ID; > -- > 2.52.0.rc1.455.g30608eb744-goog > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation 2025-11-14 17:33 ` [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation Ian Rogers @ 2025-11-14 18:57 ` Namhyung Kim 2025-11-14 19:32 ` Ian Rogers 0 siblings, 1 reply; 11+ messages in thread From: Namhyung Kim @ 2025-11-14 18:57 UTC (permalink / raw) To: Ian Rogers Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Eric Biggers, Pablo Galindo On Fri, Nov 14, 2025 at 09:33:29AM -0800, Ian Rogers wrote: > On Fri, Nov 14, 2025 at 1:29 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > It was reported that python backtrace with JIT dump was broken after the > > change to built-in SHA-1 implementation. It seems python generates the > > same JIT code for each function. They will become separate DSOs but the > > contents are the same. Only difference is in the symbol name. > > > > But this caused a problem that every JIT'ed DSOs will have the same > > build-ID which makes perf confused. And it resulted in no python > > symbols (from JIT) in the output. > > The lookup of a DSO involves the build ID and the filename. I'm > confused as to why things weren't deduplicated and why no symbols > rather than repeatedly the same symbol? I don't know, but that's the symptom in the original bug report in the python github (see Links: below). I guess the behavior is non-deterministic. > > > Looking back at the original code before the conversion, it used the > > load_addr as well as the code section to distinguish each DSO. I think > > we should do the same or use symbol table as an additional input for > > SHA-1. > > Hmm.. the build ID for the contents of the code should be a constant. > As the build ID is a note for the entire ELF file then something is > wrong with the filename handling it seems. When it tries to load symbols from a DSO, it prefer reading from the build-ID cache than the file system since it trusts build-IDs more than the path name. See dso__load() and binary_type_symtab[]. So having multiple DSO's with the same build-ID can be a problem if they are in the build-ID cache. Normally `perf inject -j` won't add the new JIT-ed DSOs to the build-ID cache but it's still possible. Thanks, Namhyung > > > This patch is a quick-and-dirty fix just to add each byte of the > > load_addr to the first 8 bytes of SHA-1 result. Probably we need to add > > sha1_update() or similar to update the existing hash value and use it > > here. I'd like something that can be backported to the stable trees > > easily. > > > > Fixes: e3f612c1d8f3945b ("perf genelf: Remove libcrypto dependency and use built-in sha1()") > > Cc: Eric Biggers <ebiggers@kernel.org> > > Cc: Pablo Galindo <pablogsal@gmail.com> > > Link: https://github.com/python/cpython/issues/139544 > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > tools/perf/util/genelf.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c > > index 591548b10e34ef6a..a412e6faf70e37f3 100644 > > --- a/tools/perf/util/genelf.c > > +++ b/tools/perf/util/genelf.c > > @@ -395,6 +395,15 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym, > > * build-id generation > > */ > > sha1(code, csize, bnote.build_id); > > + /* FIXME: update the SHA-1 hash using additional contents */ > > + bnote.build_id[0] += (load_addr >> 0) & 0xff; > > + bnote.build_id[1] += (load_addr >> 8) & 0xff; > > + bnote.build_id[2] += (load_addr >> 16) & 0xff; > > + bnote.build_id[3] += (load_addr >> 24) & 0xff; > > + bnote.build_id[4] += (load_addr >> 32) & 0xff; > > + bnote.build_id[5] += (load_addr >> 40) & 0xff; > > + bnote.build_id[6] += (load_addr >> 48) & 0xff; > > + bnote.build_id[7] += (load_addr >> 56) & 0xff; > > bnote.desc.namesz = sizeof(bnote.name); /* must include 0 termination */ > > bnote.desc.descsz = sizeof(bnote.build_id); > > bnote.desc.type = NT_GNU_BUILD_ID; > > -- > > 2.52.0.rc1.455.g30608eb744-goog > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation 2025-11-14 18:57 ` Namhyung Kim @ 2025-11-14 19:32 ` Ian Rogers 2025-11-14 23:24 ` Namhyung Kim 2025-11-16 7:22 ` Fangrui Song 0 siblings, 2 replies; 11+ messages in thread From: Ian Rogers @ 2025-11-14 19:32 UTC (permalink / raw) To: Namhyung Kim, maskray Cc: Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Eric Biggers, Pablo Galindo On Fri, Nov 14, 2025 at 10:57 AM Namhyung Kim <namhyung@kernel.org> wrote: > > On Fri, Nov 14, 2025 at 09:33:29AM -0800, Ian Rogers wrote: > > On Fri, Nov 14, 2025 at 1:29 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > It was reported that python backtrace with JIT dump was broken after the > > > change to built-in SHA-1 implementation. It seems python generates the > > > same JIT code for each function. They will become separate DSOs but the > > > contents are the same. Only difference is in the symbol name. > > > > > > But this caused a problem that every JIT'ed DSOs will have the same > > > build-ID which makes perf confused. And it resulted in no python > > > symbols (from JIT) in the output. > > > > The lookup of a DSO involves the build ID and the filename. I'm > > confused as to why things weren't deduplicated and why no symbols > > rather than repeatedly the same symbol? > > I don't know, but that's the symptom in the original bug report in the > python github (see Links: below). I guess the behavior is > non-deterministic. > > > > > > Looking back at the original code before the conversion, it used the > > > load_addr as well as the code section to distinguish each DSO. I think > > > we should do the same or use symbol table as an additional input for > > > SHA-1. > > > > Hmm.. the build ID for the contents of the code should be a constant. > > As the build ID is a note for the entire ELF file then something is > > wrong with the filename handling it seems. > > When it tries to load symbols from a DSO, it prefer reading from the > build-ID cache than the file system since it trusts build-IDs more than > the path name. See dso__load() and binary_type_symtab[]. > > So having multiple DSO's with the same build-ID can be a problem if they > are in the build-ID cache. Normally `perf inject -j` won't add the new > JIT-ed DSOs to the build-ID cache but it's still possible. +Fangrui I'm surprised that build IDs don't include symbol names but: ``` $ cat a.s .text .global main .global foo main: foo: ret $ cat b.s .text .global main .global bar main: bar: ret $ gcc -Wl,--build-id a.s -o a.out $ gcc -Wl,--build-id b.s -o b.out $ readelf -n a.out ... Build ID: 9dd0371b953db5d72929af5d98552e4ee1043616 ... $ readelf -n b.out ... Build ID: 9dd0371b953db5d72929af5d98552e4ee1043616 ... ``` so ugh. Perhaps we need to have jitdump make a single object file (and so 1 build ID) but with multiple unique symbols. Thanks, Ian > Thanks, > Namhyung > > > > > > This patch is a quick-and-dirty fix just to add each byte of the > > > load_addr to the first 8 bytes of SHA-1 result. Probably we need to add > > > sha1_update() or similar to update the existing hash value and use it > > > here. I'd like something that can be backported to the stable trees > > > easily. > > > > > > Fixes: e3f612c1d8f3945b ("perf genelf: Remove libcrypto dependency and use built-in sha1()") > > > Cc: Eric Biggers <ebiggers@kernel.org> > > > Cc: Pablo Galindo <pablogsal@gmail.com> > > > Link: https://github.com/python/cpython/issues/139544 > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > > --- > > > tools/perf/util/genelf.c | 9 +++++++++ > > > 1 file changed, 9 insertions(+) > > > > > > diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c > > > index 591548b10e34ef6a..a412e6faf70e37f3 100644 > > > --- a/tools/perf/util/genelf.c > > > +++ b/tools/perf/util/genelf.c > > > @@ -395,6 +395,15 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym, > > > * build-id generation > > > */ > > > sha1(code, csize, bnote.build_id); > > > + /* FIXME: update the SHA-1 hash using additional contents */ > > > + bnote.build_id[0] += (load_addr >> 0) & 0xff; > > > + bnote.build_id[1] += (load_addr >> 8) & 0xff; > > > + bnote.build_id[2] += (load_addr >> 16) & 0xff; > > > + bnote.build_id[3] += (load_addr >> 24) & 0xff; > > > + bnote.build_id[4] += (load_addr >> 32) & 0xff; > > > + bnote.build_id[5] += (load_addr >> 40) & 0xff; > > > + bnote.build_id[6] += (load_addr >> 48) & 0xff; > > > + bnote.build_id[7] += (load_addr >> 56) & 0xff; > > > bnote.desc.namesz = sizeof(bnote.name); /* must include 0 termination */ > > > bnote.desc.descsz = sizeof(bnote.build_id); > > > bnote.desc.type = NT_GNU_BUILD_ID; > > > -- > > > 2.52.0.rc1.455.g30608eb744-goog > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation 2025-11-14 19:32 ` Ian Rogers @ 2025-11-14 23:24 ` Namhyung Kim 2025-11-14 23:58 ` Ian Rogers 2025-11-16 7:22 ` Fangrui Song 1 sibling, 1 reply; 11+ messages in thread From: Namhyung Kim @ 2025-11-14 23:24 UTC (permalink / raw) To: Ian Rogers Cc: maskray, Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Eric Biggers, Pablo Galindo On Fri, Nov 14, 2025 at 11:32:52AM -0800, Ian Rogers wrote: > On Fri, Nov 14, 2025 at 10:57 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > On Fri, Nov 14, 2025 at 09:33:29AM -0800, Ian Rogers wrote: > > > On Fri, Nov 14, 2025 at 1:29 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > > > It was reported that python backtrace with JIT dump was broken after the > > > > change to built-in SHA-1 implementation. It seems python generates the > > > > same JIT code for each function. They will become separate DSOs but the > > > > contents are the same. Only difference is in the symbol name. > > > > > > > > But this caused a problem that every JIT'ed DSOs will have the same > > > > build-ID which makes perf confused. And it resulted in no python > > > > symbols (from JIT) in the output. > > > > > > The lookup of a DSO involves the build ID and the filename. I'm > > > confused as to why things weren't deduplicated and why no symbols > > > rather than repeatedly the same symbol? > > > > I don't know, but that's the symptom in the original bug report in the > > python github (see Links: below). I guess the behavior is > > non-deterministic. > > > > > > > > > Looking back at the original code before the conversion, it used the > > > > load_addr as well as the code section to distinguish each DSO. I think > > > > we should do the same or use symbol table as an additional input for > > > > SHA-1. > > > > > > Hmm.. the build ID for the contents of the code should be a constant. > > > As the build ID is a note for the entire ELF file then something is > > > wrong with the filename handling it seems. > > > > When it tries to load symbols from a DSO, it prefer reading from the > > build-ID cache than the file system since it trusts build-IDs more than > > the path name. See dso__load() and binary_type_symtab[]. > > > > So having multiple DSO's with the same build-ID can be a problem if they > > are in the build-ID cache. Normally `perf inject -j` won't add the new > > JIT-ed DSOs to the build-ID cache but it's still possible. > > +Fangrui > > I'm surprised that build IDs don't include symbol names but: > ``` > $ cat a.s > .text > .global main > .global foo > main: > foo: > ret > $ cat b.s > .text > .global main > .global bar > main: > bar: > ret > $ gcc -Wl,--build-id a.s -o a.out > $ gcc -Wl,--build-id b.s -o b.out > $ readelf -n a.out > ... > Build ID: 9dd0371b953db5d72929af5d98552e4ee1043616 > ... > $ readelf -n b.out > ... > Build ID: 9dd0371b953db5d72929af5d98552e4ee1043616 > ... > ``` > so ugh. Perhaps we need to have jitdump make a single object file (and > so 1 build ID) but with multiple unique symbols. Right, that'd be better. But I'm afraid some JIT code could spread to many segments so it's not possible to create a map to cover all areas due to conflicts with other libraries. Thanks, Namhyung ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation 2025-11-14 23:24 ` Namhyung Kim @ 2025-11-14 23:58 ` Ian Rogers 0 siblings, 0 replies; 11+ messages in thread From: Ian Rogers @ 2025-11-14 23:58 UTC (permalink / raw) To: Namhyung Kim Cc: maskray, Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Eric Biggers, Pablo Galindo On Fri, Nov 14, 2025 at 3:24 PM Namhyung Kim <namhyung@kernel.org> wrote: > > On Fri, Nov 14, 2025 at 11:32:52AM -0800, Ian Rogers wrote: > > On Fri, Nov 14, 2025 at 10:57 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > On Fri, Nov 14, 2025 at 09:33:29AM -0800, Ian Rogers wrote: > > > > On Fri, Nov 14, 2025 at 1:29 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > > > > > It was reported that python backtrace with JIT dump was broken after the > > > > > change to built-in SHA-1 implementation. It seems python generates the > > > > > same JIT code for each function. They will become separate DSOs but the > > > > > contents are the same. Only difference is in the symbol name. > > > > > > > > > > But this caused a problem that every JIT'ed DSOs will have the same > > > > > build-ID which makes perf confused. And it resulted in no python > > > > > symbols (from JIT) in the output. > > > > > > > > The lookup of a DSO involves the build ID and the filename. I'm > > > > confused as to why things weren't deduplicated and why no symbols > > > > rather than repeatedly the same symbol? > > > > > > I don't know, but that's the symptom in the original bug report in the > > > python github (see Links: below). I guess the behavior is > > > non-deterministic. > > > > > > > > > > > > Looking back at the original code before the conversion, it used the > > > > > load_addr as well as the code section to distinguish each DSO. I think > > > > > we should do the same or use symbol table as an additional input for > > > > > SHA-1. > > > > > > > > Hmm.. the build ID for the contents of the code should be a constant. > > > > As the build ID is a note for the entire ELF file then something is > > > > wrong with the filename handling it seems. > > > > > > When it tries to load symbols from a DSO, it prefer reading from the > > > build-ID cache than the file system since it trusts build-IDs more than > > > the path name. See dso__load() and binary_type_symtab[]. > > > > > > So having multiple DSO's with the same build-ID can be a problem if they > > > are in the build-ID cache. Normally `perf inject -j` won't add the new > > > JIT-ed DSOs to the build-ID cache but it's still possible. > > > > +Fangrui > > > > I'm surprised that build IDs don't include symbol names but: > > ``` > > $ cat a.s > > .text > > .global main > > .global foo > > main: > > foo: > > ret > > $ cat b.s > > .text > > .global main > > .global bar > > main: > > bar: > > ret > > $ gcc -Wl,--build-id a.s -o a.out > > $ gcc -Wl,--build-id b.s -o b.out > > $ readelf -n a.out > > ... > > Build ID: 9dd0371b953db5d72929af5d98552e4ee1043616 > > ... > > $ readelf -n b.out > > ... > > Build ID: 9dd0371b953db5d72929af5d98552e4ee1043616 > > ... > > ``` > > so ugh. Perhaps we need to have jitdump make a single object file (and > > so 1 build ID) but with multiple unique symbols. > > Right, that'd be better. But I'm afraid some JIT code could spread to > many segments so it's not possible to create a map to cover all areas > due to conflicts with other libraries. I'm not familiar with any JITs doing this. JITs often have to run on Windows where there is a reserve for the code cache and then commits to actually use individual pages. On Linux you can do a large PROT_NONE mmap and then mprotect pages within this code cache region (we should have perf events on this). Apple has some optimizations with the MAP_JIT mmap flag. SBCL has a code cache within the executable itself. On x86 you could spread code around within +/-2GB, on RISC it'd be a pain due to offset limitations. Anyway, it sounds like something is off when we are writing the executable and likely having too many of them. There is also something off with dsos__find as it should only give a dso if the name and build ID are matching. The sample should have an IP we turn into a map within maps, the dso of the map should differ for each of the call chain IPs (even though they are < page_size()) and the symbol lookup should work. If we're not going to maintain that jitdump build IDs behave like build IDs then there is little point using a SHA-1 hash and patch series like: https://lore.kernel.org/lkml/20251016205126.2882625-1-irogers@google.com/ are even more tedious to land :-) Thanks, Ian ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation 2025-11-14 19:32 ` Ian Rogers 2025-11-14 23:24 ` Namhyung Kim @ 2025-11-16 7:22 ` Fangrui Song 2025-11-17 16:58 ` Ian Rogers 1 sibling, 1 reply; 11+ messages in thread From: Fangrui Song @ 2025-11-16 7:22 UTC (permalink / raw) To: Ian Rogers Cc: Namhyung Kim, maskray, Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Eric Biggers, Pablo Galindo On Fri, Nov 14, 2025 at 11:33 AM Ian Rogers <irogers@google.com> wrote: > > On Fri, Nov 14, 2025 at 10:57 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > On Fri, Nov 14, 2025 at 09:33:29AM -0800, Ian Rogers wrote: > > > On Fri, Nov 14, 2025 at 1:29 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > > > It was reported that python backtrace with JIT dump was broken after the > > > > change to built-in SHA-1 implementation. It seems python generates the > > > > same JIT code for each function. They will become separate DSOs but the > > > > contents are the same. Only difference is in the symbol name. > > > > > > > > But this caused a problem that every JIT'ed DSOs will have the same > > > > build-ID which makes perf confused. And it resulted in no python > > > > symbols (from JIT) in the output. > > > > > > The lookup of a DSO involves the build ID and the filename. I'm > > > confused as to why things weren't deduplicated and why no symbols > > > rather than repeatedly the same symbol? > > > > I don't know, but that's the symptom in the original bug report in the > > python github (see Links: below). I guess the behavior is > > non-deterministic. > > > > > > > > > Looking back at the original code before the conversion, it used the > > > > load_addr as well as the code section to distinguish each DSO. I think > > > > we should do the same or use symbol table as an additional input for > > > > SHA-1. > > > > > > Hmm.. the build ID for the contents of the code should be a constant. > > > As the build ID is a note for the entire ELF file then something is > > > wrong with the filename handling it seems. > > > > When it tries to load symbols from a DSO, it prefer reading from the > > build-ID cache than the file system since it trusts build-IDs more than > > the path name. See dso__load() and binary_type_symtab[]. > > > > So having multiple DSO's with the same build-ID can be a problem if they > > are in the build-ID cache. Normally `perf inject -j` won't add the new > > JIT-ed DSOs to the build-ID cache but it's still possible. > > +Fangrui > > I'm surprised that build IDs don't include symbol names but: > ``` > $ cat a.s > .text > .global main > .global foo > main: > foo: > ret > $ cat b.s > .text > .global main > .global bar > main: > bar: > ret > $ gcc -Wl,--build-id a.s -o a.out > $ gcc -Wl,--build-id b.s -o b.out > $ readelf -n a.out > ... > Build ID: 9dd0371b953db5d72929af5d98552e4ee1043616 > ... > $ readelf -n b.out > ... > Build ID: 9dd0371b953db5d72929af5d98552e4ee1043616 > ... > ``` > so ugh. Perhaps we need to have jitdump make a single object file (and > so 1 build ID) but with multiple unique symbols. > > Thanks, > Ian Looks like a GNU ld issue. Other linkers should give different build IDs. .symtab/.strtab/.shstrtab are special sections and BFD seem to skip their content for the build ID. Raised https://sourceware.org/bugzilla/show_bug.cgi?id=33633 ("ld --build-id does not use symtab/strtab content") > > > > > Thanks, > > Namhyung > > > > > > > > > This patch is a quick-and-dirty fix just to add each byte of the > > > > load_addr to the first 8 bytes of SHA-1 result. Probably we need to add > > > > sha1_update() or similar to update the existing hash value and use it > > > > here. I'd like something that can be backported to the stable trees > > > > easily. > > > > > > > > Fixes: e3f612c1d8f3945b ("perf genelf: Remove libcrypto dependency and use built-in sha1()") > > > > Cc: Eric Biggers <ebiggers@kernel.org> > > > > Cc: Pablo Galindo <pablogsal@gmail.com> > > > > Link: https://github.com/python/cpython/issues/139544 > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > > > --- > > > > tools/perf/util/genelf.c | 9 +++++++++ > > > > 1 file changed, 9 insertions(+) > > > > > > > > diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c > > > > index 591548b10e34ef6a..a412e6faf70e37f3 100644 > > > > --- a/tools/perf/util/genelf.c > > > > +++ b/tools/perf/util/genelf.c > > > > @@ -395,6 +395,15 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym, > > > > * build-id generation > > > > */ > > > > sha1(code, csize, bnote.build_id); > > > > + /* FIXME: update the SHA-1 hash using additional contents */ > > > > + bnote.build_id[0] += (load_addr >> 0) & 0xff; > > > > + bnote.build_id[1] += (load_addr >> 8) & 0xff; > > > > + bnote.build_id[2] += (load_addr >> 16) & 0xff; > > > > + bnote.build_id[3] += (load_addr >> 24) & 0xff; > > > > + bnote.build_id[4] += (load_addr >> 32) & 0xff; > > > > + bnote.build_id[5] += (load_addr >> 40) & 0xff; > > > > + bnote.build_id[6] += (load_addr >> 48) & 0xff; > > > > + bnote.build_id[7] += (load_addr >> 56) & 0xff; > > > > bnote.desc.namesz = sizeof(bnote.name); /* must include 0 termination */ > > > > bnote.desc.descsz = sizeof(bnote.build_id); > > > > bnote.desc.type = NT_GNU_BUILD_ID; > > > > -- > > > > 2.52.0.rc1.455.g30608eb744-goog > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation 2025-11-16 7:22 ` Fangrui Song @ 2025-11-17 16:58 ` Ian Rogers 0 siblings, 0 replies; 11+ messages in thread From: Ian Rogers @ 2025-11-17 16:58 UTC (permalink / raw) To: Fangrui Song Cc: Namhyung Kim, Arnaldo Carvalho de Melo, James Clark, Jiri Olsa, Adrian Hunter, Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users, Eric Biggers, Pablo Galindo On Sat, Nov 15, 2025 at 11:21 PM Fangrui Song <maskray@sourceware.org> wrote: > > On Fri, Nov 14, 2025 at 11:33 AM Ian Rogers <irogers@google.com> wrote: > > > > On Fri, Nov 14, 2025 at 10:57 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > On Fri, Nov 14, 2025 at 09:33:29AM -0800, Ian Rogers wrote: > > > > On Fri, Nov 14, 2025 at 1:29 AM Namhyung Kim <namhyung@kernel.org> wrote: > > > > > > > > > > It was reported that python backtrace with JIT dump was broken after the > > > > > change to built-in SHA-1 implementation. It seems python generates the > > > > > same JIT code for each function. They will become separate DSOs but the > > > > > contents are the same. Only difference is in the symbol name. > > > > > > > > > > But this caused a problem that every JIT'ed DSOs will have the same > > > > > build-ID which makes perf confused. And it resulted in no python > > > > > symbols (from JIT) in the output. > > > > > > > > The lookup of a DSO involves the build ID and the filename. I'm > > > > confused as to why things weren't deduplicated and why no symbols > > > > rather than repeatedly the same symbol? > > > > > > I don't know, but that's the symptom in the original bug report in the > > > python github (see Links: below). I guess the behavior is > > > non-deterministic. > > > > > > > > > > > > Looking back at the original code before the conversion, it used the > > > > > load_addr as well as the code section to distinguish each DSO. I think > > > > > we should do the same or use symbol table as an additional input for > > > > > SHA-1. > > > > > > > > Hmm.. the build ID for the contents of the code should be a constant. > > > > As the build ID is a note for the entire ELF file then something is > > > > wrong with the filename handling it seems. > > > > > > When it tries to load symbols from a DSO, it prefer reading from the > > > build-ID cache than the file system since it trusts build-IDs more than > > > the path name. See dso__load() and binary_type_symtab[]. > > > > > > So having multiple DSO's with the same build-ID can be a problem if they > > > are in the build-ID cache. Normally `perf inject -j` won't add the new > > > JIT-ed DSOs to the build-ID cache but it's still possible. > > > > +Fangrui > > > > I'm surprised that build IDs don't include symbol names but: > > ``` > > $ cat a.s > > .text > > .global main > > .global foo > > main: > > foo: > > ret > > $ cat b.s > > .text > > .global main > > .global bar > > main: > > bar: > > ret > > $ gcc -Wl,--build-id a.s -o a.out > > $ gcc -Wl,--build-id b.s -o b.out > > $ readelf -n a.out > > ... > > Build ID: 9dd0371b953db5d72929af5d98552e4ee1043616 > > ... > > $ readelf -n b.out > > ... > > Build ID: 9dd0371b953db5d72929af5d98552e4ee1043616 > > ... > > ``` > > so ugh. Perhaps we need to have jitdump make a single object file (and > > so 1 build ID) but with multiple unique symbols. > > > > Thanks, > > Ian > > Looks like a GNU ld issue. Other linkers should give different build IDs. > > .symtab/.strtab/.shstrtab are special sections and BFD seem to skip > their content for the build ID. > Raised https://sourceware.org/bugzilla/show_bug.cgi?id=33633 ("ld > --build-id does not use symtab/strtab content") Thanks Fangrui! I repeated my test above and found for GNU ld and bfd the build IDs didn't vary, but for gold and lld they do vary. So it looks like in perf we need to update the sha1 calculation: https://web.git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/genelf.c?h=perf-tools-next#n397 to include at least the symbol information. I don't know if it is specified how the build ID should be calculated. It seems like it should be a specified thing to stop tampering with files, but I also see each linker produce a different value for my test. I'm wondering if the sha1 calculation should really be including other sections in the jitdump generated ELF files? For example, the .eh_frame could be useful as that has unwinding implications. Thanks, Ian > > > > > > > > > Thanks, > > > Namhyung > > > > > > > > > > > > This patch is a quick-and-dirty fix just to add each byte of the > > > > > load_addr to the first 8 bytes of SHA-1 result. Probably we need to add > > > > > sha1_update() or similar to update the existing hash value and use it > > > > > here. I'd like something that can be backported to the stable trees > > > > > easily. > > > > > > > > > > Fixes: e3f612c1d8f3945b ("perf genelf: Remove libcrypto dependency and use built-in sha1()") > > > > > Cc: Eric Biggers <ebiggers@kernel.org> > > > > > Cc: Pablo Galindo <pablogsal@gmail.com> > > > > > Link: https://github.com/python/cpython/issues/139544 > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > > > > --- > > > > > tools/perf/util/genelf.c | 9 +++++++++ > > > > > 1 file changed, 9 insertions(+) > > > > > > > > > > diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c > > > > > index 591548b10e34ef6a..a412e6faf70e37f3 100644 > > > > > --- a/tools/perf/util/genelf.c > > > > > +++ b/tools/perf/util/genelf.c > > > > > @@ -395,6 +395,15 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym, > > > > > * build-id generation > > > > > */ > > > > > sha1(code, csize, bnote.build_id); > > > > > + /* FIXME: update the SHA-1 hash using additional contents */ > > > > > + bnote.build_id[0] += (load_addr >> 0) & 0xff; > > > > > + bnote.build_id[1] += (load_addr >> 8) & 0xff; > > > > > + bnote.build_id[2] += (load_addr >> 16) & 0xff; > > > > > + bnote.build_id[3] += (load_addr >> 24) & 0xff; > > > > > + bnote.build_id[4] += (load_addr >> 32) & 0xff; > > > > > + bnote.build_id[5] += (load_addr >> 40) & 0xff; > > > > > + bnote.build_id[6] += (load_addr >> 48) & 0xff; > > > > > + bnote.build_id[7] += (load_addr >> 56) & 0xff; > > > > > bnote.desc.namesz = sizeof(bnote.name); /* must include 0 termination */ > > > > > bnote.desc.descsz = sizeof(bnote.build_id); > > > > > bnote.desc.type = NT_GNU_BUILD_ID; > > > > > -- > > > > > 2.52.0.rc1.455.g30608eb744-goog > > > > > ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-11-17 16:58 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-11-14 9:29 [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation Namhyung Kim 2025-11-14 9:29 ` [PATCH 2/2] perf test: Add python JIT dump test Namhyung Kim 2025-11-14 17:44 ` Ian Rogers 2025-11-14 19:03 ` Namhyung Kim 2025-11-14 17:33 ` [PATCH 1/2] perf jitdump: Add load_addr to build-ID generation Ian Rogers 2025-11-14 18:57 ` Namhyung Kim 2025-11-14 19:32 ` Ian Rogers 2025-11-14 23:24 ` Namhyung Kim 2025-11-14 23:58 ` Ian Rogers 2025-11-16 7:22 ` Fangrui Song 2025-11-17 16:58 ` Ian Rogers
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox