|
From: Evan L. <sa2...@cy...> - 2007-08-09 08:36:48
|
Thanks for the explanation; that's a lot clearer.
John Reiser wrote:
> Evan Lavelle wrote:
>> uint8_t *cptr = (uint8_t *)&src;
>
>> // **BODGE** fixes valgrind error messages: is there a better
>> // way to do this?
>> // if(Dbits == 128)
>> // Dbits = 80;
>> for(int i = 0; i < Dbits; i += 8) {
>> mask = *cptr++; // <--- VALGRIND REPORTS ERROR HERE
>> mask <<= i;
>> *dst |= mask;
>> }
>
> Is that a true copy+paste of the source code, with no overriding of
> operators, and an accurate transcription of the error location?
What I hadn't shown was that 'mask' and 'dst' are of a class type (for
storing/handling big integers). "mask = *cptr++" invokes a constructor,
which takes the "*cptr" data. This is eventually compared against zero,
so that I can call an optimised routine for setting memory to 0, if
necessary. There are also errors reported at the next line ("mask <<=
i"). This is because the class left-shift operator does a comparison
operation, and both these comparisons lead to a "conditional jump or
move" error report for the uninitialised bytes. Apologies for not making
this clearer last time; this is the start of the error trail, not the
end. The start is more interesting, because it makes it clear that a
long double causes the problem.
So, in one sense, valgrind is perfectly correct in reporting an error.
However, there's no actual error here; the end result is simply that a
long double gets exactly the same value as another long double,
including the uninitialised bytes. It doesn't actually matter what the
result of the "conditional jump or move" is - they're just different
ways to move around the undefined bytes.
Of course, I can't expect Valgrind to know that there is no higher-level
semantic error here. However, if it was possible to tell Valgrind that a
long double was 16 bytes, as reported by sizeof, then the problem goes away.
> The workaround for such cases is always the same:
> memset(&src, 0, sizeof(src));
> which obviously must be done before any existing use of src.
I had assumed that this wouldn't work, because 'src' is always a
compiler-generated long double, and my reasoning was that writing a long
double into initialised memory would mark the top bytes as
uninitialised. However, you're right; test code below. This code is also
a good short summary of the problem. It assumes that the size of a long
double is 'sizeof(long double)', and allocates this many bytes, and
initialises the first BYTES_TO_INIT bytes in the array. It then loads a
long double into the array, and carries out some ops on the top byte of
the array. If BYTES_TO_INIT is set to sizeof(long double), valgrind
doesn't complain; if it's set to anything less, you get 7 errors.
Thanks -
Evan
================================================
// compile as C-99: ie. 'gcc -std=c99 -o test', etc
#include <stdio.h>
#include <stdint.h>
#include <string.h>
#define LDSIZE sizeof(long double)
// ** valgrind reports errors only if this is less than LDSIZE **
#define BYTES_TO_INIT (LDSIZE-1) // [0, LDSIZE]
int main(void) {
int i;
uint8_t storage[LDSIZE] __attribute__ ((aligned (32)));
memset(storage, 0xaa, BYTES_TO_INIT);
// display initialised storage
printf("%d-byte array (%d bytes initialised):\n", LDSIZE,
BYTES_TO_INIT);
for(i=0; i<LDSIZE; ++i)
printf("%02x ", storage[i]);
printf("\n");
// load a long double into 'storage'
long double *ldptr = (long double *)storage;
*ldptr = 1.0L;
// display over-written storage
printf("%d-byte array, containing 1.0L:\n", LDSIZE);
for(i=0; i<LDSIZE; ++i)
printf("%02x ", storage[i]);
printf("\n");
if((storage[LDSIZE-1] & 0xaa) == 0xaa) // ops for valgrind
printf("top byte is 0xaa\n");
else
printf("top byte is %02x\n", storage[LDSIZE-1]);
}
|