1. Patchew
  2. Xen
  3. View series

[XEN PATCH] automation/eclair: add deviation for MISRA C:2012 Rule 16.3

Federico Serafini posted 1 patch 2 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/b1d2b64c8117d61ea42cf4e9feae128541eb0b61.1708348799.git.federico.serafini@bugseng.com
automation/eclair_analysis/ECLAIR/deviations.ecl | 4 ++++
docs/misra/deviations.rst                        | 6 ++++++
2 files changed, 10 insertions(+)
[XEN PATCH] automation/eclair: add deviation for MISRA C:2012 Rule 16.3
Posted by Federico Serafini 2 months, 2 weeks ago 
Update ECLAIR configuration to consider safe switch clauses ending
with __{get,put}_user_bad().

Update docs/misra/deviations.rst accordingly.

Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
---
 automation/eclair_analysis/ECLAIR/deviations.ecl | 4 ++++
 docs/misra/deviations.rst                        | 6 ++++++
 2 files changed, 10 insertions(+)

diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl
index fd32ff8a9c..831b5d4c67 100644
--- a/automation/eclair_analysis/ECLAIR/deviations.ecl
+++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
@@ -368,6 +368,10 @@ safe."
 -config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
 -doc_end
 
+-doc_begin="Switch clauses ending with constructs \"__get_user_bad()\" and \"__put_user_bad()\" are safe: they denote an unreachable program point."
+-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/__(put|get)_user_bad\\(\\);/))))"}
+-doc_end
+
 -doc_begin="Switch clauses not ending with the break statement are safe if an
 explicit comment indicating the fallthrough intention is present."
 -config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(^(?s).*/\\* [fF]all ?through.? \\*/.*$,0..1))))"}
diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst
index 123c78e20a..58f4fac18e 100644
--- a/docs/misra/deviations.rst
+++ b/docs/misra/deviations.rst
@@ -307,6 +307,12 @@ Deviations related to MISRA C:2012 Rules:
      - Switch clauses ending with failure method \"BUG()\" are safe.
      - Tagged as `safe` for ECLAIR.
 
+   * - R16.3
+     - Switch clauses ending with constructs
+       \"__get_user_bad()\" and \"__put_user_bad()\" are safe:
+       they denote an unreachable program point.
+     - Tagged as `safe` for ECLAIR.
+
    * - R16.3
      - Existing switch clauses not ending with the break statement are safe if
        an explicit comment indicating the fallthrough intention is present.
-- 
2.34.1
Re: [XEN PATCH] automation/eclair: add deviation for MISRA C:2012 Rule 16.3
Posted by Jan Beulich 2 months, 2 weeks ago 
On 19.02.2024 14:24, Federico Serafini wrote:
> Update ECLAIR configuration to consider safe switch clauses ending
> with __{get,put}_user_bad().
> 
> Update docs/misra/deviations.rst accordingly.
> 
> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>

As mentioned I'm not happy with this, not the least because of it being
unclear why these two would be deviated, when there's no sign of a
similar deviation for, say, __bad_atomic_size(). Imo this approach
doesn't scale, and that's already leaving aside that the purpose of
identically named (pseudo-)helpers could differ between architectures,
thus putting under question ...

> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
> @@ -368,6 +368,10 @@ safe."
>  -config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
>  -doc_end
>  
> +-doc_begin="Switch clauses ending with constructs \"__get_user_bad()\" and \"__put_user_bad()\" are safe: they denote an unreachable program point."
> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/__(put|get)_user_bad\\(\\);/))))"}
> +-doc_end

... the global scope of such a deviation. While it may not be a good idea,
even within an arch such (pseudo-)helpers could be used for multiple
distinct purposes.

Jan
Re: [XEN PATCH] automation/eclair: add deviation for MISRA C:2012 Rule 16.3
Posted by Federico Serafini 2 months, 2 weeks ago 
On 19/02/24 14:43, Jan Beulich wrote:
> On 19.02.2024 14:24, Federico Serafini wrote:
>> Update ECLAIR configuration to consider safe switch clauses ending
>> with __{get,put}_user_bad().
>>
>> Update docs/misra/deviations.rst accordingly.
>>
>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
> 
> As mentioned I'm not happy with this, not the least because of it being
> unclear why these two would be deviated, when there's no sign of a
> similar deviation for, say, __bad_atomic_size(). Imo this approach
> doesn't scale, and that's already leaving aside that the purpose of
> identically named (pseudo-)helpers could differ between architectures,
> thus putting under question ...
> 
>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>> @@ -368,6 +368,10 @@ safe."
>>   -config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
>>   -doc_end
>>   
>> +-doc_begin="Switch clauses ending with constructs \"__get_user_bad()\" and \"__put_user_bad()\" are safe: they denote an unreachable program point."
>> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/__(put|get)_user_bad\\(\\);/))))"}
>> +-doc_end
> 
> ... the global scope of such a deviation. While it may not be a good idea,
> even within an arch such (pseudo-)helpers could be used for multiple
> distinct purposes.

Would you agree with adding the missing break statement after
the uses of __{put,get}_user_bad() (as it is already happening for
uses of __bad_atomic_size())?

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)
Re: [XEN PATCH] automation/eclair: add deviation for MISRA C:2012 Rule 16.3
Posted by Jan Beulich 2 months, 2 weeks ago 
On 19.02.2024 15:59, Federico Serafini wrote:
> On 19/02/24 14:43, Jan Beulich wrote:
>> On 19.02.2024 14:24, Federico Serafini wrote:
>>> Update ECLAIR configuration to consider safe switch clauses ending
>>> with __{get,put}_user_bad().
>>>
>>> Update docs/misra/deviations.rst accordingly.
>>>
>>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>>
>> As mentioned I'm not happy with this, not the least because of it being
>> unclear why these two would be deviated, when there's no sign of a
>> similar deviation for, say, __bad_atomic_size(). Imo this approach
>> doesn't scale, and that's already leaving aside that the purpose of
>> identically named (pseudo-)helpers could differ between architectures,
>> thus putting under question ...
>>
>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>> @@ -368,6 +368,10 @@ safe."
>>>   -config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
>>>   -doc_end
>>>   
>>> +-doc_begin="Switch clauses ending with constructs \"__get_user_bad()\" and \"__put_user_bad()\" are safe: they denote an unreachable program point."
>>> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/__(put|get)_user_bad\\(\\);/))))"}
>>> +-doc_end
>>
>> ... the global scope of such a deviation. While it may not be a good idea,
>> even within an arch such (pseudo-)helpers could be used for multiple
>> distinct purposes.
> 
> Would you agree with adding the missing break statement after
> the uses of __{put,get}_user_bad() (as it is already happening for
> uses of __bad_atomic_size())?

I probably wouldn't mind that (despite being a little pointless).
Perhaps declaring them as noreturn would also help?

Jan
Re: [XEN PATCH] automation/eclair: add deviation for MISRA C:2012 Rule 16.3
Posted by Federico Serafini 2 months, 1 week ago 
On 19/02/24 16:06, Jan Beulich wrote:
> On 19.02.2024 15:59, Federico Serafini wrote:
>> On 19/02/24 14:43, Jan Beulich wrote:
>>> On 19.02.2024 14:24, Federico Serafini wrote:
>>>> Update ECLAIR configuration to consider safe switch clauses ending
>>>> with __{get,put}_user_bad().
>>>>
>>>> Update docs/misra/deviations.rst accordingly.
>>>>
>>>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>>>
>>> As mentioned I'm not happy with this, not the least because of it being
>>> unclear why these two would be deviated, when there's no sign of a
>>> similar deviation for, say, __bad_atomic_size(). Imo this approach
>>> doesn't scale, and that's already leaving aside that the purpose of
>>> identically named (pseudo-)helpers could differ between architectures,
>>> thus putting under question ...
>>>
>>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>> @@ -368,6 +368,10 @@ safe."
>>>>    -config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
>>>>    -doc_end
>>>>    
>>>> +-doc_begin="Switch clauses ending with constructs \"__get_user_bad()\" and \"__put_user_bad()\" are safe: they denote an unreachable program point."
>>>> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/__(put|get)_user_bad\\(\\);/))))"}
>>>> +-doc_end
>>>
>>> ... the global scope of such a deviation. While it may not be a good idea,
>>> even within an arch such (pseudo-)helpers could be used for multiple
>>> distinct purposes.
>>
>> Would you agree with adding the missing break statement after
>> the uses of __{put,get}_user_bad() (as it is already happening for
>> uses of __bad_atomic_size())?
> 
> I probably wouldn't mind that (despite being a little pointless).
> Perhaps declaring them as noreturn would also help?

Yes, it will help.
Is there any reason to have long as __get_user_bad()'s return value?
It would be nicer to declare it as a void function and then add the
noreturn attribute.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)
Re: [XEN PATCH] automation/eclair: add deviation for MISRA C:2012 Rule 16.3
Posted by Jan Beulich 2 months, 1 week ago 
On 20.02.2024 09:16, Federico Serafini wrote:
> On 19/02/24 16:06, Jan Beulich wrote:
>> On 19.02.2024 15:59, Federico Serafini wrote:
>>> On 19/02/24 14:43, Jan Beulich wrote:
>>>> On 19.02.2024 14:24, Federico Serafini wrote:
>>>>> Update ECLAIR configuration to consider safe switch clauses ending
>>>>> with __{get,put}_user_bad().
>>>>>
>>>>> Update docs/misra/deviations.rst accordingly.
>>>>>
>>>>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>>>>
>>>> As mentioned I'm not happy with this, not the least because of it being
>>>> unclear why these two would be deviated, when there's no sign of a
>>>> similar deviation for, say, __bad_atomic_size(). Imo this approach
>>>> doesn't scale, and that's already leaving aside that the purpose of
>>>> identically named (pseudo-)helpers could differ between architectures,
>>>> thus putting under question ...
>>>>
>>>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>> @@ -368,6 +368,10 @@ safe."
>>>>>    -config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
>>>>>    -doc_end
>>>>>    
>>>>> +-doc_begin="Switch clauses ending with constructs \"__get_user_bad()\" and \"__put_user_bad()\" are safe: they denote an unreachable program point."
>>>>> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/__(put|get)_user_bad\\(\\);/))))"}
>>>>> +-doc_end
>>>>
>>>> ... the global scope of such a deviation. While it may not be a good idea,
>>>> even within an arch such (pseudo-)helpers could be used for multiple
>>>> distinct purposes.
>>>
>>> Would you agree with adding the missing break statement after
>>> the uses of __{put,get}_user_bad() (as it is already happening for
>>> uses of __bad_atomic_size())?
>>
>> I probably wouldn't mind that (despite being a little pointless).
>> Perhaps declaring them as noreturn would also help?
> 
> Yes, it will help.
> Is there any reason to have long as __get_user_bad()'s return value?
> It would be nicer to declare it as a void function and then add the
> noreturn attribute.

That's a historical leftover, which can be changed. Xen originally
derived quite a bit of code from Linux. If you go look at Linux 2.6.16,
you'll find why it was declared that way.

Jan
Re: [XEN PATCH] automation/eclair: add deviation for MISRA C:2012 Rule 16.3
Posted by Federico Serafini 2 months, 1 week ago 
On 20/02/24 09:31, Jan Beulich wrote:
> On 20.02.2024 09:16, Federico Serafini wrote:
>> On 19/02/24 16:06, Jan Beulich wrote:
>>> On 19.02.2024 15:59, Federico Serafini wrote:
>>>> On 19/02/24 14:43, Jan Beulich wrote:
>>>>> On 19.02.2024 14:24, Federico Serafini wrote:
>>>>>> Update ECLAIR configuration to consider safe switch clauses ending
>>>>>> with __{get,put}_user_bad().
>>>>>>
>>>>>> Update docs/misra/deviations.rst accordingly.
>>>>>>
>>>>>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>>>>>
>>>>> As mentioned I'm not happy with this, not the least because of it being
>>>>> unclear why these two would be deviated, when there's no sign of a
>>>>> similar deviation for, say, __bad_atomic_size(). Imo this approach
>>>>> doesn't scale, and that's already leaving aside that the purpose of
>>>>> identically named (pseudo-)helpers could differ between architectures,
>>>>> thus putting under question ...
>>>>>
>>>>>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl
>>>>>> @@ -368,6 +368,10 @@ safe."
>>>>>>     -config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/BUG\\(\\);/))))"}
>>>>>>     -doc_end
>>>>>>     
>>>>>> +-doc_begin="Switch clauses ending with constructs \"__get_user_bad()\" and \"__put_user_bad()\" are safe: they denote an unreachable program point."
>>>>>> +-config=MC3R1.R16.3,reports+={safe, "any_area(end_loc(any_exp(text(/__(put|get)_user_bad\\(\\);/))))"}
>>>>>> +-doc_end
>>>>>
>>>>> ... the global scope of such a deviation. While it may not be a good idea,
>>>>> even within an arch such (pseudo-)helpers could be used for multiple
>>>>> distinct purposes.
>>>>
>>>> Would you agree with adding the missing break statement after
>>>> the uses of __{put,get}_user_bad() (as it is already happening for
>>>> uses of __bad_atomic_size())?
>>>
>>> I probably wouldn't mind that (despite being a little pointless).
>>> Perhaps declaring them as noreturn would also help?
>>
>> Yes, it will help.
>> Is there any reason to have long as __get_user_bad()'s return value?
>> It would be nicer to declare it as a void function and then add the
>> noreturn attribute.
> 
> That's a historical leftover, which can be changed. Xen originally
> derived quite a bit of code from Linux. If you go look at Linux 2.6.16,
> you'll find why it was declared that way.

Thank you, I'll send a v2.

-- 
Federico Serafini, M.Sc.

Software Engineer, BUGSENG (http://bugseng.com)

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.