> On 25 Apr 2024, at 14:32, Nicola Vetrini <nicola.vetr...@bugseng.com> wrote:
>
> On 2024-04-25 11:40, Federico Serafini wrote:
>> On 25/04/24 02:06, Stefano Stabellini wrote:
>>> On Tue, 23 Apr 2024, Federico Serafini wrote:
>>>> From: Simone Ballarin <simone.balla...@bugseng.com>
>>>> Introduce accepted_guidelines.sh: a script to autogenerate the
>>>> configuration file accepted.ecl from docs/misra/rules.rst which enables
>>>> all accepted guidelines.
>>>> Introduce monitored.ecl: a manual selection of accepted guidelines
>>>> which are clean or almost clean, it is intended to be used for the
>>>> analyses triggered by commits.
>>>> Reorganize tagging.ecl:
>>>> -Remove "accepted" tags: keeping track of accepted guidelines tagging
>>>> them as "accepted" in the configuration file tagging.ecl is no
>>>> longer needed since docs/rules.rst is keeping track of them.
>>>> -Tag more guidelines as clean.
>>>> Reorganize eclair pipelines:
>>>> - Set1, Set2, Set3 are now obsolete: remove the corresponding
>>>> pipelines and ecl files.
>>>> - Amend scheduled eclair pipeline to use accepted.ecl.
>>>> - Amend triggered eclair pipeline to use monitored.ecl.
>>>> Rename and improve action_check_clean_regressions.sh to print a
>>>> diagnostic in case a commit introduces a violation of a clean guideline.
>>>> An example of diagnostic is the following:
>>>> Failure: 13 regressions found for clean guidelines
>>>> service MC3R1.R8.2: (required) Function types shall be in prototype form
>>>> with named parameters:
>>>> violation: 13
>>>> Signed-off-by: Simone Ballarin <simone.balla...@bugseng.com>
>>>> Signed-off-by: Federico Serafini <federico.seraf...@bugseng.com>
>>>> Signed-off-by: Alessandro Zucchelli <alessandro.zucche...@bugseng.com>
>>> Fantastic work, thank you!
>>> Reviewed-by: Stefano Stabellini <sstabell...@kernel.org>
>>> Is this patch safe to commit now? Or would it cause gitlab-ci breakage?
>> Yes, it is safe because the ECLAIR analysis is still allowed to fail.
>> Committing this patch wouldn't break the CI but it will highlight some
>> regressions with the orange badge and the following messages:
>> arm:
>> Failure: 5 regressions found for clean guidelines
>> service MC3R1.R1.1: (required) The program shall contain no violations of
>> the standard C syntax and constraints, and shall not exceed the
>> implementation's translation limits:
>> violation: 5
>
Hi Nicola,
> +Cc ARM maintainers, Luca Fancellu
>
> after some investigation on these regressions on R1.1, there are some
> concerns with the current code:
> 2209c1e35b479dff8ed3d3161001ffdefa0a704e introduced the struct
>
> struct membanks {
> unsigned int nr_banks;
> unsigned int max_banks;
> struct membank bank[];
> };
>
> with a flexible array member at the end; this is well-defined in C99, but
> what is non-standard (a GNU extension) is having this struct as a member to
> another struct/union in a position other than the last.
>
> This is still supported by GNU (albeit reliance on this is undocumented for
> Xen), but what is concerning here is the following (taken from [1]):
>
> "A structure containing a C99 flexible array member, or a union containing
> such a structure, is not the last field of another structure, for example:
>
> struct flex { int length; char data[]; };
>
> struct mid_flex { int m; struct flex flex_data; int n; };
>
> In the above, accessing a member of the array mid_flex.flex_data.data[] might
> have undefined behavior. Compilers do not handle such a case consistently.
> Any code relying on this case should be modified to ensure that flexible
> array members only end up at the ends of structures.
> Please use the warning option -Wflex-array-member-not-at-end to identify all
> such cases in the source code and modify them. This extension is now
> deprecated."
>
> It looks like this option, and the corresponding deprecation of the
> extension, will be available starting from GCC 14, so this might impact
> future compiler updates for Xen builds.
>
> Linux is also concerned about this (see [2]), so I think the changes in
> struct layout introduced by the series [3] should be reviewed to determine
> whether this change was deliberate or happened as a byproduct. If this is
> determined not to be a legitimate concern, then this can be documented as an
> use of the GNU extension.
Thanks for letting us know, so the change was deliberate, the effect probably
not, I guess we need to find a way to fix this in order to use this interface.
Cheers,
Luca
