Invalid read of size 4 valgrind

Question

I ran my code with valgrind and got an "invalid read of size 4" error. Here is what I exactly got :

==15103==    at 0x10F74C: NoeudHydro<noeud*>::GetNumero() const (NoeudHydro.h:97)
==15103==    by 0x10ECE1: std::ostream& operator<< <noeud*>(std::ostream&, NoeudHydro<noeud*>) (NoeudHydro.h:191)
==15103==    by 0x10E15A: main (main.cpp:71)
==15103==  Address 0x5b88450 is 0 bytes inside a block of size 240 free'd
==15103==    at 0x4C3123B: operator delete(void*) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15103==    by 0x110943: __gnu_cxx::new_allocator<NoeudHydro<noeud*> >::deallocate(NoeudHydro<noeud*>*, unsigned long) (new_allocator.h:125)
==15103==    by 0x1105C1: std::allocator_traits<std::allocator<NoeudHydro<noeud*> > >::deallocate(std::allocator<NoeudHydro<noeud*> >&, NoeudHydro<noeud*>*, unsigned long) (alloc_traits.h:462)
==15103==    by 0x10FBE7: std::_Vector_base<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >::_M_deallocate(NoeudHydro<noeud*>*, unsigned long) (stl_vector.h:180)
==15103==    by 0x112102: void std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >::_M_realloc_insert<NoeudHydro<noeud*> >(__gnu_cxx::__normal_iterator<NoeudHydro<noeud*>*, std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > > >, NoeudHydro<noeud*>&&) (vector.tcc:448)
==15103==    by 0x111E47: void std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >::emplace_back<NoeudHydro<noeud*> >(NoeudHydro<noeud*>&&) (vector.tcc:105)
==15103==    by 0x111CCF: std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >::push_back(NoeudHydro<noeud*>&&) (stl_vector.h:954)
==15103==    by 0x111AE6: AjoutNoeudHydro(std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >&, noeud*, char, int, int*) (passerelleRT.cpp:48)
==15103==    by 0x1119EC: ParcoursAxe(axe*, std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >&, int*, int) (passerelleRT.cpp:36)
==15103==    by 0x10E0E1: main (main.cpp:68)
==15103==  Block was alloc'd at
==15103==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==15103==    by 0x1109D3: __gnu_cxx::new_allocator<NoeudHydro<noeud*> >::allocate(unsigned long, void const*) (new_allocator.h:111)
==15103==    by 0x110680: std::allocator_traits<std::allocator<NoeudHydro<noeud*> > >::allocate(std::allocator<NoeudHydro<noeud*> >&, unsigned long) (alloc_traits.h:436)
==15103==    by 0x10FE55: std::_Vector_base<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >::_M_allocate(unsigned long) (stl_vector.h:172)
==15103==    by 0x111FC5: void std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >::_M_realloc_insert<NoeudHydro<noeud*> >(__gnu_cxx::__normal_iterator<NoeudHydro<noeud*>*, std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > > >, NoeudHydro<noeud*>&&) (vector.tcc:406)
==15103==    by 0x111E47: void std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >::emplace_back<NoeudHydro<noeud*> >(NoeudHydro<noeud*>&&) (vector.tcc:105)
==15103==    by 0x111CCF: std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >::push_back(NoeudHydro<noeud*>&&) (stl_vector.h:954)
==15103==    by 0x111AE6: AjoutNoeudHydro(std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >&, noeud*, char, int, int*) (passerelleRT.cpp:48)
==15103==    by 0x1119EC: ParcoursAxe(axe*, std::vector<NoeudHydro<noeud*>, std::allocator<NoeudHydro<noeud*> > >&, int*, int) (passerelleRT.cpp:36)
==15103==    by 0x10E0E1: main (main.cpp:68)

here is my code:

#include <iostream>
#include "NoeudHydro.h"
#include "passerelleRT.h"
using namespace std;

bool EstNoeudReit(PTNoeud Nd){
    PTNoeud Ndsuivant=Nd->suiv;
    return SontConfondus(Nd,Ndsuivant);
}

bool SontConfondus(PTNoeud Nd1, PTNoeud Nd2){
    return Nd1->pos[0]==Nd2->pos[0] && Nd1->pos[1]==Nd2->pos[1] && Nd1->pos[2]==Nd2->pos[2];
}

void ParcoursAxe(PTAxe AxeCourant,vector< NoeudHydro<PTNoeud> >& SysHydro, int TabEqui[],int temps){
    PTNoeud NdSRCourant=AxeCourant->premNoeud;
    NdSRCourant=NdSRCourant->NdDistSurAxeFils;

    while(NdSRCourant!=NULL){
        if(NdSRCourant->NdDistSurAxeFils!=NULL){ //Si c'est un Nd Ramif/Reit
            if(EstNoeudReit(NdSRCourant)){
                AjoutNoeudHydro(SysHydro,NdSRCourant,'2',temps,TabEqui);
                PTNoeud NdTemp=NdSRCourant->suiv;
                while(SontConfondus(NdTemp,NdSRCourant)){
                    TabEqui[NdTemp->num]=SysHydro.size()-1; //le numero du NoeudHydro venant d etre ajoute
                                                           //est la taille du vecteur -1
                    SysHydro[SysHydro.size()+1].SetEquivalentSysRac(NdTemp);
                    NdTemp=NdTemp->suiv;
                }
            }//end if EstNoeudReit

            else{ //C est un Nd Ramif
                AjoutNoeudHydro(SysHydro,NdSRCourant,'1',temps,TabEqui);
            }

        }//end if Nd Ramif/Reit

        else{
            AjoutNoeudHydro(SysHydro,NdSRCourant,'0',temps,TabEqui);
        }
        NdSRCourant=NdSRCourant->suiv;
    }
}

void AjoutNoeudHydro(vector< NoeudHydro<PTNoeud> >& SysHydro,PTNoeud NdSR, char typeNd, int temps,int TabEqui[]){

    //Ajout dans le vecteur
    SysHydro.push_back(NoeudHydro<PTNoeud> (NdSR,SysHydro.size(),typeNd,temps));
    //Relation Pere-Fils
    SysHydro.back().SetPere(SysHydro[TabEqui[NdSR->prec->num]]);
    SysHydro[TabEqui[NdSR->prec->num]].SetFils(SysHydro.back()); 

    //Equivalences
    TabEqui[NdSR->num]=SysHydro.size()-1;
}



int main(){

    int tempsTotal=28;

    /*-----------------------------------------------------*/
    /*SOME CODE THAT DOES WORK AND NEEDED FOR THE NEXT PART*/
    /*-----------------------------------------------------*/


    cout<<"------------------------------------------"<<endl;
    cout<<"-------------------HYDRO------------------"<<endl;
    cout<<"------------------------------------------"<<endl;

    // //HYDRO
    NoeudHydro<PTNoeud> *Nd;
    Nd=new NoeudHydro<PTNoeud> (SRG.GetSR()->premAxe->suivant->premNoeud,0,'0',tempsTotal);

    vector< NoeudHydro<PTNoeud> > SysHydro;
    int TabEqui[27]; 
    TabEqui[0]=-9;
    TabEqui[4]=0;
    SysHydro.push_back(*Nd);

    ParcoursAxe(SRG.GetSR()->premAxe->suivant,SysHydro,TabEqui,tempsTotal);

    for(int i=0;i<SysHydro.size();i++){
        cout<<SysHydro[i]<<endl;
        cout<<endl;
    }
    delete Nd;
    return 0;  
} 

and the class where there is a problem :

#ifndef NOEUDHYDRO
#define NOEUDHYDRO

#include <iostream>
#include <vector>

#include "fonctions_Info_Element.h"

using namespace std;

template <typename T>
class NoeudHydro{

    private :
        int numero; //numero dans l hydro
        int typeRacine;
        int ordreRamif;
        char typeNd; //(noeud simple =0, ramif = 1, reit = 2)
        float coordonnes[3]; //coordonnees
        double diametre;
        int age;
        float distanceApex;

        double conductAxiale; //conductance axiale entre le pere et lui meme
        double conductRadiale;

        NoeudHydro<T>* Pere;
        vector<NoeudHydro*> Fils; //NoeudHydro suivant. Le premier est le suivant sur la meme racine. 
                                  //Les autres sont les NoeudHydro suivant sur les axes fils (s il y a)
        vector<T> EquivalentSysRac; //Liste des Noeud/Seg du SysRac equivalents

    public : 
        //CONSTRUCTEURS/DESTRUCTEUR
        NoeudHydro(); //par defaut
        NoeudHydro(T& Element,int num, char typeNd,  int temps);
        ~NoeudHydro();

        //ACCESSEURS
        int GetNumero() const;
        NoeudHydro<T>* GetPere() const;
        NoeudHydro<T>* GetFils(int i) const; 
        T GetEquivalentSysRac(int i) const;
        int GetNbEquivalentSysRac() const;

        void SetNumero(int i);
        void SetPere(NoeudHydro<T>& Pere);
        void SetFils(NoeudHydro<T>& Fils);

};

template <typename T>
NoeudHydro<T>::NoeudHydro(T& Element,int num, char typeNd, int temps){
    this->Pere=NULL;

    this->numero=num;
    this->typeRacine=TypeRacineElement(Element);
    this->ordreRamif=OrdreRamifElement(Element);
    this->typeNd=typeNd;

    for(int i=0;i<3;i++){
        this->coordonnes[i]=CoordonneesElement(Element,i);
    }

    this->diametre=DiametreElement(Element);
    this->age=AgeElement(Element,temps);
    this->EquivalentSysRac.push_back(Element);
    cout<<"NoeudHydro "<<this->numero<<" cree"<<endl;
}

template <typename T>
NoeudHydro<T>::~NoeudHydro(){    cout<<"NoeudHydro "<<this->numero<<" detruit"<<endl;
}



//ACCESSEURS

template <typename T>
int NoeudHydro<T>::GetNumero() const{
    return this->numero;
}

template <typename T>
NoeudHydro<T>* NoeudHydro<T>::GetPere() const{
    return this->Pere;
}

template <typename T>
NoeudHydro<T>* NoeudHydro<T>::GetFils(int i) const{
    return this->Fils[i];
}

template <typename T>
void NoeudHydro<T>::SetNumero(int i){
    this->numero=i;
}

template <typename T>
void NoeudHydro<T>::SetPere(NoeudHydro& NdPere){
    this->Pere=&NdPere;
}

template <typename T>
void NoeudHydro<T>::SetFils(NoeudHydro& NdFils){
    this->Fils.push_back(&NdFils);
}


//Operateur
template <typename T>
ostream& operator<<(ostream& stream, NoeudHydro<T> Nd){
    if(Nd.GetPere()!=NULL){cout<<"Noeud Pere : "<<Nd.GetPere()->GetNumero()<<endl;} 
    for(int i=0;i<Nd.GetNbFils();i++){
        cout<<"Fils numero "<<i<<" : noeud "<<Nd.GetFils(i)->GetNumero()<<endl;
    } 
    return stream;
}
#endif

The error seems to come from GetNumero(), but there is no error if I do

NoeudHydro<PTNoeud> Nd(parameter needed);
Nd.GetNumero();

Only when I do Nd.GetPere()->GetNumero();

And can not understand why and I also noticed that the code actually does work. (If I didn't use valgrind I would I have continued the code). Does somebody have something to say ?

NB : PTNoeud is the same as noeud*

Answer 1

Thanks a lot. I used smart pointers and it works. I would just ask if I did it properly (you made me discover smart pointers) :

void NoeudHydro<T>::SetPere(NoeudHydro& NdPere){
    shared_ptr<NoeudHydro<T> >temp (new NoeudHydro<T>(NdPere));
    this->Pere=temp;
}

template <typename T>
void NoeudHydro<T>::SetFils(NoeudHydro& NdFils){
    shared_ptr<NoeudHydro<T> >temp (new NoeudHydro<T>(NdFils));
    Fils.push_back(temp);
}

with shared_ptr<NoeudHydro<T> > Pere; and vector<shared_ptr<NoeudHydro<T>>> Fils;. And also I don't understand why when the vector is resized, the pointers get invalidated ?

Answer 2

It is difficult to say exactly what the error is without being able to easily track down the errors to the line numbers.

And can not understand why and I also noticed that the code actually does work. (If I didn't use valgrind I would I have continued the code). Does somebody have something to say ?

That is the nature of undefined behaviour (UB). It would be great if UB meant that the application always crashed with a nice and easy to debug coredump. In reality UB means that almost anything can happen. It could run without side effects for years and years and then suddenly crash.

Getting back to the problem. Valgrind is telling you that you are accessing something that was deleted. It is also telling you that this all happened in emplace_back. This probably means that the vector is being resized and something is going wrong during the resizing.

My guess is that your problem is with your parent/child raw pointers. When the vector is resized this might be invalidating these pointers. If this is right then you will need to either modify the assignment operator/copy consructor/move constructor or else just consider using smart pointers.