AddressSanitizer: attempting double-free

[sothy]

Hi,

I am developing test application. The sample application is :

++++++++++++++++++++++++++++++++++++++++++++

#include <fluid/of10msg.hh>
#include <iostream>

#include <fluid/of10msg.hh>
#include <fluid/of10/openflow-10.h>

using namespace fluid_msg;

of10::PacketOut packet_generation()
{
  of10::PacketOut po(1,of10::OFP_NO_BUFFER,of10::OFPP_CONTROLLER);
  of10::OutputAction act(1, 1024); // = new of13::OutputAction();
  po.add_action(act);
  std::vector<uint8_t> buf={100,20,30,56,67,78,98};
  po.data(buf.data(), buf.size());
  return po;

}



int main() {
   // int a[2] = {1, 0};
   // int b=a[2];

  std::vector<std::unique_ptr<of10::PacketOut>> vec;
  //={} ;
  std::cout<<"Test" << std::endl;

  for (int i=0; i<3; i++)
  {
       //of10::PacketOut p1=packet_generation();
       std::unique_ptr<of10::PacketOut> ptr=std::make_unique<of10::PacketOut>(packet_generation());
       vec.push_back(std::move(ptr));
  }

  std::cout<< "finished" << std::endl;
  return 1;
}

++++++++++++++++++++++++++++++++++++++++++++++

When I compile with g++-6 with -g -fsanitize=address option. It compiled well and could nt run. It gives some errors messages:

Test
finished
================================================== ===============
==3908==ERROR: AddressSanitizer: attempting double-free on 0x60200000efb0 in thread T0:
#0 0x7f00bdf57630 in operator delete(void*) (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc8630)
#1 0x7f00bdc50121 in fluid_msg::PacketOutCommon::~PacketOutCommon() fluid/ofcommon/msg.cc:308
/c++/6/bits/stl_vector.h:426
#11 0x4020c5 in main /home/sothy/netbricks/bitbucket/ofd/test/test2.cc:40
#12 0x7f00bd29582f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
#13 0x401a08 in _start (/home/sothy/netbricks/bitbucket/ofd/test/test2+0x401a08)

0x60200000efb0 is located 0 bytes inside of 7-byte region [0x60200000efb0,0x60200000efb7)
freed by thread T0 here:
#0 0x7f00bdf57630 in operator delete(void*) (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc8630)
#1 0x7f00bdc50121 in fluid_msg::PacketOutCommon::~PacketOutCommon() fluid/ofcommon/msg.cc:308
#2 0x402054 in main /home/sothy/netbricks/bitbucket/ofd/test/test2.cc:47
#3 0x7f00bd29582f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

previously allocated by thread T0 here:
#0 0x7f00bdf56fb0 in operator new(unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc7fb0)
#1 0x7f00bdc50a24 in fluid_msg::PacketOutCommon::data(void*, unsigned long) fluid/ofcommon/msg.cc:399

SUMMARY: AddressSanitizer: double-free (/usr/lib/x86_64-linux-gnu/libasan.so.3+0xc8630) in operator delete(void*)
==3908==ABORTING

++++++++++++++++++++++++++++++++++
I am using opensource library for PacketOutCommon

libfluid_msg/msg.cc at a1568eb67773a5efd7c13d4905e1e8977feb9764 * OpenNetworkingFoundation/libfluid_msg * GitHub

My understanding PacektOutCommon has only DTOR. I am not sure what to add copy construcutor, copy assignment, move construcutor and move assignment?
Thanks for your help

Best regards
Sothy

[algorism]

I'm guessing that your problem has something to do with passing buf.data() (a pointer) to PacketOut::data, which (I'm assuming) just stores that pointer. But it's a pointer to local data that will disappear when the function returns.

EDIT: I just had a peek at the library code and it actually DOES copy the data, so the above is wrong.

Still, it does seem weird to me that you're returning an entire class (as opposed to a pointer). Maybe that has something to do with it.

[Salem]

Rule of three (C++ programming) - Wikipedia
A double-free suggests that you don't have proper copy constructors.

Your default copy-ctor will perform a shallow copy (member for member assignment), unless you specify your own to do the right thing.

This does NOT work for pointers, because you end up with two objects pointing at the same bit of memory.

A natural consequence of this is double free when the second objects gets destructed.

[sothy]

Yes Salem. Th at is what I did. See I modified the code in order to have copy-ctor.

PacketOutCommon::PacketOutCommon(const PacketOutCommon& other) : OFMsg(other)
{

      
     this->buffer_id_== other.buffer_id_;
     this->actions_len_ ==other.actions_len_;
     this->actions_ == other.actions_;
     memcmp(this->data_,other.data_,this->data_len_);
     
     this->data_len_=other.data_len_;



}

After that when I compile, I am getting following compilaortion error:

 g++-6 test2.cc -lfluid_msg -o test2 -g -fsanitize=address
/tmp/ccmher9T.o: In function `fluid_msg::of10::PacketOut::PacketOut(fluid_msg::of10::PacketOut const&)':
/usr/local/include/fluid/of10msg.hh:230: undefined reference to `fluid_msg::PacketOutCommon::PacketOutCommon(fluid_msg::PacketOutCommon const&)'
collect2: error: ld returned 1 exit status

[Salem]

> See I modified the code in order to have copy-ctor.
It's a shame you filled it with comparisons rather than assignments.

> memcmp(this->data_,other.data_,this->data_len_);
Surely you need to allocate some new memory for this->data_ ?
Not to mention use memcpy rather than memcmp.

You see, this is why you're getting duplicate free's, because the dump copy ctor just does
this->data_ = other.data_;

> undefined reference to `fluid_msg::PacketOutCommon::PacketOutCommon(fluid _msg::PacketOutCommon const&)'
Did you declare it inside the right namespace?

[sothy]

I completely modified the source code.

PacketOutCommon::PacketOutCommon(const PacketOutCommon& other) : OFMsg(other)
{

    
     this->buffer_id_== other.buffer_id_;
     this->actions_len_ ==other.actions_len_;
     this->actions_ == other.actions_;

     if (this->data_len_) {
       ::operator delete (this->data_);
     }
     if (other.data_len_) {
        this->data_ = ::operator new(other.data_len_);
        memcpy(this->data_, other.data_,other.data_len_);
     }
     else
        this->data_ = NULL;


     this->data_len_=other.data_len_;



}

I declared right place. Based on the error, PacketOut called PacketOutCommon copy CTOR. But it could not find why? I suspect inheritance. and copy CTOR, or rule of three or something?

[Salem]

> this->buffer_id_== other.buffer_id_;
Why == ?

> But it could not find why?
Because it's C++.

[sothy]

Sorry for this silly mistake. I corrected.
Still same problem. why?

[Salem]

Study this example.

#include <iostream>
#include <iomanip>
#include <sstream>
#include <cstring>
using namespace std;

class base {
  int *mPtr;
public:
  base() {
    mPtr = nullptr;
  }
  base(int n) {
    mPtr = new int;
    *mPtr = n;
  }
  base(const base &other) {
    if ( other.mPtr ) {
      mPtr = new int;
      *mPtr = *other.mPtr;
    } else {
      mPtr = nullptr;
    }
  }
  ~base() {
    delete mPtr;
  }
};

class d1 : public base {
  char *mString;
public:
  d1() {
    mString = nullptr;
  }
  d1(const char *s) {
    mString = new char[strlen(s)+1];
    strcpy(mString,s);
  }
  d1(const d1 &other) {
    if ( other.mString ) {
      mString = new char[strlen(other.mString)+1];
      strcpy(mString,other.mString);
    } else {
      mString = nullptr;
    }
  }
  const char *get() {
    return mString;
  }
  ~d1() {
    delete [] mString;
  }
};

class d2 : public base {
  char *mString;
public:
  d2() {
    mString = nullptr;
  }
  d2(const char *s) {
    mString = new char[strlen(s)+1];
    strcpy(mString,s);
  }
  d2(const d2 &other) {
    //!! WRONG WRONG WRONG
    mString = other.mString;
  }
  const char *get() {
    return mString;
  }
  ~d2() {
    delete [] mString;
  }
};

int main ( ) {
  d1 a("hello");
  d2 b("world");
  cout << a.get() << " " << b.get() << endl;
  d1 c = a;
  d2 d = b;
  cout << c.get() << " " << d.get() << endl;  // same printed result

  cout << "a@" << reinterpret_cast<const void*>(a.get()) << endl;
  cout << "c@" << reinterpret_cast<const void*>(c.get()) << endl;
  cout << "Proper copy ctor has made a unique copy" << endl;
  cout << "b@" << reinterpret_cast<const void*>(b.get()) << endl;
  cout << "d@" << reinterpret_cast<const void*>(d.get()) << endl;
  cout << "OOPSY, both point to same string => DOUBLE FREE" << endl;
}


$ g++ -g -std=c++11 -fsanitize=address foo.cpp
$ ./a.out 
hello world
hello world
a@0x60200000eff0
c@0x60200000efb0
Proper copy ctor has made a unique copy
b@0x60200000efd0
d@0x60200000efd0
OOPSY, both point to same string => DOUBLE FREE
=================================================================
==31192==ERROR: AddressSanitizer: attempting double-free on 0x60200000efd0 in thread T0:
    #0 0x7ff17affacaa in operator delete[](void*) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99caa)
    #1 0x4018af in d2::~d2() /home/sc/Documents/foo.cpp:74
    #2 0x401261 in main /home/sc/Documents/foo.cpp:80
    #3 0x7ff17a61f82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)
    #4 0x400e88 in _start (/home/sc/Documents/a.out+0x400e88)

0x60200000efd0 is located 0 bytes inside of 6-byte region [0x60200000efd0,0x60200000efd6)
freed by thread T0 here:
    #0 0x7ff17affacaa in operator delete[](void*) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x99caa)
    #1 0x4018af in d2::~d2() /home/sc/Documents/foo.cpp:74
    #2 0x401246 in main /home/sc/Documents/foo.cpp:83
    #3 0x7ff17a61f82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

previously allocated by thread T0 here:
    #0 0x7ff17affa6b2 in operator new[](unsigned long) (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x996b2)
    #1 0x401740 in d2::d2(char const*) /home/sc/Documents/foo.cpp:63
    #2 0x401058 in main /home/sc/Documents/foo.cpp:80
    #3 0x7ff17a61f82f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2082f)

SUMMARY: AddressSanitizer: double-free ??:0 operator delete[](void*)
==31192==ABORTING

[sothy]

Thanks Salem. But your code is good example for double free error. When I correct this error, I am now in another error undefined reference. Either it should be some error in copy CTOR or somewhere else. Regading my copy CTOR seems ok. It uses memcpy compared to your code.Otherwise, everything is fine.
Thanks

[Salem]

> When I correct this error, I am now in another error undefined reference

g++-6 test2.cc -lfluid_msg -o test2 -g -fsanitize=address
/tmp/ccmher9T.o: In function `fluid_msg::of10::PacketOut::PacketOut(fluid_msg::of10::PacketOut const&)':
/usr/local/include/fluid/of10msg.hh:230: undefined reference to `fluid_msg::PacketOutCommon::PacketOutCommon(fluid_msg::PacketOutCommon const&)'
collect2: error: ld returned 1 exit status

So where did you edit the code?

Did you edit libfluid_msg/msg.cc and msg.hh ?

Because you use a library (that's the -lfluid_msg in your command line), you need to REBUILD the library if you make changes to the code base for that library.

[sothy]

yes i edited in msg.hh and msg.cc and rebuilt it

[sothy]

And It is not working . same error

[sothy]

@Salem, after cleaning libfluid_msg manually, recompiled, It worked. Thanks