From 58407e041e62a221d2ac823a42c41b23b2aa984b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 May 2022 15:32:15 -0700 Subject: tests: using custom GIT_EXEC_PATH breaks --valgrind tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a regression in b7d11a0f5d2 (tests: exercise the RUNTIME_PREFIX feature, 2021-07-24) where tests that want to set up and test a "git" wrapper in $PATH conflicted with the t/bin/valgrind wrapper(s) doing the same. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t0060-path-utils.sh | 4 ++-- t/test-lib.sh | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 2fe6ae6a4e..aa35350b6f 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -542,7 +542,7 @@ test_lazy_prereq CAN_EXEC_IN_PWD ' ./git rev-parse ' -test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' ' +test_expect_success !VALGRIND,RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' ' mkdir -p pretend/bin pretend/libexec/git-core && echo "echo HERE" | write_script pretend/libexec/git-core/git-here && cp "$GIT_EXEC_PATH"/git$X pretend/bin/ && @@ -550,7 +550,7 @@ test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD 'RUNTIME_PREFIX works' ' echo HERE >expect && test_cmp expect actual' -test_expect_success RUNTIME_PREFIX,CAN_EXEC_IN_PWD '%(prefix)/ works' ' +test_expect_success !VALGRIND,RUNTIME_PREFIX,CAN_EXEC_IN_PWD '%(prefix)/ works' ' mkdir -p pretend/bin && cp "$GIT_EXEC_PATH"/git$X pretend/bin/ && git config yes.path "%(prefix)/yes" && diff --git a/t/test-lib.sh b/t/test-lib.sh index 531cef097d..7f3d323e93 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1666,6 +1666,7 @@ test -n "$USE_LIBPCRE2" && test_set_prereq PCRE test -n "$USE_LIBPCRE2" && test_set_prereq LIBPCRE2 test -z "$NO_GETTEXT" && test_set_prereq GETTEXT test -n "$SANITIZE_LEAK" && test_set_prereq SANITIZE_LEAK +test -n "$GIT_VALGRIND_ENABLED" && test_set_prereq VALGRIND if test -z "$GIT_TEST_CHECK_CACHE_TREE" then -- cgit v1.2.3 From 29d8e21d6e97d5363233a960e7730e0afc26a8b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 May 2022 15:32:16 -0700 Subject: log test: skip a failing mkstemp() test under valgrind MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Skip a test added in f1e3df31699 (t: increase test coverage of signature verification output, 2020-03-04) when running under valgrind. Due to valgrind's interception of mkstemp() this test will fail with: + pwd + TMPDIR=[...]/t/trash directory.t4202-log/bogus git log --show-signature -n1 plain-fail ==7696== VG_(mkstemp): failed to create temp file: [...]/t/trash directory.t4202-log/bogus/valgrind_proc_7696_cmdline_d545ddcf [... 10 more similar lines omitted ..] valgrind: Startup or configuration error: valgrind: Can't create client cmdline file in [...]/t/trash directory.t4202-log/bogus/valgrind_proc_7696_cmdline_6e542d1d valgrind: Unable to start up properly. Giving up. error: last command exited with $?=1 Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- t/t4202-log.sh | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/t/t4202-log.sh b/t/t4202-log.sh index be07407f85..6e66352558 100755 --- a/t/t4202-log.sh +++ b/t/t4202-log.sh @@ -1992,10 +1992,13 @@ test_expect_success GPG 'log --show-signature for merged tag with GPG failure' ' git tag -s -m signed_tag_msg signed_tag_fail && git checkout plain-fail && git merge --no-ff -m msg signed_tag_fail && - TMPDIR="$(pwd)/bogus" git log --show-signature -n1 plain-fail >actual && - grep "^merged tag" actual && - grep "^No signature" actual && - ! grep "^gpg: Signature made" actual + if ! test_have_prereq VALGRIND + then + TMPDIR="$(pwd)/bogus" git log --show-signature -n1 plain-fail >actual && + grep "^merged tag" actual && + grep "^No signature" actual && + ! grep "^gpg: Signature made" actual + fi ' test_expect_success GPGSM 'log --graph --show-signature for merged tag x509' ' -- cgit v1.2.3 From 4627c67fa68d5669be511962a6437a11c0db3c99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 May 2022 15:32:18 -0700 Subject: object-file: fix a unpack_loose_header() regression in 3b6a8db3b03 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a regression in my 3b6a8db3b03 (object-file.c: use "enum" return type for unpack_loose_header(), 2021-10-01) revealed both by running the test suite with --valgrind, and with the amended "git fsck" test. In practice this regression in v2.34.0 caused us to claim that we couldn't parse the header, as opposed to not being able to unpack it. Before the change in the C code the test_cmp added here would emit: -error: unable to unpack header of ./objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 +error: unable to parse header of ./objects/e6/9de29bb2d1d6434b8b29ae775ad8c2e48c5391 I.e. we'd proceed to call parse_loose_header() on the uninitialized "hdr" value, and it would have been very unlikely for that uninitialized memory to be a valid git object. The other callers of unpack_loose_header() were already checking the enum values exhaustively. See 3b6a8db3b03 and 5848fb11acd (object-file.c: return ULHR_TOO_LONG on "header too long", 2021-10-01). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- object-file.c | 8 ++++++-- t/t1006-cat-file.sh | 10 ++++++++-- t/t1450-fsck.sh | 13 +++++++++++-- 3 files changed, 25 insertions(+), 6 deletions(-) diff --git a/object-file.c b/object-file.c index 5ffbf3d4fd..b5d1d12b68 100644 --- a/object-file.c +++ b/object-file.c @@ -2623,8 +2623,12 @@ int read_loose_object(const char *path, goto out; } - if (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr), - NULL) < 0) { + switch (unpack_loose_header(&stream, map, mapsize, hdr, sizeof(hdr), + NULL)) { + case ULHR_OK: + break; + case ULHR_BAD: + case ULHR_TOO_LONG: error(_("unable to unpack header of %s"), path); goto out; } diff --git a/t/t1006-cat-file.sh b/t/t1006-cat-file.sh index 1b85207694..dadf3b1458 100755 --- a/t/t1006-cat-file.sh +++ b/t/t1006-cat-file.sh @@ -681,7 +681,7 @@ test_expect_success 'cat-file -t and -s on corrupt loose object' ' # Setup and create the empty blob and its path empty_path=$(git rev-parse --git-path objects/$(test_oid_to_path "$EMPTY_BLOB")) && - git hash-object -w --stdin other.blob && @@ -722,7 +722,13 @@ test_expect_success 'cat-file -t and -s on corrupt loose object' ' # content out as-is. Try to make it zlib-invalid. mv -f other.blob "$empty_path" && test_must_fail git fsck 2>err.fsck && - grep "^error: inflate: data stream error (" err.fsck + cat >expect <<-EOF && + error: inflate: data stream error (incorrect header check) + error: unable to unpack header of ./$empty_path + error: $empty_blob: object corrupt or missing: ./$empty_path + EOF + grep "^error: " err.fsck >actual && + test_cmp expect actual ) ' diff --git a/t/t1450-fsck.sh b/t/t1450-fsck.sh index de50c0ea01..ab7f31f1dc 100755 --- a/t/t1450-fsck.sh +++ b/t/t1450-fsck.sh @@ -774,10 +774,19 @@ test_expect_success 'fsck finds problems in duplicate loose objects' ' # no "-d" here, so we end up with duplicates git repack && # now corrupt the loose copy - file=$(sha1_file "$(git rev-parse HEAD)") && + oid="$(git rev-parse HEAD)" && + file=$(sha1_file "$oid") && rm "$file" && echo broken >"$file" && - test_must_fail git fsck + test_must_fail git fsck 2>err && + + cat >expect <<-EOF && + error: inflate: data stream error (incorrect header check) + error: unable to unpack header of $file + error: $oid: object corrupt or missing: $file + EOF + grep "^error: " err >actual && + test_cmp expect actual ) ' -- cgit v1.2.3 From 7c898554d7a80e5d70ee8816a23dc425d2f1729c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 12 May 2022 15:32:17 -0700 Subject: commit-graph.c: don't assume that stat() succeeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix code added in 8d84097f965 (commit-graph: expire commit-graph files, 2019-06-18) to check the return value of the stat() system call. Not doing so caused us to use uninitialized memory in the "Bloom generation is limited by --max-new-filters" test in t4216-log-bloom.sh: + rm -f trace.event + pwd + GIT_TRACE2_EVENT=[...]/t/trash directory.t4216-log-bloom/limits/trace.event git commit-graph write --reachable --split=replace --changed-paths --max-new-filters=2 ==24835== Syscall param utimensat(times[0].tv_sec) points to uninitialised byte(s) ==24835== at 0x499E65A: __utimensat64_helper (utimensat.c:34) ==24835== by 0x4999142: utime (utime.c:36) ==24835== by 0x552BE0: mark_commit_graphs (commit-graph.c:2213) ==24835== by 0x550822: write_commit_graph (commit-graph.c:2424) ==24835== by 0x54E3A0: write_commit_graph_reachable (commit-graph.c:1681) ==24835== by 0x4374BB: graph_write (commit-graph.c:269) ==24835== by 0x436F7D: cmd_commit_graph (commit-graph.c:326) ==24835== by 0x407B9A: run_builtin (git.c:465) ==24835== by 0x406651: handle_builtin (git.c:719) ==24835== by 0x407575: run_argv (git.c:786) ==24835== by 0x406410: cmd_main (git.c:917) ==24835== by 0x511F09: main (common-main.c:56) ==24835== Address 0x1ffeffde70 is on thread 1's stack ==24835== in frame #1, created by utime (utime.c:25) ==24835== Uninitialised value was created by a stack allocation ==24835== at 0x552B50: mark_commit_graphs (commit-graph.c:2201) ==24835== [...] error: last command exited with $?=126 not ok 137 - Bloom generation is limited by --max-new-filters This would happen as we stat'd the non-existing ".git/objects/info/commit-graph" file. Let's fix mark_commit_graphs() to check the stat()'s return value, and while we're at it fix another case added in the same commit to do the same. The caller in expire_commit_graphs() would have been less likely to run into this, as it's operating on files it just got from readdir(), but it could still happen due to a race with e.g. a concurrent "rm -rf" of the commit-graph files. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- commit-graph.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/commit-graph.c b/commit-graph.c index 441b36016b..2b52818731 100644 --- a/commit-graph.c +++ b/commit-graph.c @@ -2206,7 +2206,8 @@ static void mark_commit_graphs(struct write_commit_graph_context *ctx) struct stat st; struct utimbuf updated_time; - stat(ctx->commit_graph_filenames_before[i], &st); + if (stat(ctx->commit_graph_filenames_before[i], &st) < 0) + continue; updated_time.actime = st.st_atime; updated_time.modtime = now; @@ -2247,7 +2248,8 @@ static void expire_commit_graphs(struct write_commit_graph_context *ctx) strbuf_setlen(&path, dirnamelen); strbuf_addstr(&path, de->d_name); - stat(path.buf, &st); + if (stat(path.buf, &st) < 0) + continue; if (st.st_mtime > expire_time) continue; -- cgit v1.2.3