Re: Results of static analysis with clang static analyser

[Rich Felker <dalias@libc.org>]

On Wed, Sep 23, 2015 at 10:02:51PM +0200, Jens Gustedt wrote:
> Am Mittwoch, den 23.09.2015, 15:38 -0400 schrieb Rich Felker:
> > On Tue, Sep 22, 2015 at 10:58:55PM -0700, Khem Raj wrote:
> > > Hi All
> > > 
> > > I have run scan-build on musl-git and here are results
> > > 
> > > http://busybox.net/~kraj/scan-build-2015-09-22-224330-15962-1/
> > 
> > At a quick glance, most of these seem to be cases of assuming system
> > calls do not store to the objects they receive pointers to.
> 
> Some of them, yes. But the one in __memalign seems to have secret
> knowledge that it may access header information preceeding the pointer
> that was received from malloc. I have no idea what a compiler in a
> freestanding environment is allowed (or not) to assume in that case.

malloc is not reserved in a freestanding environment. LLVM/clang used
to get this wrong and badly broke musl, but it was fixed sometime
around 3.4 or 3.5 I think.

> Perhaps it would be cleaner to have a malloc_helper function that
> returns the veritable start of the reserved chunk and then the user
> interface wrappers such as malloc and friends return that address plus
> the necessary offset.

It might, but that probably add an extra layer of call. I'll see if I
can come up with a reasonable way to do something like this when I get
around to overhauling malloc, though, since it seems useful.

> > This makes
> > them false positives, but if llvm is actually making that same
> > assumption when optimizing that could be a bug in itself. Hopefully
> > it's just treating it as "unknown" whether the object is stored to,
> > rather than "definitely not accessed".
> 
> The one in pthread_create I always struggle with. I remember that I
> had myself once convinced (or was it you?) that the bad case can't
> happen, but I was not able to reproduce the argument spontaneously.

From my perspective, this one is simply a bug in the static analysis.
At line 218, pointer arithmetic was performed on `stack` to get `tsd`.
If `stack` were null this would be UB, and if `stack` is not null then
you cannot get a null pointer without the arithmetic having invoked
UB, so you can conclude that `tsd` is not null.

Of course such UB could happen if inputs are invalid (e.g. attribute
containing a size larger than the actual size of the stack) or if
internal state is invalid (e.g. if the computation of `need` wraps),
so maybe it's useful to have this analysis take place.

Rich