1

I define this structure:

struct s_molecule
{
  std::string res_name;
  std::vector<t_particle> my_particles;
  std::vector<t_bond> my_bonds;
  std::vector<t_angle> my_angles;
  std::vector<t_dihedral> my_dihedrals;

  s_molecule& operator=(const s_molecule &to_assign)
  {
    res_name = to_assign.res_name;
    my_particles = to_assign.my_particles;
    my_bonds = to_assign.my_bonds;
    my_angles = to_assign.my_angles;
    my_dihedrals = to_assign.my_dihedrals;
    return *this;
  }
};

and these structures:

typedef struct s_particle
{
  t_coordinates position;
  double charge;
  double mass;
  std::string name;
  std::vector<t_lj_param>::iterator my_particle_kind_iter;

  s_particle& operator=(const s_particle &to_assign)
  {
    position = to_assign.position;
    charge = to_assign.charge;
    mass = to_assign.mass;
    name = to_assign.name;
    my_particle_kind_iter = to_assign.my_particle_kind_iter;
    return *this;
  }
} t_particle;

struct s_bond
{
  t_particle * particle_1;
  t_particle * particle_2;
  std::vector<t_bond_param>::iterator my_bond_kind_iter;

  s_bond& operator=(const s_bond &to_assign)
  {
    particle_1 = to_assign.particle_1;
    particle_2 = to_assign.particle_2;
    my_bond_kind_iter = to_assign.my_bond_kind_iter;
    return *this;
  }
};

and then in my code I return a pointer to an s_molecule (typedef'd to t_molecule, but still).

Using this pointer I can get this code to work:

for  (unsigned int i = 0;
      i < current_molecule->my_particles.size();
      i++)
    {
      std::cout << "Particle " 
        << current_molecule->my_particles[i].name << std::endl
            << "Charge: " 
        << current_molecule->my_particles[i].charge << std::endl
        << "Mass: " 
        << current_molecule->my_particles[i].mass << std::endl
        << "Particle Kind Name: " 
        << (*current_molecule->my_particles[i].my_particle_kind_iter).atom_kind_name 
        << std::endl
        << "x: " << current_molecule->my_particles[i].position.x 
        << " y: " << current_molecule->my_particles[i].position.y
    #ifdef USE_3D_GEOM
        << "z: " << current_molecule->my_particles[i].position.z
    #endif
        << std::endl;
    }

If I replace it with:

for  (std::vector<t_particle>::iterator it = current_molecule->my_particles.begin();
      it !=current_molecule->my_particles.end();
      it++)
    {
      std::cout << "Particle " 
        << (*it).name << std::endl
            << "Charge: " 
        << (*it).charge << std::endl
        << "Mass: " 
        << (*it).mass << std::endl
        << "Particle Kind Name: " 
        << (*(*it).my_particle_kind_iter).atom_kind_name 
        << std::endl
        << "x: " << (*it).position.x 
        << " y: " << (*it).position.y
    #ifdef USE_3D_GEOM
        << "z: " << (*it).position.z
    #endif
        << std::endl;
    }

I now get nasty segfaults...

Not to put too much here, but I'm also getting segfaults when I tried to do this:

std::cout << "Bond ATOMS : " 
          << (*current_molecule).my_bonds[0].particle_1->name
          << std::endl

Again, current_molecule is a pointer to a s_molecule structure, which contains arrays of structures, which in turn either directly have vars or are pointers. I can't get these multiple layers of indirection to work. Suggestions on fixing these segfaults.

FYI I'm compiling on Linux Centos 5.4 with g++ and using a custom makefile system.

Jason R. Mick
  • 5,177
  • 4
  • 40
  • 69
  • 2
    `std::vector::iterator my_particle_kind_iter;` does not seem to be initialized anywhere. I would suggest using debugger to figure out where the crash is coming from. – Anycorn Jun 25 '10 at 01:32
  • I assign it elsewhere. it has a value i can print from inside other functions. Sorry for not mentioning that! Even if I just put this line though: for (unsigned int i = 0; i < current_molecule->my_particles.size(); i++) { std::cout << "Particle " << current_molecule->my_particles[i].name << std::endl; } I still get crashes... – Jason R. Mick Jun 25 '10 at 01:36
  • Also make some typedefs for those iterator types. And don't be afraid to use `->` with iterators. – Ben Voigt Jun 25 '10 at 01:37
  • In other words if I index my vector by using [] and then access the structure member via '.' it works. If I use an iterator and dereference it and access the member via '.' it does NOT work. – Jason R. Mick Jun 25 '10 at 01:37
  • can you compile with "-g" and run your program through gdb? this should tell you what is crashing – Anycorn Jun 25 '10 at 01:38
  • @ Ben Voigt Thanks! Will make typedefs for the iterator types. But I don't think that will fix my issue. I tried using -> as well instead of (*it). Still segfaulted. Only thing that worked was ....[i]. where ...[i] is the vector at the same member as the iterator. I know that sounds crazy. I'm at my wit's end... – Jason R. Mick Jun 25 '10 at 01:40
  • @ aaa trying that now... – Jason R. Mick Jun 25 '10 at 01:42
  • 2
    This code is much too long and complicated. Simplify it until the bug disappears or you see what the problem is. If you still don't see it, post *the whole thing*. – Beta Jun 25 '10 at 01:43
  • @aaa debugger output: Program received signal SIGSEGV, Segmentation fault. 0x0000003a9d48e54e in std::operator<< , std::allocator > () from /usr/lib64/libstdc++.so.6 – Jason R. Mick Jun 25 '10 at 01:47
  • try setting bbreak in the first line of the loop and doing step until you crash. to set a breakpoint, use `break file.cpp: 11 ` for example, to step, use `s` – Anycorn Jun 25 '10 at 01:53
  • @beta I would like to but the code is several files long. Here's is a simpler example typedef struct s_a{ int var_1 } t_a; typedef struct s_b{ std::vector vect_1 } t_b; ... get a pointer a ptr_1 to an initialized t_b ... std::vector::iterator it = (*ptr_1).vect_1.begin(); //segfaults... //std::cout << "VAR 1 " << (*it).var_1 << std::endl; //runs... std::cout << "VAR 1 " << ptr_1->vect_1[0].var_1 << std::endl; ...hopefully that is clearer... – Jason R. Mick Jun 25 '10 at 01:53
  • @aaa thanks. setting breakpoint in main.cc where those "cout" statements live. – Jason R. Mick Jun 25 '10 at 01:55
  • 704 { return __normal_iterator(_M_current + __n); } (gdb) __normal_iterator (this=0x7fff592b1c70, __i=@0x7fff592b1c78) at /usr/lib/gcc/x86_64-redhat-linux/4.1.2/../../../../include/c++/4.1.2/bits/stl_iterator.h:650 650 __normal_iterator(const _Iterator& __i) : _M_current(__i) { } (gdb) __gnu_cxx::__normal_iterator > >::operator* (this=0x7fff592b1ca0) at /usr/lib/gcc/x86_64-redhat-linux/4.1.2/../../../../include/c++/4.1.2/bits/stl_iterator.h:664 664 { return *_M_current; } gdb – Jason R. Mick Jun 25 '10 at 02:01
  • 2
    Your copy constructors are not canonical which is a bug waiting to happen. They need a `if (this == &rhs) return *this;` clause. – msw Jun 25 '10 at 02:01
  • sorry... how am I supposed to put code in a comment? I thought I read to use code tags... obviously not working O_o – Jason R. Mick Jun 25 '10 at 02:03
  • @msw Do you think that's my issue here? I don't see where I'm copying anything via assignment – Jason R. Mick Jun 25 '10 at 02:04
  • I just ran your simplified example (after putting in some missing semicolons-- you didn't try running this yourself). It doesn't segfault if I initialize the `t_b` correctly. Just make a copy of the files and start simplifying. – Beta Jun 25 '10 at 02:04
  • @msw also, what var is rhs? or is that something predefined – Jason R. Mick Jun 25 '10 at 02:06
  • @beta alright ill try that. in the meantime any ideas are still appreciated – Jason R. Mick Jun 25 '10 at 02:07
  • after you assign iterator, do you ever modify container? iterator may become stale if container is reallocated. Check your particle vector to make sure iterator and vector correspond – Anycorn Jun 25 '10 at 02:23
  • @Jason: (You can put code into comments using the back-ticks.) I might be missing something, but I don't see why you're overloading the assignment operators. At a first glance they seem to be doing what the compiler-generated ones do. (Oh, and BTW, @msw, that's - pardon me French - bullshit. The _canonical_ assignment operators use copy ctors and `swap()`.) – sbi Jun 25 '10 at 05:53
  • 6
    @Jason: __Can you please replace this code by one piece of code that we can paste into our editors, compile, and run, and then see the problem you're seeing?__ If you cannot reproduce this in a small example, the problem is elsewhere. Try to reduce the code by commenting out code piece by piece until the problem disappears. Look at the last piece of code you commented. – sbi Jun 25 '10 at 05:55
  • I would also suggest to write it->field instead of (*it).field. It makes the code slightly more readable ;) – zvrba Jun 25 '10 at 06:33
  • 1
    @sbi: +1, Cannot be said often enough. Pseudocode too often hides the real problem in the parts that have been cut away. – DevSolar Jun 25 '10 at 13:30
  • So, if your question has been answered elsewhere, be so kind as to provide the explanation here and close the question. – DevSolar Jun 29 '10 at 05:09
  • http://stackoverflow.com/questions/3136586/weird-pointer-issue-in-c/3136667#3136667 ...sorry about that, should have provided that :D Program is working like a charm when I switch my pointers to array indices and made some new methods to work based on returning a structure at a particular index! Apparently when the vector resizes, pointers inside structures therein get invalidated... Kudos to DeadMG who figured this out. – Jason R. Mick Jun 29 '10 at 22:20

4 Answers4

0

@sbi Thanks for the good advice! I believe you are right -- the assignment overloaded operator is unnecessary and should be scrapped.

I've followed the approach of commenting out stuff and am very confused. Basically in the function that passes the pointer to my particular molecule to the main function to print, I can see all the data in that molecule (bonds, particles, name, etc) perfectly, printing with cout's.

Once I pass it to the main as a ptr, if I use that ptr with an iterator I get a segfault. In other words. Also for some reason the bond data (which I can freely print in my funct that returns to the pointer) also segfaults if I try to print it, even if I use the [] to index the vector of bonds (which works for the particle vector).

That's the best info I can give for now.

  • 2
    1) Use the "edit" function to update your question instead of posting answers (which aren't). 2) If you've simplified the code, don't be afraid to replace your original code example with the simplified one. – DevSolar Jun 25 '10 at 06:52
  • Are you returning a pointer to a local variable? If you are doing that the object will be destroyed before you can dereference the pointer, and that will be UB --which means it will crash or not depending on the mood of the system. – David Rodríguez - dribeas Jun 25 '10 at 07:55
  • @DevSolar. I'm sorry :( Searched for a delete button but didnt see one. It was like 3 a.m. Definitely will work on simplified version later @David No its a ptr to a class variable. The class is very much in existence (it contains the function that returns the molecule). And like I said... I can at least access some of the members if I used [] indexing, vs. iterator indexing of the vector (this only works with the particle structure, for some reason.... – Jason R. Mick Jun 25 '10 at 12:36
0

A wild guess: Are you using shared libraries. I remember having difficulties passing STL-containers back and forth across shared library boundaries.

S.C. Madsen
  • 5,100
  • 5
  • 32
  • 50
0

Jason (OP) was asked in a comment by David Rodríguez:

Are you returning a pointer to a local variable?

Jason answered:

No its a ptr to a class variable. The class is very much in existence (it contains the function that returns the molecule).

Unless you're talking of a true class variable (qualified as static), the fact that the class exists doesn't have much to do with it. Instances of a class exist, and they might have ceased to exist even if you just called a function on them.

As such, the question is:

  • Does the instance of the class that returned the pointer current_molecule still exist?
  • Or is current_molecule qualified as static, i.e. being a true class variable?

If the answer to both questions is "no", you're in Undefined County.

At this point, it becomes very important that you post source code that can be used by us here to actually reproduce the problem; it might well be located in source you aren't showing us.

DevSolar
  • 67,862
  • 21
  • 134
  • 209
  • This has been answered in another thread... what was happening was the vector was dynamically resizing when new elements were added, so pointers to sub vector elements in previous elements were getting screwed up. Switching to indices... 99 percent sure that will work! – Jason R. Mick Jun 28 '10 at 23:07
  • http://stackoverflow.com/questions/3136586/weird-pointer-issue-in-c/3136667#3136667 ...sorry about that, should have provided that :D Program is working like a charm when I switch my pointers to array indices and made some new methods to work based on returning a structure at a particular index! Apparently when the vector resizes, pointers inside structures therein get invalidated... Kudos to DeadMG who figured this out. – Jason R. Mick Jun 29 '10 at 22:21
0

Again, this issue was answered here: Weird Pointer issue in C++ by DeadMG. Sorry for the double post.

Community
  • 1
  • 1
Jason R. Mick
  • 5,177
  • 4
  • 40
  • 69