View Full Version : [c++] Valgrind rileva diversi errori di scrittura
Sto realizzando una classe che rappresenta una matrice templata. Ormai sono a buon punto, ma eseguendo valgrind mi accorgo che ci sono un pò di errori (nessun memory leak per ora). Provando il copy constructor ricevo i seguenti errori:
Invalid write of size 4
==2410== at 0x4010C4: miamatrice<int>::miamatrice(miamatrice<int> const&) (in /media/Storage/Progetto c++/mymatrix2/a.out)
==2410== by 0x400CB6: main (in /media/Storage/Progetto c++/mymatrix2/a.out)
==2410== Address 0x59441f4 is 0 bytes after a block of size 4 alloc'd
==2410== at 0x4C2596C: operator new(unsigned long) (vg_replace_malloc.c:220)
==2410== by 0x40104C: miamatrice<int>::miamatrice(miamatrice<int> const&) (in /media/Storage/Progetto c++/mymatrix2/a.out)
==2410== by 0x400CB6: main (in /media/Storage/Progetto c++/mymatrix2/a.out)
La classe è così definita:
miamatrice(const miamatrice &other) {
nr_=other.nr_;
nc_=other.nc_;
m_=new T*[nr_];
for (int righe=0; righe<nr_; ++righe)
{
m_[righe]=new T(nc_);
for (int colonne=0; colonne < nc_; ++colonne)
{
m_[righe][colonne]=other.m_[righe][colonne];
}
}
}
dati privati:
T** m_;
unsigned int nr_;
unsigned int nc_;
e costruttore utilizzato per costruire la prima delle due matrici:
miamatrice(unsigned int nr, unsigned int nc){
nr_=nr;
nc_=nc;
m_=new T*[nr];
for (int i=0;i<nr;++i) m_[i]=new T[nc];
}
Mi sembra non ci siano altri metodi che possano causare questi errori. Che cosa genere questa invalid write? a me sembra di ciclare correttamente la matrice!
Di errori ne ho molti altri cominciare a risolvere questo sarebbe un buon inizio :mc:
m_[righe]=new T(nc_);
forse
m_[righe]=new T[nc_]; ;)
è giusto, non so perchè vedendo dei vecchi esempi mi è venuto da mettere con le parentesi tonde. Sono incappato in un problema più misterioso però. Se creo una matrice specificando le dimensioni ma non la inizializzo e la stampo non stampa una matrice di tutti 0, ma alcuni 0 e alcuni numeri "casuali". La cosa strana è che se dopo questa matrice ne creo un'altra con lo stesso procedimento questa sarà di tutti 0. Il costruttore che accetta le lunghezze in ingresso l'ho messo nel post di prima, cosa posso aver sbagliato?
Grazie per l'aiuto :)
E' normale. L'allocazione non azzera i valori memoria.
Per la seconda volta, probabilmente la matrice viene allocata nella stessa posizione in cui prima c'erano tutti zero (magari proprio perché avevi inizializzato la matrice precedente con tutti zero).
E' normale. L'allocazione non azzera i valori memoria.
Per la seconda volta, probabilmente la matrice viene allocata nella stessa posizione in cui prima c'erano tutti zero (magari proprio perché avevi inizializzato la matrice precedente con tutti zero).
Grazie allora ho fatto giusto! Ma i problemi non finiscono mai :D
Ho definito l'operatore ++ , e provando a ciclarlo nel main effettivamente stampa tutti i valori correttamente, ma dopo aver beccato l'ultimo elemento valgrind mi rivela un errore di invalid read! Non credo che il for nel main vada oltre la dimensione della matrice, infatti se lo ciclo una volta di meno mi stampa tutti gli elementi tranne uno e non dà nessun errore :doh:
questa è la funzione di incremento dell' operatore:
iterator operator++(int) {
iterator(*this);
if (act_c >= colonne-1){
act_c=0;
act_r++;
ptr=&(supporto[act_r][act_c]);
}
else {
if (act_r < righe){
act_c++;
ptr=&(supporto[act_r][act_c]);
}
}
return *this;
}
e questo è il ciclo nel main che si occupa di stamparlo:
for(int i=0; i<A.getRows()* A.getColumns(); i++){
cout<<*ita<<endl;
++ita;
}
cosa può generare questo invalid read?
Comunque grazie mille per le dritte, sono state di grande aiuto ;)
f (act_c >= colonne){
O più semplicemente:
counter++;
ptr=&(supporto[counter / righe][counter % colonne];
f (act_c >= colonne){
O più semplicemente:
counter++;
ptr=&(supporto[counter / righe][counter % colonne];
il secondo metodo che mi hai consigliato mi sembra molto meglio del primo (meno oneroso e più semplice) ma purtroppo gli errori sono rimasti.
fare invece act_c>=colonne non va bene perchè se le colonne sono 6 act_c deve andare da 0 a 5. :help:
Se rimangono allora il problema è altrove, prova a fare un po' di debugging condizionale
Se rimangono allora il problema è altrove, prova a fare un po' di debugging condizionale
ho risolto il problema era nel main :)
ho risolto il problema era nel main :)
Era l'ultimo ++ dell'iteratore ? In effetti ne fa uno in più.
Era l'ultimo ++ dell'iteratore ? In effetti ne fa uno in più.
si esatto, ho invertito l'ordine delle due istruzioni nel ciclo e ho messo la prima stampa prima del ciclo
Allora, sto affrontando l'end dell'iteratore. Ho capito che devo puntare fuori dalla matrice per esempio il primo elemento (quindi prima colonna) di una riga oltre la matrice.
iterator end() {
T* ptr;
return ptr;
}
il problema è che non so che tipo di valore ritornare nel metodo end, perchè dando un'occhiata ad alcuni testi mi era sembrato che fosse possibile ritornare un puntatore al dato ma non ci sono riuscito. Considerando che il mio begin è questo:
iterator begin() {
return iterator(m_,nr_ , nc_);
}
ovvero il begin chiama il costruttore di iterator e gli dice di creare una matrice con nr_ righe e nc_colonne. Non potrei fare lo stesso nel metodo end, perchè quello è un costruttore che crea un nuovo iteratore che punta alla prima cella della matrice. Cosa posso fare allora?
come non detto dovrei aver fatto l'end dell'iterator. Mi sembra che l'end funzioni bene poichè se faccio un for o un while il ciclo termina correttamente però chiamare l'end genera un invalid read. Ecco come ho definito l'end:
iterator end() {
return iterator(m_,nr_,nc_,nr_,0);
}
in questo modo l'end chiama il costruttore che crea un iterator che punta una riga dopo l'ultima riga della matrice e alla cella zero. ecco il costruttore:
iterator(T** mat, unsigned int rows, unsigned int cols, unsigned int it_rows, unsigned int it_cols){
righe=rows;
colonne=cols;
act_r=it_rows;
act_c=it_cols;
supporto=mat;
ptr=&mat[act_r][act_c];
}
cosa c'è di sbagliato?
ptr=&mat[act_r][act_c];
Devi accedere alla matrice solo nel momento in cui usi l'operatore * o ->, ptr te lo devi calcolare solo in quel momento.
ptr=&mat[act_r][act_c];
Devi accedere alla matrice solo nel momento in cui usi l'operatore * o ->, ptr te lo devi calcolare solo in quel momento.
Dici quindi che l'assegnamento ptr=&supporto[act_r][act_c] dovrebbe andare solo nell operatore * o -> ( e in ++ ??) ? in questo modo però quando ciclo l'iteratore non trova mai che iteratore == matrice.end
Questo perchè matrice.end non va ad aggiornare il puntatore, visto che l'assegnamento è rimasto solo negli operatori sopra citati
Dici quindi che l'assegnamento ptr=&supporto[act_r][act_c] dovrebbe andare solo nell operatore * o -> ( e in ++ ??) ? in questo modo però quando ciclo l'iteratore non trova mai che iteratore == matrice.end
Basta non confrontare il puntatore, ma solamente gli indici ;)
Come nell'esempio che ti ho fatto prima...usa solamente un contatore. end() semplicemente lo inizializzerà a righe*colonne.
Basta non confrontare il puntatore, ma solamente gli indici ;)
Come nell'esempio che ti ho fatto prima...usa solamente un contatore. end() semplicemente lo inizializzerà a righe*colonne.
stai dicendo che nell'operatore di confronto invece che confrontare i puntatori devo confrontare solo gli indici? in questo modo se due iteratori diversi puntano a matrici diverse ma sono casualmente allo stesso numero di righe e colonne l'operatore di uguaglianza ci casca
Questo lo puoi evitare andando a confrontare l'indirizzo di matrice[0][0].
Grazie ai tuoi preziosi consigli dovrei aver risolto l'iteratore. Stavo cominciando a vedere un pò di eccezioni e per prima cosa ho pensato di gestire le eccezioni che possono scaturire dal copy constructor. L'ho definita così:
miamatrice(const miamatrice &other) {
nr_=other.getRows();
nc_=other.nc_;
m_=new T*[nr_];
int protettore;
try{
protettore=0;
for (int righe=0; righe<nr_; ++righe)
{
m_[righe]=new T[nc_];
protettore++;
for (int colonne=0; colonne < nc_; ++colonne)
{
m_[righe][colonne]=other.m_[righe][colonne];
}
}
} catch (std::exception& e)
{
for(int i=0; i<=protettore; i++)
delete[] m_[i];
throw;
}
}
Il problema è che oltre a non sapere se così va bene non so neanche come testarlo. Come posso creare un test che lanci un eccezione dal costruttore di copia?
L'unica eccezione che può venire fuori lì è quella nella new:
std::bad_alloc
Secondo me è una di quelle eccezioni che puoi lasciare ingestita o al limite controllarla a livelli più alti. Questo perché è talmente grave che pregiudica l'operatività sia del programma, ma anche solitamente del sistema. Quindi il tuo programma termina comunque. Certo sarebbe bene correre ai ripari, ma come fai senza poter allocare nuova memoria ?
mi hanno detto che praticamente devo gestire solo le eccezioni che fanno riferimento alle new. Quindi mi sa che devo farlo per forza anche se magari non ha molto senso :doh:
Mi hanno detto anche che per gestire questo tipo eccezione dovevo memorizzare quanti vettori erano già stati istanziati in modo tale da cancellare solo quelli, io ho cercato di fare questo tramite il contatore protettore e poi nel catch facendo il delete dei vettori coperti dal contatore.
protettore non è uguale righe - 1 ? Ha poco senso...
Per testarlo...l'unica cosa è lanciare tu manualmente l'eccezione ;)
protettore non è uguale righe - 1 ? Ha poco senso...
Per testarlo...l'unica cosa è lanciare tu manualmente l'eccezione ;)
cioè metto una throw nel costruttore di copia?
ho provato a mettere una throw nel costruttore di copia. All'inizio mi dava segmentation fault perchè probabilmente andava a cancellare dati che non esistevano. Ora con questo codice non da segmentation fault però valgrind segnala invalid read e invalid free (ma nessun memory leak). Il codice è questo:
miamatrice(const miamatrice &other) {
nr_=other.getRows();
nc_=other.nc_;
m_=new T*[nr_];
int protettore;
try{
protettore=0;
for (int righe=0; righe<nr_; ++righe)
{
m_[righe]=new T[nc_];
protettore++;
if(righe==1) throw std::exception();
for (int colonne=0; colonne < nc_; ++colonne)
{
m_[righe][colonne]=other.m_[righe][colonne];
}
}
} catch (std::exception& e)
{
cout<<"rilevata eccezione, ci penso io"<<endl;
for(int i=0; i<=protettore; i++){
delete[] m_[i];
}
}
}
protettore come ti ho già detto non ti serve a niente.
L'eccezione da lanciare è std::bad_alloc
E' chiaro che ti da invalid red e invalid free. protettore è uguale a 2 se righe è uguale a 1. Inoltre se lancia l'eccezione l'ultima riga non te la deve avere allocata.
protettore come ti ho già detto non ti serve a niente.
L'eccezione da lanciare è std::bad_alloc
E' chiaro che ti da invalid red e invalid free. protettore è uguale a 2 se righe è uguale a 1. Inoltre se lancia l'eccezione l'ultima riga non te la deve avere allocata.
se protettore non serve come faccio a sapere quante righe cancellare? e le istruzioni che ho scritto nel blocco catch sono ragionevoli?
se protettore non serve come faccio a sapere quante righe cancellare? e le istruzioni che ho scritto nel blocco catch sono ragionevoli?
Come ti ho detto c'è un errore in catch. Proprio per il numero di righe da cancellare.
Per protettore: protettore è sempre uguale a righe + 1.
Per protettore: protettore è sempre uguale a righe + 1.
Ooops...a righe...al momento in cui può arrivare l'eccezione. L'eccezione andrebbe lanciata subito prima della new per essere coerenti con il meccanismo.
Ooops...a righe...al momento in cui può arrivare l'eccezione. L'eccezione andrebbe lanciata subito prima della new per essere coerenti con il meccanismo.
ho provato a spostare la throw prima della new e mantenendo il protettore che cresce di pari passo con righe e nel catch (i<protettore) mi dà questi errori: 16 allocs, 20 frees
e quindi diversi errori di invalid free e cose del genere.
se io invece nel catch non cancello nulla mi dice 16 allocs, 16 frees ma genera comunque degli errori nel distruttore. Sono confuso sembra che non fare nulla nel catch sia meglio che cancellare quanto era stato istanziato!
ho provato a spostare la throw prima della new e mantenendo il protettore che cresce di pari passo con righe e nel catch (i<protettore) mi dà questi errori: 16 allocs, 20 frees
e quindi diversi errori di invalid free e cose del genere.
se io invece nel catch non cancello nulla mi dice 16 allocs, 16 frees ma genera comunque degli errori nel distruttore. Sono confuso sembra che non fare nulla nel catch sia meglio che cancellare quanto era stato istanziato!
ho capito che è importante mettere il throw dopo il ciclo nel catch. In questo modo tutti gli errori scompaiono però mi dice 9 allocs, 3 free :muro:
miamatrice(const miamatrice &other)
{
nr_=other.getRows();
nc_=other.nc_;
int righe=-1;
try{
m_=new T*[nr_];
for (righe = 0; righe<nr_; ++righe)
{
if(righe==3) throw std::bad_alloc(); //questo ovviamente va levato nelal versione definitiva
m_[righe]=new T[nc_];
for (int colonne=0; colonne < nc_; ++colonne)
{
m_[righe][colonne]=other.m_[righe][colonne];
}
}
}
catch (std::bad_alloc& e)
{
cout<<"rilevata eccezione, ci penso io su riga " << righe << endl;
for(int i=0; i<righe; i++){
delete[] m_[i];
}
if(righe > 0)
{
delete[] m_;
}
nr_ = nc_ = 0;
throw e; //il chiamante deve sapere che c'è stata una eccezione
}
}
Per il distruttore ti basta controllare il valore di nr_ e nc_ e non deallocare niente in tal caso.
ho provato a fare come hai detto, ora non mi da più errori ma ci sono dei memory leak:
/a.out
==2105== Memcheck, a memory error detector
==2105== Copyright (C) 2002-2009, and GNU GPL'd, by Julian Seward et al.
==2105== Using Valgrind-3.5.0-Debian and LibVEX; rerun with -h for copyright info
==2105== Command: ./a.out
==2105==
rilevata eccezione, ci penso io su riga 2
terminate called after throwing an instance of 'std::bad_alloc'
what(): std::bad_alloc
==2105==
==2105== HEAP SUMMARY:
==2105== in use at exit: 332 bytes in 6 blocks
==2105== total heap usage: 10 allocs, 4 frees, 396 bytes allocated
==2105==
==2105== LEAK SUMMARY:
==2105== definitely lost: 0 bytes in 0 blocks
==2105== indirectly lost: 0 bytes in 0 blocks
==2105== possibly lost: 272 bytes in 2 blocks
==2105== still reachable: 60 bytes in 4 blocks
==2105== suppressed: 0 bytes in 0 blocks
==2105== Rerun with --leak-check=full to see details of leaked memory
==2105==
==2105== For counts of detected and suppressed errors, rerun with: -v
==2105== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 4 from 4)
Aborted
per quanto riguarda il controllo nel distruttore mi sembra un'ottima soluzione.
E' normale che ci siano dei memory leak vista l'eccezione non gestita.
Gestisci l'eccezione nel chiamante e i memory leak dovrebbero sparire.
E' normale che ci siano dei memory leak vista l'eccezione non gestita.
Gestisci l'eccezione nel chiamante e i memory leak dovrebbero sparire.
Hai ragione ho gestito l'eccezione nel chiamante e adesso è tutto a posto, grazie!
ora provo a gestire le eventuali eccezioni nell'operatore di assegnamento
vBulletin® v3.6.4, Copyright ©2000-2025, Jelsoft Enterprises Ltd.