Help with STL

Help with STL

[Giovani Faccin]

Hi all!

I'm quite puzzled by this, perhaps one of the gurus on the list might
be able to help ;)

I have this member function inside a class:


calc_atom_forc(Atom_Classical *atm, std::list<Atom_Classical*> *neighbours)


Where the Atom_Classical object is essentially a struct holding data
plus a few member functions. The neighbours pointer points to a
std::list of pointers to Atom_Classical objects that are stored
outside this class.

Now situation "A". Let's say that right upon entering the
calc_atom_forc routine, I run this loop:

calc_atom_forc(Atom_Classical *atm, std::list<Atom_Classical*> *neighbours)
{
    for ( std::list<Atom_Classical*>::iterator a =
neighbours->begin(); a != neighbours->end(); a++)
    {
        cout << (*a)->x() << endl;
    };
}

This will work fine, and the program will print the value returned by
the x() function of each Atom_Classical object. So far so good.

Now we get into situation B. I need to manipulate a bit those
pointers. This is the idea:
calc_atom_forc(Atom_Classical *atm, std::list<Atom_Classical*> *neighbours)
{
        //Allocating a vector that will hold the guys we need to
change inside this routine
	std::vector<Atom_Classical> changed_atms;

	for (std::list<Atom_Classical*>::iterator neigh =
neighbours->begin(); neigh != neighbours->end(); neigh++)
	{	
               If ( criteria_that_does_not_matter_now == true)
               {
                        //Pushing a copy of the original
Atom_Classical pointed by iterator neigh into the changed_atms vector:
                        changed_atms.push_back( *(*neigh) );
                        //Printing debug info:
                        cout << "Before changing atom " <<
(*neigh)->id() << " we had: " << (*neigh)->posi() << endl;

                        //Removed here some code that would change the
data on the "changed_atms[changed_atms.size() -1]" object

			//Now we'll make the corresponding neigh pointer point to the
modified atom on the newly created changed_atoms vector, instead of
the atom in the original neighbours list:
			(*neigh) = &(changed_atms[changed_atms.size() -1]);
                        //Printing debug info:
                        cout << "After changing atom " <<
(*neigh)->id() << " we have: " << (*neigh)->posi() << endl;
		};
	};
}

This also works fine. So in situation B, I'm having some of the
pointers pointed by the neighbours list point to the objects inside
the changed_atms container I've created inside this function. Notice
that the neighbours list is not allocated inside this function, all I
have is a pointer to it. I know that when the function ends, the
original neighbours list will have invalid pointers. That does not
matter, because that list will be deleted as soon as this routine
ends.

Here comes the trouble. What I need to do is run situation B, then
situation A. If I do this, I get a segfault error. This is how it's
like:

calc_atom_forc(Atom_Classical *atm, std::list<Atom_Classical*> *neighbours)
{
	// START OF CODE FROM SITUATION B, IDENTICAL TO THE ONE SHOWN PREVIOUSLY:

        //Allocating a vector that will hold the guys we need to
change inside this routine
	std::vector<Atom_Classical> changed_atms;

	for (std::list<Atom_Classical*>::iterator neigh =
neighbours->begin(); neigh != neighbours->end(); neigh++)
	{	
               If ( criteria_that_does_not_matter_now == true)
               {
                        //Pushing a copy of the original
Atom_Classical pointed by iterator neigh into the changed_atms vector:
                        changed_atms.push_back( *(*neigh) );
                        //Printing debug info:
                        cout << "Before changing atom " <<
(*neigh)->id() << " we had: " << (*neigh)->posi() << endl;

                        //Removed here some code that would change the
data on the "changed_atms[changed_atms.size() -1]" object

			//Now we'll make the corresponding neigh pointer point to the
modified atom on the newly created changed_atoms vector, instead of
the atom in the original neighbours list:
			(*neigh) = &(changed_atms[changed_atms.size() -1]);
                        //Printing debug info:
                        cout << "After changing atom " <<
(*neigh)->id() << " we have: " << (*neigh)->posi() << endl;
		};
	};

       // END OF CODE FROM SITUATION B.
       // START OF CODE FROM SITUATION A, IDENTICAL TO THE ONE SHOWN PREVIOUSLY:

       for ( std::list<Atom_Classical*>::iterator a =
neighbours->begin(); a != neighbours->end(); a++)
       {
            cout << (*a)->x() << endl;   --> Program crashes upon
reaching this line for the first time.
       };

       // END OF CODE FROM SITUATION A.
}

What is so strange here is that I can test the code in part B, and
it's working (I can see it through the couts on the screen). Then I
get on part A and get a segfault while accessing the same list that
had worked fine just a few lines before.

What's wrong here?

Thank you very much!

Giovani Faccin

Re: Help with STL

[John Love-Jensen]

Hi Giovani,

Your code worked just fine on my machine.

I had to mock up Atom_Classical, since you did not provide the source to
that.

Maybe if you could provide a compile-able, run-able small code example that
exhibits the problem, that could be useful.

Thanks,
--Eljay

Re: Help with STL

[Giovani Faccin]

[-- Attachment #1: Type: text/plain, Size: 7516 bytes --]

Hi!

I'm still working on that code problem. Got an interesting development
here, and would like to ask your opinion.

I've stripped the source code and made a small version that shows the problem.

If I run this small version inside valgrind, this is the ouput I get:

5
4
3
2
1
Before changing atom 5 we had: 4  now we have -1
Before changing atom 4 we had: 3  now we have -2
5
4
3
2
1

Which is precisely what is expected. Now I take the same program and
run it outside valgrind:

5
4
3
2
1
Before changing atom 5 we had: 4  now we have -1
Before changing atom 4 we had: 3  now we have -2
47499490531776
4
3
2
1

Notice the 47499490531776 that appears insted of 5. This is what is
making my program crash.

Valgrind complains about this:

==7331== Invalid read of size 8
(suppressed useless dependencies)
==7331==    by 0x402A15:
Potential_Openmx::calc_atom_forc(Atom_Classical*,
std::list<Atom_Classical*, std::allocator<Atom_Classical*> >*)
(Potential_Openmx.cpp:86)

==7331==  Address 0x58f45d8 is 8 bytes inside a block of size 152 free'd
(suppressed useless dependencies)
==7331==    by 0x40276F:
Potential_Openmx::calc_atom_forc(Atom_Classical*,
std::list<Atom_Classical*, std::allocator<Atom_Classical*> >*)
(Potential_Openmx.cpp:62)

Looking at Potential_Openmx.cpp:86 we have:

cout << (*a)->id() << endl;

So this error appears when we are printing the data. When this
happens, we print the 47499490531776.

Looking at std::allocator<Atom_Classical*> >*)
(Potential_Openmx.cpp:62) we have:

changed_atms.push_back( *(*neigh) );

This happens when we copy an object inside a vector. Again, this
should not be a problem (at least I think so!). Notice that on the
same block of code I do things like (*neigh)->use_some_function() and
everything works fine.

I'm attaching the code to the email in case someone would like to give
it a try. Open the Openmx.cpp class, and you'll find the code I'm
talking about. The other classes are very small, stripped pieces of
code that are there just to define the needed objects. The main()
function appears in the mss.cpp file.

Thank you very much!

Giovani




2008/9/9 Giovani Faccin <faccin.giovani@gmail.com>:
> Hi all!
>
> I'm quite puzzled by this, perhaps one of the gurus on the list might
> be able to help ;)
>
> I have this member function inside a class:
>
>
> calc_atom_forc(Atom_Classical *atm, std::list<Atom_Classical*> *neighbours)
>
>
> Where the Atom_Classical object is essentially a struct holding data
> plus a few member functions. The neighbours pointer points to a
> std::list of pointers to Atom_Classical objects that are stored
> outside this class.
>
> Now situation "A". Let's say that right upon entering the
> calc_atom_forc routine, I run this loop:
>
> calc_atom_forc(Atom_Classical *atm, std::list<Atom_Classical*> *neighbours)
> {
>    for ( std::list<Atom_Classical*>::iterator a =
> neighbours->begin(); a != neighbours->end(); a++)
>    {
>        cout << (*a)->x() << endl;
>    };
> }
>
> This will work fine, and the program will print the value returned by
> the x() function of each Atom_Classical object. So far so good.
>
> Now we get into situation B. I need to manipulate a bit those
> pointers. This is the idea:
> calc_atom_forc(Atom_Classical *atm, std::list<Atom_Classical*> *neighbours)
> {
>        //Allocating a vector that will hold the guys we need to
> change inside this routine
>        std::vector<Atom_Classical> changed_atms;
>
>        for (std::list<Atom_Classical*>::iterator neigh =
> neighbours->begin(); neigh != neighbours->end(); neigh++)
>        {
>               If ( criteria_that_does_not_matter_now == true)
>               {
>                        //Pushing a copy of the original
> Atom_Classical pointed by iterator neigh into the changed_atms vector:
>                        changed_atms.push_back( *(*neigh) );
>                        //Printing debug info:
>                        cout << "Before changing atom " <<
> (*neigh)->id() << " we had: " << (*neigh)->posi() << endl;
>
>                        //Removed here some code that would change the
> data on the "changed_atms[changed_atms.size() -1]" object
>
>                        //Now we'll make the corresponding neigh pointer point to the
> modified atom on the newly created changed_atoms vector, instead of
> the atom in the original neighbours list:
>                        (*neigh) = &(changed_atms[changed_atms.size() -1]);
>                        //Printing debug info:
>                        cout << "After changing atom " <<
> (*neigh)->id() << " we have: " << (*neigh)->posi() << endl;
>                };
>        };
> }
>
> This also works fine. So in situation B, I'm having some of the
> pointers pointed by the neighbours list point to the objects inside
> the changed_atms container I've created inside this function. Notice
> that the neighbours list is not allocated inside this function, all I
> have is a pointer to it. I know that when the function ends, the
> original neighbours list will have invalid pointers. That does not
> matter, because that list will be deleted as soon as this routine
> ends.
>
> Here comes the trouble. What I need to do is run situation B, then
> situation A. If I do this, I get a segfault error. This is how it's
> like:
>
> calc_atom_forc(Atom_Classical *atm, std::list<Atom_Classical*> *neighbours)
> {
>        // START OF CODE FROM SITUATION B, IDENTICAL TO THE ONE SHOWN PREVIOUSLY:
>
>        //Allocating a vector that will hold the guys we need to
> change inside this routine
>        std::vector<Atom_Classical> changed_atms;
>
>        for (std::list<Atom_Classical*>::iterator neigh =
> neighbours->begin(); neigh != neighbours->end(); neigh++)
>        {
>               If ( criteria_that_does_not_matter_now == true)
>               {
>                        //Pushing a copy of the original
> Atom_Classical pointed by iterator neigh into the changed_atms vector:
>                        changed_atms.push_back( *(*neigh) );
>                        //Printing debug info:
>                        cout << "Before changing atom " <<
> (*neigh)->id() << " we had: " << (*neigh)->posi() << endl;
>
>                        //Removed here some code that would change the
> data on the "changed_atms[changed_atms.size() -1]" object
>
>                        //Now we'll make the corresponding neigh pointer point to the
> modified atom on the newly created changed_atoms vector, instead of
> the atom in the original neighbours list:
>                        (*neigh) = &(changed_atms[changed_atms.size() -1]);
>                        //Printing debug info:
>                        cout << "After changing atom " <<
> (*neigh)->id() << " we have: " << (*neigh)->posi() << endl;
>                };
>        };
>
>       // END OF CODE FROM SITUATION B.
>       // START OF CODE FROM SITUATION A, IDENTICAL TO THE ONE SHOWN PREVIOUSLY:
>
>       for ( std::list<Atom_Classical*>::iterator a =
> neighbours->begin(); a != neighbours->end(); a++)
>       {
>            cout << (*a)->x() << endl;   --> Program crashes upon
> reaching this line for the first time.
>       };
>
>       // END OF CODE FROM SITUATION A.
> }
>
> What is so strange here is that I can test the code in part B, and
> it's working (I can see it through the couts on the screen). Then I
> get on part A and get a segfault while accessing the same list that
> had worked fine just a few lines before.
>
> What's wrong here?
>
> Thank you very much!
>
> Giovani Faccin
>

[-- Attachment #2: mss-error.tar.gz --]
[-- Type: application/x-gzip, Size: 5920 bytes --]

Re: Help with STL

[John Fine]

The problem is in the line

(*neigh) = &(changed_atms[changed_atms.size() -1]);

You are taking the address of an element of a vector and keeping that 
address long past the time it becomes invalid.

As soon as the vector grows, that address becomes invalid.

Even if the vector didn't grow, I think you are keeping the address 
after the whole vector goes out of scope.  But I'm not sure of that.  I 
am sure you are using the address while the vector is still in scope, 
but after it may have grown.  That's bad enough.