address violations of MISRA C rules

[XEN PATCH 4/5] automation/eclair_analysis: address remaining violations of MISRA C Rule 20.12

Posted by Nicola Vetrini 5 months, 3 weeks ago

The DEFINE macro in asm-offsets.c (for all architectures) still generates
violations despite the file(s) being excluded from compliance, due to the
fact that in its expansion it sometimes refers entities in non-excluded files.
These corner cases are deviated by the configuration.

No functional change.

Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index cf62a874d928..f29db9e08248 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -483,6 +483,12 @@ leads to a violation of the Rule are deviated."
 -config=MC3R1.R20.12,macros+={deliberate, "name(GENERATE_CASE)&&loc(file(deliberate_generate_case))"}
 -doc_end
 
+-doc_begin="The macro DEFINE is defined and used in excluded files asm-offsets.c.
+This may still cause violations if entities outside these files are referred to
+in the expansion."
+-config=MC3R1.R20.12,macros+={deliberate, "name(DEFINE)&&loc(file(asm_offsets))"}
+-doc_end
+
 #
 # Series 21.
 #
-- 
2.34.1

Re: [XEN PATCH 4/5] automation/eclair_analysis: address remaining violations of MISRA C Rule 20.12

Posted by Stefano Stabellini 5 months, 1 week ago

On Sat, 1 Jun 2024, Nicola Vetrini wrote:
> The DEFINE macro in asm-offsets.c (for all architectures) still generates
> violations despite the file(s) being excluded from compliance, due to the
> fact that in its expansion it sometimes refers entities in non-excluded files.
> These corner cases are deviated by the configuration.
> 
> No functional change.
> 
> Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>

Re: [XEN PATCH 4/5] automation/eclair_analysis: address remaining violations of MISRA C Rule 20.12

Posted by Jan Beulich 5 months, 3 weeks ago

On 01.06.2024 12:16, Nicola Vetrini wrote:
> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -483,6 +483,12 @@ leads to a violation of the Rule are deviated."
>  -config=MC3R1.R20.12,macros+={deliberate, "name(GENERATE_CASE)&&loc(file(deliberate_generate_case))"}
>  -doc_end
>  
> +-doc_begin="The macro DEFINE is defined and used in excluded files asm-offsets.c.
> +This may still cause violations if entities outside these files are referred to
> +in the expansion."
> +-config=MC3R1.R20.12,macros+={deliberate, "name(DEFINE)&&loc(file(asm_offsets))"}
> +-doc_end

Can you give an example of such a reference? Nothing _in_ asm-offsets.c
should be referenced, I'd think. Only stuff in asm-offsets.h as _generated
from_ asm-offsets.c will, of course, be.

Jan

Re: [XEN PATCH 4/5] automation/eclair_analysis: address remaining violations of MISRA C Rule 20.12

Posted by Nicola Vetrini 5 months, 3 weeks ago

On 2024-06-03 07:58, Jan Beulich wrote:
> On 01.06.2024 12:16, Nicola Vetrini wrote:
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -483,6 +483,12 @@ leads to a violation of the Rule are deviated."
>>  -config=MC3R1.R20.12,macros+={deliberate, 
>> "name(GENERATE_CASE)&&loc(file(deliberate_generate_case))"}
>>  -doc_end
>> 
>> +-doc_begin="The macro DEFINE is defined and used in excluded files 
>> asm-offsets.c.
>> +This may still cause violations if entities outside these files are 
>> referred to
>> +in the expansion."
>> +-config=MC3R1.R20.12,macros+={deliberate, 
>> "name(DEFINE)&&loc(file(asm_offsets))"}
>> +-doc_end
> 
> Can you give an example of such a reference? Nothing _in_ asm-offsets.c
> should be referenced, I'd think. Only stuff in asm-offsets.h as 
> _generated
> from_ asm-offsets.c will, of course, be.
> 

Perhaps I could have expressed that more clearly. What I meant is that 
there are some arguments to DEFINE that are not part of asm-offsets.c, 
therefore they end up in the violation report, but are not actually 
relevant, because the macro DEFINE is actually what we want to exclude.

See for instance at the link below VCPU_TRAP_{NMI,MCE}, which are 
defined in asm/domain.h and used as arguments to DEFINE inside 
asm-offsets.c.

https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/staging/X86_64-BUGSENG/676/PROJECT.ecd;/by_service/MC3R1.R20.12.html

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [XEN PATCH 4/5] automation/eclair_analysis: address remaining violations of MISRA C Rule 20.12

Posted by Jan Beulich 5 months, 3 weeks ago

On 03.06.2024 09:13, Nicola Vetrini wrote:
> On 2024-06-03 07:58, Jan Beulich wrote:
>> On 01.06.2024 12:16, Nicola Vetrini wrote:
>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> @@ -483,6 +483,12 @@ leads to a violation of the Rule are deviated."
>>>  -config=MC3R1.R20.12,macros+={deliberate, 
>>> "name(GENERATE_CASE)&&loc(file(deliberate_generate_case))"}
>>>  -doc_end
>>>
>>> +-doc_begin="The macro DEFINE is defined and used in excluded files 
>>> asm-offsets.c.
>>> +This may still cause violations if entities outside these files are 
>>> referred to
>>> +in the expansion."
>>> +-config=MC3R1.R20.12,macros+={deliberate, 
>>> "name(DEFINE)&&loc(file(asm_offsets))"}
>>> +-doc_end
>>
>> Can you give an example of such a reference? Nothing _in_ asm-offsets.c
>> should be referenced, I'd think. Only stuff in asm-offsets.h as 
>> _generated
>> from_ asm-offsets.c will, of course, be.
>>
> 
> Perhaps I could have expressed that more clearly. What I meant is that 
> there are some arguments to DEFINE that are not part of asm-offsets.c, 
> therefore they end up in the violation report, but are not actually 
> relevant, because the macro DEFINE is actually what we want to exclude.
> 
> See for instance at the link below VCPU_TRAP_{NMI,MCE}, which are 
> defined in asm/domain.h and used as arguments to DEFINE inside 
> asm-offsets.c.
> 
> https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/staging/X86_64-BUGSENG/676/PROJECT.ecd;/by_service/MC3R1.R20.12.html

I'm afraid I still don't understand: The file being supposed to be
excluded from scanning, why does it even show up in that report?

Jan

Re: [XEN PATCH 4/5] automation/eclair_analysis: address remaining violations of MISRA C Rule 20.12

Posted by Nicola Vetrini 5 months, 3 weeks ago

On 2024-06-03 20:52, Jan Beulich wrote:
> On 03.06.2024 09:13, Nicola Vetrini wrote:
>> On 2024-06-03 07:58, Jan Beulich wrote:
>>> On 01.06.2024 12:16, Nicola Vetrini wrote:
>>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>> @@ -483,6 +483,12 @@ leads to a violation of the Rule are deviated."
>>>>  -config=MC3R1.R20.12,macros+={deliberate,
>>>> "name(GENERATE_CASE)&&loc(file(deliberate_generate_case))"}
>>>>  -doc_end
>>>> 
>>>> +-doc_begin="The macro DEFINE is defined and used in excluded files
>>>> asm-offsets.c.
>>>> +This may still cause violations if entities outside these files are
>>>> referred to
>>>> +in the expansion."
>>>> +-config=MC3R1.R20.12,macros+={deliberate,
>>>> "name(DEFINE)&&loc(file(asm_offsets))"}
>>>> +-doc_end
>>> 
>>> Can you give an example of such a reference? Nothing _in_ 
>>> asm-offsets.c
>>> should be referenced, I'd think. Only stuff in asm-offsets.h as
>>> _generated
>>> from_ asm-offsets.c will, of course, be.
>>> 
>> 
>> Perhaps I could have expressed that more clearly. What I meant is that
>> there are some arguments to DEFINE that are not part of asm-offsets.c,
>> therefore they end up in the violation report, but are not actually
>> relevant, because the macro DEFINE is actually what we want to 
>> exclude.
>> 
>> See for instance at the link below VCPU_TRAP_{NMI,MCE}, which are
>> defined in asm/domain.h and used as arguments to DEFINE inside
>> asm-offsets.c.
>> 
>> https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/staging/X86_64-BUGSENG/676/PROJECT.ecd;/by_service/MC3R1.R20.12.html
> 
> I'm afraid I still don't understand: The file being supposed to be
> excluded from scanning, why does it even show up in that report?
> 
> Jan

The report is made up of several source code locations. Three of them 
are within asm-offsets.c, which is excluded from compliance but still 
analyzed, but one references a macro definition in another file (e.g., 
VCPU_TRAP_NMI from asm/domain.h). So in this case the exclusion of 
asm-offsets.c is not enough for the report not to be shown.

The configuration is telling the tool that the macro DEFINE from 
asm-offsets is meant to be deviated, which is what is needed in this 
case.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)

Re: [XEN PATCH 4/5] automation/eclair_analysis: address remaining violations of MISRA C Rule 20.12

Posted by Jan Beulich 5 months, 3 weeks ago

On 03.06.2024 21:12, Nicola Vetrini wrote:
> On 2024-06-03 20:52, Jan Beulich wrote:
>> On 03.06.2024 09:13, Nicola Vetrini wrote:
>>> On 2024-06-03 07:58, Jan Beulich wrote:
>>>> On 01.06.2024 12:16, Nicola Vetrini wrote:
>>>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>> @@ -483,6 +483,12 @@ leads to a violation of the Rule are deviated."
>>>>>  -config=MC3R1.R20.12,macros+={deliberate,
>>>>> "name(GENERATE_CASE)&&loc(file(deliberate_generate_case))"}
>>>>>  -doc_end
>>>>>
>>>>> +-doc_begin="The macro DEFINE is defined and used in excluded files
>>>>> asm-offsets.c.
>>>>> +This may still cause violations if entities outside these files are
>>>>> referred to
>>>>> +in the expansion."
>>>>> +-config=MC3R1.R20.12,macros+={deliberate,
>>>>> "name(DEFINE)&&loc(file(asm_offsets))"}
>>>>> +-doc_end
>>>>
>>>> Can you give an example of such a reference? Nothing _in_ 
>>>> asm-offsets.c
>>>> should be referenced, I'd think. Only stuff in asm-offsets.h as
>>>> _generated
>>>> from_ asm-offsets.c will, of course, be.
>>>>
>>>
>>> Perhaps I could have expressed that more clearly. What I meant is that
>>> there are some arguments to DEFINE that are not part of asm-offsets.c,
>>> therefore they end up in the violation report, but are not actually
>>> relevant, because the macro DEFINE is actually what we want to 
>>> exclude.
>>>
>>> See for instance at the link below VCPU_TRAP_{NMI,MCE}, which are
>>> defined in asm/domain.h and used as arguments to DEFINE inside
>>> asm-offsets.c.
>>>
>>> https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/staging/X86_64-BUGSENG/676/PROJECT.ecd;/by_service/MC3R1.R20.12.html
>>
>> I'm afraid I still don't understand: The file being supposed to be
>> excluded from scanning, why does it even show up in that report?
> 
> The report is made up of several source code locations. Three of them 
> are within asm-offsets.c, which is excluded from compliance but still 
> analyzed, but one references a macro definition in another file (e.g., 
> VCPU_TRAP_NMI from asm/domain.h). So in this case the exclusion of 
> asm-offsets.c is not enough for the report not to be shown.

But the (would-be-)violation is in asm-offsets.c. The other locations
pointed at are providing context. To report a violation, it should be
enough to exclude the file where the violation itself is?

Jan

Re: [XEN PATCH 4/5] automation/eclair_analysis: address remaining violations of MISRA C Rule 20.12

Posted by Nicola Vetrini 5 months, 3 weeks ago

On 2024-06-03 23:24, Jan Beulich wrote:
> On 03.06.2024 21:12, Nicola Vetrini wrote:
>> On 2024-06-03 20:52, Jan Beulich wrote:
>>> On 03.06.2024 09:13, Nicola Vetrini wrote:
>>>> On 2024-06-03 07:58, Jan Beulich wrote:
>>>>> On 01.06.2024 12:16, Nicola Vetrini wrote:
>>>>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>>> @@ -483,6 +483,12 @@ leads to a violation of the Rule are 
>>>>>> deviated."
>>>>>>  -config=MC3R1.R20.12,macros+={deliberate,
>>>>>> "name(GENERATE_CASE)&&loc(file(deliberate_generate_case))"}
>>>>>>  -doc_end
>>>>>> 
>>>>>> +-doc_begin="The macro DEFINE is defined and used in excluded 
>>>>>> files
>>>>>> asm-offsets.c.
>>>>>> +This may still cause violations if entities outside these files 
>>>>>> are
>>>>>> referred to
>>>>>> +in the expansion."
>>>>>> +-config=MC3R1.R20.12,macros+={deliberate,
>>>>>> "name(DEFINE)&&loc(file(asm_offsets))"}
>>>>>> +-doc_end
>>>>> 
>>>>> Can you give an example of such a reference? Nothing _in_
>>>>> asm-offsets.c
>>>>> should be referenced, I'd think. Only stuff in asm-offsets.h as
>>>>> _generated
>>>>> from_ asm-offsets.c will, of course, be.
>>>>> 
>>>> 
>>>> Perhaps I could have expressed that more clearly. What I meant is 
>>>> that
>>>> there are some arguments to DEFINE that are not part of 
>>>> asm-offsets.c,
>>>> therefore they end up in the violation report, but are not actually
>>>> relevant, because the macro DEFINE is actually what we want to
>>>> exclude.
>>>> 
>>>> See for instance at the link below VCPU_TRAP_{NMI,MCE}, which are
>>>> defined in asm/domain.h and used as arguments to DEFINE inside
>>>> asm-offsets.c.
>>>> 
>>>> https://saas.eclairit.com:3787/fs/var/local/eclair/XEN.ecdf/ECLAIR_normal/staging/X86_64-BUGSENG/676/PROJECT.ecd;/by_service/MC3R1.R20.12.html
>>> 
>>> I'm afraid I still don't understand: The file being supposed to be
>>> excluded from scanning, why does it even show up in that report?
>> 
>> The report is made up of several source code locations. Three of them
>> are within asm-offsets.c, which is excluded from compliance but still
>> analyzed, but one references a macro definition in another file (e.g.,
>> VCPU_TRAP_NMI from asm/domain.h). So in this case the exclusion of
>> asm-offsets.c is not enough for the report not to be shown.
> 
> But the (would-be-)violation is in asm-offsets.c. The other locations
> pointed at are providing context. To report a violation, it should be
> enough to exclude the file where the violation itself is?
> 

In general that may not be the safest choice, or it can limit the 
granularity of the configuration (not in the specific case, but the 
mechanism is general).
It's a design aspect of the tool to show a report unless all its 
locations are not meant to be shown.

-- 
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)