summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJunio C Hamano <gitster@pobox.com>2025-04-08 11:43:12 -0700
committerJunio C Hamano <gitster@pobox.com>2025-04-08 11:43:12 -0700
commitb97b360c514acd0f5a148524a85bcdb583dbe914 (patch)
treedc16900efcc7576ec5211f9c340e18bd01dd9020
parent9d22ac51228304102deb62f30c3ecba6377e1237 (diff)
parent5633aa3af1282cad5161174f17867399e58b2a54 (diff)
Merge branch 'en/assert-wo-side-effects'
Ensure what we write in assert() does not have side effects, and introduce ASSERT() macro to mark those that cannot be mechanically checked for lack of side effects. * en/assert-wo-side-effects: treewide: replace assert() with ASSERT() in special cases ci: add build checking for side-effects in assert() calls git-compat-util: introduce ASSERT() macro
-rw-r--r--Makefile4
-rwxr-xr-xci/check-unsafe-assertions.sh18
-rwxr-xr-xci/run-static-analysis.sh2
-rw-r--r--diffcore-rename.c2
-rw-r--r--git-compat-util.h9
-rw-r--r--merge-ort.c4
-rw-r--r--merge-recursive.c2
-rw-r--r--object-file.c2
-rw-r--r--parallel-checkout.c2
-rw-r--r--scalar.c4
-rw-r--r--sequencer.c2
11 files changed, 42 insertions, 9 deletions
diff --git a/Makefile b/Makefile
index ac32d2d0bd..9f8aeffb68 100644
--- a/Makefile
+++ b/Makefile
@@ -2263,6 +2263,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 0000000000..233bd9dfbc
--- /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 >&2 "ERROR: The compiler could not verify the following assert()"
+ echo >&2 " calls are free of side-effects. Please replace with"
+ echo >&2 " ASSERT() calls."
+ 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 0d51e5ce0e..ae714e020a 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/diffcore-rename.c b/diffcore-rename.c
index 5002e896aa..8077283fc7 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));
+ ASSERT(!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/git-compat-util.h b/git-compat-util.h
index cf733b38ac..f8562996ed 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -1460,6 +1460,8 @@ 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__)
+/* ASSERT: like assert(), but won't be compiled out with NDEBUG */
+#define ASSERT(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__)
@@ -1592,4 +1594,11 @@ static inline void *container_of_or_null_offset(void *ptr, size_t offset)
*/
#define NOT_CONSTANT(expr) ((expr) || false_but_the_compiler_does_not_know_it_)
extern int false_but_the_compiler_does_not_know_it_;
+
+#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
diff --git a/merge-ort.c b/merge-ort.c
index 2b7d86aa4e..c41f466577 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 ==
+ ASSERT(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 &&
+ ASSERT(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 884ccf99a5..4fbbece922 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));
+ ASSERT(!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 726e41a047..4fb3cd9dcb 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));
+ ASSERT(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 7cc6b30528..57c2dcaa8f 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));
+ ASSERT(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 da42b4be0c..d359f08bb8 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());
+ ASSERT(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());
+ ASSERT(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 ad0ab75c8d..c625a39111 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 ||
+ ASSERT(!(opts->signoff || opts->no_commit ||
opts->record_origin || should_edit(opts) ||
opts->committer_date_is_author_date ||
opts->ignore_date));