[coverity] Pointer to local outside scope (RETURN_LOCAL)

[Nico Schlömer]

I fed Dolfin into Coverity and got a bunch of critical errors out.

For example: File dolfin/source/dolfin/la/PETScVector.cpp, line 282:

CID 16374 (#1 of 1): Pointer to local outside scope (RETURN_LOCAL)
5. use_invalid: Using block, which points to an out-of-scope variable tmp.

[Prof Garth Wells]

There's not much anyone without access to coverity can do about this. Moreover, the listed message looks to me like a false positive - I don't see a problem on line 282.

[Lawrence Mitchell]

FWIW, that is a problem, the code in question is:

if ( m == 0 ) {
    double tmp = 0;
    block = &tmp;   // make block point to local var tmp
}
// tmp now out of scope
if ( ... || m == 0 ) {
   // block points to garbage
   ierr = VecGetValues(..., block);
   ...
}

[Martin Sandve Alnæs]

It's not a false warning. This is undefined behaviour, because block at XXX can point to tmp at YYY which is out of scope. Can be fixed by moving tmp to ZZZ, or setting block to null if that's allowed by VecGetValues.

  //double tmp = 0.0; // ZZZ

  // Handle case that m = 0 (VecGetValues is collective -> must be
  // called be all processes)
  if (m == 0)
  {
    _rows = &_m;
    double tmp = 0.0; // YYY
    block = &tmp;
  }

  // Use VecGetValues if no ghost points, otherwise check for ghost
  // values
  if (ghost_global_to_local.empty() || m == 0)
  {
    ierr = VecGetValues(_x, _m, _rows, block);  // XXX
    if (ierr != 0) petsc_error(ierr, __FILE__, "VecGetValues");
  }

[Prof Garth Wells]

@martinal This code that Lawrence points out it an issue, but it's not line 282 (in the dev version) as in the report. Line 282 is fine.

[Martin Sandve Alnæs]

Yes it is. Line 282 in master is this:

    ierr = VecGetValues(_x, _m, _rows, block);

which is where the block is accessed.

[Martin Sandve Alnæs]

But I agree with the sentiment in general, to fix the coverity warnings we will need more than line numbers, i.e. the dolfin commit id that coverity was run on. Maybe Nico can fix them and/or post the warnings here.

[Prof Garth Wells]

Remove some work-around code for PETSc 3.2, fixes Issue #287.

→ <<cset bfd2e74a66cb>>

[Nico Schlömer]

The Coverity report sums up 27 "high impact issues" like use-after-free, and a total of 1352 issues. Many of them are in the auto-generated Python-wrapper and thus not technically Dolfin bugs, but there are some actual issues like this one. I could filter them and post all of them here automatically but that would still flood this issue tracker I think.

If you let me know your Coverity user names I could share the results for you; alternatively you could run Dolfin through your own Coverity dashboard; it's a reasonably fast process.

[Martin Sandve Alnæs]

I managed to view the issues quickly: Log in to www.coverity.com with github account, search for "dolfin" project, click nschloe/dolfin, click "view issues" or something like that.

Not going through them myself now, just wanted to see. It's free for open source projects.