git/list[1] front-page[2] threads[3] people[4] search[5] about
wed 2026-10-07 17:17 UTC

Re: [PATCH 4/4] Makefile: turn on NO_MMAP when building with LSan

From
Junio C Hamano <gitster@pobox.com>
Date
Mar 7, 2026, 01:14 UTC
Message-ID
<xmqqqzpwv3t7.fsf@gitster.g>
In-Reply-To
<20260305231305.GD2901305@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 8 quoted lines
> @@ -1600,6 +1600,7 @@ BASIC_CFLAGS += -DSHA1DC_FORCE_ALIGNED_ACCESS
>  endif
>  ifneq ($(filter leak,$(SANITIZERS)),)
>  BASIC_CFLAGS += -O0
> +NO_MMAP = CatchMapLeaks
>  SANITIZE_LEAK = YesCompiledWithIt
>  endif
>  ifneq ($(filter address,$(SANITIZERS)),)

And of course, this "breaks" the leaks job at CI without being the true culprit.

    https://github.com/git/git/actions/runs/22786105918/job/66103114142
My bisection between v2.52.0 and v2.53.0 with the following
    $ git bisect start v2.53.0 v2.52.0
    $ git bisect run sh :doit

where :doit has the shell script attached at the end of this message blames this commit. I didn't dig further than that.

commit 4c89d31494bff4bde6079a0e0821f1437e37d07b
Author: Patrick Steinhardt <ps@pks.im>
Date:   Sun Nov 23 19:59:37 2025 +0100
    streaming: rely on object sources to create object stream
    
    When creating an object stream we first look up the object info and, if
    it's present, we call into the respective backend that contains the
    object to create a new stream for it.
    
    This has the consequence that, for loose object source, we basically
    iterate through the object sources twice: we first discover that the
    file exists as a loose object in the first place by iterating through
    all sources. And, once we have discovered it, we again walk through all
    sources to try and map the object. The same issue will eventually also
    surface once the packfile store becomes per-object-source.
    
    Furthermore, it feels rather pointless to first look up the object only
    to then try and read it.
    
    Refactor the logic to be centered around sources instead. Instead of
    first reading the object, we immediately ask the source to create the
    object stream for us. If the object exists we get stream, otherwise
    we'll try the next source.
    
    Like this we only have to iterate through sources once. But even more
    importantly, this change also helps us to make the whole logic
    pluggable. The object read stream subsystem does not need to be aware of
    the different source backends anymore, but eventually it'll only have to
    call the source's callback function.
    
    Note that at the current point in time we aren't fully there yet:
    
      - The packfile store still sits on the object database level and is
        thus agnostic of the sources.
    
      - We still have to call into both the packfile store and the loose
        object source.
    
    But both of these issues will soon be addressed.
    
    This refactoring results in a slight change to semantics: previously, it
    was `odb_read_object_info_extended()` that picked the source for us, and
    it would have favored packed (non-deltified) objects over loose objects.
    And while we still favor packed over loose objects for a single source
    with the new logic, we'll now favor a loose object from an earlier
    source over a packed object from a later source.
    
    Ultimately this shouldn't matter though: the stream doesn't indicate to
    the caller which source it is from and whether it was created from a
    packed or loose object, so such details are opaque to the caller. And
    other than that we should be able to assume that two objects with the
    same object ID should refer to the same content, so the streamed data
    would be the same, too.
    
    Signed-off-by: Patrick Steinhardt <ps@pks.im>
    Signed-off-by: Junio C Hamano <gitster@pobox.com>
 streaming.c | 65 +++++++++++++++++++++++--------------------------------------
 1 file changed, 24 insertions(+), 41 deletions(-)
---- >8 ----
#!/bin/sh
git apply -3 <"$0" || {
	git reset --hard
	exit 125
}
(
	export SANITIZE=leak GIT_TEST_PASSING_SANITIZE_LEAK=true 
	make NO_MMAP=CatchMapLeaks CC=clang &&
	cd t && sh t1060-object-corruption.sh
)
status=$?

make distclean git reset --hard

exit $status
diff --git i/compat/mmap.c w/compat/mmap.c
index 2fe1c7732e..1a118711f7 100644
--- i/compat/mmap.c
+++ w/compat/mmap.c
@@ -38,7 +38,7 @@ void *git_mmap(void *start, size_t length, int prot, int flags, int fd, off_t of
 	return start;
 }
 
-int git_munmap(void *start, size_t length)
+int git_munmap(void *start, size_t length UNUSED)
 {
 	free(start);
 	return 0;
Previous: Ramsay JonesNext: Junio C Hamano
Message 21 of 25 in “memory leak when cloning a repository”
  1. Jacob KellerMar 5, 2026
  2. Jeff KingMar 5, 2026
  3. 0/4 plugging some mmap() leaksJeff King, Mar 5, 2026
  4. 1/4 check_connected(): delay opening new_packJeff King, Mar 5, 2026
  5. 2/4 check_connected(): fix leak of pack-index mmapJeff King, Mar 5, 2026
  6. 3/4 pack-revindex: avoid double-loading .rev filesJeff King, Mar 5, 2026
  7. 4/4 Makefile: turn on NO_MMAP when building with LSanJeff King, Mar 5, 2026
  8. Jacob KellerMar 5, 2026
  9. Jacob KellerMar 5, 2026
  10. Jacob KellerMar 5, 2026
  11. Ramsay JonesMar 6, 2026
  12. Jacob KellerMar 6, 2026
  13. Jeff KingMar 6, 2026
  14. 5/4 meson: turn on NO_MMAP when building with LSanJeff King, Mar 6, 2026
  15. Ramsay JonesMar 6, 2026
  16. Ramsay JonesMar 6, 2026
  17. Junio C HamanoMar 6, 2026
  18. Ramsay JonesMar 6, 2026
  19. Junio C HamanoMar 6, 2026
  20. Ramsay JonesMar 6, 2026
  21. Junio C HamanoMar 7, 2026
  22. Junio C HamanoMar 7, 2026
  23. 5/4 object-file: fix mmap() leak in odb_source_loose_read_object_stream()Jeff King, Mar 7, 2026
  24. Junio C HamanoMar 7, 2026
  25. Patrick SteinhardtMar 10, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.