Package: abyss
Version: 2.0.2-1
Severity: important
Tags: sid + patch
Justification: FTBFS
User: debian-m...@lists.debian.org
Usertags: mips-patch
Forwarded: https://github.com/bcgsc/abyss/pull/139

Hi,

Package abyss FTBFS on mips and other big-endian architectures with followind 
error:
> Kmer.cpp: In function 'Seq load(const uint8_t*)':
> Kmer.cpp:193:14: error: 'SEQ_WORDS' was not declared in this scope
>   copy(s, s + SEQ_WORDS, reverse_iterator<uint64_t*>(d));
>               ^~~~~~~~~
> Kmer.cpp: In function 'void storeReverse(uint8_t*, Seq)':
> Kmer.cpp:239:14: error: 'SEQ_WORDS' was not declared in this scope
>   copy(s, s + SEQ_WORDS,
              ^~~~~~~~~

build log: 
https://buildd.debian.org/status/fetch.php?pkg=abyss&arch=mips&ver=2.0.2-1&stamp=1478005755

After adding the missing SEQ_WORDS define for big-endian the build passed,
but the following tests failed:
> FAIL: common_kmer
> FAIL: BloomFilter
> FAIL: Konnector_DBGBloomAlgorithms
> FAIL: Konnector_konnector
> FAIL: PairedDBG_LoadAlgorithm
> FAIL: PairedDBG_KmerPair

Comparison of values returned by load and storeReverse methods between mipsel
and mips showed that the data contained in variable seq (type of std::bitset) is
in wrong byte order.

Conversion of arrays bigger than architecture's word size into bitset is working
by going sequentially through memory and converting the values into binary word
by word. It does not care about endianness.
So, on 32bit big-endian architectures the conversion must be done in 32bit
chunks and every chunk must be copied in reverse byte order.

The existing code in load and storeReverse methods was suitable for 64bit
big-endian architectures, it copies 64bit chunks of the array in reverse order.

The attached patch fixes the conversion of array to bitset (and vice versa) by
changing the uint64_t type to size_t in load and storeReverse methods.
This enables to use the same code on 32 and 64 bit architectures.

With this patch I was able to build abyss successfully on mips. This patch was
successfully tested on 64bit big-endian mips too.

Thanks,
Daniel
--- abyss-2.0.2.orig/Common/Kmer.cpp
+++ abyss-2.0.2/Common/Kmer.cpp
@@ -188,9 +188,9 @@ static Seq load(const uint8_t *src)
 	Seq seq;
 #if MAX_KMER > 96
 # if WORDS_BIGENDIAN
-	const uint64_t *s = reinterpret_cast<const uint64_t*>(src);
-	uint64_t *d = reinterpret_cast<uint64_t*>(&seq + 1);
-	copy(s, s + SEQ_WORDS, reverse_iterator<uint64_t*>(d));
+	const size_t *s = reinterpret_cast<const size_t*>(src);
+	size_t *d = reinterpret_cast<size_t*>(&seq + 1);
+	copy(s, s + Kmer::NUM_BYTES/sizeof(size_t), reverse_iterator<size_t*>(d));
 # else
 	uint8_t *d = reinterpret_cast<uint8_t*>(&seq);
 	memcpy(d, src, sizeof seq);
@@ -234,10 +234,10 @@ static void storeReverse(uint8_t *dest,
 {
 #if MAX_KMER > 96
 # if WORDS_BIGENDIAN
-	const uint64_t *s = reinterpret_cast<const uint64_t*>(&seq);
-	uint64_t *d = reinterpret_cast<uint64_t*>(dest);
-	copy(s, s + SEQ_WORDS,
-			reverse_iterator<uint64_t*>(d + SEQ_WORDS));
+	const size_t *s = reinterpret_cast<const size_t*>(&seq);
+	size_t *d = reinterpret_cast<size_t*>(dest);
+	copy(s, s + Kmer::NUM_BYTES/sizeof(size_t),
+			reverse_iterator<size_t*>(d +  Kmer::NUM_BYTES/sizeof(size_t)));
 	reverse(dest, dest + Kmer::NUM_BYTES);
 # else
 	memcpy(dest, &seq, Kmer::NUM_BYTES);

Reply via email to