Re: MISRA C:2012 D4.11 caution on staging

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 11/10/2023 02:15, Andrew Cooper wrote:
> On 10/10/2023 5:31 pm, Nicola Vetrini wrote:
> > Hi,
> >
> > as you can see from [1], there's a MISRA C guideline, D4.11, that is
> > supposed to be clean
> > (i.e., have no reports), but has a caution on an argument to memcpy
> > (the second argument might be null according to the checker, given a
> > set of assumptions on
> > the control flow). To access the report just click on the second link
> > in the log, which should take you to a webpage with a list of
> > MISRA guidelines. Click on D4.11 and you'll see the full report, which
> > I pasted below for convenience.
> >
> > If the finding is genuine, then some countermeasure needs to be taken
> > against this
> > possible bug, otherwise it needs to be motivated why the field
> > config->handle can't
> > be null at that point.
> > The finding is likely the result of an improvement made to the
> > checker, because the first
> > analysis I can see that spots it happened when rc1 has been tagged,
> > but that commit does not
> > touch the involved files.
> >
> > [1] https://gitlab.com/xen-project/xen/-/jobs/5251222578
> That's a false positive, but I'm not entirely surprised that the 
> checker
> struggled to see it.
>
> First,
>
> ASSERT(is_system_domain(d) ? config == NULL : config != NULL);
>
> All system domains (domid >= 0x7ff0, inc IDLE) pass a NULL config.  All
> other domains pass a real config.
>
> Next,
>
> /* DOMID_{XEN,IO,etc} (other than IDLE) are sufficiently constructed. 
> */
> if ( is_system_domain(d) && !is_idle_domain(d) )
>     return d;
>
> So at this point we only have the IDLE domain and real domains.
>
> And finally, the complained-about construct is inside an:
>
> if ( !is_idle_domain(d) )
>     ...
>
> hence config cannot be NULL, but only because of the way in which
> is_{system,idle}_domain() interact.
>
> ~Andrew
Ok. I think this should be documented as a false positive using the 
comment deviation mechanism
in the false-positive-eclair.json file (once a name for the prefix is 
settled on in the
other thread).

--
Nicola Vetrini, BSc
Software Engineer, BUGSENG srl (https://bugseng.com)