[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: Bug#889147: abyss: PairedDBG_LoadAlgorithm test fails on sparc64 due to strict alignment violation



user debian-sparc@lists.debian.org
usertags sparc64
thanks

(You missed the User: pseudoheader)

> On 2 Feb 2018, at 14:46, David Matthew Mattli <dmm@mattli.us> wrote:
> 
> Package: abyss
> Severity: normal
> Tags: patch upstream
> Usertags: sparc64
> 
> Dear Maintainer,
> 
> This package currently FTBFS on sparc64 due to the
> PairedDBG_LoadAlgorithm test failing with a SIGBUS. The Kmer.load and
> Kmer.storeReverse methods in the Common/Kmer.cpp file cast a uint8_t*
> to a size_t* without ensuring the pointer value has the proper
> alignment.
> 
> To fix this I added an aligned stack allocated buffer and memcpy to
> that. Stack allocated is appropriate because the buffer size is
> small(32 bytes) and known at compile time.

Hi,
As a sparc64 porter, thanks for fixing packages! Just a couple of
comments on the patch.

> --- a/Common/Kmer.cpp
> +++ b/Common/Kmer.cpp
> @@ -188,9 +188,10 @@
>  	Seq seq;
>  #if MAX_KMER > 96
>  # if WORDS_BIGENDIAN
> -	const size_t *s = reinterpret_cast<const size_t*>(src);
> +	size_t buf[Kmer::NUM_BYTES];

Should be divided by sizeof(size_t). Also, why not call it s? That should
reduce the number of changes.

> +	memcpy(buf, src, Kmer::NUM_BYTES);
>  	size_t *d = reinterpret_cast<size_t*>(&seq + 1);
> -	copy(s, s + Kmer::NUM_BYTES/sizeof(size_t), reverse_iterator<size_t*>(d));
> +	copy(buf, buf + 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);
> @@ -235,9 +236,10 @@
>  #if MAX_KMER > 96
>  # if WORDS_BIGENDIAN
>  	const size_t *s = reinterpret_cast<const size_t*>(&seq);
> -	size_t *d = reinterpret_cast<size_t*>(dest);
> +	size_t d[Kmer::NUM_BYTES];

Ditto for the size (this time you used the same name as before).

>  	copy(s, s + Kmer::NUM_BYTES/sizeof(size_t),
>  			reverse_iterator<size_t*>(d +  Kmer::NUM_BYTES/sizeof(size_t)));
> +	memcpy(dest, d, Kmer::NUM_BYTES);
>  	reverse(dest, dest + Kmer::NUM_BYTES);
>  # else
>  	memcpy(dest, &seq, Kmer::NUM_BYTES);

Regards,
James


Reply to: