From patchwork Wed Jan 1 20:12:39 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13924260 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EE79A17548 for ; Wed, 1 Jan 2025 20:12:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762362; cv=none; b=KJFnxNq7bWdGdspZ4U+/2ycANxKNnsBzFWeZdMb3ztovFf5Sz0WVjxYopTCNj8I7ECRH/6mpIH1B+dx6Rk4H1LuJa51pPgthFWOKoPbOv0EuPUk31AJwOf2AP/VWJKq5ljqvNQGrLccBPpPajXyVaq+/cXbdFUPGjdLa24ycCCA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762362; c=relaxed/simple; bh=0Is+TP1bBaWX+RdaL9l6aKSBmp6DSgEnEA1/y0C8/do=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=Yq/L6vS/oFp7SaZswLYahYzHFGQst58YfTdD4JVXUZ6qy+Rk2G5RAJJXKtTHp9i1+IWh345isrmLyKTyipac7UOCCvNmSiAV/w/2fUHBs+SGqYLW0sXBPySaspc7fIMdUzyR53bR18miKDJWzoHTV6SjQTph6PyRSRM4leP8AhM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=iFzzMvqA; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="iFzzMvqA" Received: (qmail 22952 invoked by uid 109); 1 Jan 2025 20:12:40 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=0Is+TP1bBaWX+RdaL9l6aKSBmp6DSgEnEA1/y0C8/do=; b=iFzzMvqAo8coTy4q/L+G9/aBcWL0tVWlHEqNmPOYyw83Ws08ryfS+whugRvknYE4Rlnz6CLdHSAMj5k1fFWYX51YGCBF8N6eqYNHKBG+w5jVezKvzjGY2q1yuXEv/jGvt8zP6m8CmWI0l973V2Ml/xJq6Wi5pR6k10acXwSs9BFFuloRcg2bwWx6aUHibYdrf5JGv5Tbe4lKIP7R+o8VrJZlsFPADlS5GBF0oNbqH1GT5Ep6msXwlF09+uaZSyUCdf+vmzlWdNRjndFem9MVe95k008LUxz0ql4XBaIAk4gm2/uedwlgLoQPZ+SerC9TgmJTxMUJOmyYhq15WKCmfA== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Wed, 01 Jan 2025 20:12:40 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11395 invoked by uid 111); 1 Jan 2025 20:12:39 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Wed, 01 Jan 2025 15:12:39 -0500 Authentication-Results: peff.net; auth=none Date: Wed, 1 Jan 2025 15:12:39 -0500 From: Jeff King To: Junio C Hamano Cc: Patrick Steinhardt , git@vger.kernel.org Subject: [PATCH 1/6] test-lib: use individual lsan dir for --stress runs Message-ID: <20250101201239.GA3305462@coredump.intra.peff.net> References: <20250101201226.GA3304465@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250101201226.GA3304465@coredump.intra.peff.net> When storing output in test-results/, we usually give each numbered run in a --stress set its own output file. But we don't do that for storing LSan logs, so something like: ./t0003-attributes.sh --stress will have many scripts simultaneously creating, writing to, and deleting the test-results/t0003-attributes.leak directory. This can cause logs from one run to be attributed to another, spurious failures when creation and deletion race, and so on. This has always been broken, but nobody noticed because it's rare to do a --stress run with LSan (since the point is for the code to run quickly many times in order to hit races). But if you're trying to find a race in the leak sanitizing code, it makes sense to use these together. We can fix it by using $TEST_RESULTS_BASE, which already incorporates the stress job suffix. Signed-off-by: Jeff King --- t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 1a67adb207..96f2dfb69d 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -331,7 +331,7 @@ TEST_RESULTS_BASE="$TEST_RESULTS_DIR/$TEST_NAME$TEST_STRESS_JOB_SFX" TEST_RESULTS_SAN_FILE_PFX=trace TEST_RESULTS_SAN_DIR_SFX=leak TEST_RESULTS_SAN_FILE= -TEST_RESULTS_SAN_DIR="$TEST_RESULTS_DIR/$TEST_NAME.$TEST_RESULTS_SAN_DIR_SFX" +TEST_RESULTS_SAN_DIR="$TEST_RESULTS_BASE.$TEST_RESULTS_SAN_DIR_SFX" TRASH_DIRECTORY="trash directory.$TEST_NAME$TEST_STRESS_JOB_SFX" test -n "$root" && TRASH_DIRECTORY="$root/$TRASH_DIRECTORY" case "$TRASH_DIRECTORY" in From patchwork Wed Jan 1 20:12:52 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13924261 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 82DA11E25F9 for ; Wed, 1 Jan 2025 20:12:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762376; cv=none; b=Nh1rGoe1U1RiOUjMHaoveES+Ztcs6t4C1Pwr+/5AvVKcYWhEGRq24zcUL83e5OkYKlUbYgD/LuOlq0hTixQcQuJ6NTLKcSUMJoCtGvaa3tJDR8Z+H5POEJqVQx/gLGU8g0NJwCT+e4fWtdzYQA949tEBQjRQvvI6NmV3FuFA0p4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762376; c=relaxed/simple; bh=o8M8lrUMrVGDHqKmSFh0aJSUfYc8UuO5EWMnYyrNCWo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=QZl/Spl1riOeEX2lZGH1yA95G5Fzguc8TcjpcuYl0GXUcn7nbelSFBm0fJzrh4JxwMF5jnruOIP7vq5X637CLWl+Rhetwf7Mx5l/i93AT9e+9El+aX5ZDBXm7oE+nFGhmtawQ+2BgTAXlIZC9EMn0mfN54d6ewBIrFniI9oOi54= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=aM96B7oZ; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="aM96B7oZ" Received: (qmail 22965 invoked by uid 109); 1 Jan 2025 20:12:53 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=o8M8lrUMrVGDHqKmSFh0aJSUfYc8UuO5EWMnYyrNCWo=; b=aM96B7oZrcuWwfig7Gxn419o4GfrxVIYr44opkUUww5z8SRfUpyzYi0MhlDmqdYn3y1oFjaonmLQPfP1m0DRNnqyiP6g2j+Kzq8/W5EDP4b8oD+llK60PesjDxDXL3cCSPxrRCoYwlqUtC/SDcZYvbOsk29s36IzJQeUmHfqlymuYORpwLdFjA0WuEIlKP4XP8cWzZoZH13yO+7yomNJhAW4/H3rNGJlaefXNGOvKO6klhpPQFwBzOcOoUPtsBJbU9QfiP5WL7pbX6wyF02cg2INOGTeuXYH3283kDUJpQyxbuK5N+Ksw2cePncDcMzYfvaLsDxZVpxmeh6rXPI7TQ== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Wed, 01 Jan 2025 20:12:53 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11403 invoked by uid 111); 1 Jan 2025 20:12:53 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Wed, 01 Jan 2025 15:12:53 -0500 Authentication-Results: peff.net; auth=none Date: Wed, 1 Jan 2025 15:12:52 -0500 From: Jeff King To: Junio C Hamano Cc: Patrick Steinhardt , git@vger.kernel.org Subject: [PATCH 2/6] Revert "index-pack: spawn threads atomically" Message-ID: <20250101201252.GB3305462@coredump.intra.peff.net> References: <20250101201226.GA3304465@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250101201226.GA3304465@coredump.intra.peff.net> This reverts commit 993d38a0669a8056d496797516e743e26b6b8b54. That commit was trying to solve a race between LSan setting up the threads stack and another thread calling exit(), by making sure that all pthread_create() calls have finished before doing any work that might trigger the exit(). But that isn't sufficient. The setup code actually runs in the individual threads themselves, not in the spawning thread's call to pthread_create(). So while it may have improved the race a bit, you can still trigger it pretty quickly with: make SANITIZE=leak cd t ./t5309-pack-delta-cycles.sh --stress Let's back out that failed attempt so we can try again. Signed-off-by: Jeff King --- builtin/index-pack.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index d773809c4c..0b62b2589f 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1336,15 +1336,13 @@ static void resolve_deltas(struct pack_idx_option *opts) base_cache_limit = opts->delta_base_cache_limit * nr_threads; if (nr_threads > 1 || getenv("GIT_FORCE_THREADS")) { init_thread(); - work_lock(); for (i = 0; i < nr_threads; i++) { int ret = pthread_create(&thread_data[i].thread, NULL, threaded_second_pass, thread_data + i); if (ret) die(_("unable to create thread: %s"), strerror(ret)); } - work_unlock(); for (i = 0; i < nr_threads; i++) pthread_join(thread_data[i].thread, NULL); cleanup_thread(); From patchwork Wed Jan 1 20:14:44 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13924262 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D8A841E1C3F for ; Wed, 1 Jan 2025 20:14:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762487; cv=none; b=O1tT3kVqAfgB8e7rLB5GYwtfxAkxkPkIo3fhd6bc+HshWwTJnmNlpBiUVtbwC1u4ItKbx/nR2WWYqCoKSZ5LutIWrhI1BptxTETRxfvinHPypCCgJLUe0us5xIftFzpQgki5aa+vBWbbDOz1mhbSAdJUAZ1XFsEPLSTxc3ePcUs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762487; c=relaxed/simple; bh=4Cfd6fxqNEakItmMkE3Twh5HB5M/A+LcG79g5ojo4qg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=cRKj76o5yb6RyFoFc6o7lXTiobJ+uCxWOsqI7n4L2hrgdCrR1eX0hrJ6mqUk+anHdXzM8XeXSltxa5oPVDI253EMQQ7wAa2IZ3pacud2iuXKIMEHAMVF7H+XzdhyEXfPQXhwb20aGpPOY2Vh+fs7Ybdwy5c1yFeTE+sjJzux+ic= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=QkZ4olhM; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="QkZ4olhM" Received: (qmail 23016 invoked by uid 109); 1 Jan 2025 20:14:45 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=4Cfd6fxqNEakItmMkE3Twh5HB5M/A+LcG79g5ojo4qg=; b=QkZ4olhMkq3nZDVJxoWOl0o3/QYipXm0++CpOOq5tV70WJgr74K8MpcOz98J+fi2BFBJq+Jwf10niPZCNM/y2Iq6gp68KKGdyD+h7QGn3TMjO0DTVqQy0QEoMzISop9FCVKxgTYiDktW5h07oYdERqNDLLJ3BkrF4TIfnmgqg7LC9SDKQOw1TiB9okImdNZb2Gc2gRD4cZB0lDlOFR3EkSYNnjiOrcQFh0BS3WpriGWDcU22kQcATuq/HAHPGoUQspTnJRfAmb/+2CtPNGYMGwldUU7y+sZ6NTqTCsZPYy7d+ZVTxnt3C9LviyZ5iclMDK1+2eTqYoEQlsLY4/YU0Q== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Wed, 01 Jan 2025 20:14:45 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11409 invoked by uid 111); 1 Jan 2025 20:14:44 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Wed, 01 Jan 2025 15:14:44 -0500 Authentication-Results: peff.net; auth=none Date: Wed, 1 Jan 2025 15:14:44 -0500 From: Jeff King To: Junio C Hamano Cc: Patrick Steinhardt , git@vger.kernel.org Subject: [PATCH 3/6] test-lib: rely on logs to detect leaks Message-ID: <20250101201444.GC3305462@coredump.intra.peff.net> References: <20250101201226.GA3304465@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250101201226.GA3304465@coredump.intra.peff.net> When we run with sanitizers, we set abort_on_error=1 so that the tests themselves can detect problems directly (when the buggy program exits with SIGABRT). This has one blind spot, though: we don't always check the exit codes for all programs (e.g., helpers like upload-pack invoked behind the scenes). For ASan and UBSan this is mostly fine; they exit as soon as they see an error, so the unexpected abort of the program causes the test to fail anyway. But for LSan, the program runs to completion, since we can only check for leaks at the end. And in that case we could miss leak reports. And thus we started checking LSan logs in faececa53f (test-lib: have the "check" mode for SANITIZE=leak consider leak logs, 2022-07-28). Originally the logs were optional, but logs are generated (and checked) always as of 8c1d6691bc (test-lib: GIT_TEST_SANITIZE_LEAK_LOG enabled by default, 2024-07-11). And we even check them for each test snippet, as of cf1464331b (test-lib: check for leak logs after every test, 2024-09-24). So now aborting on error is superfluous for LSan! We can get everything we need by checking the logs. And checking the logs is actually preferable, since it gives us more control over silencing false positives (something we do not yet do, but will soon). So let's tell LSan to just exit normally, even if it finds leaks. We can do so with exitcode=0, which also suppresses the abort_on_error flag. Signed-off-by: Jeff King --- t/test-lib.sh | 1 + 1 file changed, 1 insertion(+) diff --git a/t/test-lib.sh b/t/test-lib.sh index 96f2dfb69d..dd2ba6e6cc 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -80,6 +80,7 @@ prepend_var ASAN_OPTIONS : detect_leaks=0 export ASAN_OPTIONS prepend_var LSAN_OPTIONS : $GIT_SAN_OPTIONS +prepend_var LSAN_OPTIONS : exitcode=0 prepend_var LSAN_OPTIONS : fast_unwind_on_malloc=0 export LSAN_OPTIONS From patchwork Wed Jan 1 20:17:21 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13924263 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B0DB04C70 for ; Wed, 1 Jan 2025 20:17:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762644; cv=none; b=jn+WbOLPaxn1Bbk9zv+JEhKKS5lmnmilwxOK8PfLSVLuz9Ex8ChQJV99UQFdWEMyAgqnfMsyuuhF7L4OccDGc4RvpRfz8yQwCKkrTNlXvcm023b3bD03SK6PCOxtc8urYJNUVkDU1t2V74qdFof9HyXqwkHk30uC6H5TzUawu6s= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762644; c=relaxed/simple; bh=Yhqv4yUc4HC6DvIsPu8HwuVZo5JKElaZwtx4t/Rmh2E=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=am07Qjnhlck78Yh1QugeXNdTdtNh8GhfXiUC1AR1fEBnYzOP+n1H8MoVZdDh4K8P5DGxQWnXyBZpFlo4XECFntR79lmLMZ8EQDl0KM5z8bM6QxfR6AbWphSTmtGloxqnsIbVu/5TJvIzxkMLTnVY0qRS2J4vL0zeFlWq8zK98fQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=TdR9sNxU; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="TdR9sNxU" Received: (qmail 23078 invoked by uid 109); 1 Jan 2025 20:17:22 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=Yhqv4yUc4HC6DvIsPu8HwuVZo5JKElaZwtx4t/Rmh2E=; b=TdR9sNxUn0VVxzNGOzUmU6pS9Wz/MOCDf4Z6sVNuQIjVF/k14mEbPG2uK53dIYBv6XcJcb28bGIKOVuuFcF3+fZn6DAd9Gc3hWJaBvtf21oNps+F8He6xvuAYqfSoeEGE69L0cMktdcAGEsbgcdZ+IRzATDpGotjseWs0WJUlKuEKETEYaRsIErnKO7rrTmXZTM/7B34vtZx6pw/D+xUs83uag5d7EAKecgGYZmnVDJ67Xb2+8q5M4kpNwFbVet5ZmVKFeV6hFpe4lmnrmkq3xghrcbva4K7sXEyecOgU7kBFZkO2PrC8HikrvJ3yRHuJUkkUoRRLN2Dt2qHUExXDQ== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Wed, 01 Jan 2025 20:17:22 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11458 invoked by uid 111); 1 Jan 2025 20:17:21 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Wed, 01 Jan 2025 15:17:21 -0500 Authentication-Results: peff.net; auth=none Date: Wed, 1 Jan 2025 15:17:21 -0500 From: Jeff King To: Junio C Hamano Cc: Patrick Steinhardt , git@vger.kernel.org Subject: [PATCH 4/6] test-lib: simplify leak-log checking Message-ID: <20250101201721.GD3305462@coredump.intra.peff.net> References: <20250101201226.GA3304465@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250101201226.GA3304465@coredump.intra.peff.net> We have a function to count the number of leaks found (actually, it is the number of processes which produced a log file). Once upon a time we cared about seeing if this number increased between runs. But we simplified that away in 95c679ad86 (test-lib: stop showing old leak logs, 2024-09-24), and now we only care if it returns any results or not. In preparation for refactoring it further, let's drop the counting function entirely, and roll it into the "is it empty" check. The outcome should be the same, but we'll be free to return a boolean "did we find anything" without worrying about somebody adding a new call to the counting function. Signed-off-by: Jeff King --- We need the extra "!" to invert the exit code of the final grep, which made my head spin a little at first. Maybe it would be less confusing if this was reporting non-empty results, as "check_test_results_has_leaks" or something? We'd have to update the callers, but there are only 2 of them. I dunno. t/test-lib.sh | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index dd2ba6e6cc..23eb26bfbe 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -340,17 +340,6 @@ case "$TRASH_DIRECTORY" in *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;; esac -# Utility functions using $TEST_RESULTS_* variables -nr_san_dir_leaks_ () { - # stderr piped to /dev/null because the directory may have - # been "rmdir"'d already. - find "$TEST_RESULTS_SAN_DIR" \ - -type f \ - -name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null | - xargs grep -lv "Unable to get registers from thread" | - wc -l -} - # If --stress was passed, run this test repeatedly in several parallel loops. if test "$GIT_TEST_STRESS_STARTED" = "done" then @@ -1181,8 +1170,14 @@ test_atexit_handler () { } check_test_results_san_file_empty_ () { - test -z "$TEST_RESULTS_SAN_FILE" || - test "$(nr_san_dir_leaks_)" = 0 + test -z "$TEST_RESULTS_SAN_FILE" && return 0 + + # stderr piped to /dev/null because the directory may have + # been "rmdir"'d already. + ! find "$TEST_RESULTS_SAN_DIR" \ + -type f \ + -name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null | + xargs grep -qv "Unable to get registers from thread" } check_test_results_san_file_ () { From patchwork Wed Jan 1 20:18:28 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13924264 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EEAC6208A0 for ; Wed, 1 Jan 2025 20:18:29 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762711; cv=none; b=lX1j+Cvx6g9ET+jSTwI7FNc++TUi9+kSbN8hcgEe4DRwG2Iezbh+0kHB/CfLFGGkQJTtqvaJz6xoyxC7JTn6Tm0RDfPrMqvFojQ+hayhjnpCUa8PObnFu6yvRdOOqHMoRdJC2aThgTTGp85tMT+i/o+CyYv1nrk2pGqs6QnLoTg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762711; c=relaxed/simple; bh=mmke7E8kk/ds2ca4e64S15GYbMccBzLD/fqTkKopWDk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=SLwQD42PBRKS1tndwZ0f42r1yKT+cGmk/1OvK7rBFszXqPl+vIeDJTenaC+8zbBWRt9EOZB8dBUkFb3AvNKecWtYJ+hXeqauHgS4epI/LE7eld3FVVxkjcuv2fNREgcAbmngvs7Tio67VGd1I74Horz8pVw2b3wqI7J3TXioHsQ= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=HKBygkvv; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="HKBygkvv" Received: (qmail 23118 invoked by uid 109); 1 Jan 2025 20:18:29 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=mmke7E8kk/ds2ca4e64S15GYbMccBzLD/fqTkKopWDk=; b=HKBygkvvheUBGhHMG7+u2k/JMpWTMnBicX+zXx0gbqG1lwjJLHMfxUU22go5x5pr4nT+fUo2sKyrijbrpWSqzLpdBbJ+F9ultQIVDkIkJ98pmkmavnXEnHmg6dIxgJiIV/MP6Dg7SWNgut5BY/GyuoXGPTTdGVWy5nHLvKDNTEo2y/KbjGqRtoFWbCqhrS38PUDyHmLtvLKR/AM+UMXOsth/3mvDOPcYria/3qsWBbhdOzXB1kjPK462COoD3vhPpJ0EHVedfAZRm/YCMDys/azuhfix18BNbuOMM9QxtN/mJs44f0AkbwWaREtw3kCohedxToiZb+XgyKlthCCLLQ== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Wed, 01 Jan 2025 20:18:29 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11471 invoked by uid 111); 1 Jan 2025 20:18:28 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Wed, 01 Jan 2025 15:18:28 -0500 Authentication-Results: peff.net; auth=none Date: Wed, 1 Jan 2025 15:18:28 -0500 From: Jeff King To: Junio C Hamano Cc: Patrick Steinhardt , git@vger.kernel.org Subject: [PATCH 5/6] test-lib: check leak logs for presence of DEDUP_TOKEN Message-ID: <20250101201828.GE3305462@coredump.intra.peff.net> References: <20250101201226.GA3304465@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250101201226.GA3304465@coredump.intra.peff.net> When we check the leak logs, our original strategy was to check for any non-empty log file produced by LSan. We later amended that to ignore noisy lines in 370ef7e40d (test-lib: ignore uninteresting LSan output, 2023-08-28). This makes it hard to ignore noise which is more than a single line; we'd have to actually parse the file to determine the meaning of each line. But there's an easy line-oriented solution. Because we always pass the dedup_token_length option, the output will contain a DEDUP_TOKEN line for each leak that has been found. So if we invert our strategy to stop ignoring useless lines and only look for useful ones, we can just count the number of DEDUP_TOKEN lines. If it's non-zero, then we found at least one leak (it would even give us a count of unique leaks, but we really only care if it is non-zero). This should yield the same outcome, but will help us build more false positive detection on top. Signed-off-by: Jeff King --- t/test-lib.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index 23eb26bfbe..c9487d0805 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1177,7 +1177,7 @@ check_test_results_san_file_empty_ () { ! find "$TEST_RESULTS_SAN_DIR" \ -type f \ -name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null | - xargs grep -qv "Unable to get registers from thread" + xargs grep -q ^DEDUP_TOKEN } check_test_results_san_file_ () { From patchwork Wed Jan 1 20:21:24 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13924265 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9E0214C70 for ; Wed, 1 Jan 2025 20:21:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762887; cv=none; b=lI7ko8Ex6I9UiS2COxi8mFXZ/XuEJeXGdhzLiw77/ujBmAKr+nQ9jtjM7Ufsy9qQ3bw0JbnxJ9hca1L2o1v5bb7vTXd4qIlqxiiPMT0WDY/d3ItxaLRdUdmFKH+i0oFnE+hYcs5NM6YR5ypneY67ppP3Zuurn3YcCCma6djSPZY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1735762887; c=relaxed/simple; bh=Ir1E6tAL3wB20C4EAbfVc1I1Vx/UvHwNn7a+XIvb5/0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=qmGBbI8Qk99dph0CA3NFnVQX/+ErNxJQsleHo7EIwWjTeeGzGTzKhnTF3qBTj+qjTdu5HWU5OhatVPr1fzz2eeptBGgfsJzoWmwK2wWIexue5XmVbrWXOR3UKdl23VJJTYEuMIqOSHwTe7URt4FESUkpz8OunIBI4Uxd2XCRBNk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=N62byAAP; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="N62byAAP" Received: (qmail 23181 invoked by uid 109); 1 Jan 2025 20:21:25 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=Ir1E6tAL3wB20C4EAbfVc1I1Vx/UvHwNn7a+XIvb5/0=; b=N62byAAPxFNgMb+yn8pubBIYS5N+PtCp29Beejb/vTNGKySDiv6qnZYtHP6UySa+Mtrk1pTs2QkaXcCxMTVYs2XSMJCfSIgeiqE8TlRgZXti4AIM+60Ii1nDwtjMrlyDj64piEC6Vy3EBAmXgbb0+dR0ggX5xfJhElUV0KSm+6x3rzMKOQncCmzXmnbAOQHt8kYXUegcYTDLc42s753qpXZPbs9t3MMLrUyVvV27BbUrts1M2BDz12kd64hgk3mQWu3iHK49kZleI5ILLH+N25H3iJgNQSmHNRbof/EsmgwjTKyq8ISN0I2MFqw4P+h7GZcssuh5MEp1i85S0lcbtQ== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Wed, 01 Jan 2025 20:21:25 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 11533 invoked by uid 111); 1 Jan 2025 20:21:24 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Wed, 01 Jan 2025 15:21:24 -0500 Authentication-Results: peff.net; auth=none Date: Wed, 1 Jan 2025 15:21:24 -0500 From: Jeff King To: Junio C Hamano Cc: Patrick Steinhardt , git@vger.kernel.org Subject: [PATCH 6/6] test-lib: ignore leaks in the sanitizer's thread code Message-ID: <20250101202124.GF3305462@coredump.intra.peff.net> References: <20250101201226.GA3304465@coredump.intra.peff.net> Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20250101201226.GA3304465@coredump.intra.peff.net> Our CI jobs sometimes see false positive leaks like this: ================================================================= ==3904583==ERROR: LeakSanitizer: detected memory leaks Direct leak of 32 byte(s) in 1 object(s) allocated from: #0 0x7fa790d01986 in __interceptor_realloc ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:98 #1 0x7fa790add769 in __pthread_getattr_np nptl/pthread_getattr_np.c:180 #2 0x7fa790d117c5 in __sanitizer::GetThreadStackTopAndBottom(bool, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:150 #3 0x7fa790d11957 in __sanitizer::GetThreadStackAndTls(bool, unsigned long*, unsigned long*, unsigned long*, unsigned long*) ../../../../src/libsanitizer/sanitizer_common/sanitizer_linux_libcdep.cpp:598 #4 0x7fa790d03fe8 in __lsan::ThreadStart(unsigned int, unsigned long long, __sanitizer::ThreadType) ../../../../src/libsanitizer/lsan/lsan_posix.cpp:51 #5 0x7fa790d013fd in __lsan_thread_start_func ../../../../src/libsanitizer/lsan/lsan_interceptors.cpp:440 #6 0x7fa790adc3eb in start_thread nptl/pthread_create.c:444 #7 0x7fa790b5ca5b in clone3 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 This is not a leak in our code, but appears to be a race between one thread calling exit() while another one is in LSan's stack setup code. You can reproduce it easily by running t0003 or t5309 with --stress (these trigger it because of the threading in git-grep and index-pack respectively). This may be a bug in LSan, but regardless of whether it is eventually fixed, it is useful to work around it so that we stop seeing these false positives. We can recognize it by the mention of the sanitizer functions in the DEDUP_TOKEN line. With this patch, the scripts mentioned above should run with --stress indefinitely. Signed-off-by: Jeff King --- One small downside here is that this just suppresses the "were there any leaks" check. If there's a real leak _and_ the race was triggered, then you'd see the racy false positive in the output. I don't think that's a big deal, since both the race and real leaks should be rare-ish, and you'd have to encounter both in the same run of a given test script. t/test-lib.sh | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/test-lib.sh b/t/test-lib.sh index c9487d0805..d1f62adbf8 100644 --- a/t/test-lib.sh +++ b/t/test-lib.sh @@ -1177,7 +1177,8 @@ check_test_results_san_file_empty_ () { ! find "$TEST_RESULTS_SAN_DIR" \ -type f \ -name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null | - xargs grep -q ^DEDUP_TOKEN + xargs grep ^DEDUP_TOKEN | + grep -qv sanitizer::GetThreadStackTopAndBottom } check_test_results_san_file_ () {