Re: [PATCH] misra: add deviation of Rule 2.1 for BUG() macro

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 2025-08-26 20:07, Dmytro Prokopchuk1 wrote:
> On 8/25/25 13:07, Jan Beulich wrote:
> > On 24.08.2025 16:56, Dmytro Prokopchuk1 wrote:
> > > --- a/docs/misra/deviations.rst
> > > +++ b/docs/misra/deviations.rst
> > > @@ -97,6 +97,19 @@ Deviations related to MISRA C:2012 Rules:
> > >          Xen expects developers to ensure code remains safe and 
> > > reliable in builds,
> > >          even when debug-only assertions like `ASSERT_UNREACHABLE() 
> > > are removed.
> > > +   * - R2.1
> > > +     - The 'BUG()' macro is intentionally used in the 
> > > 'prepare_acpi()' function
> > > +       in specific build configuration (when the config CONFIG_ACPI 
> > > is not
> > > +       defined) to trigger an error if ACPI-related features are 
> > > used incorrectly.
> > > +     - Tagged as `deliberate` for ECLAIR.
> > With
> >
> > #define acpi_disabled true
> >
> > in xen/acpi.h I don't see why we even have that inline stub. When it's 
> > dropped
> > and the declaration left in place without #ifdef CONFIG_ACPI around 
> > it, the
> > compiler will DCE the code (much like we arrange for in many other 
> > places). No
> > deviation needed then.
> >
> > If such a deviation was to be added, it would need disambiguating. A 
> > function
> > of the given name could appear in x86 as well. That wouldn't be 
> > covered by the
> > Eclair config then, but it would be covered by the text here.
> >
> > > +   * - R2.1
> > > +     - The 'BUG()' macro is intentionally used in 'gicv3_do_LPI'() 
> > > and
> > > +       'gicv3_its_setup_collection()' functions in specific build 
> > > configuration
> > > +       (when the config CONFIG_HAS_ITS is not defined) to catch and 
> > > prevent any
> > > +       unintended execution of code that should only run when ITS is 
> > > available.
> > > +     - Tagged as `deliberate` for ECLAIR.
> > I didn't look at this, but I would very much hope that something 
> > similar could
> > be done there as well.
> >
> > Jan
> After small changes related to prepare_acpi() function, Misra R2.1
> violation has gone. The compiler really does DCE:
>
>      if ( acpi_disabled <<< this is TRUE )
>      {
>          rc = prepare_dtb_hwdom(d, kinfo);
>          if ( rc < 0 )
>              return rc;
> #ifdef CONFIG_HAS_PCI
>          rc = pci_host_bridge_mappings(d);
> #endif
>      }
>      else
>          rc = prepare_acpi(d, kinfo); <<< DCE
>
> I will publish it as separate patch.
> Thanks to Jan, I really appreciate his help.
>
>
> The situation with functions gicv3_do_LPI(),
> gicv3_its_setup_collection() and config CONFIG_HAS_ITS is little bit
> different.
> The compiler can do DCE in case when config CONFIG_HAS_ITS is "y", and
> Misra R2.1 violation related to these functions also can be resolved.
> Actually, no changes in source code need for that.
> But Eclair detects these violations because config CONFIG_HAS_ITS is
> "n", and source code is really compiled with inline stub functions 
> (with
> BUG() macro).
> This is because config CONFIG_HAS_ITS is "experimental/unsupported"
>
>      config HAS_ITS
>              bool "GICv3 ITS MSI controller support (UNSUPPORTED)" if
>   UNSUPPORTED
>          depends on GICV3 && !NEW_VGIC && !ARM_32
>
> and to enable it need to set additional config: "CONFIG_UNSUPPORTED=y".
>
> I tried to test it (added "CONFIG_UNSUPPORTED=y" into
> automation/gitlab-ci/analyze.yaml file). You can see my CI pipeline:
> https://eclair-analysis-logs.xenproject.org/fs/var/local/eclair/xen-project.ecdf/xen-project/people/dimaprkp4k/xen/ECLAIR_normal/rule_2.1_gicv3_its_host_has_its_v2/ARM64/11144854092/PROJECT.ecd;/by_service.html#service&kind
>
> Unfortunately, I observed +6 new violations with that additional config
> "CONFIG_UNSUPPORTED=y".
>
> I don't know how and why these EXTRA_XEN_CONFIG were selected in the
> file 'automation/gitlab-ci/analyze.yaml'. And are we able to add new
> configs here ?....
You'll have to ask Stefano about that, but I doubt at this stage. Those 
set of configs for Arm and X86 has been selected ~2 years ago.
> So, I see the next plan (just from my point of view):
> 1. Add "CONFIG_UNSUPPORTED=y" and resolve new violations.
> 2. Continue with proposed deviation
> 3. ... ?
>
> Thank you in advance.
> Dmytro.
--
Nicola Vetrini, B.Sc.
Software Engineer
BUGSENG (https://bugseng.com)
LinkedIn: https://www.linkedin.com/in/nicola-vetrini-a42471253