[PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Namhyung Kim]

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.  But it'd
be better to use contents of symtab and strtab instead as it aligns with
some linker behaviors.

This patch adds a buffer to save all the contents in a single place for
SHA-1 calculation.  Probably we need to add sha1_update() or similar to
update the existing hash value with different contents and use it here.
But it's out of scope for this change and 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>
Cc: Fangrui Song <maskray@sourceware.org>
Link: https://github.com/python/cpython/issues/139544
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
v2) use symtab/strtab instead of load_addr

 tools/perf/util/genelf.c | 32 ++++++++++++++++++++++++++++++--
 1 file changed, 30 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/genelf.c b/tools/perf/util/genelf.c
index 591548b10e34ef6a..a1cd5196f4ec8f88 100644
--- a/tools/perf/util/genelf.c
+++ b/tools/perf/util/genelf.c
@@ -173,6 +173,8 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym,
 	Elf_Shdr *shdr;
 	uint64_t eh_frame_base_offset;
 	char *strsym = NULL;
+	void *build_id_data = NULL, *tmp;
+	int build_id_data_len;
 	int symlen;
 	int retval = -1;
 
@@ -251,6 +253,14 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym,
 	shdr->sh_flags = SHF_EXECINSTR | SHF_ALLOC;
 	shdr->sh_entsize = 0;
 
+	build_id_data = malloc(csize);
+	if (build_id_data == NULL) {
+		warnx("cannot allocate build-id data");
+		goto error;
+	}
+	memcpy(build_id_data, code, csize);
+	build_id_data_len = csize;
+
 	/*
 	 * Setup .eh_frame_hdr and .eh_frame
 	 */
@@ -334,6 +344,15 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym,
 	shdr->sh_entsize = sizeof(Elf_Sym);
 	shdr->sh_link = unwinding ? 6 : 4; /* index of .strtab section */
 
+	tmp = realloc(build_id_data, build_id_data_len + sizeof(symtab));
+	if (tmp == NULL) {
+		warnx("cannot allocate build-id data");
+		goto error;
+	}
+	memcpy(tmp + build_id_data_len, symtab, sizeof(symtab));
+	build_id_data = tmp;
+	build_id_data_len += sizeof(symtab);
+
 	/*
 	 * setup symbols string table
 	 * 2 = 1 for 0 in 1st entry, 1 for the 0 at end of symbol for 2nd entry
@@ -376,6 +395,15 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym,
 	shdr->sh_flags = 0;
 	shdr->sh_entsize = 0;
 
+	tmp = realloc(build_id_data, build_id_data_len + symlen);
+	if (tmp == NULL) {
+		warnx("cannot allocate build-id data");
+		goto error;
+	}
+	memcpy(tmp + build_id_data_len, strsym, symlen);
+	build_id_data = tmp;
+	build_id_data_len += symlen;
+
 	/*
 	 * setup build-id section
 	 */
@@ -394,7 +422,7 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym,
 	/*
 	 * build-id generation
 	 */
-	sha1(code, csize, bnote.build_id);
+	sha1(build_id_data, build_id_data_len, bnote.build_id);
 	bnote.desc.namesz = sizeof(bnote.name); /* must include 0 termination */
 	bnote.desc.descsz = sizeof(bnote.build_id);
 	bnote.desc.type   = NT_GNU_BUILD_ID;
@@ -439,7 +467,7 @@ jit_write_elf(int fd, uint64_t load_addr __maybe_unused, const char *sym,
 	(void)elf_end(e);
 
 	free(strsym);
-
+	free(build_id_data);
 
 	return retval;
 }
-- 
2.52.0.460.gd25c4c69ec-goog

[PATCH v2 2/2] perf test: Add python JIT dump test

[Namhyung Kim]

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>
---
v2 changes)
* check availability of -Xperf_jit properly  (Pablo)
* use setup_python.sh  (Ian)
* do not use a separate script file  (Ian)
* add cleanup function  (Ian)

 tools/perf/tests/shell/jitdump-python.sh | 81 ++++++++++++++++++++++++
 1 file changed, 81 insertions(+)
 create mode 100755 tools/perf/tests/shell/jitdump-python.sh

diff --git a/tools/perf/tests/shell/jitdump-python.sh b/tools/perf/tests/shell/jitdump-python.sh
new file mode 100755
index 0000000000000000..ae86203b14a22b4e
--- /dev/null
+++ b/tools/perf/tests/shell/jitdump-python.sh
@@ -0,0 +1,81 @@
+#!/bin/bash
+# python profiling with jitdump
+# SPDX-License-Identifier: GPL-2.0
+
+SHELLDIR=$(dirname $0)
+# shellcheck source=lib/setup_python.sh
+. "${SHELLDIR}"/lib/setup_python.sh
+
+OUTPUT=$(${PYTHON} -Xperf_jit -c 'import os, sys; print(os.getpid(), sys.is_stack_trampoline_active())' 2> /dev/null)
+PID=${OUTPUT% *}
+HAS_PERF_JIT=${OUTPUT#* }
+
+rm -f /tmp/jit-${PID}.dump 2> /dev/null
+if [ "${HAS_PERF_JIT}" != "True" ]; then
+    echo "SKIP: python JIT dump is not available"
+    exit 2
+fi
+
+PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXXX)
+
+cleanup() {
+    echo "Cleaning up files..."
+    rm -f ${PERF_DATA} ${PERF_DATA}.jit /tmp/jit-${PID}.dump /tmp/jitted-${PID}-*.so 2> /dev/null
+
+    trap - EXIT TERM INT
+}
+
+trap_cleanup() {
+    echo "Unexpected termination"
+    cleanup
+    exit 1
+}
+
+trap trap_cleanup EXIT TERM INT
+
+echo "Run python with -Xperf_jit"
+cat <<EOF | perf record -k 1 -g --call-graph dwarf -o "${PERF_DATA}" \
+		 -- ${PYTHON} -Xperf_jit
+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)
+EOF
+
+# extract PID of the target process from the data
+_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"
+DEBUGINFOD_URLS='' 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 function/module name"
+NUM=$(perf report -i "${PERF_DATA}.jit" -s sym | grep -cE 'py::(foo|bar|baz):<stdin>')
+
+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
+
+cleanup
+
+if [ "${NUM}" -eq 0 ]; then
+    exit 1
+fi
-- 
2.52.0.460.gd25c4c69ec-goog

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Eric Biggers]

On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> 
> Looking back at the original code before the conversion, it used the
> load_addr as well as the code section to distinguish each DSO.  But it'd
> be better to use contents of symtab and strtab instead as it aligns with
> some linker behaviors.
> 
> This patch adds a buffer to save all the contents in a single place for
> SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> update the existing hash value with different contents and use it here.
> But it's out of scope for this change and 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>
> Cc: Fangrui Song <maskray@sourceware.org>
> Link: https://github.com/python/cpython/issues/139544
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

That commit actually preserved the behavior of the existing variant of
gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
was always broken, and it was just never noticed because the alternative
variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?

The MD5 variant of gen_build_id() just hashed the load_addr concatenated
with the code.  That's not what this patch does, though.  So just to
clarify, you'd actually like to go with a third approach rather than
just restoring the original hash(load_addr || code) approach?

Also, I missed that you had actually changed the hash algorithm.  I had
assumed the perf folks were were pushing SHA-1 because they were already
using it.  Given that the algorithm changed, there must not be any
backwards compatibility concerns here, and you should switch to a modern
hash algorithm such as SHA-256 instead.

I'd be glad to add an incremental API if you need it, but I'm confused
why you want SHA-1 and not a modern hash algorithm.

- Eric

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Ian Rogers]

On Tue, Nov 25, 2025 at 11:29 AM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> >
> > Looking back at the original code before the conversion, it used the
> > load_addr as well as the code section to distinguish each DSO.  But it'd
> > be better to use contents of symtab and strtab instead as it aligns with
> > some linker behaviors.
> >
> > This patch adds a buffer to save all the contents in a single place for
> > SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> > update the existing hash value with different contents and use it here.
> > But it's out of scope for this change and 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>
> > Cc: Fangrui Song <maskray@sourceware.org>
> > Link: https://github.com/python/cpython/issues/139544
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
>
> That commit actually preserved the behavior of the existing variant of
> gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
> was always broken, and it was just never noticed because the alternative
> variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?
>
> The MD5 variant of gen_build_id() just hashed the load_addr concatenated
> with the code.  That's not what this patch does, though.  So just to
> clarify, you'd actually like to go with a third approach rather than
> just restoring the original hash(load_addr || code) approach?
>
> Also, I missed that you had actually changed the hash algorithm.  I had
> assumed the perf folks were were pushing SHA-1 because they were already
> using it.  Given that the algorithm changed, there must not be any
> backwards compatibility concerns here, and you should switch to a modern
> hash algorithm such as SHA-256 instead.
>
> I'd be glad to add an incremental API if you need it, but I'm confused
> why you want SHA-1 and not a modern hash algorithm.

Hi Eric,

Thanks for the help with the hash functions! There's a bit more
context in this thread:
https://lore.kernel.org/linux-perf-users/CAP-5=fWLgaWsv82dcPajVk=UmBbmwyEd7OVp6psZQ4TiXh-Meg@mail.gmail.com/

So genelf is trying to take snippets of jitted code and create ELF
files from them for the purpose of symbolizing in perf. The buildid
hash being used is SHA1 and I think the MD5 support was removed as
unnecessary. The problem this patch is addressing is that a JIT may
create many identical stubs which then end up being deduplicated into
the same buildid as only the code is hashed. The BFD linker seems to
have the same issue (Fangrui filed a bug), gold and lld appear to hash
the symbols (which Namhyung adds to genelf here) but still yield
different build id for the same source assembly code. It is possible
to hash the address of the symbol rather than the symbol itself, but I
think the intent for the code should be to best match what a compiler
and linker would generate. The problem there is that this differs for
every linker :-)

Something that is unfortunate in the code now is copying/concatenating
all the build data for the sake of producing the hash. It would be
nice if the code could incrementally build up the sha1 hash to avoid
the copying. I don't know if there is functionality for this
currently.

Thanks,
Ian

> - Eric

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Eric Biggers]

On Tue, Nov 25, 2025 at 06:55:39PM -0800, Ian Rogers wrote:
> On Tue, Nov 25, 2025 at 11:29 AM Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> > >
> > > Looking back at the original code before the conversion, it used the
> > > load_addr as well as the code section to distinguish each DSO.  But it'd
> > > be better to use contents of symtab and strtab instead as it aligns with
> > > some linker behaviors.
> > >
> > > This patch adds a buffer to save all the contents in a single place for
> > > SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> > > update the existing hash value with different contents and use it here.
> > > But it's out of scope for this change and 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>
> > > Cc: Fangrui Song <maskray@sourceware.org>
> > > Link: https://github.com/python/cpython/issues/139544
> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> >
> > That commit actually preserved the behavior of the existing variant of
> > gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
> > was always broken, and it was just never noticed because the alternative
> > variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?
> >
> > The MD5 variant of gen_build_id() just hashed the load_addr concatenated
> > with the code.  That's not what this patch does, though.  So just to
> > clarify, you'd actually like to go with a third approach rather than
> > just restoring the original hash(load_addr || code) approach?
> >
> > Also, I missed that you had actually changed the hash algorithm.  I had
> > assumed the perf folks were were pushing SHA-1 because they were already
> > using it.  Given that the algorithm changed, there must not be any
> > backwards compatibility concerns here, and you should switch to a modern
> > hash algorithm such as SHA-256 instead.
> >
> > I'd be glad to add an incremental API if you need it, but I'm confused
> > why you want SHA-1 and not a modern hash algorithm.
> 
> Hi Eric,
> 
> Thanks for the help with the hash functions! There's a bit more
> context in this thread:
> https://lore.kernel.org/linux-perf-users/CAP-5=fWLgaWsv82dcPajVk=UmBbmwyEd7OVp6psZQ4TiXh-Meg@mail.gmail.com/
> 
> So genelf is trying to take snippets of jitted code and create ELF
> files from them for the purpose of symbolizing in perf. The buildid
> hash being used is SHA1 and I think the MD5 support was removed as
> unnecessary. The problem this patch is addressing is that a JIT may
> create many identical stubs which then end up being deduplicated into
> the same buildid as only the code is hashed. The BFD linker seems to
> have the same issue (Fangrui filed a bug), gold and lld appear to hash
> the symbols (which Namhyung adds to genelf here) but still yield
> different build id for the same source assembly code. It is possible
> to hash the address of the symbol rather than the symbol itself, but I
> think the intent for the code should be to best match what a compiler
> and linker would generate. The problem there is that this differs for
> every linker :-)
> 
> Something that is unfortunate in the code now is copying/concatenating
> all the build data for the sake of producing the hash. It would be
> nice if the code could incrementally build up the sha1 hash to avoid
> the copying. I don't know if there is functionality for this
> currently.

Again, I can add support for incremental hashing if you need it.  But I
don't understand why you want the hash function to be SHA-1.  Also,
given that this seems to be a regression fix, it's surprising that
you're suddenly changing the inputs to the hash entirely, instead of
just going back to hashing load_addr concatenated with the code for now.

- Eric

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Namhyung Kim]

Hello,

On Tue, Nov 25, 2025 at 07:04:52PM -0800, Eric Biggers wrote:
> On Tue, Nov 25, 2025 at 06:55:39PM -0800, Ian Rogers wrote:
> > On Tue, Nov 25, 2025 at 11:29 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > >
> > > On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> > > >
> > > > Looking back at the original code before the conversion, it used the
> > > > load_addr as well as the code section to distinguish each DSO.  But it'd
> > > > be better to use contents of symtab and strtab instead as it aligns with
> > > > some linker behaviors.
> > > >
> > > > This patch adds a buffer to save all the contents in a single place for
> > > > SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> > > > update the existing hash value with different contents and use it here.
> > > > But it's out of scope for this change and 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>
> > > > Cc: Fangrui Song <maskray@sourceware.org>
> > > > Link: https://github.com/python/cpython/issues/139544
> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > >
> > > That commit actually preserved the behavior of the existing variant of
> > > gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
> > > was always broken, and it was just never noticed because the alternative
> > > variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?
> > >
> > > The MD5 variant of gen_build_id() just hashed the load_addr concatenated
> > > with the code.  That's not what this patch does, though.  So just to
> > > clarify, you'd actually like to go with a third approach rather than
> > > just restoring the original hash(load_addr || code) approach?
> > >
> > > Also, I missed that you had actually changed the hash algorithm.  I had
> > > assumed the perf folks were were pushing SHA-1 because they were already
> > > using it.  Given that the algorithm changed, there must not be any
> > > backwards compatibility concerns here, and you should switch to a modern
> > > hash algorithm such as SHA-256 instead.
> > >
> > > I'd be glad to add an incremental API if you need it, but I'm confused
> > > why you want SHA-1 and not a modern hash algorithm.
> > 
> > Hi Eric,
> > 
> > Thanks for the help with the hash functions! There's a bit more
> > context in this thread:
> > https://lore.kernel.org/linux-perf-users/CAP-5=fWLgaWsv82dcPajVk=UmBbmwyEd7OVp6psZQ4TiXh-Meg@mail.gmail.com/
> > 
> > So genelf is trying to take snippets of jitted code and create ELF
> > files from them for the purpose of symbolizing in perf. The buildid
> > hash being used is SHA1 and I think the MD5 support was removed as
> > unnecessary. The problem this patch is addressing is that a JIT may
> > create many identical stubs which then end up being deduplicated into
> > the same buildid as only the code is hashed. The BFD linker seems to
> > have the same issue (Fangrui filed a bug), gold and lld appear to hash
> > the symbols (which Namhyung adds to genelf here) but still yield
> > different build id for the same source assembly code. It is possible
> > to hash the address of the symbol rather than the symbol itself, but I
> > think the intent for the code should be to best match what a compiler
> > and linker would generate. The problem there is that this differs for
> > every linker :-)
> > 
> > Something that is unfortunate in the code now is copying/concatenating
> > all the build data for the sake of producing the hash. It would be
> > nice if the code could incrementally build up the sha1 hash to avoid
> > the copying. I don't know if there is functionality for this
> > currently.
> 
> Again, I can add support for incremental hashing if you need it.  But I
> don't understand why you want the hash function to be SHA-1.  Also,
> given that this seems to be a regression fix, it's surprising that
> you're suddenly changing the inputs to the hash entirely, instead of
> just going back to hashing load_addr concatenated with the code for now.

Historically MD5 or SHA1 was the only choice for build-ID in linkers.
I don't know if it's changed lately though.  But because of that we
support build-IDs up to 20 bytes and it's in the UAPI too (well.. it's
implicit in PERF_RECORD_MMAP2).  I think we can support other hashs if
they fit into it, but SHA-1 would be the right choice for now.

About load_addr, I don't think it's the right fix.  We cannot have the
exactly same build-IDs as before since we already switched to SHA-1.
What we actually want is a way to generate unique IDs for each DSO.
For that purpose, it'd be nice to use sym/str-tables as Fangrui said
instead of using load_addr which seems to be a quick hack in the past.

So I think I can merge this change for stable fix.  And add incremental
SHA-1 (thanks in advance!) and update the code to it later.

Thanks,
Namhyung

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Eric Biggers]

On Thu, Nov 27, 2025 at 01:17:01PM -0800, Namhyung Kim wrote:
> Hello,
> 
> On Tue, Nov 25, 2025 at 07:04:52PM -0800, Eric Biggers wrote:
> > On Tue, Nov 25, 2025 at 06:55:39PM -0800, Ian Rogers wrote:
> > > On Tue, Nov 25, 2025 at 11:29 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > >
> > > > On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> > > > >
> > > > > Looking back at the original code before the conversion, it used the
> > > > > load_addr as well as the code section to distinguish each DSO.  But it'd
> > > > > be better to use contents of symtab and strtab instead as it aligns with
> > > > > some linker behaviors.
> > > > >
> > > > > This patch adds a buffer to save all the contents in a single place for
> > > > > SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> > > > > update the existing hash value with different contents and use it here.
> > > > > But it's out of scope for this change and 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>
> > > > > Cc: Fangrui Song <maskray@sourceware.org>
> > > > > Link: https://github.com/python/cpython/issues/139544
> > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > >
> > > > That commit actually preserved the behavior of the existing variant of
> > > > gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
> > > > was always broken, and it was just never noticed because the alternative
> > > > variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?
> > > >
> > > > The MD5 variant of gen_build_id() just hashed the load_addr concatenated
> > > > with the code.  That's not what this patch does, though.  So just to
> > > > clarify, you'd actually like to go with a third approach rather than
> > > > just restoring the original hash(load_addr || code) approach?
> > > >
> > > > Also, I missed that you had actually changed the hash algorithm.  I had
> > > > assumed the perf folks were were pushing SHA-1 because they were already
> > > > using it.  Given that the algorithm changed, there must not be any
> > > > backwards compatibility concerns here, and you should switch to a modern
> > > > hash algorithm such as SHA-256 instead.
> > > >
> > > > I'd be glad to add an incremental API if you need it, but I'm confused
> > > > why you want SHA-1 and not a modern hash algorithm.
> > > 
> > > Hi Eric,
> > > 
> > > Thanks for the help with the hash functions! There's a bit more
> > > context in this thread:
> > > https://lore.kernel.org/linux-perf-users/CAP-5=fWLgaWsv82dcPajVk=UmBbmwyEd7OVp6psZQ4TiXh-Meg@mail.gmail.com/
> > > 
> > > So genelf is trying to take snippets of jitted code and create ELF
> > > files from them for the purpose of symbolizing in perf. The buildid
> > > hash being used is SHA1 and I think the MD5 support was removed as
> > > unnecessary. The problem this patch is addressing is that a JIT may
> > > create many identical stubs which then end up being deduplicated into
> > > the same buildid as only the code is hashed. The BFD linker seems to
> > > have the same issue (Fangrui filed a bug), gold and lld appear to hash
> > > the symbols (which Namhyung adds to genelf here) but still yield
> > > different build id for the same source assembly code. It is possible
> > > to hash the address of the symbol rather than the symbol itself, but I
> > > think the intent for the code should be to best match what a compiler
> > > and linker would generate. The problem there is that this differs for
> > > every linker :-)
> > > 
> > > Something that is unfortunate in the code now is copying/concatenating
> > > all the build data for the sake of producing the hash. It would be
> > > nice if the code could incrementally build up the sha1 hash to avoid
> > > the copying. I don't know if there is functionality for this
> > > currently.
> > 
> > Again, I can add support for incremental hashing if you need it.  But I
> > don't understand why you want the hash function to be SHA-1.  Also,
> > given that this seems to be a regression fix, it's surprising that
> > you're suddenly changing the inputs to the hash entirely, instead of
> > just going back to hashing load_addr concatenated with the code for now.
> 
> Historically MD5 or SHA1 was the only choice for build-ID in linkers.
> I don't know if it's changed lately though.  But because of that we
> support build-IDs up to 20 bytes and it's in the UAPI too (well.. it's
> implicit in PERF_RECORD_MMAP2).  I think we can support other hashs if
> they fit into it, but SHA-1 would be the right choice for now.
> 
> About load_addr, I don't think it's the right fix.  We cannot have the
> exactly same build-IDs as before since we already switched to SHA-1.
> What we actually want is a way to generate unique IDs for each DSO.
> For that purpose, it'd be nice to use sym/str-tables as Fangrui said
> instead of using load_addr which seems to be a quick hack in the past.
> 
> So I think I can merge this change for stable fix.  And add incremental
> SHA-1 (thanks in advance!) and update the code to it later.

If it needs to be 20 bytes, you should just use a truncated SHA-256.

The only use for SHA-1 is backwards compatibility.  Which it seems you
don't need.

- Eric

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Namhyung Kim]

On Thu, Nov 27, 2025 at 02:18:22PM -0800, Eric Biggers wrote:
> On Thu, Nov 27, 2025 at 01:17:01PM -0800, Namhyung Kim wrote:
> > Hello,
> > 
> > On Tue, Nov 25, 2025 at 07:04:52PM -0800, Eric Biggers wrote:
> > > On Tue, Nov 25, 2025 at 06:55:39PM -0800, Ian Rogers wrote:
> > > > On Tue, Nov 25, 2025 at 11:29 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > > >
> > > > > On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> > > > > >
> > > > > > Looking back at the original code before the conversion, it used the
> > > > > > load_addr as well as the code section to distinguish each DSO.  But it'd
> > > > > > be better to use contents of symtab and strtab instead as it aligns with
> > > > > > some linker behaviors.
> > > > > >
> > > > > > This patch adds a buffer to save all the contents in a single place for
> > > > > > SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> > > > > > update the existing hash value with different contents and use it here.
> > > > > > But it's out of scope for this change and 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>
> > > > > > Cc: Fangrui Song <maskray@sourceware.org>
> > > > > > Link: https://github.com/python/cpython/issues/139544
> > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > >
> > > > > That commit actually preserved the behavior of the existing variant of
> > > > > gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
> > > > > was always broken, and it was just never noticed because the alternative
> > > > > variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?
> > > > >
> > > > > The MD5 variant of gen_build_id() just hashed the load_addr concatenated
> > > > > with the code.  That's not what this patch does, though.  So just to
> > > > > clarify, you'd actually like to go with a third approach rather than
> > > > > just restoring the original hash(load_addr || code) approach?
> > > > >
> > > > > Also, I missed that you had actually changed the hash algorithm.  I had
> > > > > assumed the perf folks were were pushing SHA-1 because they were already
> > > > > using it.  Given that the algorithm changed, there must not be any
> > > > > backwards compatibility concerns here, and you should switch to a modern
> > > > > hash algorithm such as SHA-256 instead.
> > > > >
> > > > > I'd be glad to add an incremental API if you need it, but I'm confused
> > > > > why you want SHA-1 and not a modern hash algorithm.
> > > > 
> > > > Hi Eric,
> > > > 
> > > > Thanks for the help with the hash functions! There's a bit more
> > > > context in this thread:
> > > > https://lore.kernel.org/linux-perf-users/CAP-5=fWLgaWsv82dcPajVk=UmBbmwyEd7OVp6psZQ4TiXh-Meg@mail.gmail.com/
> > > > 
> > > > So genelf is trying to take snippets of jitted code and create ELF
> > > > files from them for the purpose of symbolizing in perf. The buildid
> > > > hash being used is SHA1 and I think the MD5 support was removed as
> > > > unnecessary. The problem this patch is addressing is that a JIT may
> > > > create many identical stubs which then end up being deduplicated into
> > > > the same buildid as only the code is hashed. The BFD linker seems to
> > > > have the same issue (Fangrui filed a bug), gold and lld appear to hash
> > > > the symbols (which Namhyung adds to genelf here) but still yield
> > > > different build id for the same source assembly code. It is possible
> > > > to hash the address of the symbol rather than the symbol itself, but I
> > > > think the intent for the code should be to best match what a compiler
> > > > and linker would generate. The problem there is that this differs for
> > > > every linker :-)
> > > > 
> > > > Something that is unfortunate in the code now is copying/concatenating
> > > > all the build data for the sake of producing the hash. It would be
> > > > nice if the code could incrementally build up the sha1 hash to avoid
> > > > the copying. I don't know if there is functionality for this
> > > > currently.
> > > 
> > > Again, I can add support for incremental hashing if you need it.  But I
> > > don't understand why you want the hash function to be SHA-1.  Also,
> > > given that this seems to be a regression fix, it's surprising that
> > > you're suddenly changing the inputs to the hash entirely, instead of
> > > just going back to hashing load_addr concatenated with the code for now.
> > 
> > Historically MD5 or SHA1 was the only choice for build-ID in linkers.
> > I don't know if it's changed lately though.  But because of that we
> > support build-IDs up to 20 bytes and it's in the UAPI too (well.. it's
> > implicit in PERF_RECORD_MMAP2).  I think we can support other hashs if
> > they fit into it, but SHA-1 would be the right choice for now.
> > 
> > About load_addr, I don't think it's the right fix.  We cannot have the
> > exactly same build-IDs as before since we already switched to SHA-1.
> > What we actually want is a way to generate unique IDs for each DSO.
> > For that purpose, it'd be nice to use sym/str-tables as Fangrui said
> > instead of using load_addr which seems to be a quick hack in the past.
> > 
> > So I think I can merge this change for stable fix.  And add incremental
> > SHA-1 (thanks in advance!) and update the code to it later.
> 
> If it needs to be 20 bytes, you should just use a truncated SHA-256.
> 
> The only use for SHA-1 is backwards compatibility.  Which it seems you
> don't need.

I think the linker (ld) --build-id still defaults to SHA-1.

Thanks,
Namhyung

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Ian Rogers]

On Tue, Dec 2, 2025 at 1:56 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, Nov 27, 2025 at 02:18:22PM -0800, Eric Biggers wrote:
> > On Thu, Nov 27, 2025 at 01:17:01PM -0800, Namhyung Kim wrote:
> > > Hello,
> > >
> > > On Tue, Nov 25, 2025 at 07:04:52PM -0800, Eric Biggers wrote:
> > > > On Tue, Nov 25, 2025 at 06:55:39PM -0800, Ian Rogers wrote:
> > > > > On Tue, Nov 25, 2025 at 11:29 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > > > >
> > > > > > On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> > > > > > >
> > > > > > > Looking back at the original code before the conversion, it used the
> > > > > > > load_addr as well as the code section to distinguish each DSO.  But it'd
> > > > > > > be better to use contents of symtab and strtab instead as it aligns with
> > > > > > > some linker behaviors.
> > > > > > >
> > > > > > > This patch adds a buffer to save all the contents in a single place for
> > > > > > > SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> > > > > > > update the existing hash value with different contents and use it here.
> > > > > > > But it's out of scope for this change and 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>
> > > > > > > Cc: Fangrui Song <maskray@sourceware.org>
> > > > > > > Link: https://github.com/python/cpython/issues/139544
> > > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > > >
> > > > > > That commit actually preserved the behavior of the existing variant of
> > > > > > gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
> > > > > > was always broken, and it was just never noticed because the alternative
> > > > > > variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?
> > > > > >
> > > > > > The MD5 variant of gen_build_id() just hashed the load_addr concatenated
> > > > > > with the code.  That's not what this patch does, though.  So just to
> > > > > > clarify, you'd actually like to go with a third approach rather than
> > > > > > just restoring the original hash(load_addr || code) approach?
> > > > > >
> > > > > > Also, I missed that you had actually changed the hash algorithm.  I had
> > > > > > assumed the perf folks were were pushing SHA-1 because they were already
> > > > > > using it.  Given that the algorithm changed, there must not be any
> > > > > > backwards compatibility concerns here, and you should switch to a modern
> > > > > > hash algorithm such as SHA-256 instead.
> > > > > >
> > > > > > I'd be glad to add an incremental API if you need it, but I'm confused
> > > > > > why you want SHA-1 and not a modern hash algorithm.
> > > > >
> > > > > Hi Eric,
> > > > >
> > > > > Thanks for the help with the hash functions! There's a bit more
> > > > > context in this thread:
> > > > > https://lore.kernel.org/linux-perf-users/CAP-5=fWLgaWsv82dcPajVk=UmBbmwyEd7OVp6psZQ4TiXh-Meg@mail.gmail.com/
> > > > >
> > > > > So genelf is trying to take snippets of jitted code and create ELF
> > > > > files from them for the purpose of symbolizing in perf. The buildid
> > > > > hash being used is SHA1 and I think the MD5 support was removed as
> > > > > unnecessary. The problem this patch is addressing is that a JIT may
> > > > > create many identical stubs which then end up being deduplicated into
> > > > > the same buildid as only the code is hashed. The BFD linker seems to
> > > > > have the same issue (Fangrui filed a bug), gold and lld appear to hash
> > > > > the symbols (which Namhyung adds to genelf here) but still yield
> > > > > different build id for the same source assembly code. It is possible
> > > > > to hash the address of the symbol rather than the symbol itself, but I
> > > > > think the intent for the code should be to best match what a compiler
> > > > > and linker would generate. The problem there is that this differs for
> > > > > every linker :-)
> > > > >
> > > > > Something that is unfortunate in the code now is copying/concatenating
> > > > > all the build data for the sake of producing the hash. It would be
> > > > > nice if the code could incrementally build up the sha1 hash to avoid
> > > > > the copying. I don't know if there is functionality for this
> > > > > currently.
> > > >
> > > > Again, I can add support for incremental hashing if you need it.  But I
> > > > don't understand why you want the hash function to be SHA-1.  Also,
> > > > given that this seems to be a regression fix, it's surprising that
> > > > you're suddenly changing the inputs to the hash entirely, instead of
> > > > just going back to hashing load_addr concatenated with the code for now.
> > >
> > > Historically MD5 or SHA1 was the only choice for build-ID in linkers.
> > > I don't know if it's changed lately though.  But because of that we
> > > support build-IDs up to 20 bytes and it's in the UAPI too (well.. it's
> > > implicit in PERF_RECORD_MMAP2).  I think we can support other hashs if
> > > they fit into it, but SHA-1 would be the right choice for now.
> > >
> > > About load_addr, I don't think it's the right fix.  We cannot have the
> > > exactly same build-IDs as before since we already switched to SHA-1.
> > > What we actually want is a way to generate unique IDs for each DSO.
> > > For that purpose, it'd be nice to use sym/str-tables as Fangrui said
> > > instead of using load_addr which seems to be a quick hack in the past.
> > >
> > > So I think I can merge this change for stable fix.  And add incremental
> > > SHA-1 (thanks in advance!) and update the code to it later.
> >
> > If it needs to be 20 bytes, you should just use a truncated SHA-256.
> >
> > The only use for SHA-1 is backwards compatibility.  Which it seems you
> > don't need.
>
> I think the linker (ld) --build-id still defaults to SHA-1.

Reviewed-by: Ian Rogers <irogers@google.com>

I think everyone is agreeing. Let's fix the python JIT issue with
what's here with SHA-1 and work upon truncated SHA-256 with
incremental hashing that's truncated to 20 bytes and without the
load_addr hack. We could also hash other ELF sections from the jitdump
output at the same time as it may be theoretically possible to have
matching object code and symbols but differing frame layouts. Thanks
Namhyung for working on the fix, Eric and Fangrui for the hashing
support and build ID insights!

Thanks,
Ian

> Thanks,
> Namhyung
>

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Eric Biggers]

On Tue, Dec 02, 2025 at 01:56:42PM -0800, Namhyung Kim wrote:
> On Thu, Nov 27, 2025 at 02:18:22PM -0800, Eric Biggers wrote:
> > On Thu, Nov 27, 2025 at 01:17:01PM -0800, Namhyung Kim wrote:
> > > Hello,
> > > 
> > > On Tue, Nov 25, 2025 at 07:04:52PM -0800, Eric Biggers wrote:
> > > > On Tue, Nov 25, 2025 at 06:55:39PM -0800, Ian Rogers wrote:
> > > > > On Tue, Nov 25, 2025 at 11:29 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > > > >
> > > > > > On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> > > > > > >
> > > > > > > Looking back at the original code before the conversion, it used the
> > > > > > > load_addr as well as the code section to distinguish each DSO.  But it'd
> > > > > > > be better to use contents of symtab and strtab instead as it aligns with
> > > > > > > some linker behaviors.
> > > > > > >
> > > > > > > This patch adds a buffer to save all the contents in a single place for
> > > > > > > SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> > > > > > > update the existing hash value with different contents and use it here.
> > > > > > > But it's out of scope for this change and 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>
> > > > > > > Cc: Fangrui Song <maskray@sourceware.org>
> > > > > > > Link: https://github.com/python/cpython/issues/139544
> > > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > > >
> > > > > > That commit actually preserved the behavior of the existing variant of
> > > > > > gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
> > > > > > was always broken, and it was just never noticed because the alternative
> > > > > > variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?
> > > > > >
> > > > > > The MD5 variant of gen_build_id() just hashed the load_addr concatenated
> > > > > > with the code.  That's not what this patch does, though.  So just to
> > > > > > clarify, you'd actually like to go with a third approach rather than
> > > > > > just restoring the original hash(load_addr || code) approach?
> > > > > >
> > > > > > Also, I missed that you had actually changed the hash algorithm.  I had
> > > > > > assumed the perf folks were were pushing SHA-1 because they were already
> > > > > > using it.  Given that the algorithm changed, there must not be any
> > > > > > backwards compatibility concerns here, and you should switch to a modern
> > > > > > hash algorithm such as SHA-256 instead.
> > > > > >
> > > > > > I'd be glad to add an incremental API if you need it, but I'm confused
> > > > > > why you want SHA-1 and not a modern hash algorithm.
> > > > > 
> > > > > Hi Eric,
> > > > > 
> > > > > Thanks for the help with the hash functions! There's a bit more
> > > > > context in this thread:
> > > > > https://lore.kernel.org/linux-perf-users/CAP-5=fWLgaWsv82dcPajVk=UmBbmwyEd7OVp6psZQ4TiXh-Meg@mail.gmail.com/
> > > > > 
> > > > > So genelf is trying to take snippets of jitted code and create ELF
> > > > > files from them for the purpose of symbolizing in perf. The buildid
> > > > > hash being used is SHA1 and I think the MD5 support was removed as
> > > > > unnecessary. The problem this patch is addressing is that a JIT may
> > > > > create many identical stubs which then end up being deduplicated into
> > > > > the same buildid as only the code is hashed. The BFD linker seems to
> > > > > have the same issue (Fangrui filed a bug), gold and lld appear to hash
> > > > > the symbols (which Namhyung adds to genelf here) but still yield
> > > > > different build id for the same source assembly code. It is possible
> > > > > to hash the address of the symbol rather than the symbol itself, but I
> > > > > think the intent for the code should be to best match what a compiler
> > > > > and linker would generate. The problem there is that this differs for
> > > > > every linker :-)
> > > > > 
> > > > > Something that is unfortunate in the code now is copying/concatenating
> > > > > all the build data for the sake of producing the hash. It would be
> > > > > nice if the code could incrementally build up the sha1 hash to avoid
> > > > > the copying. I don't know if there is functionality for this
> > > > > currently.
> > > > 
> > > > Again, I can add support for incremental hashing if you need it.  But I
> > > > don't understand why you want the hash function to be SHA-1.  Also,
> > > > given that this seems to be a regression fix, it's surprising that
> > > > you're suddenly changing the inputs to the hash entirely, instead of
> > > > just going back to hashing load_addr concatenated with the code for now.
> > > 
> > > Historically MD5 or SHA1 was the only choice for build-ID in linkers.
> > > I don't know if it's changed lately though.  But because of that we
> > > support build-IDs up to 20 bytes and it's in the UAPI too (well.. it's
> > > implicit in PERF_RECORD_MMAP2).  I think we can support other hashs if
> > > they fit into it, but SHA-1 would be the right choice for now.
> > > 
> > > About load_addr, I don't think it's the right fix.  We cannot have the
> > > exactly same build-IDs as before since we already switched to SHA-1.
> > > What we actually want is a way to generate unique IDs for each DSO.
> > > For that purpose, it'd be nice to use sym/str-tables as Fangrui said
> > > instead of using load_addr which seems to be a quick hack in the past.
> > > 
> > > So I think I can merge this change for stable fix.  And add incremental
> > > SHA-1 (thanks in advance!) and update the code to it later.
> > 
> > If it needs to be 20 bytes, you should just use a truncated SHA-256.
> > 
> > The only use for SHA-1 is backwards compatibility.  Which it seems you
> > don't need.
> 
> I think the linker (ld) --build-id still defaults to SHA-1.

But generating exactly the same build IDs as the linker isn't a
requirement, it sounds like?

- Eric

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Ian Rogers]

On Tue, Dec 2, 2025 at 3:27 PM Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Tue, Dec 02, 2025 at 01:56:42PM -0800, Namhyung Kim wrote:
> > On Thu, Nov 27, 2025 at 02:18:22PM -0800, Eric Biggers wrote:
> > > On Thu, Nov 27, 2025 at 01:17:01PM -0800, Namhyung Kim wrote:
> > > > Hello,
> > > >
> > > > On Tue, Nov 25, 2025 at 07:04:52PM -0800, Eric Biggers wrote:
> > > > > On Tue, Nov 25, 2025 at 06:55:39PM -0800, Ian Rogers wrote:
> > > > > > On Tue, Nov 25, 2025 at 11:29 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > > > > >
> > > > > > > On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> > > > > > > >
> > > > > > > > Looking back at the original code before the conversion, it used the
> > > > > > > > load_addr as well as the code section to distinguish each DSO.  But it'd
> > > > > > > > be better to use contents of symtab and strtab instead as it aligns with
> > > > > > > > some linker behaviors.
> > > > > > > >
> > > > > > > > This patch adds a buffer to save all the contents in a single place for
> > > > > > > > SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> > > > > > > > update the existing hash value with different contents and use it here.
> > > > > > > > But it's out of scope for this change and 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>
> > > > > > > > Cc: Fangrui Song <maskray@sourceware.org>
> > > > > > > > Link: https://github.com/python/cpython/issues/139544
> > > > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > > > >
> > > > > > > That commit actually preserved the behavior of the existing variant of
> > > > > > > gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
> > > > > > > was always broken, and it was just never noticed because the alternative
> > > > > > > variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?
> > > > > > >
> > > > > > > The MD5 variant of gen_build_id() just hashed the load_addr concatenated
> > > > > > > with the code.  That's not what this patch does, though.  So just to
> > > > > > > clarify, you'd actually like to go with a third approach rather than
> > > > > > > just restoring the original hash(load_addr || code) approach?
> > > > > > >
> > > > > > > Also, I missed that you had actually changed the hash algorithm.  I had
> > > > > > > assumed the perf folks were were pushing SHA-1 because they were already
> > > > > > > using it.  Given that the algorithm changed, there must not be any
> > > > > > > backwards compatibility concerns here, and you should switch to a modern
> > > > > > > hash algorithm such as SHA-256 instead.
> > > > > > >
> > > > > > > I'd be glad to add an incremental API if you need it, but I'm confused
> > > > > > > why you want SHA-1 and not a modern hash algorithm.
> > > > > >
> > > > > > Hi Eric,
> > > > > >
> > > > > > Thanks for the help with the hash functions! There's a bit more
> > > > > > context in this thread:
> > > > > > https://lore.kernel.org/linux-perf-users/CAP-5=fWLgaWsv82dcPajVk=UmBbmwyEd7OVp6psZQ4TiXh-Meg@mail.gmail.com/
> > > > > >
> > > > > > So genelf is trying to take snippets of jitted code and create ELF
> > > > > > files from them for the purpose of symbolizing in perf. The buildid
> > > > > > hash being used is SHA1 and I think the MD5 support was removed as
> > > > > > unnecessary. The problem this patch is addressing is that a JIT may
> > > > > > create many identical stubs which then end up being deduplicated into
> > > > > > the same buildid as only the code is hashed. The BFD linker seems to
> > > > > > have the same issue (Fangrui filed a bug), gold and lld appear to hash
> > > > > > the symbols (which Namhyung adds to genelf here) but still yield
> > > > > > different build id for the same source assembly code. It is possible
> > > > > > to hash the address of the symbol rather than the symbol itself, but I
> > > > > > think the intent for the code should be to best match what a compiler
> > > > > > and linker would generate. The problem there is that this differs for
> > > > > > every linker :-)
> > > > > >
> > > > > > Something that is unfortunate in the code now is copying/concatenating
> > > > > > all the build data for the sake of producing the hash. It would be
> > > > > > nice if the code could incrementally build up the sha1 hash to avoid
> > > > > > the copying. I don't know if there is functionality for this
> > > > > > currently.
> > > > >
> > > > > Again, I can add support for incremental hashing if you need it.  But I
> > > > > don't understand why you want the hash function to be SHA-1.  Also,
> > > > > given that this seems to be a regression fix, it's surprising that
> > > > > you're suddenly changing the inputs to the hash entirely, instead of
> > > > > just going back to hashing load_addr concatenated with the code for now.
> > > >
> > > > Historically MD5 or SHA1 was the only choice for build-ID in linkers.
> > > > I don't know if it's changed lately though.  But because of that we
> > > > support build-IDs up to 20 bytes and it's in the UAPI too (well.. it's
> > > > implicit in PERF_RECORD_MMAP2).  I think we can support other hashs if
> > > > they fit into it, but SHA-1 would be the right choice for now.
> > > >
> > > > About load_addr, I don't think it's the right fix.  We cannot have the
> > > > exactly same build-IDs as before since we already switched to SHA-1.
> > > > What we actually want is a way to generate unique IDs for each DSO.
> > > > For that purpose, it'd be nice to use sym/str-tables as Fangrui said
> > > > instead of using load_addr which seems to be a quick hack in the past.
> > > >
> > > > So I think I can merge this change for stable fix.  And add incremental
> > > > SHA-1 (thanks in advance!) and update the code to it later.
> > >
> > > If it needs to be 20 bytes, you should just use a truncated SHA-256.
> > >
> > > The only use for SHA-1 is backwards compatibility.  Which it seems you
> > > don't need.
> >
> > I think the linker (ld) --build-id still defaults to SHA-1.
>
> But generating exactly the same build IDs as the linker isn't a
> requirement, it sounds like?

I agree, I don't think it is required and every linker is generating
different build IDs from my experimentation. I think we can fix this
as a follow up though.

Thanks,
Ian

> - Eric

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Namhyung Kim]

On Tue, Dec 02, 2025 at 03:26:59PM -0800, Ian Rogers wrote:
> On Tue, Dec 2, 2025 at 1:56 PM Namhyung Kim <namhyung@kernel.org> wrote:
> >
> > On Thu, Nov 27, 2025 at 02:18:22PM -0800, Eric Biggers wrote:
> > > On Thu, Nov 27, 2025 at 01:17:01PM -0800, Namhyung Kim wrote:
> > > > Hello,
> > > >
> > > > On Tue, Nov 25, 2025 at 07:04:52PM -0800, Eric Biggers wrote:
> > > > > On Tue, Nov 25, 2025 at 06:55:39PM -0800, Ian Rogers wrote:
> > > > > > On Tue, Nov 25, 2025 at 11:29 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > > > > >
> > > > > > > On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> > > > > > > >
> > > > > > > > Looking back at the original code before the conversion, it used the
> > > > > > > > load_addr as well as the code section to distinguish each DSO.  But it'd
> > > > > > > > be better to use contents of symtab and strtab instead as it aligns with
> > > > > > > > some linker behaviors.
> > > > > > > >
> > > > > > > > This patch adds a buffer to save all the contents in a single place for
> > > > > > > > SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> > > > > > > > update the existing hash value with different contents and use it here.
> > > > > > > > But it's out of scope for this change and 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>
> > > > > > > > Cc: Fangrui Song <maskray@sourceware.org>
> > > > > > > > Link: https://github.com/python/cpython/issues/139544
> > > > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > > > >
> > > > > > > That commit actually preserved the behavior of the existing variant of
> > > > > > > gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
> > > > > > > was always broken, and it was just never noticed because the alternative
> > > > > > > variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?
> > > > > > >
> > > > > > > The MD5 variant of gen_build_id() just hashed the load_addr concatenated
> > > > > > > with the code.  That's not what this patch does, though.  So just to
> > > > > > > clarify, you'd actually like to go with a third approach rather than
> > > > > > > just restoring the original hash(load_addr || code) approach?
> > > > > > >
> > > > > > > Also, I missed that you had actually changed the hash algorithm.  I had
> > > > > > > assumed the perf folks were were pushing SHA-1 because they were already
> > > > > > > using it.  Given that the algorithm changed, there must not be any
> > > > > > > backwards compatibility concerns here, and you should switch to a modern
> > > > > > > hash algorithm such as SHA-256 instead.
> > > > > > >
> > > > > > > I'd be glad to add an incremental API if you need it, but I'm confused
> > > > > > > why you want SHA-1 and not a modern hash algorithm.
> > > > > >
> > > > > > Hi Eric,
> > > > > >
> > > > > > Thanks for the help with the hash functions! There's a bit more
> > > > > > context in this thread:
> > > > > > https://lore.kernel.org/linux-perf-users/CAP-5=fWLgaWsv82dcPajVk=UmBbmwyEd7OVp6psZQ4TiXh-Meg@mail.gmail.com/
> > > > > >
> > > > > > So genelf is trying to take snippets of jitted code and create ELF
> > > > > > files from them for the purpose of symbolizing in perf. The buildid
> > > > > > hash being used is SHA1 and I think the MD5 support was removed as
> > > > > > unnecessary. The problem this patch is addressing is that a JIT may
> > > > > > create many identical stubs which then end up being deduplicated into
> > > > > > the same buildid as only the code is hashed. The BFD linker seems to
> > > > > > have the same issue (Fangrui filed a bug), gold and lld appear to hash
> > > > > > the symbols (which Namhyung adds to genelf here) but still yield
> > > > > > different build id for the same source assembly code. It is possible
> > > > > > to hash the address of the symbol rather than the symbol itself, but I
> > > > > > think the intent for the code should be to best match what a compiler
> > > > > > and linker would generate. The problem there is that this differs for
> > > > > > every linker :-)
> > > > > >
> > > > > > Something that is unfortunate in the code now is copying/concatenating
> > > > > > all the build data for the sake of producing the hash. It would be
> > > > > > nice if the code could incrementally build up the sha1 hash to avoid
> > > > > > the copying. I don't know if there is functionality for this
> > > > > > currently.
> > > > >
> > > > > Again, I can add support for incremental hashing if you need it.  But I
> > > > > don't understand why you want the hash function to be SHA-1.  Also,
> > > > > given that this seems to be a regression fix, it's surprising that
> > > > > you're suddenly changing the inputs to the hash entirely, instead of
> > > > > just going back to hashing load_addr concatenated with the code for now.
> > > >
> > > > Historically MD5 or SHA1 was the only choice for build-ID in linkers.
> > > > I don't know if it's changed lately though.  But because of that we
> > > > support build-IDs up to 20 bytes and it's in the UAPI too (well.. it's
> > > > implicit in PERF_RECORD_MMAP2).  I think we can support other hashs if
> > > > they fit into it, but SHA-1 would be the right choice for now.
> > > >
> > > > About load_addr, I don't think it's the right fix.  We cannot have the
> > > > exactly same build-IDs as before since we already switched to SHA-1.
> > > > What we actually want is a way to generate unique IDs for each DSO.
> > > > For that purpose, it'd be nice to use sym/str-tables as Fangrui said
> > > > instead of using load_addr which seems to be a quick hack in the past.
> > > >
> > > > So I think I can merge this change for stable fix.  And add incremental
> > > > SHA-1 (thanks in advance!) and update the code to it later.
> > >
> > > If it needs to be 20 bytes, you should just use a truncated SHA-256.
> > >
> > > The only use for SHA-1 is backwards compatibility.  Which it seems you
> > > don't need.
> >
> > I think the linker (ld) --build-id still defaults to SHA-1.
> 
> Reviewed-by: Ian Rogers <irogers@google.com>
> 
> I think everyone is agreeing. Let's fix the python JIT issue with
> what's here with SHA-1 and work upon truncated SHA-256 with
> incremental hashing that's truncated to 20 bytes and without the
> load_addr hack. We could also hash other ELF sections from the jitdump
> output at the same time as it may be theoretically possible to have
> matching object code and symbols but differing frame layouts. Thanks
> Namhyung for working on the fix, Eric and Fangrui for the hashing
> support and build ID insights!

Yep, thanks Ian for your careful review!

Namhyung

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Fangrui Song]

On Tue, Dec 2, 2025 at 1:56 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> On Thu, Nov 27, 2025 at 02:18:22PM -0800, Eric Biggers wrote:
> > On Thu, Nov 27, 2025 at 01:17:01PM -0800, Namhyung Kim wrote:
> > > Hello,
> > >
> > > On Tue, Nov 25, 2025 at 07:04:52PM -0800, Eric Biggers wrote:
> > > > On Tue, Nov 25, 2025 at 06:55:39PM -0800, Ian Rogers wrote:
> > > > > On Tue, Nov 25, 2025 at 11:29 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > > > > >
> > > > > > On Tue, Nov 25, 2025 at 12:07:46AM -0800, Namhyung Kim 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.
> > > > > > >
> > > > > > > Looking back at the original code before the conversion, it used the
> > > > > > > load_addr as well as the code section to distinguish each DSO.  But it'd
> > > > > > > be better to use contents of symtab and strtab instead as it aligns with
> > > > > > > some linker behaviors.
> > > > > > >
> > > > > > > This patch adds a buffer to save all the contents in a single place for
> > > > > > > SHA-1 calculation.  Probably we need to add sha1_update() or similar to
> > > > > > > update the existing hash value with different contents and use it here.
> > > > > > > But it's out of scope for this change and 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>
> > > > > > > Cc: Fangrui Song <maskray@sourceware.org>
> > > > > > > Link: https://github.com/python/cpython/issues/139544
> > > > > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > > > > >
> > > > > > That commit actually preserved the behavior of the existing variant of
> > > > > > gen_build_id() that was under #ifdef BUILD_ID_SHA.  So I guess that code
> > > > > > was always broken, and it was just never noticed because the alternative
> > > > > > variant of gen_build_id() under #ifdef BUILD_ID_MD5 was used instead?
> > > > > >
> > > > > > The MD5 variant of gen_build_id() just hashed the load_addr concatenated
> > > > > > with the code.  That's not what this patch does, though.  So just to
> > > > > > clarify, you'd actually like to go with a third approach rather than
> > > > > > just restoring the original hash(load_addr || code) approach?
> > > > > >
> > > > > > Also, I missed that you had actually changed the hash algorithm.  I had
> > > > > > assumed the perf folks were were pushing SHA-1 because they were already
> > > > > > using it.  Given that the algorithm changed, there must not be any
> > > > > > backwards compatibility concerns here, and you should switch to a modern
> > > > > > hash algorithm such as SHA-256 instead.
> > > > > >
> > > > > > I'd be glad to add an incremental API if you need it, but I'm confused
> > > > > > why you want SHA-1 and not a modern hash algorithm.
> > > > >
> > > > > Hi Eric,
> > > > >
> > > > > Thanks for the help with the hash functions! There's a bit more
> > > > > context in this thread:
> > > > > https://lore.kernel.org/linux-perf-users/CAP-5=fWLgaWsv82dcPajVk=UmBbmwyEd7OVp6psZQ4TiXh-Meg@mail.gmail.com/
> > > > >
> > > > > So genelf is trying to take snippets of jitted code and create ELF
> > > > > files from them for the purpose of symbolizing in perf. The buildid
> > > > > hash being used is SHA1 and I think the MD5 support was removed as
> > > > > unnecessary. The problem this patch is addressing is that a JIT may
> > > > > create many identical stubs which then end up being deduplicated into
> > > > > the same buildid as only the code is hashed. The BFD linker seems to
> > > > > have the same issue (Fangrui filed a bug), gold and lld appear to hash
> > > > > the symbols (which Namhyung adds to genelf here) but still yield
> > > > > different build id for the same source assembly code. It is possible
> > > > > to hash the address of the symbol rather than the symbol itself, but I
> > > > > think the intent for the code should be to best match what a compiler
> > > > > and linker would generate. The problem there is that this differs for
> > > > > every linker :-)
> > > > >
> > > > > Something that is unfortunate in the code now is copying/concatenating
> > > > > all the build data for the sake of producing the hash. It would be
> > > > > nice if the code could incrementally build up the sha1 hash to avoid
> > > > > the copying. I don't know if there is functionality for this
> > > > > currently.
> > > >
> > > > Again, I can add support for incremental hashing if you need it.  But I
> > > > don't understand why you want the hash function to be SHA-1.  Also,
> > > > given that this seems to be a regression fix, it's surprising that
> > > > you're suddenly changing the inputs to the hash entirely, instead of
> > > > just going back to hashing load_addr concatenated with the code for now.
> > >
> > > Historically MD5 or SHA1 was the only choice for build-ID in linkers.
> > > I don't know if it's changed lately though.  But because of that we
> > > support build-IDs up to 20 bytes and it's in the UAPI too (well.. it's
> > > implicit in PERF_RECORD_MMAP2).  I think we can support other hashs if
> > > they fit into it, but SHA-1 would be the right choice for now.
> > >
> > > About load_addr, I don't think it's the right fix.  We cannot have the
> > > exactly same build-IDs as before since we already switched to SHA-1.
> > > What we actually want is a way to generate unique IDs for each DSO.
> > > For that purpose, it'd be nice to use sym/str-tables as Fangrui said
> > > instead of using load_addr which seems to be a quick hack in the past.
> > >
> > > So I think I can merge this change for stable fix.  And add incremental
> > > SHA-1 (thanks in advance!) and update the code to it later.
> >
> > If it needs to be 20 bytes, you should just use a truncated SHA-256.
> >
> > The only use for SHA-1 is backwards compatibility.  Which it seems you
> > don't need.
>
> I think the linker (ld) --build-id still defaults to SHA-1.
>
> Thanks,
> Namhyung
>

In all GNU ld compatible linkers I am aware of, --build-id is an alias
for --build-id=sha1.
However, lld (since https://reviews.llvm.org/D121531 ) and mold
actually truncate BLAKE3 to 20 bytes for "sha1".

Re: [PATCH v2 1/2] perf jitdump: Add sym/str-tables to build-ID generation

[Namhyung Kim]

On Tue, 25 Nov 2025 00:07:46 -0800, Namhyung Kim 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.
> 
> [...]
Applied to perf-tools-next, thanks!

Best regards,
Namhyung