GMP 5.1.1: Valgrind reports incorrect read in __gmpn_copyd (called from __gmpz_mul_2exp)

[Torbjorn Granlund]

Alexander Kruppa <akruppa at gmail.com> writes:

     0x0000000000401a6f <+175>:	retq
     0x0000000000401a70 <+176>:	sub    $0x10,%rdx
  => 0x0000000000401a74 <+180>:	movaps (%rsi),%xmm0
     0x0000000000401a77 <+183>:	jb     0x401afe <__gmpn_copyd+318>
     0x0000000000401a7d <+189>:	nopl   (%rax)
  
  so it seems to be a 16-byte transfer. Maybe Valgrind processes this
  instruction as two transfers of 8 bytes each, and fails to realize
  that this is a half-valid, half-invalid access and thus
  --partial-loads-ok=yes fails to suppress it? If that should be the
  case, it would be a bug in Valgrind, imho - notwithstanding that
  --partial-loads-ok=yes is a last resort hack.
  
The docs for --partial-loads-ok=yes is somewhat ambiguous.  It talks
about "words" wich might exclude 16-byte quantities.

  At any rate, if this error is by design for efficiency reasons, I
  think it would be nice to have a configure option to make GMP choose
  code that strictly adheres to correct memory access. Memory checkers
  are not a silver bullet, but they are useful enough for debugging that
  imho it would be a shame to reduce their utility with deliberate false
  positives.
  
Hmm.  I prefer to either see valgrind fixed, or somehow annotate the
code to communicate the size promotion to valgrind.

I envision much more size promotion code in the future.  Now as SIMD
becomes 256 bits, and will probably widen more in the future, not using
it will really hurt performance.  And then we will read the first and
last naturally aligned block whether part of the operand or not.
Disabling this for valgrind seems like an idea that will worsen with
time...

We should make sure this is really a valid 16-bit aligned read before we
report any valgrind bug.

-- 
Torbjörn