From mboxrd@z Thu Jan 1 00:00:00 1970 X-Msuck: nntp://news.gmane.org/gmane.linux.lib.musl.general/8567 Path: news.gmane.org!not-for-mail From: Matt Avery Newsgroups: gmane.linux.lib.musl.general Subject: Re: Results of static analysis with clang static analyser Date: Fri, 25 Sep 2015 11:35:27 -0400 Message-ID: <560569BF.4020505@akamai.com> References: <20150923193809.GE17773@brightrain.aerifal.cx> <1443038571.23868.17.camel@inria.fr> <20150924003442.GG17773@brightrain.aerifal.cx> <1443079354.23868.25.camel@inria.fr> Reply-To: musl@lists.openwall.com NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/alternative; boundary="------------090600020802040202070606" X-Trace: ger.gmane.org 1443195359 18848 80.91.229.3 (25 Sep 2015 15:35:59 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Fri, 25 Sep 2015 15:35:59 +0000 (UTC) To: musl@lists.openwall.com Original-X-From: musl-return-8579-gllmg-musl=m.gmane.org@lists.openwall.com Fri Sep 25 17:35:45 2015 Return-path: Envelope-to: gllmg-musl@m.gmane.org Original-Received: from mother.openwall.net ([195.42.179.200]) by plane.gmane.org with smtp (Exim 4.69) (envelope-from ) id 1ZfV2G-0006NC-4w for gllmg-musl@m.gmane.org; Fri, 25 Sep 2015 17:35:44 +0200 Original-Received: (qmail 20426 invoked by uid 550); 25 Sep 2015 15:35:40 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: Original-Received: (qmail 20399 invoked from network); 25 Sep 2015 15:35:39 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=akamai.com; s=a1; t=1443195327; bh=Em1cPVgc1OpJGGGEdURv1lGcZ41j52+HBDDdqu76tu8=; l=4855; h=To:References:From:Date:In-Reply-To:From; b=BUBgdXCfXIAlOhB8cgBLyoDOVyNueZJ4gWmkLJcXbAH9PkApECTWwHPDvF2eaFpXA 279zdlcSahrafyA0sgeiLL/dLd5sNxA/cxms0cYztCRGLcunJM+0l1oZKWtdaNMb5G j8weTBPQgMClW5LG9li1H4pYLZ5jKOTTKcQEqMdw= X-Enigmail-Draft-Status: N1110 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.2.0 In-Reply-To: <1443079354.23868.25.camel@inria.fr> Xref: news.gmane.org gmane.linux.lib.musl.general:8567 Archived-At: This is a multi-part message in MIME format. --------------090600020802040202070606 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit 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 : 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 > > --------------090600020802040202070606 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 7bit 
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:
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

--------------090600020802040202070606--