| Message ID | 4aa692a50a5d08d24560f02fdc36911965fc860b.1724850701.git.federico.serafini@bugseng.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [XEN] xen/bitmap: remove comment-based deviations | expand |
On 28.08.2024 15:12, Federico Serafini wrote: > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl > @@ -565,6 +565,10 @@ of this macro do not lead to developer confusion, and can thus be deviated." > -config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^count_args_$))))"} > -doc_end > > +-doc_begin="The expansion of an argument surrounded by tokens '{', '}' and ';' is safe." > +-config=MC3R1.R20.7,expansion_context+={safe, "left_right(^[\\{;]$,^[;\\}]$)"} > +-doc_end Not the least because this is quite a bit wider than ... > --- a/xen/include/xen/bitmap.h > +++ b/xen/include/xen/bitmap.h > @@ -103,13 +103,10 @@ extern int bitmap_allocate_region(unsigned long *bitmap, int pos, int order); > #define bitmap_switch(nbits, zero, small, large) \ > unsigned int n__ = (nbits); \ > if (__builtin_constant_p(nbits) && !n__) { \ > - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ > zero; \ > } else if (__builtin_constant_p(nbits) && n__ <= BITS_PER_LONG) { \ > - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ > small; \ > } else { \ > - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ > large; \ > } ... what's needed here, I wonder if we're not opening up avenues to problems by generally permitting that pattern. Plus in the description I'm missing a statement to the effect of why this is (always) safe. Jan
On Wed, 28 Aug 2024, Federico Serafini wrote: > Update ECLAIR configuration of MISRA C:2012 Rule 20.7 ("Expressions > resulting from the expansion of macro parameters shall be enclosed in > parentheses") to tag as 'safe' the expansions of arguments surrounded > by the following tokens: '{', '}' and ';'. > > Remove redundant comment-based deviations. > > Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> I am missing some context: is this done to cover other cases similar to the one below (bitmap_allocate_region) or is this done only to get rid of the three SAF-7-safe in-code comments? > --- > automation/eclair_analysis/ECLAIR/deviations.ecl | 4 ++++ > docs/misra/deviations.rst | 5 +++++ > xen/include/xen/bitmap.h | 3 --- > 3 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl > index 1aa8277066..f37329973f 100644 > --- a/automation/eclair_analysis/ECLAIR/deviations.ecl > +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl > @@ -565,6 +565,10 @@ of this macro do not lead to developer confusion, and can thus be deviated." > -config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^count_args_$))))"} > -doc_end > > +-doc_begin="The expansion of an argument surrounded by tokens '{', '}' and ';' is safe." > +-config=MC3R1.R20.7,expansion_context+={safe, "left_right(^[\\{;]$,^[;\\}]$)"} > +-doc_end > + > -doc_begin="Uses of variadic macros that have one of their arguments defined as > a macro and used within the body for both ordinary parameter expansion and as an > operand to the # or ## operators have a behavior that is well-understood and > diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst > index d51aa422b5..d529726464 100644 > --- a/docs/misra/deviations.rst > +++ b/docs/misra/deviations.rst > @@ -512,6 +512,11 @@ Deviations related to MISRA C:2012 Rules: > refactoring it to add parentheses breaks its functionality. > - Tagged as `safe` for ECLAIR. > > + * - R20.7 > + - The expansion of an argument surrounded by tokens '{', '}' and ';' is > + safe. > + - Tagged as `safe` for ECLAIR. > + > * - R20.12 > - Variadic macros that use token pasting often employ the gcc extension > `ext_paste_comma`, as detailed in `C-language-toolchain.rst`, which is > diff --git a/xen/include/xen/bitmap.h b/xen/include/xen/bitmap.h > index 5d668053b0..4b642cd420 100644 > --- a/xen/include/xen/bitmap.h > +++ b/xen/include/xen/bitmap.h > @@ -103,13 +103,10 @@ extern int bitmap_allocate_region(unsigned long *bitmap, int pos, int order); > #define bitmap_switch(nbits, zero, small, large) \ > unsigned int n__ = (nbits); \ > if (__builtin_constant_p(nbits) && !n__) { \ > - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ > zero; \ > } else if (__builtin_constant_p(nbits) && n__ <= BITS_PER_LONG) { \ > - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ > small; \ > } else { \ > - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ > large; \ > } > > -- > 2.34.1 >
On 28/08/24 16:50, Jan Beulich wrote: > On 28.08.2024 15:12, Federico Serafini wrote: >> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl >> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl >> @@ -565,6 +565,10 @@ of this macro do not lead to developer confusion, and can thus be deviated." >> -config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^count_args_$))))"} >> -doc_end >> >> +-doc_begin="The expansion of an argument surrounded by tokens '{', '}' and ';' is safe." >> +-config=MC3R1.R20.7,expansion_context+={safe, "left_right(^[\\{;]$,^[;\\}]$)"} >> +-doc_end > > Not the least because this is quite a bit wider than ... > >> --- a/xen/include/xen/bitmap.h >> +++ b/xen/include/xen/bitmap.h >> @@ -103,13 +103,10 @@ extern int bitmap_allocate_region(unsigned long *bitmap, int pos, int order); >> #define bitmap_switch(nbits, zero, small, large) \ >> unsigned int n__ = (nbits); \ >> if (__builtin_constant_p(nbits) && !n__) { \ >> - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ >> zero; \ >> } else if (__builtin_constant_p(nbits) && n__ <= BITS_PER_LONG) { \ >> - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ >> small; \ >> } else { \ >> - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ >> large; \ >> } > > ... what's needed here, I wonder if we're not opening up avenues to > problems by generally permitting that pattern. Plus in the description > I'm missing a statement to the effect of why this is (always) safe. The rational of the rule is that if a macro argument expands to an expression, there may be problems related to operator precedence, e.g.: #define A(x, y) x * y A(1+1, 2+2) will expand to: 1+1 * 2+2 Yes, the deviation is more general and wider than what is needed for the specific case but it is safe: if the expanded argument is between one of the aforementioned tokens, then there are no operators involved and no precedence issues. I can add some details in a v2.
On 29/08/24 02:42, Stefano Stabellini wrote: > On Wed, 28 Aug 2024, Federico Serafini wrote: >> Update ECLAIR configuration of MISRA C:2012 Rule 20.7 ("Expressions >> resulting from the expansion of macro parameters shall be enclosed in >> parentheses") to tag as 'safe' the expansions of arguments surrounded >> by the following tokens: '{', '}' and ';'. >> >> Remove redundant comment-based deviations. >> >> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> > > I am missing some context: is this done to cover other cases similar to > the one below (bitmap_allocate_region) or is this done only to get rid > of the three SAF-7-safe in-code comments? This is done to cover also other cases that may occur in the future. More details here: https://lists.xenproject.org/archives/html/xen-devel/2024-08/msg01392.html
On 30.08.2024 10:13, Federico Serafini wrote: > On 28/08/24 16:50, Jan Beulich wrote: >> On 28.08.2024 15:12, Federico Serafini wrote: >>> --- a/automation/eclair_analysis/ECLAIR/deviations.ecl >>> +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl >>> @@ -565,6 +565,10 @@ of this macro do not lead to developer confusion, and can thus be deviated." >>> -config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^count_args_$))))"} >>> -doc_end >>> >>> +-doc_begin="The expansion of an argument surrounded by tokens '{', '}' and ';' is safe." >>> +-config=MC3R1.R20.7,expansion_context+={safe, "left_right(^[\\{;]$,^[;\\}]$)"} >>> +-doc_end >> >> Not the least because this is quite a bit wider than ... >> >>> --- a/xen/include/xen/bitmap.h >>> +++ b/xen/include/xen/bitmap.h >>> @@ -103,13 +103,10 @@ extern int bitmap_allocate_region(unsigned long *bitmap, int pos, int order); >>> #define bitmap_switch(nbits, zero, small, large) \ >>> unsigned int n__ = (nbits); \ >>> if (__builtin_constant_p(nbits) && !n__) { \ >>> - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ >>> zero; \ >>> } else if (__builtin_constant_p(nbits) && n__ <= BITS_PER_LONG) { \ >>> - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ >>> small; \ >>> } else { \ >>> - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ >>> large; \ >>> } >> >> ... what's needed here, I wonder if we're not opening up avenues to >> problems by generally permitting that pattern. Plus in the description >> I'm missing a statement to the effect of why this is (always) safe. > > The rational of the rule is that if a macro argument expands to an > expression, there may be problems related to operator precedence, e.g.: > > #define A(x, y) x * y > > A(1+1, 2+2) will expand to: 1+1 * 2+2 > > Yes, the deviation is more general and wider than what is needed for > the specific case but it is safe: if the expanded argument is between > one of the aforementioned tokens, then there are no operators involved > and no precedence issues. > > I can add some details in a v2. Please do,taking into consideration also language extensions that we use, e.g. the statements-as-expressions one (where figure braces exist inside an expression). Jan
diff --git a/automation/eclair_analysis/ECLAIR/deviations.ecl b/automation/eclair_analysis/ECLAIR/deviations.ecl index 1aa8277066..f37329973f 100644 --- a/automation/eclair_analysis/ECLAIR/deviations.ecl +++ b/automation/eclair_analysis/ECLAIR/deviations.ecl @@ -565,6 +565,10 @@ of this macro do not lead to developer confusion, and can thus be deviated." -config=MC3R1.R20.7,reports+={safe, "any_area(any_loc(any_exp(macro(^count_args_$))))"} -doc_end +-doc_begin="The expansion of an argument surrounded by tokens '{', '}' and ';' is safe." +-config=MC3R1.R20.7,expansion_context+={safe, "left_right(^[\\{;]$,^[;\\}]$)"} +-doc_end + -doc_begin="Uses of variadic macros that have one of their arguments defined as a macro and used within the body for both ordinary parameter expansion and as an operand to the # or ## operators have a behavior that is well-understood and diff --git a/docs/misra/deviations.rst b/docs/misra/deviations.rst index d51aa422b5..d529726464 100644 --- a/docs/misra/deviations.rst +++ b/docs/misra/deviations.rst @@ -512,6 +512,11 @@ Deviations related to MISRA C:2012 Rules: refactoring it to add parentheses breaks its functionality. - Tagged as `safe` for ECLAIR. + * - R20.7 + - The expansion of an argument surrounded by tokens '{', '}' and ';' is + safe. + - Tagged as `safe` for ECLAIR. + * - R20.12 - Variadic macros that use token pasting often employ the gcc extension `ext_paste_comma`, as detailed in `C-language-toolchain.rst`, which is diff --git a/xen/include/xen/bitmap.h b/xen/include/xen/bitmap.h index 5d668053b0..4b642cd420 100644 --- a/xen/include/xen/bitmap.h +++ b/xen/include/xen/bitmap.h @@ -103,13 +103,10 @@ extern int bitmap_allocate_region(unsigned long *bitmap, int pos, int order); #define bitmap_switch(nbits, zero, small, large) \ unsigned int n__ = (nbits); \ if (__builtin_constant_p(nbits) && !n__) { \ - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ zero; \ } else if (__builtin_constant_p(nbits) && n__ <= BITS_PER_LONG) { \ - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ small; \ } else { \ - /* SAF-7-safe Rule 20.7 non-parenthesized macro argument */ \ large; \ }
Update ECLAIR configuration of MISRA C:2012 Rule 20.7 ("Expressions resulting from the expansion of macro parameters shall be enclosed in parentheses") to tag as 'safe' the expansions of arguments surrounded by the following tokens: '{', '}' and ';'. Remove redundant comment-based deviations. Signed-off-by: Federico Serafini <federico.serafini@bugseng.com> --- automation/eclair_analysis/ECLAIR/deviations.ecl | 4 ++++ docs/misra/deviations.rst | 5 +++++ xen/include/xen/bitmap.h | 3 --- 3 files changed, 9 insertions(+), 3 deletions(-)