git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/2] log: UNLEAK original pending objects

From
Andrzej Hunt via GitGitGadget <gitgitgadget@gmail.com>
Date
Sep 18, 2021, 13:49 UTC
Message-ID
<aad3fe7381ced5eeff9c8d57ce90911bc59e3923.1631972978.git.gitgitgadget@gmail.com>
In-Reply-To
<pull.1092.git.git.1631972978.gitgitgadget@gmail.com>
From: Andrzej Hunt <ajrhunt@google.com>

cmd_show() uses objects to point to rev.pending's objects. Later, it might detach rev.pending's objects: when handling an OBJ_COMMIT it will reset rev.pending (without freeing its objects). Detaching (as opposed to freeing) is necessary because cmd_show() continues iterating over the original objects array.

We choose to UNLEAK because there's no real advantage to cleaning up properly (cmd_show() exits immediately after looping over these objects). A number of alternatives exist, but are all significantly more complex for no gain:

Alternative 1:
  Convert objects into an object_array, and memcpy rev.pending into it
  (followed by detaching rev.pending immediately - making objects the
  owner of what used to be rev.pending). Then we could safely
  objects_array_clear() at the end of cmd_show(). And we can rely on
  a preexisting UNLEAK(rev) to avoid having to clean up rev.pending.
  This is a more complex and riskier approach vs a simple UNLEAK,
  and doesn't add any user-visible value.
Alternative 2:
  A variation on alternative 1. We make objects own the object_array as
  before. Once we're done, we free the new rev.pending array (which
  might be empty), and we memcpy objects back into rev.pending, relying
  on the existin UNLEAK(rev) to avoid having to free rev.pending.
ASAN output from t0000:
Direct leak of 41 byte(s) in 1 object(s) allocated from:
    #0 0x487504 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:437:3
    #1 0x9e4ef8 in xstrdup wrapper.c:29:14
    #2 0x86395d in add_object_array_with_path object.c:366:17
    #3 0x9264fc in add_pending_object_with_path revision.c:330:2
    #4 0x91c4e0 in handle_revision_arg_1 revision.c:2086:2
    #5 0x91bfcd in handle_revision_arg revision.c:2093:12
    #6 0x91ff5a in setup_revisions revision.c:2780:7
    #7 0x5a7678 in cmd_log_init_finish builtin/log.c:206:9
    #8 0x5a4f18 in cmd_log_init builtin/log.c:278:2
    #9 0x5a55d1 in cmd_show builtin/log.c:646:2
    #10 0x4cff30 in run_builtin git.c:461:11
    #11 0x4cdb00 in handle_builtin git.c:713:3
    #12 0x4cf527 in run_argv git.c:780:4
    #13 0x4cd426 in cmd_main git.c:911:19
    #14 0x6b2eb5 in main common-main.c:52:11
    #15 0x7f74fc9bd349 in __libc_start_main (/lib64/libc.so.6+0x24349)
SUMMARY: AddressSanitizer: 41 byte(s) leaked in 1 allocation(s).
Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
---
 builtin/log.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/builtin/log.c b/builtin/log.c
index 6faaddf17a6..769ee6a9258 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -702,7 +702,8 @@ int cmd_show(int argc, const char **argv, const char *prefix)
 			ret = error(_("unknown type: %d"), o->type);
 		}
 	}
-	free(objects);
+	UNLEAK(objects);
+
 	return ret;
 }
 
-- 
gitgitgadget
Previous: Jeff KingNext: Carlo Arenas
Message 10 of 15 in “Squash leaks in t0000”
  1. 0/2 Squash leaks in t0000Andrzej Hunt via GitGitGadget, Sep 18, 2021
  2. 1/2 log: UNLEAK rev to silence a large number of leaksAndrzej Hunt via GitGitGadget, Sep 18, 2021
  3. Carlo Marcelo Arenas BelónSep 18, 2021
  4. Andrzej HuntSep 19, 2021
  5. Ævar Arnfjörð BjarmasonSep 19, 2021
  6. Carlo Marcelo Arenas BelónSep 19, 2021
  7. Eric SunshineSep 20, 2021
  8. Carlo Marcelo Arenas BelónSep 20, 2021
  9. Jeff KingSep 21, 2021
  10. 2/2 log: UNLEAK original pending objectsAndrzej Hunt via GitGitGadget, Sep 18, 2021
  11. Carlo ArenasSep 18, 2021
  12. Andrzej HuntSep 19, 2021
  13. Ævar Arnfjörð BjarmasonSep 19, 2021
  14. Junio C HamanoSep 20, 2021
  15. Ævar Arnfjörð BjarmasonSep 21, 2021

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.