Re: [XEN PATCH][for-4.19 v2 2/2] docs/misra: add deviations.rst to document additional deviations.

[Nicola Vetrini <nicola.vetrini@xxxxxxxxxxx>]

On 13/10/2023 01:14, Stefano Stabellini wrote:
> On Wed, 11 Oct 2023, Nicola Vetrini wrote:
> > On 11/10/2023 17:00, Nicola Vetrini wrote:
> > > > > > > > +
> > > > > > > > +   * - R2.1
> > > > > > > > +     - The compiler implementation guarantees that the
> > > > > > > > unreachable code
> > > > > > > > is
> > > > > > > > +       removed. Constant expressions and unreachable branches of
> > > > > > > > if and
> > > > > > > > switch
> > > > > > > > +       statements are expected.
> > > > > > > > +     - Tagged as `safe` for ECLAIR.
> > > > > > > > +
> > > > > > > > +   * - R2.1
> > > > > > > > +     - Some functions are intended not to be referenced.
> > > > > > > > +     - Tagged as `deliberate` for ECLAIR.
> > > > > > >
> > > > > > > What does it mean "some functions" in this case? Should we list
> > > > > > > which
> > > > > > > functions?
> > > > > > >
> > > > > >
> > > > > > Well, there are a lot, typically resulting from build configurations
> > > > > > that do
> > > > > > not
> > > > > > use them, or because they are used only in asm code. I can mention
> > > > > > these
> > > > > > reasons in the
> > > > > > document, to make it easier to understand.
> > > > >
> > > > > Yes, I think we need to clarify further this point, because saying "Some
> > > > > functions" doesn't help the reader understand:
> > > > > - whether all functions can be not referenced
> > > > > - which subset of functions can be not referenced
> > > > >
> > > > > How to distinguish between? How do we know whether a certain patch is
> > > > > violating the rule or not?
> > > > >
> > > > > If there is a clear list of functions that can be not referenced, then
> > > > > we should list them here. If there is a methodology we can use to
> > > > > distinguish between them (e.g. functions called from asm only) then we
> > > > > can write the methodology here. Either way it is fine as long as the
> > > > > criteria to know if it is OK if a function is not referenced is clear.
> > > >
> > > > Aren't they more or less the one we tagged with SAF-1-safe because
> > > > there were no prototype? If so, we could use the same tags.
> > > >
> > > > We could introduce an extra tags for the others. An alternative would
> > > > be to add an attribute (e.g. asmcall) to mark each function used by
> > > > assembly.
> > > >
> > > > Cheers,
> > >
> > > Both suggestion do have some value. As it is, it's not distinguishable
> > > what causes a
> > > function to be unreferenced in a certain analysis config. However:
> > >
> > > - functions only used by asm code can be specified in the ECLAIR
> > > config so that they will
> > >   have an extra fake reference as far as the checker is concerned. I
> > > can do that on a
> > >   separate patch and list them in deviations.rst. An attribute seems a
> > > good way to signal the
> > >   intention.
> > > - Functions that have no reference only in the current analysis should
> > > have their declaration
> > >   #ifdef-ed out in the configurations where they are not used, in an
> > > ideal world.
> > > - Truly unreferenced functions should be removed, or justified
> >
> > Especially the last two appear somewhat tricky to disentangle, as they 
> > do
> > require knowledge of
> > possible code paths.
> First let me premise that if we are unsure on how to proceed on this 
> you
> can resend this patch series without this item ("Some functions are
> intended not to be referenced"), so at least the rest can go in now.
>
> On this specific point, I think we should only make clear and
> unmistakable statements. For instance, I think it is OK to say that all
> the functions only used by asm code are exceptions (ideally they would
> have a asmcall tag as Julien suggested) because that is deterministic.
>
> Functions that have no references in a specific kconfig configuration
> should have their definition #ifdef'ed (not necessarily the
> declaration, I think we have already clarified that it is OK to have a
> declaration without definition.)
>
> Truly unreferenced functions should be removed.
>
> In conclusion, I think we should only have "functions only called from
> asm code" as a deviation here.
I agree on leaving this out of the patch for now.

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