From: Luca Fancellu <Luca.Fancellu@arm.com>
To: Nicola Vetrini <nicola.vetrini@bugseng.com>
Cc: Federico Serafini <federico.serafini@bugseng.com>,
Stefano Stabellini <sstabellini@kernel.org>,
"xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
"consulting@bugseng.com" <consulting@bugseng.com>,
Simone Ballarin <simone.ballarin@bugseng.com>,
Doug Goldstein <cardoe@cardoe.com>,
Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>,
Julien Grall <julien@xen.org>,
Bertrand Marquis <Bertrand.Marquis@arm.com>,
Michal Orzel <michal.orzel@amd.com>,
Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>
Subject: Re: [XEN PATCH] automation/eclair: reorganize pipelines
Date: Fri, 26 Apr 2024 08:34:23 +0000 [thread overview]
Message-ID: <6DA0DCCE-6638-4F9F-83DB-A6CC1ED91BD8@arm.com> (raw)
In-Reply-To: <8be082b6d22d61c0b14910680d3833a7@bugseng.com>
> On 25 Apr 2024, at 14:32, Nicola Vetrini <nicola.vetrini@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.ballarin@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.ballarin@bugseng.com>
>>>> Signed-off-by: Federico Serafini <federico.serafini@bugseng.com>
>>>> Signed-off-by: Alessandro Zucchelli <alessandro.zucchelli@bugseng.com>
>>> Fantastic work, thank you!
>>> Reviewed-by: Stefano Stabellini <sstabellini@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
next prev parent reply other threads:[~2024-04-26 8:34 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-23 15:14 [XEN PATCH] automation/eclair: reorganize pipelines Federico Serafini
2024-04-25 0:06 ` Stefano Stabellini
2024-04-25 9:40 ` Federico Serafini
2024-04-25 13:32 ` Nicola Vetrini
2024-04-26 8:34 ` Luca Fancellu [this message]
2024-04-25 20:12 ` Stefano Stabellini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6DA0DCCE-6638-4F9F-83DB-A6CC1ED91BD8@arm.com \
--to=luca.fancellu@arm.com \
--cc=Bertrand.Marquis@arm.com \
--cc=Volodymyr_Babchuk@epam.com \
--cc=alessandro.zucchelli@bugseng.com \
--cc=cardoe@cardoe.com \
--cc=consulting@bugseng.com \
--cc=federico.serafini@bugseng.com \
--cc=julien@xen.org \
--cc=michal.orzel@amd.com \
--cc=nicola.vetrini@bugseng.com \
--cc=simone.ballarin@bugseng.com \
--cc=sstabellini@kernel.org \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).