Hi! Thank you very much for the bug report and fix! Applied to master, will be part of 2.1.4.
Cheers, Andy On Mon 28 Jul 2014 16:35, "Ben Rocer" <fleab...@mail.com> writes: > [resubmitting to bug-guile@gnu.org as debbugs seems to have eaten my > first mail] > > When I tried to reproduce this bug on a 32-bit x86 system, I got an > abort in the function bytevector_large_set(); I think this is also > where the bug is. > > Specifically, there are two bugs in these two consecutive lines in > bytevector_large_set(): > > value_size = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size)) / (8 * c_size); > if (SCM_UNLIKELY (value_size > c_size)) > > In the first line, there is an off-by-one error in the calculation of > value_size; it gives the wrong answer if mpz_sizeinbase() is a > multiple of (8 * c_size) (see > https://gmplib.org/manual/Integer-Import-and-Export.html). > > Secondly, this calculation gives the number of (c_size-byte) *words* > required to hold c_mpz, not the number of bytes. So the check in the > next line should be (c_size * value_size > c_size), or equivalently > (value_size > 1). > > Since bytevector-u64-set! also calls bytevector_large_set, it > may be possible to reproduce this bug on 64 bit systems too; e.g > (bytevector-u64-set! (make-bytevector 8) 0 (expt 2 64) (endianness big)) > [untested] > > > --- a/libguile/bytevectors.c > +++ b/libguile/bytevectors.c > @@ -867,10 +867,10 @@ bytevector_large_set (char *c_bv, size_t c_size, int > signed_p, > memset (c_bv, 0, c_size); > else > { > - size_t word_count, value_size; > + size_t word_count, value_words; > > - value_size = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size)) / (8 * c_size); > - if (SCM_UNLIKELY (value_size > c_size)) > + value_words = (mpz_sizeinbase (c_mpz, 2) + (8 * c_size) - 1) / (8 * > c_size); > + if (SCM_UNLIKELY (value_words > 1)) > { > err = -2; > goto finish;