2

The struct is defined as such:

struct section_{
    int start;
    ...
};

For reasons I will not go into, I need to pass a pointer to the struct to a function that accepts a void*. The function looks like this:

void* my_fun(void* sec){
    section_* section = (section_*)sec;
    int start = section->start; // <---- valgrind complains here
    ...
}

I have a std::vector<section_*> and I need to call my_fun on each of the elements of this vector. I do this like so:

std::vector<section_*> sections = get_sections();
for (int i = 0; i < sections.size(); ++i){
    my_fun((void*)sections[i]);
}

The get_sections() function looks something like:

std::vector<section_*> get_sections(){
    std::vector<section_*> sections;
    section_ sec1;
    sec1.start = 0;
    ...
    sections.push_back(&sec1);
    return sections;
}

I've tracked the problem down to the line in my_fun that says

int start = section->start;

The error says:

==3512== Invalid read of size 4
==3512==    at 0x41A970: my_fun(void*)
...
==3512==  Address 0xffeffa2a0 is on thread 1's stack
==3512==  14160 bytes below stack pointer

Even though I'm getting an invalid read, I am still able to access the members of the struct inside my_fun and they are the correct values. Why is that?

I realize that this code is in little bits and pieces, but the actual code is much more complex and lengthy than what I'm showing, though I think I'm showing all the relevant parts. I hope the info I gave is enough.

asrjarratt
  • 314
  • 6
  • 17
  • did you try using [static_casting](http://www.cplusplus.com/doc/tutorial/typecasting/), It looks like a casting problem. – Vtik May 31 '16 at 13:51
  • @Vtik I've never used that before... I will look into that. Thanks – asrjarratt May 31 '16 at 13:53
  • 1
    This `sections.push_back(&sec1); return sections;` returns a pointer to a local variable. Like the message says *==3512== Address 0xffeffa2a0 is on thread 1's stack* – Bo Persson May 31 '16 at 13:53
  • @Vtik I did `section_* section = static_cast(sec);`, but that didn't change anything. – asrjarratt May 31 '16 at 14:07

1 Answers1

2

As mentioned in the comments by @BoPersson, you add a local variable to the vector:

std::vector<section_*> get_sections(){
    std::vector<section_*> sections;
    section_ sec1;    // <- Local, temporary variable 
    sec1.start = 0;
    ...
    sections.push_back(&sec1); // <- Address of local var
    return sections;
    // When this function ends, sec1 is no longer valid
}

You can either use new to create sec1 (deleteing it later). Or change the vector type to std::vector<section_> sections;.

001
  • 13,291
  • 5
  • 35
  • 66
  • Also, why was I able to access the struct members (giving the correct values) if `sec1` was no longer valid? – asrjarratt May 31 '16 at 14:19
  • Ok, that makes sense. So, if I change the vector type to std::vector sections, I can pass &sections[i] to my_fun, yes? – asrjarratt May 31 '16 at 14:24
  • 2
    You were just getting lucky. [This answer](http://stackoverflow.com/a/6445794/669576) might help. – 001 May 31 '16 at 14:24
  • Just changed my code and ran it. This fixed it. Thanks a lot! – asrjarratt May 31 '16 at 14:25
  • 2
    @JohnnyMopp I consider it very **un**lucky that the program appeared to work as intended. – eerorika May 31 '16 at 15:00
  • @user2079303 Agreed. A hidden bug that might show itself later is not lucky. – 001 May 31 '16 at 15:12
  • @JohnnyMopp I guess push_back copies the contents of the variable into the vector, so adding a local variable to the vector should not be a issue right? Could you please clarify. Ref: [link](http://stackoverflow.com/questions/9304480/safe-to-return-a-vector-populated-with-local-variables) – Shyam Sep 23 '16 at 16:07
  • @Shyam The problem here is the OP is adding a pointer to a local var. If you are not doing that, you should be ok. – 001 Sep 23 '16 at 16:53
  • ah! got it. Thanks @JohnnyMopp – Shyam Sep 23 '16 at 19:09