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

[Nicola Vetrini <nicola.vetrini@bugseng.com>]

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