Valgrind: Uninitialised value was created by a heap allocation

16,846

Solution 1

    case 8 : //suma
        sscanf (mystring,"%s %s %s", charv1, str, charv2);
        v1 = strtol((charv1+1) , NULL , 10);
        v2 = strtol((charv2+1) , NULL , 10);
        addorsub = add(v[v1-1], v[v2-1]);
        printf("SUMA: %d %d\n", v1, v2);
        print(addorsub);
        break;

This is your problem. The pointer addorsub has already been malloc'd, but the add() function returns a vector pointer which is malloc'd. So the vadd vector pointer inside of the add() function is overwriting the addorsub pointer, which makes the allocated memory already in the addorsub pointer just...disappear into thin air.

The same thing happens with the sub() function. It's actually impossible to destroy the allocated memory after the return statement, so you have to destroy the memory in the pointer outside of the add() or sub() functions before you call those functions in order to preserve your memory.

So in the main() function, the incr() function, the decr() function, etc., you need to take out the memory allocation for anything that receives a pointer for add() or sub(), as well as making sure that if it's inside of a loop that you destroy any memory currently allocated before reassigning the pointer to a different address.

UPDATE: Memory leaks killed, now I need to know how can I initialize this statement:
   vector *addorsub = (vector*)malloc(sizeof(*addorsub));

You won't. Just make it

    vector * addorsub = NULL;

And make sure that addorsub gets a call to free() in every iteration of the loop.

Solution 2

The main issue is that the program is crashing before it has a chance to free the memory. A big part of the problem seems to be here:

    line[strlen(line)] = ','; /* Replaces the end ] with a , */

it should be

    line[strlen(line)-1] = ','; /* Replaces the end ] with a , */

There are various other memory issues as well. For example, addorsub is allocated but not freed.

In general, when using valgrind, try to start with the first error and work your way down. Looking at the end is misleading, since earlier errors can have the side-effect of causing the later ones.

16,846

Cheknov

Updated on July 14, 2022

Comments

Cheknov 6 months

UPDATE: Memory leaks killed, now I need to know how can I initialize this statement:

vector *addorsub = (vector*)malloc(sizeof(*addorsub));

This is what I get from valgrind:

[email protected]:~/Escritorio/valgrind/vg$ valgrind --tool=memcheck --leak-check=full --track-origins=yes ./eda.exe
==6129== Memcheck, a memory error detector
==6129== Copyright (C) 2002-2011, and GNU GPL'd, by Julian Seward et al.
==6129== Using Valgrind-3.7.0 and LibVEX; rerun with -h for copyright info
==6129== Command: ./eda.exe
==6129== 
==6129== Conditional jump or move depends on uninitialised value(s)
==6129==    at 0x4C2A7E4: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==6129==    by 0x4015E7: destroy_vector(vector*) (metbasicos.c:17)
==6129==    by 0x4014E2: main (main.c:175)
==6129==  Uninitialised value was created by a heap allocation
==6129==    at 0x4C2B6CD: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==6129==    by 0x400D59: main (main.c:87)
==6129== 
==6129== 
==6129== HEAP SUMMARY:
==6129==     in use at exit: 0 bytes in 0 blocks
==6129==   total heap usage: 2 allocs, 2 frees, 16 bytes allocated
==6129== 
==6129== All heap blocks were freed -- no leaks are possible
==6129== 
==6129== For counts of detected and suppressed errors, rerun with: -v
==6129== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 2 from 2)

And this is the main program:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <math.h>
#include "metbasicos.h"
#include "metintermedios.h"
#include "metavanzados.h"
//NumsVector, funcion que nos devuelve el numero de "numeros" que hay en cada vector del .txt,
//es decir, los n floats por cada vector
int NumsVector(char *linea, int size){
    int numsvector = 1; //Inicializamos a 1 ya que no podemos suponer valor maximo segun enunciado, pero si minimo >= 1
    int n;
    for(n = 2; n<= size; n++){ //como ya suponemos que el primer valor despues del corchete es un numero y ya lo hemos contado, empezamos en 2
        if (linea[n] != '[' && linea[n] != ']'){
            if(linea[n] == 44){
                numsvector = numsvector + 1;
            }
        }
    }
    return numsvector;
}
    int main(){
    int n =0, i = 0;
    scanf("%d\n", &n);
    vector **v = (vector **)malloc(sizeof(vector*) * n);
    for(i = 0; i<n; ++i) { 
        char *line = NULL, ch;
        int it = 0 ;
        line = (char*) malloc (2*sizeof(char)) ;
        *line = '\0' ;
        while((ch=getchar()) != '\n')
        {
        *(line+it) = ch ;
        it++ ;
        line = (char*) realloc(line, (2*sizeof(char)) + it ) ;
        }
        *(line+it) = '\0';
        int read = strlen(line);
        int numsvector = NumsVector(line, read);
        float* nfloat;  //sabemos el tamanyo del vector que hemos leido, creamos array de floats y lo llenamos de los floats
        //empieza el proceso para obtener los floats a partir de string de chars
        nfloat = (float*)malloc(numsvector*sizeof(float));
        int j = 0;
        line[strlen(line)] = ','; /* Replaces the end ] with a , */
        char *p = line + 1; /* creates a new pointer, pointing after the first [ in the original string */
        do
        {
            sscanf(p, "%f,", &nfloat[j]); /* grabs up to the next comma as a float */       
            while (*(p++) != ',') ; /* moves pointer forward to next comma */
        }
        while (++j < numsvector); /* stops when you've got the expected number */     
        v[i] = create_vector(numsvector, nfloat);//conseguimos almacenar el contenido del string en un vector del tipo float (nfloat)
        int aux;
        for(aux = 0; aux<numsvector; ++aux){ //test de que cada elemento se ha guardado bien y printa todos los elementos ok
            printf("V[%d]->data[%d] = : %.1f\n", i, aux, v[i]->data[aux]); //test de que la memoria se almacena bien, luego se borra
        }
        free(line);
        free(nfloat);
    }
    char mystring [21];
    char str[10], charv1[6], charv2[6];
    int operation = 0;
    char simbol[4]; /* Can be +, - and dot */
    mystring[0] = str[0] = charv1[0] = charv2[0] = simbol[0] = 'a';
    for(i = 0; i<21; i++){
        mystring[i] = 'a';
    }
    for(i = 0; i<6; i++)
    {
        charv1[i] = 'a';
        charv2[i] = 'a';
    }
    for (i = 0; i < 10; i++)
    {
            str[i] = 'a';
    }
    for (i = 0; i < 4; i++)
    {
            simbol[i] = 'a';
    }
    vector *addorsub = (vector*)malloc(sizeof(*addorsub));
    fgets (mystring , 21 , stdin);
    do {
        sscanf (mystring,"%s",str);
        int res = strlen (str);
        //int res = strncmp(str, "incr", 10);
        if(mystring[0] == 'p') operation = 1;
        else if(mystring[0] == 'i') operation = 2;
        else if(mystring[0] == 'd' && mystring[1] == 'i') operation = 4;
        else if(mystring[0] == 'd' && mystring[1] == 'e') operation = 5;
        else if(res == 9) operation = 6;
        else if(res == 4 && mystring[0] == 'n') operation = 7;
        else{
            sscanf (mystring,"%s %s",str, simbol);
            if (simbol[0] == '+') operation = 8;
            else if(simbol[0] == '-') operation = 9;
            else operation = 3;
        }
        int v1 = 0, v2 = 0;
        float returnresult = 0.0;
        switch(operation)
        {
        case 1 :
            sscanf (mystring,"%s %s",str, charv1);
            v1 = strtol((charv1+1) , NULL , 10);
            printf("PRINT: %d\n", v1);
            print(v[v1-1]);
            break;
        case 2 :
            sscanf (mystring,"%s %s %s",str, charv1, charv2);
            v1 = strtol((charv1+1) , NULL , 10);
            v2 = strtol((charv2+1) , NULL , 10);
            printf("INCREASE: %d %d\n", v1, v2);
            incr(v[v1-1], v[v2-1]);
            break;
        case 3 :
            sscanf (mystring,"%s %s %s",charv1, str, charv2);
            v1 = strtol((charv1+1) , NULL , 10);
            v2 = strtol((charv2+1) , NULL , 10);
            returnresult = dot(v[v1-1], v[v2-1]);
            printf("DOT: %d %d\n", v1, v2);
            printf("%f\n", returnresult);
            break;
        case 4 :
            sscanf (mystring,"%s %s %s", str, charv1, charv2);
            v1 = strtol((charv1+1) , NULL , 10);
            v2 = strtol((charv2+1) , NULL , 10);
            returnresult = distance(v[v1-1], v[v2-1]);
            printf("%f\n", returnresult);
            break;
        case 5 :
            sscanf (mystring,"%s %s %s",str, charv1, charv2);
            v1 = strtol((charv1+1) , NULL , 10);
            v2 = strtol((charv2+1) , NULL , 10);
            decr(v[v1-1], v[v2-1]);
            break;
        case 6 :
            sscanf (mystring,"%s %s",str, charv1);
            v1 = strtol((charv1+1) , NULL , 10);
            normalize(v[v1-1]);
            break;
        case 7 :
            sscanf (mystring,"%s %s",str, charv1);
            v1 = strtol((charv1+1) , NULL , 10);
            returnresult = norm(v[v1-1]);
            printf("%f\n", returnresult);
            break;
        case 8 : //suma
            sscanf (mystring,"%s %s %s", charv1, str, charv2);
            v1 = strtol((charv1+1) , NULL , 10);
            v2 = strtol((charv2+1) , NULL , 10);
            addorsub = add(v[v1-1], v[v2-1]);
            printf("SUMA: %d %d\n", v1, v2);
            print(addorsub);
            break;
        case 9 :
            sscanf (mystring,"%s %s %s", charv1, str, charv2);
            v1 = strtol((charv1+1) , NULL , 10);
            v2 = strtol((charv2+1) , NULL , 10);
            addorsub = sub(v[v1-1], v[v2-1]);
            printf("resta: %d %d\n", v1, v2);
            print(addorsub);
            break;    
       default :
            printf("operation value is: %d\n", operation);
            break;
        }
        operation = 0;
    } while (fgets (mystring , 21 , stdin) != NULL);
    for (i = 0; i < n; ++i)
    {
        destroy_vector(v[i]);
    }
    free(v);
}

I checked all malloc and frees, but I think I'm leaving anything ...

any ideas? thank you very much.

EDIT:

Input example (int a .txt file as stdin):

3
[9.3,1.2,87.9]
[1.0,1.0]
[0.0,0.0,1.0]
v1 + v2
v3 - v1
incr v3 v1
decr v1 v3
decr v1 v3
v2 dot v3
norm v3
distance v1 v3
normalize v3
print v3

Struct:

typedef struct {
    float* data;
    int size;
} vector;

metbasicos.c:

#include "metbasicos.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <math.h>
/* Metodos Básicos */
vector *create_vector(int n, float* data){
    vector *newvect = (vector*)malloc(sizeof(*newvect));
    newvect->data = (float*)malloc(n*sizeof(float));
    memcpy(newvect->data, data, sizeof(float) * n);
    newvect->size = n;
    return newvect;
}
void destroy_vector(vector* v){
    free(v->data);
    free(v);
}
void print(vector* v){
    int size = v->size, i;
    for (i = 0; i < size; ++i)
    {
        if(i == 0) printf("[%.1f,", v->data[i]);
        else if(i == (size-1)) printf("%.1f]\n", v->data[i]);
        else printf("%.1f,", v->data[i]);
    }
}

metintermedios.c:

#include "metintermedios.h"
#include "metbasicos.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <math.h>
/* Metodos Intermedios */
float dotDiferentSizes(vector* v1, vector* v2, int smax, int smin){
    float prod = 0.0;
    int i;
    for(i = 0; i < smin; i++){
        prod = prod + (v1->data[i])*(v2->data[i]); // += means add to product
    }
    for(i = smin; i < smax; i++){
        prod += (v1->data[i])*0; // += means add to product
    }
    return prod;
}
float dot(vector* v1, vector* v2){
    int smax = (v1->size), smin = 0;
    int v1size = smax;
    int v2size = (v2->size);
    float product = 0.0;
    if (v2size > smax) {
        smax = v2size; //max_size checking
        smin = v1size; //min_size checking
    }
    else if (v2size < smax){
            smin = v2->size;
    }
    else {
        if(v1size == v2size){
            smin = smax;
        }
    }
    // compute
    if(smax == smin){
        int i;
        for(i = 0; i < smin; i++){
            product += (v1->data[i])*(v2->data[i]); // += means add to product
        }
    }
    else{
        if(v1size == smax && v1size!= smin){
            product = dotDiferentSizes(v1,v2,smax,smin); //v1>v2
        }
        if(v2size == smax && v2size!= smin){
            product = dotDiferentSizes(v2,v1,smax,smin); //v2>v1 OJU nomes canviem l'ordre en que posem els parametres, la funcio es identica.
        }
    }
    return product;
}
float norm(vector* v){
    int size = v->size, i;
    float norm = 0.0;
    for(i= 0; i < size; i++){
        norm += (v->data[i])*(v->data[i]);
    }
    norm = sqrt( norm );
    return norm;
}
void normalize(vector* v){
    int size = v->size, i;
    float norma = 0.0;
    norma = norm(v);
    for(i= 0; i< size; i++){
        v->data[i] = v->data[i] / norma;
    }
    print(v);
}

metavanzados.c:

#include "metavanzados.h"
#include "metintermedios.h"
#include "metbasicos.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <math.h>
/* Metodos Avanzados */
vector* add(vector* v1, vector* v2){
    vector *vadd = (vector*)malloc(sizeof(*vadd));
    int v1size, v2size, i;
    v1size = v1->size;
    int size = v1size;
    v2size = v2->size;
    if(v2size > v1size) {
        size = v2size;
        vadd = create_vector(size, v2->data);
        for(i = 0; i < v1size; i++){
            vadd->data[i] += v1->data[i];
        }
    }
    else {
        vadd = create_vector(size, v1->data);
        for(i = 0; i < v1size; i++){
            vadd->data[i] += v2->data[i];
        }
    }
    return(vadd);
    destroy_vector(vadd);
}
vector* sub(vector* v1, vector* v2){
    vector *vsub = (vector*)malloc(sizeof(*vsub));
    int v1size, v2size, i;
    v1size = v1->size;
    int size = v1size;
    v2size = v2->size;
    if(v2size > v1size) {
        size = v2size;
        vsub = create_vector(size, v2->data);
        for(i = 0; i < v1size; i++){
            vsub->data[i] = v1->data[i] - vsub->data[i]; /* restamos siempre v1 - v2*/
        } /* en el bucle forzamos a restar v1 - v2, evitando el caso v2 - v1*/
        for(i = v1size; i < size; i++){
            vsub->data[i] = (v2->data[i])*(-1);
        }
    }
    else { /* v1size >= v2size */
        vsub = create_vector(size, v1->data);
        for(i = 0; i < v2size; i++){
            vsub->data[i] -= v2->data[i];
        }
    }
    return(vsub);
    destroy_vector(vsub);
}
void incr(vector* source, vector* other){
    int smax, i, ssize = source->size, osize = other->size;
    vector *vincr = (vector*)malloc(sizeof(*vincr));
    if(ssize > osize) smax = ssize;
    else {
        if(ssize < osize) smax = osize;
        else smax = ssize;
    }
    vincr = add(source, other);
    if(ssize > osize){
        for(i = 0; i < smax; i++){
            source->data[i] = vincr->data[i];
        }
    }
    else{
        source->data = (float*)realloc(source->data, sizeof(float) * smax);
        source->size = smax;    
        for(i = 0; i < smax; i++){
            source->data[i] = vincr->data[i];
        }
    }
    print(source);
    destroy_vector(vincr);
}
void decr(vector* source, vector* other){
    int smax, i, ssize = source->size, osize = other->size;
    if(ssize > osize) smax = ssize;
    else {
        if(ssize < osize) smax = osize;
        else smax = ssize;
    }
    vector *vdecr = (vector*)malloc(sizeof(*vdecr));
    vdecr = sub(source, other);
    if(ssize > osize){
        for(i = 0; i < smax; i++){
            source->data[i] = vdecr->data[i];
        }
    }
    else{
        source->data = (float*)realloc(source->data, sizeof(float) * smax);
        source->size = smax;    
        for(i = 0; i < smax; i++){
            source->data[i] = vdecr->data[i];
        }
    }
    print(source);
    destroy_vector(vdecr);
}
float distance(vector* v1, vector* v2){
    int i;
    float dist = 0.0;
    vector *vdist = (vector*)malloc(sizeof(*vdist));
    vdist = sub(v1, v2);
    for(i = 0; i<= vdist->size; i++){
        vdist->data[i] = (vdist->data[i])*(vdist->data[i]);
        dist += vdist->data[i];
    }
    dist = sqrt( dist );
    return dist;
    destroy_vector(vdist);
}

This is all the code.

Vaughn Cato about 9 years

Regardless of whether the output is what you want, it is still crashing, as indicated by this output from valgrind: Process terminating with default action of signal 11 (SIGSEGV). If you fix the crash then that will allow the program to reach the free() statements.

Cheknov about 9 years

Thank you very much, has been helpful to further optimize the code, but I keep getting the same error in valgrind. I think the error may come from the do .. while statement
Cheknov about 9 years

I was right, the problem came from the "do-while", I'll update the post with the new information from valgrind.
Cheknov about 9 years

Wow, That is, it must be that. But don't know how can I return the vadd and then free it... Could you give an example or give some clue on how to remove allocated memory for vadd / vsub? Thank you very very very muuuchhh
Cheknov about 9 years

I think I solved the problem by replacing the do...while statement by a single while loop. I updated the main post with the new valgrind warnings, do you know how can I fix the Conditional jump or move depends on uninitialised value(s)? Thank you.
S.M.Mousavi about 7 years

thanks for tip about fix problems in up-to-down approach