[petsc-dev] clang --analyze messages on PETSc

[Barry Smith]

   For the first n years of PETSc we always forced a malloc of at least size 1. The problem is that this allows code to use pointers that they shouldn't use and leads to bugs that can go undetected for a long time.


    I'm inclined to turn off the --analze error message for zero pointer ops and leave the code alone.

  Barry

> On Apr 15, 2015, at 3:32 AM, Lisandro Dalcin <dalcinl at gmail.com> wrote:
> 
> I have to competing branches attempting to fix the issues with PetscMalloc.
> 
> 1) https://bitbucket.org/petsc/petsc/branch/dalcinl/petscmalloc-0
> 
> Currently, PetscMallocAlign just calls malloc(size) and generates an
> error if the result is NULL. Given that PetscMalloc is a direct call
> to PetscMallocAlign() in optimized builds, and this has been the case
> at least since release 3.5, it is interesting to note that malloc(0)
> returning NULL seems to be rare nowadays.
> 
> This branch follows Matt's ideas by fixing PetscMallocAlign(0) to
> return NULL. However, I DO NOT like this behavior.
> 
> 2) https://bitbucket.org/petsc/petsc/branch/dalcinl/petscmalloc-1
> 
> This one no longer special-cases zero-sized allocations. It fixes all
> the PetscMallocK() macros.
> 
> If we ever find a system where system malloc(0) returns NULL, we can
> use an obvious approach like in Python sources:
> https://hg.python.org/cpython/file/582e8e71f635/Objects/obmalloc.c#l53
> 
> Matt argued that this could lead to inaccurate memory logging, but
> hey, this is just for for some special systems, and I do not expect
> that 1 byte for each malloc(0) call will be relevant.
> 
> All that being said, I vote for option (2).
> 
> 
> 
> On 15 April 2015 at 03:51, Jed Brown <jed at jedbrown.org> wrote:
>> Matthew Knepley <knepley at gmail.com> writes:
>>> I am arguing for uniformity. With the current system, all internal PETSc
>>> mallocs (we are supposed to use PetscMallocK everywhere)
>>> will return NULL for 0 size argument. On the other hand, if you just
>>> forward the request to malloc, the standard says it can return
>>> a valid pointer OR NULL, so now we have different behavior. What is worse
>>> is that this "valid" pointer can be freed, but it cannot
>>> be dereferenced, and there is no way to determine that it cannot be
>>> dereferenced, unlike NULL. I see absolutely no good argument
>>> for this situation.
>> 
>> You have to store the size somewhere anyway and usually looping
>> constructs are empty if the array has length zero.  Guaranteeing NULL
>> means you can have logic like
>> 
>>  if (a) a[0] = 1;
>> 
>> or
>> 
>>  if (len) a[0] = 1;
>> 
>> If a could be non-NULL, then there is only one way to write the check
>> (and it encourages using logic constructs with fewer branches).
> 
> 
> 
> -- 
> Lisandro Dalcin
> ============
> Research Scientist
> Computer, Electrical and Mathematical Sciences & Engineering (CEMSE)
> Numerical Porous Media Center (NumPor)
> King Abdullah University of Science and Technology (KAUST)
> http://numpor.kaust.edu.sa/
> 
> 4700 King Abdullah University of Science and Technology
> al-Khawarizmi Bldg (Bldg 1), Office # 4332
> Thuwal 23955-6900, Kingdom of Saudi Arabia
> http://www.kaust.edu.sa
> 
> Office Phone: +966 12 808-0459