New/delete vs malloc/free

[wwmayer]

In C++ you never should release memory with free() that was allocated with new() or release it with delete/delete[] that was allocated with malloc(). Although it works in most cases it's bad practice and there is no guarantee that it always works.

Recently I have built FreeCAD with the compiler option -fsanitize=address that is able to detect flaws like above. When running the unit tests there FreeCAD crashes and in the terminal a message appears like: alloc-dealloc-mismatch (malloc vs operator delete [])

I was able to reduce the crash to this little snippet:

from pivy import coin

node = coin.SoCoordinate3()
pts = []
pts.append([0, 0, 0])
node.point.setValues(pts)

In order to better locate the problem I have built pivy locally and then the utility directly pointed me to the line in the source code that causes the crash. It's the function _wrap_SoMFVec3f_setValues__SWIG_1 which contains this block:

arg3 = static_cast< int >(val3);
  {
    int len;
    if (PySequence_Check(obj3)) {
      len = PySequence_Length(obj3);
      temp4 = (float (*)[3]) malloc(len*3*sizeof(float));
      convert_SoMFVec3f_array(obj3, len, temp4);  
      arg4 = temp4;
    } else {
      PyErr_SetString(PyExc_TypeError, "expected a sequence.");
      arg4 = NULL;
    }
  }
  (arg1)->setValues(arg2,arg3,(float const (*)[3])arg4);
  resultobj = SWIG_Py_Void();
  {
    if (arg4) {
      delete[] arg4; 
    }
  }
  return resultobj;

As you can see the memory for temp4 is allocated with malloc, assigned to arg4 and this frees it with delete[] instead of free().

Since this is generated code I had a look at the file SoMFVec3f.i:

%typemap(in) const float xyz[][3] (float (*temp)[3]) {
  int len;
  if (PySequence_Check($input)) {
    len = PySequence_Length($input);
    temp = (float (*)[3]) malloc(len*3*sizeof(float));
    convert_SoMFVec3f_array($input, len, temp);  
    $1 = temp;
  } else {
    PyErr_SetString(PyExc_TypeError, "expected a sequence.");
    $1 = NULL;
  }
}

/* free the list */
%typemap(freearg) const float xyz[][3] {
  if ($1) { delete[] $1; }
}

So to fix the problem the freearg must be fixed with:

/* free the list */
%typemap(freearg) const float xyz[][3] {
  if ($1) { free($1); }
}

The same fix must be applied to SoMFVec3d.i, SoMFVec2f.i and SoMFVec4f.i. Other classes are not affected as far as I can see.

[looooo]

@wwmayer is this critical? Should it be included for the 0.19 bundles?

[wwmayer]

No, it's not critical. As said with a normally built FreeCAD version everything works.

It's just when building FreeCAD with the above compiler option there are checks at runtime to figure out many kind of memory issues. This option is mainly useful for developers.

[looooo]

I think we can close this one.