Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 25 Sep 2015 11:35:27 -0400
From: Matt Avery <mavery@...mai.com>
To: musl@...ts.openwall.com
Subject: Re: Results of static analysis with clang static analyser

I'm not sure how keen you are to use asserts, but adding them helps the static analyzer not evaluate impossible paths. From the docs <http://clang-analyzer.llvm.org/scan-build.html>:

ALWAYS analyze a project in its "debug" configuration

Most projects can be built in a "debug" mode that enables assertions.
Assertions are picked up by the static analyzer to prune infeasible paths, which
in some cases can greatly reduce the number of false positives (bogus error
reports) emitted by the tool.

Adding an assert(tsd != 0) after the arithmetic operation, or even better,  assert(stack > __pthread_tsd_size) before should be enough?  
-Matt



On 09/24/2015 03:22 AM, Jens Gustedt wrote:
> Am Mittwoch, den 23.09.2015, 20:34 -0400 schrieb Rich Felker:
>> On Wed, Sep 23, 2015 at 10:02:51PM +0200, Jens Gustedt wrote:
>>> 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.
> I wouldn'd call this a bug. This also assumes that the analyser has do
> "know" from somewhere that `stack` is a pointer that is sufficiently
> far from the 0 address, so the result of the arithmetic can never be a
> 0 valued pointer.
>
> So the problem here is that we use a pointer value that is the result
> of arithmetic to hold the state of a conditional execution.
>
> AFAICS, we could completely avoid that by placing a goto after line
> 220 to jump to line 251. Then the initialization of tsd and the `if
> (!tsd)` conditional (not the code inside) could be omitted.
>
> Jens
>
>


Content of type "text/html" skipped

Powered by blists - more mailing lists

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.