[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Proposal for deviations in static analyser findings


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Bertrand Marquis <Bertrand.Marquis@xxxxxxx>
  • Date: Wed, 19 Oct 2022 07:52:59 +0000
  • Accept-language: en-GB, en-US
  • Arc-authentication-results: i=2; mx.microsoft.com 1; spf=pass (sender ip is 63.35.35.123) smtp.rcpttodomain=lists.xenproject.org smtp.mailfrom=arm.com; dmarc=pass (p=none sp=none pct=100) action=none header.from=arm.com; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com; arc=pass (0 oda=1 ltdi=1 spf=[1,1,smtp.mailfrom=arm.com] dkim=[1,1,header.d=arm.com] dmarc=[1,1,header.from=arm.com])
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
  • Arc-message-signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=7Zg7ZG6Wf7lkUiNk7DKWDDPlFday/yqNF7zo/Bwp0p8=; b=aZJt0qG1JMQVNXSKGXALOCLJlVys6P9ihEaKoD+i6rTAlJdHxcIgYNEZ17UA6m7RMlnPBLuAYs9h9pFRoBIeOMDd2kMWkHsUVg7V+zuH7GJuTmfLDEdXgNk+JVYKNZyylQE1JCywwM4fkAj+eaf66+e4EVWTFwcBde4UxalSB6W+VePPNHaZ7ZhKGXt8LTuThu/551BAJaZrWB5YsAgfUEzlkCzCZdaCth8Vzbs/CQ1hk/S6myNoeUg+oeIayoGAK5ZtzVMwKxxFW2SaUAARnI7kEfXqZbURTrlhfxBa6Vd0ncQ/chhC8reAXJlrKhtZcW0f4o1c6ubJoeBbkxcq9w==
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=7Zg7ZG6Wf7lkUiNk7DKWDDPlFday/yqNF7zo/Bwp0p8=; b=DmTrloEK1etz6Y5elOchM6Kg/i2RUrC3l9qk7u8AC4CnIZw2at97yFi2ps4RuY3zH0SNRmZyVhZm4nbj7AmcUDUNpZyYpgkO+QwMiZEJwAFJ0Eo4TYj5D/mzNnaqYFZOBk2FAiLmAv0Iu19IskHhgm/NFFRcoN+0y1rqnn8Qxa9LRDr+WIikypVzRVhN1GU7jzrlmwMbeT69wXhO2zCZqT6oNFbE+TywXCkczV/lMeFNAg4fUJQyOYkuyyVR7FtOF2T0oYN4AM3QRQJmkIZrebVCUlPGSJYH8ruaZ9XFKDCT0FW5LNU8f5pzDQko6FX4DQJsP7hDvGeqlWCrA4Tjsw==
  • Arc-seal: i=2; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=pass; b=Ui2/BiiROAJ1XfpEj7orFj2Q9ygfUfLihGpb1izAfc1X9NArK65GJq7KIKXgd26AVn3qMdSlSrKwEA1B8m0gosnnkAbGLam9+2cdwNWEJo5TNgQI+iD0MI+G88cy3uW9VZf7pqtIo+a92Gh8Ilq76oAHNSVonUsi1BMIiHi0UnUlzANkbWngNkJMoZuXONK+uoCJHQAUdNSAWP9eprQjg/hT2F/lMrsCQlIvATOEXGlHGbHxtJeDIGpU2HPbPZ6WF4wXApcck7d9L5mPiBA/4ajCDNxqa912PJgX2tW2ZogOalu33MNC/PKfA0eVSuzm/Xa8Z9y7/U+2oUTkB1PEMQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=LC5ZhMjjz+ZMsCPUqV32siZH3deBmO0dIDp4NiZYcNccLdGempFVTGiJ39gT2oNTZKLWSgY/w6evWBjI33LP8sCnWvGwdKxOBLVdH7Fx88LVtw8KwqfTK+n/xy8pwwXiTbhBzdb4C3f6Y7x9YxXi9o134WrfchwEh7Bk1OEgW0upxueSyApmDZIZFKoXaLaPxrTDP9JD3XhLcMNIJZQ/3IrCcGlDXVSPPXPr1RAlIdlkZkJqDlDGuCNH7Rrjjdx/a/2el4i5UEzU1+nOfugNctMeJNMKBCMo2Rp1xbBHKJFgOk/7Rpd3UsD3oXHyneEYqicFkvK+0JNy74Mzzq2xRg==
  • Authentication-results-original: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Cc: Luca Fancellu <Luca.Fancellu@xxxxxxx>, Wei Chen <Wei.Chen@xxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Wed, 19 Oct 2022 07:53:28 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
  • Nodisclaimer: true
  • Original-authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=arm.com;
  • Thread-index: AQHY3lPHM2Tyba3hCkeKMW07GDOb2a4L9DeAgAAnewCAABdGAIAIGdUAgAADdICAAAu3gIAA8h6AgAAU34A=
  • Thread-topic: Proposal for deviations in static analyser findings

Hi Jan,

> On 19 Oct 2022, at 07:38, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> 
> On 18.10.2022 18:11, Bertrand Marquis wrote:
>>> On 18 Oct 2022, at 16:29, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>> On 18.10.2022 17:17, Luca Fancellu wrote:
>>>>> On 13 Oct 2022, at 12:34, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>> On 13.10.2022 12:11, Luca Fancellu wrote:
>>>>>>> On 13 Oct 2022, at 08:50, Jan Beulich <jbeulich@xxxxxxxx> wrote:
>>>>>>> On 12.10.2022 18:00, Luca Fancellu wrote:
>>>>>>>> Entries in the database should never be removed, even if they are not 
>>>>>>>> used
>>>>>>>> anymore in the code (if a patch is removing or modifying the faulty 
>>>>>>>> line).
>>>>>>>> This is to make sure that numbers are not reused which could lead to 
>>>>>>>> conflicts
>>>>>>>> with old branches or misleading justifications.
>>>>>>> 
>>>>>>> Can we add provisions for shrinking such entries to e.g. just their "id"
>>>>>>> line? Or is the intention to be able to re-use such an entry if a 
>>>>>>> matching
>>>>>>> instance appears again later?
>>>>>> 
>>>>>> I prefer to don’t shrink it, the name itself is not very long, even 
>>>>>> using many digits of the incremental
>>>>>> number, it removes also the dependency on the file name.
>>>>> 
>>>>> Name length isn't relevant here, and I have no idea what dependency on a
>>>>> file name you're thinking of. My question is a scalability one: Over time
>>>>> the table will grow large. If all entries remain there in full forever,
>>>>> table size may become unwieldy.
>>>> 
>>>> Ok I misunderstood your question, now I understand what you are asking, we 
>>>> could remove the content
>>>> of the “analyser” dictionary for sure, because if there is not anymore a 
>>>> link with the current code.
>>>> 
>>>> Regarding removing the “name” and “text”, could it be that at some point 
>>>> we can introduce in the code
>>>> a violation that requires the same justification provided by the “orphan” 
>>>> entry?
>>>> In that case we could reuse that entry without creating a new one that 
>>>> will only waste space.
>>>> What is the opinion on this?
>>> 
>>> Well, yes, this is the one case that I, too, was wondering about. It's not
>>> clear to me whether it wouldn't be better to allocate a fresh ID in such a
>>> case.
>> 
>> For traceability and release handling I think it is important that:
>> - we never reuse the same ID in any case
>> - we keep IDs in the database even if there is no occurrence in xen code as 
>> this will prevent adding a new ID if an existing one can be reused
>> 
>> In a certification context, when a justification has been validated and 
>> agreed it will make life a lot easier to not modify it and reuse it in the 
>> future if it is needed.
>> From our point of view, having a clear and simple way of handling those will 
>> make backports a lot easier.
> 
> Isn't validation of a justification connected to the affected code? If so,
> every new instance will need validation, while an orphan entry is entirely
> meaningless.

Hopefully most our justifications should be for some “common” deviations
we allow and as a consequence be reused several times.
An orphan might be meaningless but could still have a meaning in a stable
branch, taking those out might make the work of people doing certification
more complex.
Anyway I think this is a corner case that we could reconsider depending on
how often it is happening but for a start it would make life easier to not 
remove
so that numbers are not reused.

> 
>>>>>> After the analysis, the source code will return as the original (with 
>>>>>> the SAF-* tag).
>>>>> 
>>>>> While you mention something similar also as step 3 in the original 
>>>>> document
>>>>> near the top, I'm afraid I don't understand what this "return as the 
>>>>> original"
>>>>> means. If you want to run the tool on an altered (comments modified) 
>>>>> source
>>>>> tree, what I'd expect you to do is clone the sources into a throw-away 
>>>>> tree,
>>>>> massage the comments, run the tool, and delete the massaged tree.
>>>>>>>> if the object doesn't have a key-value, then the corresponding in-code
>>>>>>>> comment won't be translated.
>>>>>>> 
>>>>>>> Iirc at least Coverity ignores certain instances of what it might 
>>>>>>> consider
>>>>>>> violations (fall-through in switch() statements in particular) in case
>>>>>>> _any_ comment is present. Therefore may I suggest that such comments be
>>>>>>> deleted (really: replaced by a blank line, to maintain correct line
>>>>>>> numbering) if there's no matching key-value pair?
>>>>>> 
>>>>>> Yes the line won’t be altered if there is no match. This to ensure the 
>>>>>> correct line
>>>>>> numbering is not affected.
>>>>> 
>>>>> "won't be altered" is the opposite of what I've been asking to consider:
>>>>> Observing that comments _regardless_ of their contents may silence 
>>>>> findings,
>>>>> the suggestion is to remove comments (leaving a blank line) when there's 
>>>>> no
>>>>> entry for the targeted tool in the table entry.
>>>> 
>>>> Why? The tag comment won’t do anything, it would act as a blank line from 
>>>> the analyser
>>>> perspective.
>>> 
>>> The _tag_ won't do anything, but as said any _comment_ may have an effect.
>> 
>> I am not sure I follow this one but in any case we can choose to anyway 
>> substitute the tag with something like /* Not applicable */.
> 
> That's still a comment, which hence may still silence a tool:
> 
>    switch ( x )
>    {
>    case a:
>        ...
>        /* SAF-<N>-safe */
>    case b:
>        ...
>        break;
>    }
> 
> is no different from
> 
>    switch ( x )
>    {
>    case a:
>        ...
>        /* fall-through */
>    case b:
>        ...
>        break;
>    }
> 
> nor
> 
>    switch ( x )
>    {
>    case a:
>        ...
>        /* Not applicable */
>    case b:
>        ...
>        break;
>    }
> 
> Only
> 
>    switch ( x )
>    {
>    case a:
>        ...
> 
>    case b:
>        ...
>        break;
>    }
> 
> will make e.g. Coverity actually point out the potentially unintended
> fall through (based on past observations). Whether that behavior is
> fall-through-specific I don't know. If it indeed is, then maybe my
> concern is void - in the long run I think we want to use the pseudo-
> keyword there in all cases anyway.

We can choose the replacement comment to be something not
considered by the tools (or even put an empty /* */).
What we cannot do is remove the line as it would change line numbers.
But apart from fallthrough I do not think any comment is considered by
any tools so this should not be an issue.

Bertrand

> 
> Jan


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.