#139 static analyzer results from clang 5.0 on pymol svn r4042 with clang patch

[Jack Howarth]

The attached clang5_static_analyzer_results.txt contains the output from the Apple Clang 5.0 static analyzer for pymol svn at r4042 with the proposed patch from https://sourceforge.net/p/pymol/patches/6/ applied. The instances which indicate garbage values should be looked at first. The dereference of a null pointer instances will require close examination as the static analyzer doesn't recognize conditionals for null pointers (so these have to be looked at individually...see http://clang-analyzer.llvm.org/faq.html#null_pointer).

[Jack Howarth]

At least some of these detections of null pointer dereferences appear to be actual coding errors. For instance in layer0/Isosurf.c....

if(!ok) {
  if(result) {
    if(result->data)
      FieldFree(result->data);
    if(result->points)
      FieldFree(result->points);
    mfree(result);
    result = NULL;
  }
}
result->gradients = NULL;
return (result);

does indeed appear to allow result to be assigned a NULL pointer and then dereferenced for the assignment of NULL to result->gradients.

layer0/Isosurf.c:211:21: warning: Access to field 'gradients' results in a dereference of a null pointer
       (loaded from variable 'result')
   result->gradients = NULL;
   ~~~~~~            ^

[Jack Howarth]

Also note the warnings issued for...

layer1/PConv.c:490:12: warning: Call to 'malloc' has an allocation size of 0 bytes
    (*f) = Alloc(float, l);
           ^~~~~~~~~~~~~~~

which caused me to notice that we have...

int a, l;

and

l = PyList_Size(obj);

in PConvPyListToFloatArray. After python 2.5, this isn't correct as...

Py_ssize_t PyList_Size(PyObject *list)
Return the length of the list object in list; this is equivalent to len(list) on a list object.

Changed in version 2.5: This function returned an int. This might require changes in your code for properly supporting 64-bit systems.

[Jack Howarth]

Another one worth looking at is...

layer1/Ray.c:3469:51: warning: Division by zero
    y = T->y_start + ((yy - T->y_start) + offset) % (render_height);    /* make sure threads write to differen...
                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~

where earlier in that code you actually test if render_height is zero at...

if(render_height) {
  offset = (T->phase * render_height / T->n_thread);
  offset = offset - (offset % T->n_thread) + T->phase;
}

but not later for the division.