Re: bwa Segmentation fault issue

[Fabian Klötzl <kloetzl@evolbio.mpg.de>]

Hi,

I much prefer an interesting coding challenge to cleaning out my flat,
so I tackled this problem.

On 01.07.2017 00:04, Nadiya Sitdykova wrote:
> 
> 2) negative argument for malloc size
> ==10017== Thread 13:
> ==10017== Argument 'size' of function malloc has a fishy (possibly
> negative) value: -9223372036854775657
> ==10017==    at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
> ==10017==    by 0x12E34A: bsw2_pair1 (bwtsw2_pair.c:123)
> ==10017==    by 0x12EC37: bsw2_pair (bwtsw2_pair.c:193)
> ==10017==    by 0x127B4E: bsw2_aln_core (bwtsw2_aux.c:621)
> ==10017==    by 0x127E90: worker (bwtsw2_aux.c:660)
> ==10017==    by 0x535D493: start_thread (pthread_create.c:333)
> 
> Thanks to debug output I knew that this happens when l_mseq = 149, end =
> 0, beg = 1

end is not 0, but a *very* small number. But that is only the symptom of
an issue earlier on.

In bsw2_stat some statistics of the given data are computed, which are
then used to calculate beg and end in bsw2_pair1, the function with the
broken malloc. The computed statistics are avg and std. However, only
values within certain boundaries are used to compute the avg. If none of
the input values fall within that interval it all boils down to avg =
0/0 = NaN. This then screws up any downstream computation. A
straightforward patch is attached to this mail.

While I was at it, I also created a patch for the malloc wrapper: size_t
is unsigned. (Technically, the valgrind message given above is also wrong.)

Feel free to modify and forward at you leisure.

Best,
Fabian

diff --git a/bwtsw2_pair.c b/bwtsw2_pair.c
index b945128..429da47 100644
--- a/bwtsw2_pair.c
+++ b/bwtsw2_pair.c
@@ -85,6 +85,9 @@ bsw2pestat_t bsw2_stat(int n, bwtsw2_t **buf, kstring_t *msg, int max_ins)
 	if (r.high < r.avg + MAX_STDDEV * r.std) r.high = (int)(r.avg + MAX_STDDEV * r.std + .499);
 	ksprintf(msg, "[%s] low and high boundaries for proper pairs: (%d, %d)\n", __func__, r.low, r.high);
 	free(isize);
+	if (isnan(r.avg) || isnan(r.std)){
+		r.failed = 1;
+	}
 	return r;
 }
 

diff --git a/malloc_wrap.c b/malloc_wrap.c
index 100b8cb..6165e04 100644
--- a/malloc_wrap.c
+++ b/malloc_wrap.c
@@ -13,7 +13,7 @@ void *wrap_calloc(size_t nmemb, size_t size,
 	void *p = calloc(nmemb, size);
 	if (NULL == p) {
 		fprintf(stderr,
-				"[%s] Failed to allocate %zd bytes at %s line %u: %s\n",
+				"[%s] Failed to allocate %zu bytes at %s line %u: %s\n",
 				func, nmemb * size, file, line, strerror(errno));
 		exit(EXIT_FAILURE);
 	}
@@ -25,7 +25,7 @@ void *wrap_malloc(size_t size,
 	void *p = malloc(size);
 	if (NULL == p) {
 		fprintf(stderr,
-				"[%s] Failed to allocate %zd bytes at %s line %u: %s\n",
+				"[%s] Failed to allocate %zu bytes at %s line %u: %s\n",
 				func, size, file, line, strerror(errno));
 		exit(EXIT_FAILURE);
 	}
@@ -37,7 +37,7 @@ void *wrap_realloc(void *ptr, size_t size,
 	void *p = realloc(ptr, size);
 	if (NULL == p) {
 		fprintf(stderr,
-				"[%s] Failed to allocate %zd bytes at %s line %u: %s\n",
+				"[%s] Failed to allocate %zu bytes at %s line %u: %s\n",
 				func, size, file, line, strerror(errno));
 		exit(EXIT_FAILURE);
 	}
@@ -49,7 +49,7 @@ char *wrap_strdup(const char *s,
 	char *p = strdup(s);
 	if (NULL == p) {
 		fprintf(stderr,
-				"[%s] Failed to allocate %zd bytes at %s line %u: %s\n",
+				"[%s] Failed to allocate %zu bytes at %s line %u: %s\n",
 				func, strlen(s), file, line, strerror(errno));
 		exit(EXIT_FAILURE);
 	}