0
votes

I am struggling hard to understand and solve the "Conditional jump or move depends on uninitialised value(s)" error valgrind points, so far with no success.

I tried initializing almost everything in my code (except for data element of the structure), even when it is obvious that it is not necessary. I tried debugging and checking that all members of every stack_t-object is initialized and has a value. All for no avail - valgrind keeps concluding that there is a memory error due to uninitialised value(s).

Funny thing is that all tests are flawless, except for the valgrind issue. I kept shrinking the scope of the code until I found the specific pieces of code that valgrind does not like.

Here it is below. This is from file stack.c:

#define GET_STACK_SIZE(x, y) (offsetof(stack_t, _data) + x * y)
typedef struct stack
{
    size_t _capacity;
    size_t _element_size;
    size_t _top_of_stack;
    char _data[1];
} stack_t;

stack_t *StackCreate(size_t capacity, size_t element_size)
{
    stack_t *new_st = NULL;
    assert(element_size > 0);
    assert(capacity > 0);
    new_st = malloc(GET_STACK_SIZE(capacity, element_size));
    if (NULL == new_st)
    {
        return NULL;
    }
    new_st->_capacity = capacity;
    new_st->_element_size = element_size;
    new_st->_top_of_stack = 0;

    return new_st;
}

stack_t *StackCopy(const stack_t *other)
{
    stack_t *new_st;
    assert(NULL != other);
    new_st = StackCreate(other->_capacity, other->_element_size);
    if (NULL == new_st)
        return NULL;
    memcpy(new_st, other, GET_STACK_SIZE(other->_capacity, other->_element_size));
    new_st->_capacity = other->_capacity;
    new_st->_element_size = other->_element_size;
    new_st->_top_of_stack = other->_top_of_stack;
    return new_st;
}

int StackEquals(const stack_t *st1, const stack_t *st2)
{
    return memcmp(st1, st2,
                  GET_STACK_SIZE(st1->_capacity, st1->_element_size)) == 0;
}

and this is from my test file:

#define NUM_OF_STACKS_IN_TEST (2)
#define CAPACITY_MULT (5)

stack_t *stacks[NUM_OF_STACKS_IN_TEST] = {NULL};

int init()
{
    int i = 0;
    for (; i < NUM_OF_STACKS_IN_TEST; ++i)
    {
        switch (i)
        {
        case 0:
            stacks[i] = StackCreate((i + 1) * CAPACITY_MULT, sizeof(char));
            if (NULL == stacks[i])
                return EXIT_FAILURE;
            break;
        case 1:
            stacks[i] = StackCopy(stacks[0]);
            if (NULL == stacks[i])
                return EXIT_FAILURE;

        default:
            break;
        }
    }
    return EXIT_SUCCESS;
}

int StackCreateTest(void)
{
    size_t i;
    for (i = 0; i<NUM_OF_STACKS_IN_TEST>> 1; ++i)
    {
        if (StackEquals(stacks[i], stacks[i + 1]) != 1)
            return EXIT_FAILURE;
    }
    return EXIT_SUCCESS;
}

int main(void)
{
    if (EXIT_SUCCESS != init())
    {
        puts("initialization failed");
    }
    RUN_TEST(StackCreateTest); /* just a macro that runs StackCreateTest function */
    end(); /* frees memory allocation */
    return EXIT_SUCCESS;
}

As far as can tell, everything looks OK. but valgrind gives this error:

==25611== Conditional jump or move depends on uninitialised value(s)
==25611==    at 0x4C35E6F: bcmp (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25611==    by 0x108C4F: StackEquals (stack.c:92)
==25611==    by 0x108D19: StackCreateTest (stack_test.c:113)
==25611==    by 0x108C83: main (stack_test.c:72)
==25611==  Uninitialised value was created by a heap allocation
==25611==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25611==    by 0x108A62: StackCreate (stack.c:46)
==25611==    by 0x108D79: init (stack_test.c:175)
==25611==    by 0x108C67: main (stack_test.c:67)
==25611== 
==25611== Conditional jump or move depends on uninitialised value(s)
==25611==    at 0x4C35E91: bcmp (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25611==    by 0x108C4F: StackEquals (stack.c:92)
==25611==    by 0x108D19: StackCreateTest (stack_test.c:113)
==25611==    by 0x108C83: main (stack_test.c:72)
==25611==  Uninitialised value was created by a heap allocation
==25611==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25611==    by 0x108A62: StackCreate (stack.c:46)
==25611==    by 0x108D79: init (stack_test.c:175)
==25611==    by 0x108C67: main (stack_test.c:67)
==25611== 
==25611== Conditional jump or move depends on uninitialised value(s)
==25611==    at 0x108D1D: StackCreateTest (stack_test.c:113)
==25611==    by 0x108C83: main (stack_test.c:72)
==25611==  Uninitialised value was created by a heap allocation
==25611==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==25611==    by 0x108A62: StackCreate (stack.c:46)
==25611==    by 0x108D79: init (stack_test.c:175)
==25611==    by 0x108C67: main (stack_test.c:67)
==25611== 
StackCreateTest is OK!
==25611== 
==25611== HEAP SUMMARY:
==25611==     in use at exit: 0 bytes in 0 blocks
==25611==   total heap usage: 3 allocs, 3 frees, 1,082 bytes allocated
==25611== 
==25611== All heap blocks were freed -- no leaks are possible
==25611== 
==25611== For counts of detected and suppressed errors, rerun with: -v
==25611== ERROR SUMMARY: 6 errors from 3 contexts (suppressed: 0 from 0)

I would appreciate any help or idea that might shed some light on the topic.

UPDATE: As kevin and KamilCuk correctly pointed out, the problem was that _data member of structure stack_t was uninitialized and thus memcpy and memcmp were actually using an uninitialized memory, as valgrind correctly detected

1
#typedef struct stack What happened with that # in front? char _data[1]; How old compiler are you using? Any reason not to use flexible array member available for 20 years? StackEquals should check if stacks are the same size.KamilCuk
"I tried initializing almost everything in my code (except for data element of the structure), even when it is obvious that it is not necessary." Why don't you think it's necessary, especially the bolded part? You're calling memcmp on malloc'd memory that hasn't been initialized to anything.Kevin
@KamilCuk Thanks. That # is a mistake and deleted. I reorganized my code so not to just dump everything here. Thanks for correcting. As for compiler, it is gcc, but c89 styleguyr79
I think you could reduce your problem to main () { char *a = malloc(1), *b = malloc(1); *a = *b; return *a == *b; } - ie. the memory by second malloc assigned to b is uninitialized, yet you read from it and then compare it. @edit removed mem* functions. Och, even char a, b = a; return a == b; works with my valgrind.KamilCuk
@Kevin and kamilCuk Thanks!! I corrected the code according to your suggestions and got read of the valgrind issue. The problem was indeed in the data part connected with the use of memcmp and memcpyguyr79

1 Answers

2
votes

Although reading uninitialized memory through char* handle is not undefined behavior (and isn't via memcpy and memcmp functions), still it is considered good practice to always initialize memory before reading.

Valgrind properly diagnoses the "issue", you new_st = malloc(GET_STACK_SIZE(capacity, element_size)); allocate the memory and do not initialize the memory in the range ( offsetof(stack_t, _data), offsetof(stack_t, _data) + x * y ]. Then the code reads from it in memcpy(new_st, other, GET_STACK_SIZE(other->_capacity, other->_element_size)); and compares it in memcmp(st1, st2, - so valgrind shows uninitialized bytes errors.

The solve would be easy - initialize the memory. Either use calloc or call memset like memset(new_st.data, 0, x * y).