Re: [XEN PATCH 05/13] automation/eclair: add deviation for usercopy.c

[Jan Beulich <jbeulich@xxxxxxxx>]

On 29.08.2023 00:27, Stefano Stabellini wrote:
> On Mon, 28 Aug 2023, Simone Ballarin wrote:
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -96,6 +96,10 @@ conform to the directive."
>>  -config=MC3R1.D4.10,reports+={safe, "first_area(text(^ \\* In this case, no 
>> inclusion guards apply and the caller is responsible.*\\*/$, begin-1))"}
>>  -doc_end
>>  
>> +-doc_begin="xen/arch/x86/usercopy.c includes itself: it is not supposed to 
>> comply with the directive"
>> +-config=MC3R1.D4.10,reports+={deliberate, 
>> "all_area(all_loc(file("^xen/arch/x86/usercopy\\.c$")))"}
>> +-doc_end
>> +
>>  #
>>  # Series 5.
>>  #
>> diff --git a/docs/misra/rules.rst b/docs/misra/rules.rst
>> index 4b1a7b02b6..45e13d0302 100644
>> --- a/docs/misra/rules.rst
>> +++ b/docs/misra/rules.rst
>> @@ -62,6 +62,8 @@ maintainers if you want to suggest a change.
>>       - Files that are intended to be included more than once do not need to
>>         conform to the directive. Files that explicitly avoid inclusion 
>> guards
>>         under specific circumstances do not need to conform the directive.
>> +       xen/arch/x86/usercopy.c includes itself: it is not supposed to comply
>> +       with the directive.
> 
> 
> We need to find a consistent way to document this kind of deviations in
> a non-ECLAIR specific way, without adding the complete list of
> deviations to rules.rst.

+1

Especially rules.rst should not be modified to add mention of individual
exceptions. That's simply not the purpose of the file, at least the way
I understand it.

> Can we use safe.json and add an in-code comment at the top of
> usercopy.c? E.g.:

Right, this ought to be the was to go. Question is whether ...

> diff --git a/xen/arch/x86/usercopy.c b/xen/arch/x86/usercopy.c
> index b8c2d1cc0b..8bb591f472 100644
> --- a/xen/arch/x86/usercopy.c
> +++ b/xen/arch/x86/usercopy.c
> @@ -1,3 +1,4 @@
> +/* SAF-1-safe */

... this (or any other) placement of the comment will actually do (not
just for Eclair).

Jan

>  /* 
>   * User address space access functions.
>   *
> 
> Otherwise, maybe we should extend safe.json to also have an extra field
> with a list of paths. For instance see "files" below:
> 
> {
>     "version": "1.0",
>     "content": [
>         {
>             "id": "SAF-0-safe",
>             "analyser": {
>                 "eclair": "MC3R1.R8.6",
>                 "coverity": "misra_c_2012_rule_8_6_violation"
>             },
>             "name": "Rule 8.6: linker script defined symbols",
>             "text": "It is safe to declare this symbol because it is defined 
> in the linker script."
>         },
>         {
>             "id": "SAF-1-safe",
>             "analyser": {
>                 "eclair": "MC3R1.D4.10"
>             },
>             "name": "Dir 4.10: files that include themselves",
>             "text": "Files purposely written to include themselves are not 
> supposed to comply with D4.10.",
>             "files": ["xen/arch/x86/usercopy.c"]
>         },
>         {
>             "id": "SAF-2-safe",
>             "analyser": {},
>             "name": "Sentinel",
>             "text": "Next ID to be used"
>         }
>     ]
> }