From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 36459 invoked by alias); 15 Mar 2017 22:37:08 -0000 Mailing-List: contact newlib-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: newlib-owner@sourceware.org Received: (qmail 36412 invoked by uid 89); 15 Mar 2017 22:37:07 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.6 required=5.0 tests=AWL,BAYES_50,KAM_LAZY_DOMAIN_SECURITY,RP_MATCHES_RCVD autolearn=no version=3.3.2 spammy=H*r:ip*192.168.2.2, sk:scanco, sk:scan.co, scancoveritycom X-HELO: OARmail.OARCORP.com Received: from oarmail.oarcorp.com (HELO OARmail.OARCORP.com) (67.63.146.244) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 15 Mar 2017 22:37:05 +0000 Received: from [192.168.1.175] (192.168.1.175) by OARmail.OARCORP.com (192.168.2.2) with Microsoft SMTP Server (TLS) id 8.3.389.2; Wed, 15 Mar 2017 17:36:59 -0500 Subject: Re: Use of initialized variable in strtod.c To: "noloader@gmail.com" , "newlib@sourceware.org" References: <788987e9-9b0d-4bfd-b40a-38c219bd8a17@oarcorp.com> <47ea65c0-130a-e138-3c7c-0c1a636d87fd@oarcorp.com> <2264cd14-0e03-2aad-f95e-562394435c0b@LGSInnovations.com> From: Joel Sherrill Message-ID: <5a3ecf1c-1f4f-cce6-6b3e-b933cf87fa28@oarcorp.com> Date: Wed, 15 Mar 2017 22:37:00 -0000 User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2017/txt/msg00183.txt.bz2 Sorry to keep replying to myself. More below. On 3/15/2017 3:03 PM, Joel Sherrill wrote: > > > On 3/15/2017 2:54 PM, Joel Sherrill wrote: >> >> >> On 3/15/2017 2:31 PM, Jeffrey Walton wrote: >>>> Does Coverity have a way in which in the code it can be marked as OK? (I'd >>>> expect some '#pragma CoverityIgnore(bits)' or the like ought to be >>>> available.) >>> >>> Yes. You have to provide a modeling file. Also see the Coverity Scan >>> FAQ entry "what is a model" at https://scan.coverity.com/faq. >> >> A model is for odd cases like where you have your >> own memory allocators or synchronization primitives. >> >> I think this is a case for what they call annotations. >> I have never written one of these but I think we would >> have to add a comment something like this ahead of the >> call: >> >> /* coverity[uninit_use_in_call] */ > > Adding this does result in silencing Coverity on this issue. > It doesn't change the fact that the uninitialized bits > variable is used on the RHS of ULtod() though. :( But this in __call_atexit.c is definitely correct. It is treating free() as a weak symbol and the only way to silence Coverity is to add an annotation. 136 /* Don't dynamically free the atexit array if free is not 137 available. */ CID 175323 (#1 of 1): Function address comparison (BAD_COMPARE) func_conv: This implicit conversion to a function pointer is suspicious: free. Did you intend to call free? 138 if (!free) 139 break; >> I will try adding that notation but we clearly need >> some guidelines as a project. Agreed. There appear to be cases where annotation is the only solution. Is adding annotation acceptable? If a solution other than annotation exists, is that the preferred option? >>> Other projects use them, like Python. See, for example, >>> https://docs.python.org/devguide/coverity.html. >>> >>>> I agree with trying to get rid of the message, but it is worth >>>> bloat to do it? (It will add instructions to either initialize bits to 0 or >>>> add the else.) >>> >>> If I am parsing things correctly, it seems like the bloat is going the >>> other way: if the code is not needed, then remove it. It will avoid >>> findings like these, and speed up the compile. >>> >>>> I would rather mark something in the code as a false >>>> positive than add code because the tool is not smart enough to know--so we >>>> might differ in philosophy there. >>> >>> Perhaps a better strategy would be to initialize all variables, and >>> then allow the optimizer to remove the unneeded writes. It will ensure >>> a program is in a good state, and avoid findings like these. >> >> I'm a middle of the road guy. I add initialization in cases >> where there are paths where it is used and doesn't otherwise >> get set. I wouldn't automatically initialize everything. >> >> In this case, "bits" is actually used on the RHS multiple >> times in ULtod() so it bothers me that it has an undefined >> value. That means the output of ULtod() is undefined in >> this case. >> >>> Another strategy is to do nothing. In this case, the same findings >>> will waste multiple developer's time, and generate additional mailing >>> list messages. >> >> Agreed. >> >>> I like dark and silent cockpits, so I don't want tools generating >>> findings, and I don't want mailing list messages. I would squash it >>> once and for all and avoid all future problems. But that's just me, >>> and I understand the Newlib project may have a different outlook on >>> things. >> >> +1 >> >> There are still 60 others issues. We should do our best to squash >> them permanently. IMO marking them with Coverity specific annotation >> just means that another static analyzer may find the same issue >> in the future. The annotation won't help. >> >>> Jeff >>> >> --joel >> > -- Joel Sherrill, Ph.D. Director of Research & Development joel.sherrill@OARcorp.com On-Line Applications Research Ask me about RTEMS: a free RTOS Huntsville AL 35806 Support Available (256) 722-9985