| Message ID | cca71f954bec4efc14b98e3ad991581441d593d7.1712238975.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [XEN] automaton/eclair: add deviations for MISRA C:2012 Rule 13.6 | expand |
On 04.04.2024 16:21, Federico Serafini wrote: > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl > @@ -333,6 +333,14 @@ of the short-circuit evaluation strategy of such logical operators." > -config=MC3R1.R13.5,reports+={disapplied,"any()"} > -doc_end > > +-doc_begin="Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the func parameters without evaluating them." > +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))))"} > +-doc_end alternative_vcall(), for now at least, is x86-only. Why blindly deviate it also for Arm? > +-doc_begin="Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very specific cases where by code inspection you can see that its usage is correct. The BUILD_BUG_ON checks check that EFI_TIME and struct xenpf_efi_time fields match." > +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^chk_fld$))))"} > +-doc_end As indicated on earlier occasions - when a macro isn't global, its deviation better wouldn't be global either. An identically named macro may be introduced elsewhere, and may not need deviating there. > --- a/docs/misra/deviations.rst > +++ b/docs/misra/deviations.rst > @@ -279,6 +279,18 @@ Deviations related to MISRA C:2012 Rules: > the short-circuit evaluation strategy for logical operators. > - Project-wide deviation; tagged as `disapplied` for ECLAIR. > > + * - R13.6 > + - Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the > + func parameters without evaluating them. Just to mention it: The return type of the function isn't really of interest there. What's being checked is that the argument types match the corresponding parameter ones, which otherwise would "naturally" be done by the compiler when seeing function calls. Might be useful to state no more and no less than what is really needed. Further, according to the discussion on the matter: alternative_call() is quite similar in this regard, just that it uses typeof(). Imo it should be deviated here as well right away, no matter whether Eclair can actually be configured to also check typeof() and alignof(). Jan
On Thu, 4 Apr 2024, Jan Beulich wrote: > Further, according to the discussion on the matter: alternative_call() is > quite similar in this regard, just that it uses typeof(). Imo it should be > deviated here as well right away, no matter whether Eclair can actually be > configured to also check typeof() and alignof(). On this specific point regarding typeof, I agree but it should be done as a separate rule (not 13.6), to make sure it matches the scanner results. It looks like the new rule will be called B.MISLEFF.
On 05.04.2024 02:04, Stefano Stabellini wrote: > On Thu, 4 Apr 2024, Jan Beulich wrote: >> Further, according to the discussion on the matter: alternative_call() is >> quite similar in this regard, just that it uses typeof(). Imo it should be >> deviated here as well right away, no matter whether Eclair can actually be >> configured to also check typeof() and alignof(). > > On this specific point regarding typeof, I agree but it should be done > as a separate rule (not 13.6), to make sure it matches the scanner > results. It looks like the new rule will be called B.MISLEFF. Fine with me, as long as things stay together (i.e. in a single commit). Jan
On 04/04/24 16:34, Jan Beulich wrote: > On 04.04.2024 16:21, Federico Serafini wrote: >> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl >> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl >> @@ -333,6 +333,14 @@ of the short-circuit evaluation strategy of such logical operators." >> -config=MC3R1.R13.5,reports+={disapplied,"any()"} >> -doc_end >> >> +-doc_begin="Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the func parameters without evaluating them." >> +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))))"} >> +-doc_end > > alternative_vcall(), for now at least, is x86-only. Why blindly deviate it also > for Arm? I'll reduce the scope to x86 only. >> +-doc_begin="Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very specific cases where by code inspection you can see that its usage is correct. The BUILD_BUG_ON checks check that EFI_TIME and struct xenpf_efi_time fields match." >> +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^chk_fld$))))"} >> +-doc_end > > As indicated on earlier occasions - when a macro isn't global, its deviation > better wouldn't be global either. An identically named macro may be introduced > elsewhere, and may not need deviating there. Noted. >> --- a/docs/misra/deviations.rst >> +++ b/docs/misra/deviations.rst >> @@ -279,6 +279,18 @@ Deviations related to MISRA C:2012 Rules: >> the short-circuit evaluation strategy for logical operators. >> - Project-wide deviation; tagged as `disapplied` for ECLAIR. >> >> + * - R13.6 >> + - Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the >> + func parameters without evaluating them. > > Just to mention it: The return type of the function isn't really of interest > there. What's being checked is that the argument types match the corresponding > parameter ones, which otherwise would "naturally" be done by the compiler when > seeing function calls. Might be useful to state no more and no less than what > is really needed. Noted.
On 05/04/24 08:06, Jan Beulich wrote: > On 05.04.2024 02:04, Stefano Stabellini wrote: >> On Thu, 4 Apr 2024, Jan Beulich wrote: >>> Further, according to the discussion on the matter: alternative_call() is >>> quite similar in this regard, just that it uses typeof(). Imo it should be >>> deviated here as well right away, no matter whether Eclair can actually be >>> configured to also check typeof() and alignof(). >> >> On this specific point regarding typeof, I agree but it should be done >> as a separate rule (not 13.6), to make sure it matches the scanner >> results. It looks like the new rule will be called B.MISLEFF. > > Fine with me, as long as things stay together (i.e. in a single commit). Noted.
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl index de9ba723fb..eebe779000 100644 --- a/automation/eclair_analysis/ECLAIR/deviations.ecl +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl @@ -333,6 +333,14 @@ of the short-circuit evaluation strategy of such logical operators." -config=MC3R1.R13.5,reports+={disapplied,"any()"} -doc_end +-doc_begin="Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the func parameters without evaluating them." +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^alternative_vcall[0-9]$))))"} +-doc_end + +-doc_begin="Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very specific cases where by code inspection you can see that its usage is correct. The BUILD_BUG_ON checks check that EFI_TIME and struct xenpf_efi_time fields match." +-config=MC3R1.R13.6,reports+={deliberate,"any_area(any_loc(any_exp(macro(^chk_fld$))))"} +-doc_end + # # Series 14 # diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst index eb5ef2bd9d..a97c31d18f 100644 --- a/docs/misra/deviations.rst +++ b/docs/misra/deviations.rst @@ -279,6 +279,18 @@ Deviations related to MISRA C:2012 Rules: the short-circuit evaluation strategy for logical operators. - Project-wide deviation; tagged as `disapplied` for ECLAIR. + * - R13.6 + - Macros alternative_vcall[0-9] use sizeof to type-check \"func\" and the + func parameters without evaluating them. + - Tagged as `deliberate` for ECLAIR. + + * - R13.6 + - Macro chk_fld is only used to introduce BUILD_BUG_ON checks in very + specific cases where by code inspection you can see that its usage is + correct. The BUILD_BUG_ON checks check that EFI_TIME and + struct xenpf_efi_time fields match. + - Tagged as `deliberate` for ECLAIR. + * - R14.2 - The severe restrictions imposed by this rule on the use of 'for' statements are not counterbalanced by the presumed facilitation of the
Update ECLAIR configuration to deviate uses of chk_fld and alternative_vcall[0-9] macros. Suggested-by: Stefano Stabellini <sstabellini@kernel.org> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> --- automation/eclair_analysis/ECLAIR/deviations.ecl | 8 ++++++++ docs/misra/deviations.rst | 12 ++++++++++++ 2 files changed, 20 insertions(+)