Skip to content

Commit 5d2c7f6

Browse files
committed
Merge branch 'jk/lsan-race-ignore-false-positive' into jch
CI jobs that run threaded programs under LSan has been giving false positives from time to time, which has been worked around. This is an alternative to the jk/lsan-race-with-barrier topic with much smaller change to the production code. * jk/lsan-race-ignore-false-positive: test-lib: ignore leaks in the sanitizer's thread code test-lib: check leak logs for presence of DEDUP_TOKEN test-lib: simplify leak-log checking test-lib: rely on logs to detect leaks Revert barrier-based LSan threading race workaround
2 parents d062ccf + b119a68 commit 5d2c7f6

File tree

6 files changed

+10
-52
lines changed

6 files changed

+10
-52
lines changed

Makefile

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -141,10 +141,6 @@ include shared.mak
141141
#
142142
# Define NO_PTHREADS if you do not have or do not want to use Pthreads.
143143
#
144-
# Define THREAD_BARRIER_PTHREAD if your system has pthread_barrier_t. Barrier
145-
# support is optional and is only helpful when building with SANITIZE=leak, as
146-
# it is used to eliminate some races in the leak-checker.
147-
#
148144
# Define NO_PREAD if you have a problem with pread() system call (e.g.
149145
# cygwin1.dll before v1.5.22).
150146
#
@@ -2083,9 +2079,6 @@ ifdef NO_PTHREADS
20832079
else
20842080
BASIC_CFLAGS += $(PTHREAD_CFLAGS)
20852081
EXTLIBS += $(PTHREAD_LIBS)
2086-
ifdef THREAD_BARRIER_PTHREAD
2087-
BASIC_CFLAGS += -DTHREAD_BARRIER_PTHREAD
2088-
endif
20892082
endif
20902083

20912084
ifdef HAVE_PATHS_H

builtin/grep.c

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,6 @@ static pthread_cond_t cond_write;
101101
/* Signalled when we are finished with everything. */
102102
static pthread_cond_t cond_result;
103103

104-
/* Synchronize the start of all threads */
105-
static maybe_thread_barrier_t start_barrier;
106-
107104
static int skip_first_line;
108105

109106
static void add_work(struct grep_opt *opt, struct grep_source *gs)
@@ -201,8 +198,6 @@ static void *run(void *arg)
201198
int hit = 0;
202199
struct grep_opt *opt = arg;
203200

204-
maybe_thread_barrier_wait(&start_barrier);
205-
206201
while (1) {
207202
struct work_item *w = get_work();
208203
if (!w)
@@ -234,7 +229,6 @@ static void start_threads(struct grep_opt *opt)
234229
pthread_cond_init(&cond_add, NULL);
235230
pthread_cond_init(&cond_write, NULL);
236231
pthread_cond_init(&cond_result, NULL);
237-
maybe_thread_barrier_init(&start_barrier, NULL, num_threads + 1);
238232
grep_use_locks = 1;
239233
enable_obj_read_lock();
240234

@@ -254,7 +248,6 @@ static void start_threads(struct grep_opt *opt)
254248
die(_("grep: failed to create thread: %s"),
255249
strerror(err));
256250
}
257-
maybe_thread_barrier_wait(&start_barrier);
258251
}
259252

260253
static int wait_all(void)
@@ -291,7 +284,6 @@ static int wait_all(void)
291284
pthread_cond_destroy(&cond_add);
292285
pthread_cond_destroy(&cond_write);
293286
pthread_cond_destroy(&cond_result);
294-
maybe_thread_barrier_destroy(&start_barrier);
295287
grep_use_locks = 0;
296288
disable_obj_read_lock();
297289

builtin/index-pack.c

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,6 @@ static pthread_mutex_t deepest_delta_mutex;
185185

186186
static pthread_key_t key;
187187

188-
static maybe_thread_barrier_t start_barrier;
189-
190188
static inline void lock_mutex(pthread_mutex_t *mutex)
191189
{
192190
if (threads_active)
@@ -211,7 +209,6 @@ static void init_thread(void)
211209
if (show_stat)
212210
pthread_mutex_init(&deepest_delta_mutex, NULL);
213211
pthread_key_create(&key, NULL);
214-
maybe_thread_barrier_init(&start_barrier, NULL, nr_threads);
215212
CALLOC_ARRAY(thread_data, nr_threads);
216213
for (i = 0; i < nr_threads; i++) {
217214
thread_data[i].pack_fd = xopen(curr_pack, O_RDONLY);
@@ -234,7 +231,6 @@ static void cleanup_thread(void)
234231
for (i = 0; i < nr_threads; i++)
235232
close(thread_data[i].pack_fd);
236233
pthread_key_delete(key);
237-
maybe_thread_barrier_destroy(&start_barrier);
238234
free(thread_data);
239235
}
240236

@@ -1104,8 +1100,6 @@ static int compare_ref_delta_entry(const void *a, const void *b)
11041100

11051101
static void *threaded_second_pass(void *data)
11061102
{
1107-
if (threads_active)
1108-
maybe_thread_barrier_wait(&start_barrier);
11091103
if (data)
11101104
set_thread_data(data);
11111105
for (;;) {

ci/lib.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -385,7 +385,6 @@ linux-musl)
385385
;;
386386
linux-leaks|linux-reftable-leaks)
387387
export SANITIZE=leak
388-
export THREAD_BARRIER_PTHREAD=1
389388
;;
390389
linux-asan-ubsan)
391390
export SANITIZE=address,undefined

t/test-lib.sh

Lines changed: 10 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ prepend_var ASAN_OPTIONS : detect_leaks=0
8080
export ASAN_OPTIONS
8181

8282
prepend_var LSAN_OPTIONS : $GIT_SAN_OPTIONS
83+
prepend_var LSAN_OPTIONS : exitcode=0
8384
prepend_var LSAN_OPTIONS : fast_unwind_on_malloc=0
8485
export LSAN_OPTIONS
8586

@@ -339,17 +340,6 @@ case "$TRASH_DIRECTORY" in
339340
*) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$TRASH_DIRECTORY" ;;
340341
esac
341342

342-
# Utility functions using $TEST_RESULTS_* variables
343-
nr_san_dir_leaks_ () {
344-
# stderr piped to /dev/null because the directory may have
345-
# been "rmdir"'d already.
346-
find "$TEST_RESULTS_SAN_DIR" \
347-
-type f \
348-
-name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null |
349-
xargs grep -lv "Unable to get registers from thread" |
350-
wc -l
351-
}
352-
353343
# If --stress was passed, run this test repeatedly in several parallel loops.
354344
if test "$GIT_TEST_STRESS_STARTED" = "done"
355345
then
@@ -1180,8 +1170,15 @@ test_atexit_handler () {
11801170
}
11811171

11821172
check_test_results_san_file_empty_ () {
1183-
test -z "$TEST_RESULTS_SAN_FILE" ||
1184-
test "$(nr_san_dir_leaks_)" = 0
1173+
test -z "$TEST_RESULTS_SAN_FILE" && return 0
1174+
1175+
# stderr piped to /dev/null because the directory may have
1176+
# been "rmdir"'d already.
1177+
! find "$TEST_RESULTS_SAN_DIR" \
1178+
-type f \
1179+
-name "$TEST_RESULTS_SAN_FILE_PFX.*" 2>/dev/null |
1180+
xargs grep ^DEDUP_TOKEN |
1181+
grep -qv sanitizer::GetThreadStackTopAndBottom
11851182
}
11861183

11871184
check_test_results_san_file_ () {

thread-utils.h

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -53,22 +53,5 @@ int dummy_pthread_init(void *);
5353
int online_cpus(void);
5454
int init_recursive_mutex(pthread_mutex_t*);
5555

56-
#ifdef THREAD_BARRIER_PTHREAD
57-
#define maybe_thread_barrier_t pthread_barrier_t
58-
#define maybe_thread_barrier_init pthread_barrier_init
59-
#define maybe_thread_barrier_wait pthread_barrier_wait
60-
#define maybe_thread_barrier_destroy pthread_barrier_destroy
61-
#else
62-
#define maybe_thread_barrier_t int
63-
static inline int maybe_thread_barrier_init(maybe_thread_barrier_t *b UNUSED,
64-
void *attr UNUSED,
65-
unsigned nr UNUSED)
66-
{
67-
errno = ENOSYS;
68-
return -1;
69-
}
70-
#define maybe_thread_barrier_wait(barrier)
71-
#define maybe_thread_barrier_destroy(barrier)
72-
#endif
7356

7457
#endif /* THREAD_COMPAT_H */

0 commit comments

Comments
 (0)