[DynInst_API:] Results of dyninst static code check

[William Cohen <wcohen@xxxxxxxxxx>]

Hi All,

At Red Hat we have access to the Coverity static analysis tools.  This weekend I ran Coverity on the dyninst srpm available from:

http://koji.fedoraproject.org/koji/buildinfo?buildID=328273

The result file is rather large (>600k) so I have put it at:

http://people.redhat.com/wcohen/dyninst/dyninst-7.99-0.17.fc18.err

Some of the errors are false positives. Below is a breakdown of the results by type annotated with the relative importance of the various errors (based on https://wiki.edubuntu.org/CoverityCheckerDictionary):

$ grep ^Error dyninst-7.99-0.17.fc18.err |sort|uniq -c
     18 Error: CHECKED_RETURN:			 medium
     10 Error: CTOR_DTOR_LEAK:			 high
     11 Error: DEADCODE:			 medium
      2 Error: DELETE_ARRAY:			 medium
     73 Error: FORWARD_NULL:			 high
      1 Error: INVALIDATE_ITERATOR:		 high
     21 Error: MISSING_BREAK:			 medium
      2 Error: NEGATIVE_RETURNS:		 medium
      1 Error: NO_EFFECT:			 medium
     30 Error: NULL_RETURNS:			 high
      2 Error: OVERRUN_DYNAMIC:			 high
      9 Error: OVERRUN_STATIC:			 high
    146 Error: RESOURCE_LEAK:			 high
      8 Error: REVERSE_INULL:			 high
      5 Error: SIGN_EXTENSION:			 medium
      4 Error: SIZEOF_MISMATCH:			 medium
      8 Error: STREAM_FORMAT_STATE:		 medium
     29 Error: UNINIT:				 high
    201 Error: UNINIT_CTOR:			 medium
     11 Error: UNREACHABLE:			 medium
     71 Error: UNUSED_VALUE:			 low
      8 Error: USE_AFTER_FREE:			 high
      1 Error: WRAPPER_ESCAPE:			 high


Looking through the MISSING_BREAK errors some are definitely false positives. However, there are a few that look legit:


/builddir/build/BUILD/dyninst-7.99/dyninst/parseAPI/src/ParserDetails.C:63: unterminated_case: This case (value 0) is not terminated by a 'break' statement.
/builddir/build/BUILD/dyninst-7.99/dyninst/instructionAPI/src/Expression.C:59: unterminated_case: This case (value 10) is not terminated by a 'break' statement.


A number of the UNUSED_VALUE errors look harmless and are due to code like the following from dataflowAPI/src/debug_dataflow.C:

  char *p;

  if ((p=getenv("DATAFLOW_DEBUG_STACKANALYSIS"))) {
    fprintf(stderr, "Enabling DataflowAPI stack analysis debugging\n");
    df_debug_stackanalysis = 1;
  }

It looks like it would be fairly straight forward to remove the "char *p;" declarations and the "p=" assignments in the code to eliminate those errors.

There should be a null check for:

/builddir/build/BUILD/dyninst-7.99/dyninst/dyninstAPI/src/linux.C:427: dereference: Dereferencing a pointer that might be null "getenv("LD_LIBRARY_PATH")" when calling "strdup". (The dereference is assumed on the basis of the 'nonnull' parameter attribute.)

A number of the NULL_RETURNS errors are because strdup() can return NULL, but the code does not check that condition.


Is the code flagged by the following error correct?

Error: OVERRUN_DYNAMIC:
/builddir/build/BUILD/dyninst-7.99/dyninst/symtabAPI/src/Object-elf.C:5160: alloc_strlen: Allocating insufficient memory for the terminating null of the string.


The OVERRUN_STATIC errors for InstructionDecoder-x86.C should be examined more closely. 


-Will