MISRA C Rule 5.5 states: "Identifiers shall be distinct from macro names".
Update ECLAIR configuration to deviate clashes: specify the macros that
should be ignored. Update deviations.rst and rules.rst accordingly.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
The conversion from a function pointer with the 'noreturn' attribute
('void noreturn (*)(...)') to a function pointer type ('void (*)(...)'
causes type incompatibility according to MISRA C Rule 11.1, which
forbids conversions between incompatible function pointer types.
The violation occurs at the call site:
smp_call_function(halt_this_cpu, NULL, 0);
where 'halt_this_cpu' with type 'void noreturn (*)(void *)' is passed to
'smp_call_function' expecting function pointer of type 'void (*)(void *)'.
The 'noreturn' attribute does not change the function calling convention
or parameter handling at runtime, making the conversion safe.
For now, configure Eclair to just treat implicit conversions that lose
the "noreturn" attribute on a function 'void (*)(void*)' as safe.
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Jan Beulich <jbeulich@suse.com> # docs
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com> # ECLAIR
MISRA C Rule 2.1 states: "A project shall not contain unreachable code".
Functions that are non-returning and are not explicitly annotated with
the `noreturn' attribute are considered a violation of this rule.
In certain cases, some functions might be non-returning in specific build
configurations (when assertions are enabled, i.e., when `NDEBUG' is not
defined). This is due to calls to `__builtin_unreachable()' in the
expansion of the macro `ASSERT_UNREACHABLE()'.
Conversely, in builds where `NDEBUG' is defined (assertions are disabled),
the macro `ASSERT_UNREACHABLE()' expands to an empty construct
(`do { } while (0)'), which does not affect the execution flow. This allows
such functions to return normally in such builds, avoiding unreachable code.
To account for that in specific builds, the `noreturn` property of
`__builtin_unreachable()` is overridden in the ECLAIR configuration to
deviate these violations.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The definition of MISRA C Rule 12.2 ("The right hand operand of a shift
operator shall lie in the range zero to one less than the width in bits
of the essential type of the left hand operand") is concerned with the
essential type of an expression, while the C Undefined Behaviour is
related to C standard types, which may be wider but not narrower than
the MISRA C essential type. For this reason, it is safe to consider the
C standard type, rather than the essential type when checking the rule.
To avoid regressions, tag the rule as clean and add it to the
monitored set.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
MISRA C Rule 10.1 states:
"Operands shall not be of an inappropriate essential type"
The unary minus operator applied to an unsigned type(s) has
a semantics (wrap around) that is well-defined by the toolchains.
Thus, this operation is deemed safe.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Victor Lira <victorm.lira@amd.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
MISRA C Directive 4.10 states that "Precautions shall be taken in order
to prevent the contents of a header file being included more than
once".
Add a SAF tag and update the comment on top of cpufeatures.h.
Add a header inclusion guard to compile.h.
Generate header guards for hypercall-defs.h
Update ECLAIR configuration to:
- extend existing deviation to other comments explicitly saying a file
is intended for multiple inclusion;
- extend existing deviation to other autogenerated files;
- tag the guidelines as clean.
Update deviations.rst accordingly.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
MISRA C Rule 21.6 states that "The Standard Library input/output
functions shall not be used".
Xen does not use the functions provided by the Standard Library,
uses -nostdlib to ensure this.
Xen implements a set of functions that share the same names as their
Standard Library equivalent. The implementation of these functions is
available in source form: if some undefined or unspecified behavior
does arise in the implementation, it falls under the jurisdiction of
other MISRA guidelines.
Update ECLAIR configuration to deviate violations of the rule,
tag it as clean and add it to the monitored set.
Updated docs/misra/deviations.rst accordingly.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Dmytro Prokopchuk <dmytro_prokopchuk1@epam.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
... in preparation for changing how they're implemented.
Update the MISRA deviations with the new path.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>
The use of "legitimately" mixes the concepts of "it was designed to do this"
and "it was correct to do this".
The latter in particular can go stale. "intended" is a better way of phrasing
this.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
This ought to have been part of the original patch, so as to avoid breaking
CI.
Fixes: 31c0d6fdf4 ("xen/bsearch: Split out of lib.h into it's own header")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Rule 11.8 states as following: "A cast shall not remove any `const' or
`volatile' qualification from the type pointed to by a pointer".
Function `__hvm_copy' in `xen/arch/x86/hvm/hvm.c' is a double-use
function, where the parameter needs to not be const because it can be
set for write or not. As it was decided a new const-only function will
lead to more developer confusion than it's worth, this violation is
addressed by deviating the function.
All cases of casting away const-ness are accompanied with a comment
explaining why it is safe given the other flags passed in; such comment is used
by the deviation in order to match the appropriate function call.
No functional change.
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Rename all instances of ECLAIR MISRA C:2012 service identifiers,
identified by the prefix MC3R1, to use the prefix MC3A2, which
refers to MISRA C:2012 Amendment 2 guidelines.
This update is motivated by the need to upgrade ECLAIR GitLab runners
that use the new naming scheme for MISRA C:2012 Amendment 2 guidelines.
Changes to the docs/misra directory are needed in order to keep
comment-based deviation up to date.
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Add deviation to address violations of MISRA C:2012 Directive 4.10
("Precautions shall be taken in order to prevent the contents of a
header file being included more than once").
This deviation suppresses the violation arising from autogenerated file
xen/include/generated/autoconf.h
No functional change.
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
MISRA Rule 20.7 states:
"Expressions resulting from the expansion of macro parameters
shall be enclosed in parentheses".
The files imported from the gnu-efi package are already deviated, yet
the macro NextMemoryDescriptor is used in non-excluded code, so a further
deviation is needed to exclude also any expansion of the macro.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
MISRA C:2012 Rule 20.7 states that "Expressions resulting from the
expansion of macro parameters shall be enclosed in parentheses".
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 M(A, B) A * B
M(1+1, 2+2) will expand to: 1+1 * 2+2
Update ECLAIR configuration to tag as 'safe' the expansions of macro
arguments surrounded tokens '{', '}' and ';', since in their presence
problems related to operator precedence can not occur.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
MISRA C Rule 18.2 states: "Subtraction between pointers shall
only be applied to pointers that address elements of the same array".
Subtractions between pointer where at least one symbol is a
symbol defined by the linker are safe and thus deviated, because
the compiler cannot exploit the undefined behaviour that would
arise from violating the rules in this case.
To create an ECLAIR configuration that contains the list of
linker-defined symbols, the script "linker-symbols.sh" is used
after a build of xen (without static analysis) is performed.
The generated file "linker_symbols.ecl" is then used as part of the
static analysis configuration.
Additional changes to the ECLAIR integration are:
- perform a build of xen without static analysis during prepare.sh
- run the scripts to generated ECL configuration during the prepare.sh,
rather than analysis.sh
- export ECLAIR_PROJECT_ROOT earlier, to allow such generation
Additionally, the macro page_to_mfn performs a subtraction that is safe,
so its uses are deviated.
No functional changes.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Prior to bd1664db7b ("xen/bitops: Introduce a multiple_bits_set() helper")
the definition of {COMPILE,RUNTIME}_CHECK was fully compliant with respect
to MISRA C Rule 20.7:
"Expressions resulting from the expansion of macro parameters shall be
enclosed in parentheses."
However, to allow testing function-like macros, parentheses on the "fn"
parameter were removed and thus new violations of the rule have been
introduced. Given the usefulness of this functionality,
it is deemed ok to deviate these two macros for this rule, because
their scope of (direct) usage is limited to just the file where they
are defined, and the possibility of misuses is unlikely.
No functional change.
Fixes: bd1664db7b ("xen/bitops: Introduce a multiple_bits_set() helper")
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Update ECLAIR configuration to deviate more cases where an
unintentional fallthrough cannot happen.
Tag Rule 16.3 as clean for arm.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
This patch modifies deviations for Directive 4.10:
"Precautions shall be taken in order to prevent the contents of
a header file being included more than once"
This patch avoids the file-based deviation for empty headers, and
replaces it with a comment-based one using the format specified in
docs/misra/safe.json.
Generated headers are not generally safe against multi-inclusions,
whether a header is safe depends on the nature of the generated code
in the header. For that reason, this patch drops the deviation for
generated headers.
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Add missing escape for the final dot of the fallthrough comment,
extend the search of a fallthrough comment up to 2 lines after the last
statement and improve the text of the justification.
Fixes: a128d8da91 ("automation/eclair: add deviations for MISRA C:2012 Rule 16.3")
Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Update ECLAIR configuration to take into account the deviations
agreed during the MISRA meetings.
While doing this, remove the obsolete "Set [123]" comments.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>
MISRA C Rule 20.7 states: "Expressions resulting from the expansion
of macro parameters shall be enclosed in parentheses".
The local helpers GRP2 and XADD in the x86 emulator use their first
argument as the constant expression for a case label. This pattern
is deviated project-wide, because it is very unlikely to induce
developer confusion and result in the wrong control flow being
carried out.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Rule 21.2 reports identifiers reserved for the C and POSIX standard
libraries: or, and, not and xor are reserved identifiers because they
constitute alternate spellings for the corresponding operators (they are
defined as macros by iso646.h); however Xen doesn't use standard library
headers, so there is no risk of overlap.
This addresses violations arising from x86_emulate/x86_emulate.c, where
label statements named as or, and and xor appear.
No functional change.
Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
Rule 13.6 states that "The operand of the `sizeof' operator shall not
contain any expression which has potential side effects".
Define service B.UNEVALEFF as an extension of Rule 13.6 to
check for unevalued side effects also for typeof and alignof operators.
Update ECLAIR configuration to deviate uses of BUILD_BUG_ON and
alternative_v?call[0-9] for both Rule 13.6 and B.UNEVALEFF.
Add service B.UNEVALEFF to the accepted.ecl guidelines to check
"violations" in the weekly analysis.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Release-Acked-By: Oleksii Kurochko <oleksii.kurochko@gmail.com>
MISRA C Rule 5.5 states that "Identifiers shall be distinct from macro
names".
Update ECLAIR configuration to deviate:
- macros expanding to their own name;
- clashes between macros and non-callable entities;
- clashes related to the selection of specific implementations of string
handling functions.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Release-Acked-by: Oleksii Kurochko <oleksii.kurochko@gmail.com>
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>
Commit ea59e7d780 ("xen/bitops: Cleanup and new infrastructure ahead of
rearrangements") introduced new violations on previously clean rules 20.9 and
20.12 (clean on ARM only, right now).
The first is introduced because CONFIG_CC_IS_CLANG in xen/self-tests.h is not
defined in the configuration under analysis. Using "defined()" instead avoids
relying on the preprocessor's behaviour upon encountering an undedfined identifier
and addresses the violation.
The violation of Rule 20.12 is due to "val" being used both as an ordinary argument
in macro RUNTIME_CHECK, and as a stringification operator.
No functional change.
Fixes: ea59e7d780 ("xen/bitops: Cleanup and new infrastructure ahead of rearrangements")
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
The parentheses in this regular expression should be doubly
escaped because they undergo expansion twice.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
[stefano: fix commit message]
Signed-off-by: Stefano Stabellini <stefano.stabellini@amd.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
The count_args_ macro violates Rule 20.7, but it can't be made
compliant with Rule 20.7 without breaking its functionality. Since
it's very unlikely for this macro to be misused, it is deviated.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
These rules are concerned with the use of facilities provided by the
C Standard Library (qsort, bsearch for rule 21.9, and those provided
by <time.h> for rule 21.10).
Xen provides in its source code its own implementation of some of these
functions and macros, therefore a justification is provided for allowing
uses of these functions in the project.
The rules are also marked as clean as a consequence.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Update ECLAIR configuration to take into account the deviations
agreed during MISRA meetings.
Amend an existing entry of Rule 14.4 in deviations-rst:
it is not a project-wide deviation.
Tag Rule 14.4 as clean for arm.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
MISRA C:2012 Rule 16.4 states that "Every switch statement shall have a
default label".
Update ECLAIR configuration to take into account the deviations
agreed during MISRA meetings.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
MISRA C Rule 16.2 states:
"A switch label shall only be used when the most closely-enclosing
compound statement is the body of a switch statement".
Since complying with this rule of the x86 emulator would lead to
a lot of code duplication, it is deemed better to exempt those
files for this guideline.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
The Xen community wants to avoid using variadic functions except for
specific circumstances where it feels appropriate by strict code review.
Add deviation for printf()-like functions.
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
These deviations deal with the following cases:
- macro arguments used directly as initializer list arguments;
- uses of the __config_enabled macro, that can't be brought
into compliance without breaking its functionality;
- exclude files that are out of scope (efi headers and cpu_idle);
- uses of alternative_{call,vcall}[0-9] macros.
The existing configuration for R20.7 is reordered so that it matches the
cases listed in its documentation comment.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Update ECLAIR configuration of MISRA C:2012 Rule 8.3 to deviate
violations involving parameter name "unused" (with an optional
numeric suffix): it makes explicit the intention of not using such
parameter within the function.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Certain macros are allowed to violate the Rule, since their meaning and
intended use is well-known to all Xen developers.
Variadic macros that rely on the GCC extension for removing a trailing
comma when token pasting the variable argument are similarly
well-understood and therefore allowed.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Update ECLAIR configuration to take into account the deviations
agreed during MISRA meetings.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Unwanted leading '+'-s cause the deviation not to work as expected,
leading to wrong analysis counts.
Fixes: 11126964d8 ("automation/eclair_analysis: avoid violation of MISRA Rule 2.1")
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The xen sources contain violations of MISRA C:2012 Rule 11.8 whose
headline states:
"A conversion shall not remove any const, volatile or _Atomic qualification
from the type pointed to by a pointer".
The macro 'container_of' violates the Rule because it casts away any
qualifier to do pointer arithmetic, but making the macro definition compliant
would add additional complexity, therefore a deviation is introduced.
Signed-off-by: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Deviate typedef names that are delberately defined multiple times.
Update docs/misra/deviations.rst accordingly.
Tag Rule 5.6 as clean.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The presence of an unlinked object file triggers a violation
of MISRA C Rule 2.1, which is deviated, as it's not part of
the final Xen binary.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Given that 'hwdom_shutdown' is a noreturn function, unreachable
breaks can be eliminated to resolve violations of Rule 2.1.
The rename s/maybe_reboot/reboot_or_halt/ is done to clarify
that the function is noreturn.
No functional change.
Signed-off-by: Nicola Vetrini <nicola.vetrini@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
The xen sources contain violations of MISRA C:2012 Rule 14.4 whose
headline states:
"The controlling expression of an if statement and the controlling
expression of an iteration-statement shall have essentially Boolean type".
Struct domain member is_dying is an anonymous enum designed to act as boolean.
Add deviation to mark its uses in controlling expressions as deliberate.
Signed-off-by: Maria Celeste Cesario <maria.celeste.cesario@bugseng.com>
Signed-off-by: Simone Ballarin <simone.ballarin@bugseng.com>
Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>
Update ECLAIR configuration to take into account the adopted files
and type "ret_t".
Update docs/misra/deviations.rst accordingly.
Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
Acked-by: Stefano Stabellini <sstabellini@kernel.org>
Dmytro Prokopchuk1
Nicola Vetrini
Stefano Stabellini
Federico Serafini
Andrew Cooper
Alessandro Zucchelli
Maria Celeste Cesario
Simone Ballarin