From patchwork Fri Mar 14 00:20:50 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 14016102 Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6105717D2 for ; Fri, 14 Mar 2025 00:20:57 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.54 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741911659; cv=none; b=Y69ODYSGwKkVuiAaTnGtwB7NHqsYmZjKzKQY8rbSb2jwjPvgqO6lEUsGRts0ABjW0NfGuKApFB7QJZRqPE9OKX/CWyJUrDHnjOFCl2GdnZCuqhyn8XARO8y3vu9CFTI9gIreFgK4fm8AClZpGpi7TcP2UC6tgpbvwxi0r9/nlj8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741911659; c=relaxed/simple; bh=9jjz5F1a8/dNZiM277C1aeXpMRBIIEehRF47/i0JKOs=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=PiFvqfOimNatWahja9dcHR9kUq3lU2fheUM9l88Raa2/vXvk2MLctIwgjGjo4PycLxU8wDgGssN5g7Z9Qyr8t19cJMy+3Y/aQmHl13GGhheyGAE+AyadFeUtoeLwzbrlMaFzDv82uspUBxvL8axrxNjbdbuMtlrpOQwKk+B7Zz8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=HP2Nwukk; arc=none smtp.client-ip=209.85.221.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="HP2Nwukk" Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-38f2f391864so892832f8f.3 for ; Thu, 13 Mar 2025 17:20:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741911654; x=1742516454; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=3ZpoFSg52lF4whc6ZSEX42NfZUMdvuxvhsh6l6nJHN0=; b=HP2Nwukk41++u5XWlNjKDgAWkJM8H5jJ7V1nQQIm9z/dYJ1kwHBX9+yOe3CZZqjxaJ Y1wwIBYleVnlQKA1w01S8hDWarlOlsBKpE+cTN1i0wOKJM2ppEnmSeY5V1sbEfnCtzPT R66XUNjbG4r3MGfGOAYo5UVbuqI3LJk2EtjNNQWn9AhExD56Ik9SEYmDo5uIrHjK0y0L abby4vIHWrWHrMpWy3KwLuwhLi5VGQ2Pgkw0G+kJg3xTXtzq5BTvNlCRtxjEUvfZlyrX 9Pr7Vm6WcG8/ryb246e1Umf/IW9qPVajWgHzH0VwmuvxebqjSepqO75rqmNjtJpolc/V Vwvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741911654; x=1742516454; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=3ZpoFSg52lF4whc6ZSEX42NfZUMdvuxvhsh6l6nJHN0=; b=J8CimVpN5X8BhSe5BDr14rVLepF2ulcJBENzevpTjvTazC1k0JsZDf77SJabFejkkq fQ1KM/+1n6DGQJGylM+DuXhJJl3e0VYt1m4sSzaScQRSXBbKA/skta6tPMMNGy8h8zp5 4OUIWqpfaXTrYcU+oVEpGoTLxjkA0vf1dkQZYqrz1BAJylFc3Uk2PA5xyUqISXLD6kdv /LZBbvpP2SA4y2rErtBvnWTirZ5A+bnQWY8irEXVRoEwd0qvNBITUG1KbHkarDYoF+QV o3CqLLTnfsrVuey4eZTpljanM1q2psq/tXJrGglSt85+HjzOXsAw/Qr9t+EYS6ADK7Ac JIAg== X-Gm-Message-State: AOJu0Yz3mSNgOm7Uk/egFEmgni9zN9J7ThSp8x4SuMhhXjRjtV5fSfS9 hkqfHaaVH7oU3up0lLRcNAoTxLJ6QCyOM/HlUkqXZMYHZY+tiFDlnSTh+w== X-Gm-Gg: ASbGncsy9kwOhTAlatwKFdjhkMWLZkS+9uuVhtd0yO9wyZpZNodTD0bnW5A2A1ZuTMI N3jrSxpJRZSYm370moxxpPSTs9r416t/FOeNjR6M0tUpWlz5f+HKKRU3d60MyHPOGWSGmrOGJgM ryu5lWY2wBRfjGZucaLp0YppXrKmgQLFDaR0GknX6IPriZoalz3itrt5BaLR6QUUSPsoiD0t06/ B9X1EEuM1eZci/f1j5uXU4oahHfS13WWMpijwZqZi+lbAunf+pWpPCWrBYLqoGkLXEPwwey8Cnu JRhB/0LClruHdHGtGvERo+FXHUqXt3sPULSHVvnN21LBRjnFBZd1WfYt X-Google-Smtp-Source: AGHT+IEAKDmrVrCPQfL2Ol/CE150cjcCKyZRkQNLHA45chtmWQ92nHkjx1MuDF9wgs+HnIUzMlYpag== X-Received: by 2002:a5d:588b:0:b0:391:4674:b136 with SMTP id ffacd0b85a97d-3971ded2b2dmr383104f8f.29.1741911654382; Thu, 13 Mar 2025 17:20:54 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-395cb318a12sm3678084f8f.75.2025.03.13.17.20.53 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Mar 2025 17:20:53 -0700 (PDT) Message-Id: <109060ccb8665c73aa0c4f73e3cbbddcd135bde4.1741911652.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 14 Mar 2025 00:20:50 +0000 Subject: [PATCH 1/3] git-compat-util: introduce BUG_IF_NOT() macro Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren Create a BUG_IF_NOT() macro which is similar to assert(), but will not be compiled out when NDEBUG is defined, and is thus safe to use even if its argument has side-effects. We will use this new macro in a subsequent commit to convert a few existing assert() invocations to BUG_IF_NOT(). In particular, we'll convert the handful of invocations which cannot be proven to be free of side effects with a simple compiler/linker hack. Signed-off-by: Elijah Newren --- git-compat-util.h | 1 + 1 file changed, 1 insertion(+) diff --git a/git-compat-util.h b/git-compat-util.h index e123288e8f1..c3415ad7e0a 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1460,6 +1460,7 @@ extern int bug_called_must_BUG; __attribute__((format (printf, 3, 4))) NORETURN void BUG_fl(const char *file, int line, const char *fmt, ...); #define BUG(...) BUG_fl(__FILE__, __LINE__, __VA_ARGS__) +#define BUG_IF_NOT(a) if (!(a)) BUG("Assertion `" #a "' failed.") __attribute__((format (printf, 3, 4))) void bug_fl(const char *file, int line, const char *fmt, ...); #define bug(...) bug_fl(__FILE__, __LINE__, __VA_ARGS__) From patchwork Fri Mar 14 00:20:51 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 14016103 Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 862A6645 for ; Fri, 14 Mar 2025 00:20:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.45 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741911660; cv=none; b=E7DKk9bHpWSGSf4XQZ5YaD9MAJzEkwlQTvv81L9H5aA/pIb3YrOzFovL6bXSlYsmfGwXj0+lLsd2dAEwQZlAh8gnIlBmH7T4HoNgXG/K7OzdOj8asrEOzhFkL0mVp54Td/YrdkLGfIgebaIKYO/0GO+q/fCUmJQSuW4ZcR/qAAk= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741911660; c=relaxed/simple; bh=mRgayroK1BcvKQ1Rwkhhb2r2ZaNecThni6T14iI6YB0=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=nrLaBaBbM316J8wh06w04nEx0aHfdJJWKxdV+ZQvSYM2V/KY+E2Fl80rL8B+BRZjQylGRSsr+pRD6aHJWUqHVeHdjd420vM/i2RbqNtApoThD6iDL4OQOLgYEPai/ntadea69nu5B/Jlpyun07O5PVecgefTw1P/MG6/8uTC5x8= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=agY239zb; arc=none smtp.client-ip=209.85.221.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="agY239zb" Received: by mail-wr1-f45.google.com with SMTP id ffacd0b85a97d-3913fdd003bso843105f8f.1 for ; Thu, 13 Mar 2025 17:20:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741911656; x=1742516456; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=b18BZcSRc2K7cJv/D+oSBt1xsZUSo9cA8FGp4u8zQRI=; b=agY239zbjajhTmXIxjRrVtzKjjS4/Nl9Wj72ICTTWJAy6vk8/ngLtUDZrWFijZIZvV 4uuNLBaNosSZ6DCS+WFDZJ1t0f8l5DNpl4ZzQ/M5giBnZK4w6q10LvVaON6+tCuYCRhc 0QB6AXL9pgY812i1U5B79V2Mk64h6bgXu3/4tsVVcDJ7FzSqs4ydmAURIxXv2+mVL+F0 tX7XuAj6os9EmGouIe6F2gvPhSLrjkvUY3dFaRPP4MponDltYtW3spQ2g5gZITLfP4OF 8Zv1GsnJokj/ZfyBZthrXbJMh2pZUIytFcj/XnJ8dQwVzuxXP0wJSfiDkfzY2pLCYUJM A0jA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741911656; x=1742516456; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=b18BZcSRc2K7cJv/D+oSBt1xsZUSo9cA8FGp4u8zQRI=; b=I896cL0okOC8qPe/oXDip0wzqZbCOvEUsw3cnCOS9TtRVEZAM3ovavgc5vIKjHOzJl /sgtxMVtWgXrRmNX5v+EHzgCMEwkV8NBGay5dffsv91269Ql1ZAKBglI4yZWIPpCvp+t vcfFShjVw+SjWFEUillxO7+Kk+TBxR3/OX+8k/ML8PJ/qcYkAKu3mC4MMpplPeYI0wPS vA7i7ybI2ZOBYWVug6uDQzEWqfTN7hWJMFb2TEvcWieVVz/c5SB0pLxv7r+VlbyukJ18 0shbYzDU6aWcARmwkrOauH80Kpx8pQjVMSUBd6Od+uGyxlpd6NnqwxP9+nNKMT+Y01lA ERpA== X-Gm-Message-State: AOJu0YxVhzHAS3L5A3xyQf6XtIO6m5Dg97i8GyII3EBpOAvgay3mi3QW VKIc9D39tc77MN0rh7y8XpC47DvR91Vttvovi588qLedNN0iuk0xOuAhsQ== X-Gm-Gg: ASbGncvSp50E+BNTAnRby1t+YPQ02lNjSkc9frC48cKCoXqWg9gb+VJqs+oqSuQYrxp ev9sGtYlH5XJlSbJT0y1mw72FFdmplA3OoqpX5j6UnNC6p13MbnTMNfin+SqStDrQ8m2DUhcVCl UHYt3PGOzeLe7b3IzeS+pYd/xxDe/IeOuOzTa+8PvdSFdg81WNXjbwZvVAnxPdXv+X9ew/Cg4aL 1ZAzm2PZLgOH7ToVjzBKMge8cot1ahagnIRQHlhMOBme+PbsqRyktAPzU33CtwQVIkldvFy4ot6 VU2Pnuo4kev6vpcZorZ7ZpwGxk0X2TrSPSRPebZbi8vQUQ== X-Google-Smtp-Source: AGHT+IETWu3r7f8/skQpVXs1kPJifZaYtDpJh2fnYrj875Ji6kEyLEBToujmhHEJfeE4wdi0Kx4oWg== X-Received: by 2002:a05:6000:1447:b0:391:65c:1b05 with SMTP id ffacd0b85a97d-3971bfce005mr519747f8f.11.1741911656384; Thu, 13 Mar 2025 17:20:56 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-395c83b6a10sm3706888f8f.36.2025.03.13.17.20.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Mar 2025 17:20:54 -0700 (PDT) Message-Id: <80dcc2ba3aa0ef72abe18f8525d571ea39ac6382.1741911652.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 14 Mar 2025 00:20:51 +0000 Subject: [PATCH 2/3] ci: add build checking for side-effects in assert() calls Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren It is a big no-no to have side-effects in an assertion, because if the assert() is compiled out, you don't get that side-effect, leading to the code behaving differently. That can be a large headache to debug. We have roughly 566 assert() calls in our codebase (my grep might have picked up things that aren't actually assert() calls, but most appeared to be). All but 9 of them can be determined by gcc to be free of side effects with a clever redefine of assert(). The current 9 appear to be free of side effects to me as well, but are too complicated for a compiler/linker to figure that since each assertion involves some kind of function call. Add a CI job which will find and report these possibly problematic assertions, and have the job suggest to the user that they replace these with BUG_IF_NOT() calls. Example output from running: ``` ERROR: The compiler could not verify the following assert() calls are free of side-effects. Please replace with BUG_IF_NOT() calls. /home/newren/floss/git/diffcore-rename.c:1409 assert(!dir_rename_count || strmap_empty(dir_rename_count)); /home/newren/floss/git/merge-ort.c:1645 assert(renames->deferred[side].trivial_merges_okay && !strset_contains(&renames->deferred[side].target_dirs, path)); /home/newren/floss/git/merge-ort.c:794 assert(omittable_hint == (!starts_with(type_short_descriptions[type], "CONFLICT") && !starts_with(type_short_descriptions[type], "ERROR")) || type == CONFLICT_DIR_RENAME_SUGGESTED); /home/newren/floss/git/merge-recursive.c:1200 assert(!merge_remote_util(commit)); /home/newren/floss/git/object-file.c:2709 assert(would_convert_to_git_filter_fd(istate, path)); /home/newren/floss/git/parallel-checkout.c:280 assert(is_eligible_for_parallel_checkout(pc_item->ce, &pc_item->ca)); /home/newren/floss/git/scalar.c:244 assert(have_fsmonitor_support()); /home/newren/floss/git/scalar.c:254 assert(have_fsmonitor_support()); /home/newren/floss/git/sequencer.c:4968 assert(!(opts->signoff || opts->no_commit || opts->record_origin || should_edit(opts) || opts->committer_date_is_author_date || opts->ignore_date)); ``` Note that if there are possibly problematic assertions, not necessarily all of them will be shown in a single run, because the compiler errors may include something like "ld: ... more undefined references to `not_supposed_to_survive' follow" instead of listing each individually. But in such cases, once you clean up a few that are shown in your first run, subsequent runs will show (some of) the ones that remain, allowing you to iteratively remove them all. Signed-off-by: Elijah Newren --- Makefile | 4 ++++ ci/check-unsafe-assertions.sh | 18 ++++++++++++++++++ ci/run-static-analysis.sh | 2 ++ git-compat-util.h | 6 ++++++ 4 files changed, 30 insertions(+) create mode 100755 ci/check-unsafe-assertions.sh diff --git a/Makefile b/Makefile index 7315507381e..57774912f18 100644 --- a/Makefile +++ b/Makefile @@ -2261,6 +2261,10 @@ ifdef WITH_BREAKING_CHANGES BASIC_CFLAGS += -DWITH_BREAKING_CHANGES endif +ifdef CHECK_ASSERTION_SIDE_EFFECTS + BASIC_CFLAGS += -DCHECK_ASSERTION_SIDE_EFFECTS +endif + ifdef INCLUDE_LIBGIT_RS # Enable symbol hiding in contrib/libgit-sys/libgitpub.a without making # us rebuild the whole tree every time we run a Rust build. diff --git a/ci/check-unsafe-assertions.sh b/ci/check-unsafe-assertions.sh new file mode 100755 index 00000000000..d66091efd22 --- /dev/null +++ b/ci/check-unsafe-assertions.sh @@ -0,0 +1,18 @@ +#!/bin/sh + +make CHECK_ASSERTION_SIDE_EFFECTS=1 >compiler_output 2>compiler_error +if test $? != 0 +then + echo "ERROR: The compiler could not verify the following assert()" >&2 + echo " calls are free of side-effects. Please replace with" >&2 + echo " BUG_IF_NOT() calls." >&2 + grep undefined.reference.to..not_supposed_to_survive compiler_error \ + | sed -e s/:[^:]*$// | sort | uniq | tr ':' ' ' \ + | while read f l + do + printf "${f}:${l}\n " + awk -v start="$l" 'NR >= start { print; if (/\);/) exit }' $f + done + exit 1 +fi +rm compiler_output compiler_error diff --git a/ci/run-static-analysis.sh b/ci/run-static-analysis.sh index 0d51e5ce0e7..ae714e020ae 100755 --- a/ci/run-static-analysis.sh +++ b/ci/run-static-analysis.sh @@ -31,4 +31,6 @@ exit 1 make check-pot +${0%/*}/check-unsafe-assertions.sh + save_good_tree diff --git a/git-compat-util.h b/git-compat-util.h index c3415ad7e0a..0aefd763751 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -1584,4 +1584,10 @@ static inline void *container_of_or_null_offset(void *ptr, size_t offset) ((uintptr_t)&(ptr)->member - (uintptr_t)(ptr)) #endif /* !__GNUC__ */ +#ifdef CHECK_ASSERTION_SIDE_EFFECTS +#undef assert +extern int not_supposed_to_survive; +#define assert(expr) ((void)(not_supposed_to_survive || (expr))) +#endif /* CHECK_ASSERTION_SIDE_EFFECTS */ + #endif From patchwork Fri Mar 14 00:20:52 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Elijah Newren X-Patchwork-Id: 14016104 Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 90BD66125 for ; Fri, 14 Mar 2025 00:20:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.54 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741911661; cv=none; b=aS7B/EYEpdhlYXswf/rJJuxgyyOEVT41aiMP8gcWyDyn7cAxvKu88T1h5dLinW14pFS5300UXH5mnv65hQUH+2RU3jgzjV6NO4PYi9beYbd/JO3TJZWU1DSnXHU9Cjwliy8cydqfDw3564v9sbVX+eZFjd7FDqiYu6A9KdhskRc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741911661; c=relaxed/simple; bh=wRQMfDQaX62xU9Sa5VNd1pS6SPdoVx6yFQicuRK5kr8=; h=Message-Id:In-Reply-To:References:From:Date:Subject:Content-Type: MIME-Version:To:Cc; b=LOVZ4JMIenRQjx6/2uNWDHQAlVDIEsiA4rP9PbAHV+UgwNIK38DmpzAnn/gkzLoNS392SJXHu+9G0xUTqqwWpyWiYtNS9L5vk+WQzXFjIw/sbSKOlhyxUE6CdBaJEQZBf4aeFN1vJLam5i1uP1OtfKH2q0vEDJzgZR8RxoxBZkA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=U73TQEt5; arc=none smtp.client-ip=209.85.221.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="U73TQEt5" Received: by mail-wr1-f54.google.com with SMTP id ffacd0b85a97d-390fdaf2897so1358384f8f.0 for ; Thu, 13 Mar 2025 17:20:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741911657; x=1742516457; darn=vger.kernel.org; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:from:to:cc:subject:date :message-id:reply-to; bh=WXA3TRQ/r5/vvweHdjLi9mt0grtT7hlGaEAEogCoozw=; b=U73TQEt58m4Vjl6OYZhwD6Nzg7n8t0lcN/ndo2bWp0WeZ5475mkSUSfcZWuLH+M95M kDDf6QyuZNvygOkndwqJg/Uvxs142n6gUQjkhAiFUqC25NGw0gSS5tjnc6k0+X6QSnuS cSql3MnHwoOmduJUiX1ya27k7bLwURaLe5zCKRzi/xvqJ1/yD2G2kHXzDF6h/T+vqs/d OunpSf/Cd/UIGgHtN5vSfzhLO0VTbo8wmNJnsbV+0NVE1ZA5UxYwV3oy271sLIeM2pEp Dcwb7FaxNP2zJlPvV42odCg5Bsz7KLCV7m0ssvZcSgBAr6yq3pLBaTGsSxDeFGHlqwah 3QVg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741911657; x=1742516457; h=cc:to:mime-version:content-transfer-encoding:fcc:subject:date:from :references:in-reply-to:message-id:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=WXA3TRQ/r5/vvweHdjLi9mt0grtT7hlGaEAEogCoozw=; b=r5jUih1mpp8Lm9+7MIMUfUs+nA06IwllmO3S0eTuUYX7gyn7NwwvyEDpI/ObbuBL2z W8FmTXgRcZNntCGLzecgygyG1sO5rZy5T+8jaw2Dr7BxYZjUC3y0hV7xCKG0xMyYxxIP C1lyGwTUQYjfkkRUbn189kCGEMpTn+locpT3xTi1ZFoUCpPa9YiYqg8FkjlhmQgywcNx MnGjCOHIsjViLC2KM8Rqon6Ugij6INSkBqdiHmgx/6xL4Y7hT7sdh9eA0+MlRqSi4mFa qinXazlBdM4U5vhZO2c0EWzee/d6V8faTfQhrFX1HK3xI2vTto7ji6o4KggDS1ARpKu3 7i6A== X-Gm-Message-State: AOJu0YwpJV+aplGfk8rKI9UNoFaK2vkEbz41qyO+PN0pGrVL8VEe3aCO 8KjllmPc8doRgu5ZkZgtM2a3rrVUzQ/kUdm424btF4HyDWrXjbXsw5bXKw== X-Gm-Gg: ASbGnctusS8iemZJUdXzjYU5Xi4T3rICp1jaosX9Ja0Z/1iewLdah9q4Fqc70tZpnEH NpZ3xUVqvgUf4hl6ipncfvvKa+xAyZHCpv6ehlc76kCB3qv1kOzfBDtFEfIiuWh+mliTkf+PFXf 6lOBaDaEqCqkt6+qP1d1UfAY2G/8zt2RrE5yvaTCx1d4LNtquoLF7XMzGO6h39KLnf3NEQ9pOx6 AwQzlnWsdo8h6l6mBJkXV2fft7rh41WnufqYCWD1ayo3P/zLGcqHprx3uFy0fysuUOOXaP+Z9rw AfTCi+3PflH5CIueBkI5I3fSfIlgT4f5PiO97sdZEPCh4w== X-Google-Smtp-Source: AGHT+IH3kzH56eBuivmpXhqdBlx2CUCQDViDkN25M7tVYgTaVLF+Xp9vX2jlV/RHIVxTp00jBImIBw== X-Received: by 2002:adf:a3c2:0:b0:390:e9e0:5cb3 with SMTP id ffacd0b85a97d-3971e2ad27dmr324114f8f.12.1741911657486; Thu, 13 Mar 2025 17:20:57 -0700 (PDT) Received: from [127.0.0.1] ([13.74.141.28]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-395cb7eb9c0sm3754939f8f.97.2025.03.13.17.20.56 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 13 Mar 2025 17:20:56 -0700 (PDT) Message-Id: <4c668039bb72e7e86ce08dc7b8e6c7f09673cee4.1741911652.git.gitgitgadget@gmail.com> In-Reply-To: References: Date: Fri, 14 Mar 2025 00:20:52 +0000 Subject: [PATCH 3/3] treewide: replace assert() with BUG_IF_NOT() in special cases Fcc: Sent Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 To: git@vger.kernel.org Cc: Elijah Newren , Elijah Newren From: Elijah Newren From: Elijah Newren When the compiler/linker cannot verify that an assert() invocation is free of side effects for us (e.g. because the assertion includes some kind of function call), replace the use of assert() with BUG_IF_NOT(). Signed-off-by: Elijah Newren --- diffcore-rename.c | 2 +- merge-ort.c | 4 ++-- merge-recursive.c | 2 +- object-file.c | 2 +- parallel-checkout.c | 2 +- scalar.c | 4 ++-- sequencer.c | 2 +- 7 files changed, 9 insertions(+), 9 deletions(-) diff --git a/diffcore-rename.c b/diffcore-rename.c index 91b77993c78..1a945945fab 100644 --- a/diffcore-rename.c +++ b/diffcore-rename.c @@ -1406,7 +1406,7 @@ void diffcore_rename_extended(struct diff_options *options, trace2_region_enter("diff", "setup", options->repo); info.setup = 0; - assert(!dir_rename_count || strmap_empty(dir_rename_count)); + BUG_IF_NOT(!dir_rename_count || strmap_empty(dir_rename_count)); want_copies = (detect_rename == DIFF_DETECT_COPY); if (dirs_removed && (break_idx || want_copies)) BUG("dirs_removed incompatible with break/copy detection"); diff --git a/merge-ort.c b/merge-ort.c index 46e78c3ffa6..3db7a911f81 100644 --- a/merge-ort.c +++ b/merge-ort.c @@ -791,7 +791,7 @@ static void path_msg(struct merge_options *opt, struct strbuf tmp = STRBUF_INIT; /* Sanity checks */ - assert(omittable_hint == + BUG_IF_NOT(omittable_hint == (!starts_with(type_short_descriptions[type], "CONFLICT") && !starts_with(type_short_descriptions[type], "ERROR")) || type == CONFLICT_DIR_RENAME_SUGGESTED); @@ -1642,7 +1642,7 @@ static int handle_deferred_entries(struct merge_options *opt, ci = strmap_get(&opt->priv->paths, path); VERIFY_CI(ci); - assert(renames->deferred[side].trivial_merges_okay && + BUG_IF_NOT(renames->deferred[side].trivial_merges_okay && !strset_contains(&renames->deferred[side].target_dirs, path)); resolve_trivial_directory_merge(ci, side); diff --git a/merge-recursive.c b/merge-recursive.c index 884ccf99a58..ab888689ae4 100644 --- a/merge-recursive.c +++ b/merge-recursive.c @@ -1197,7 +1197,7 @@ static void print_commit(struct repository *repo, struct commit *commit) struct pretty_print_context ctx = {0}; ctx.date_mode.type = DATE_NORMAL; /* FIXME: Merge this with output_commit_title() */ - assert(!merge_remote_util(commit)); + BUG_IF_NOT(!merge_remote_util(commit)); repo_format_commit_message(repo, commit, " %h: %m %s", &sb, &ctx); fprintf(stderr, "%s\n", sb.buf); strbuf_release(&sb); diff --git a/object-file.c b/object-file.c index 726e41a0475..8ef4813eb63 100644 --- a/object-file.c +++ b/object-file.c @@ -2706,7 +2706,7 @@ static int index_stream_convert_blob(struct index_state *istate, struct strbuf sbuf = STRBUF_INIT; assert(path); - assert(would_convert_to_git_filter_fd(istate, path)); + BUG_IF_NOT(would_convert_to_git_filter_fd(istate, path)); convert_to_git_filter_fd(istate, path, fd, &sbuf, get_conv_flags(flags)); diff --git a/parallel-checkout.c b/parallel-checkout.c index 7cc6b305281..4d2fa6d7374 100644 --- a/parallel-checkout.c +++ b/parallel-checkout.c @@ -277,7 +277,7 @@ static int write_pc_item_to_fd(struct parallel_checkout_item *pc_item, int fd, ssize_t wrote; /* Sanity check */ - assert(is_eligible_for_parallel_checkout(pc_item->ce, &pc_item->ca)); + BUG_IF_NOT(is_eligible_for_parallel_checkout(pc_item->ce, &pc_item->ca)); filter = get_stream_filter_ca(&pc_item->ca, &pc_item->ce->oid); if (filter) { diff --git a/scalar.c b/scalar.c index da42b4be0cc..173286110ea 100644 --- a/scalar.c +++ b/scalar.c @@ -241,7 +241,7 @@ static int add_or_remove_enlistment(int add) static int start_fsmonitor_daemon(void) { - assert(have_fsmonitor_support()); + BUG_IF_NOT(have_fsmonitor_support()); if (fsmonitor_ipc__get_state() != IPC_STATE__LISTENING) return run_git("fsmonitor--daemon", "start", NULL); @@ -251,7 +251,7 @@ static int start_fsmonitor_daemon(void) static int stop_fsmonitor_daemon(void) { - assert(have_fsmonitor_support()); + BUG_IF_NOT(have_fsmonitor_support()); if (fsmonitor_ipc__get_state() == IPC_STATE__LISTENING) return run_git("fsmonitor--daemon", "stop", NULL); diff --git a/sequencer.c b/sequencer.c index ad0ab75c8d4..98a7657b398 100644 --- a/sequencer.c +++ b/sequencer.c @@ -4965,7 +4965,7 @@ static int pick_commits(struct repository *r, ctx->reflog_message = sequencer_reflog_action(opts); if (opts->allow_ff) - assert(!(opts->signoff || opts->no_commit || + BUG_IF_NOT(!(opts->signoff || opts->no_commit || opts->record_origin || should_edit(opts) || opts->committer_date_is_author_date || opts->ignore_date));