Re: [XEN PATCH] docs/misra: document the C dialect and translation toolchain assumptions.

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 16 Jun 2023, Roberto Bagnara wrote:
> > > +   * - Implicit conversion from a pointer to an incompatible pointer
> > > +     - ARM64, X86_64
> > > +     - Non-documented GCC extension.
> > 
> > Is this related to -Wincompatible-pointer-types?
> 
> In my opinion, this does not specify what the result of the
> conversion is.

Fair enough. However, if -Wincompatible-pointer-types and "Implicit
conversion from a pointer to an incompatible pointer" are related, it
would add -Wincompatible-pointer-types as extra info about it. See also
below.


> > > +   * - Pointer to a function is converted to a pointer to an object or a
> > > pointer to an object is converted to a pointer to a function
> > > +     - X86_64
> > > +     - Non-documented GCC extension.
> > 
> > Is this J.5.7 of n1570?
> > https://www.iso-9899.info/n1570.html
> 
> This says that function pointer casts are a common extension.
> What we need here is documentation for GCC that assures us
> that the extension is implemented and what its semantics is.
> 
> > Or maybe we should link https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83584
> 
> My opinion is that this might not be accepted by an assessor:
> if I was an assessor, I would not accept it.

I understand your point and I think it is valid. My observation was
that it is better to provide as much information for these undocumented
extensions as we can. Not necessarily to help with an assessors, but for
a new engineer working on this project, reading this document and
understanding what can be done. 

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83584 might not be an
official documentation of the extension but it is better than no
documentation at all. Even better might be a code example.

I am not saying we should document ourselves what GCC failed to
document. I am only saying we should add enough description to
understand what we are talking about.

For instance, I read "Pointer to a function is converted to a pointer to
an object or a pointer to an object is converted to a pointer to a
function" and I have an idea about what this is but I am not really
sure. I googled the sentence and found information on Stackoverflow. I
think it is better to link
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=83584 or a couple of
sentences from it, although it might not be official.


> > > +   * - Ill-formed source detected by the parser
> > 
> > As we are documenting compiler extensions that we are using, I am a bit
> > confused by the name of this category of compiler extensions, and the
> > reason why they are bundled together. After all, they are all separate
> > compiler extensions? Should each of them have their own row?
> 
> Agreed.
> 
> > > +     - ARM64, X86_64
> > > +     - token pasting of ',' and __VA_ARGS__ is a GNU extension:
> > > +          see Section "6.21 Macros with a Variable Number of Arguments"
> > > of GCC_MANUAL.
> > > +       must specify at least one argument for '...' parameter of variadic
> > > macro:
> > > +          see Section "6.21 Macros with a Variable Number of Arguments"
> > > of GCC_MANUAL.
> > > +       void function should not return void expression:
> > 
> > I understand that GCC does a poor job at documenting several of these
> > extensions. In fact a few of them are not even documented at all.
> > However, if they are extensions, they should be described for what they
> > do, not for the rule they violate. What do you think?
> 
> The point is that we don't know what they do.  We might make observations,
> and our observations might substantiate what we believe they do.
> But this would not allow us to generalize them.
>
> > For example, in this case maybe we should say "void function can return
> > a void expression" ?
> 
> We can certainly say that, but this might not convince an assessor.
> One possibility would be to submit patches to the GCC manual and see
> whether they are accepted.

I think we have two different target audiences for this document. One
target is an assessors, and I understand that extra unofficial
information might not help there.

However another target is the community. This document should help the
Xen community write better code, not just the assessors raise red flags.
Right? It should help us have better compiler compatibility, and making
sure that we are clear about the C dialect we use. Actually, I think
this document could be of great help. Do you agree?

>From that point of view "void function should not return void
expression" is not understandable. At least I don't understand it.

A different approach would be to say:

- this is a MISRA C violation or compiler warning/error
- it is not C99 compliant
- it is not documented behavior by GCC

Not try to describe what the extension is at all, and instead focus on
what the MISRA C violation or compiler warning is.

I think it is OK to go down that route, but in that case we need to
reorganize the document so that:
- all documented extensions are referred to as extensions
- all undocumented extensions are referred to by the warning they
  trigger

I think that we would be OK but honestly I prefer the current approach
and we just need to add a few extra words to better explain what the
undocumented extensions are. Not to replace the GCC manual but simply
because otherwise we are not understanding each other (at least I am not
understanding.)


> > > +          see the documentation for -Wreturn-type in Section "3.8 Options
> > > to Request or Suppress Warnings" of GCC_MANUAL.
> > > +       use of GNU statement expression extension from macro expansion:
> > > +          see Section "6.1 Statements and Declarations in Expressions" of
> > > GCC_MANUAL.
> > > +       invalid application of sizeof to a void type:
> > > +          see Section "6.24 Arithmetic on void- and Function-Pointers" of
> > > GCC_MANUAL.
> > > +       redeclaration of already-defined enum is a GNU extension:
> > > +          see Section "6.49 Incomplete enum Types" of GCC_MANUAL.
> > > +       static function is used in an inline function with external
> > > linkage:
> > > +          non-documented GCC extension.
> > 
> > I am not sure if I follow about this one. Did you mean "static is used
> > in an inline function with external linkage" ?
> 
> An inline function with external linkage can be inlined everywhere.
> If that calls a static functions, which is not available everywhere,
> the behavior is not defined.

Got it. Can we add this sentence you wrote to the doc?


> > > +       struct may not be nested in a struct due to flexible array member:
> > > +          see Section "6.18 Arrays of Length Zero" of GCC_MANUAL.
> > > +       struct may not be used as an array element due to flexible array
> > > member:
> > > +          see Section "6.18 Arrays of Length Zero" of GCC_MANUAL.
> > > +       ISO C restricts enumerator values to the range of int:
> > > +          non-documented GCC extension.
> > 
> > Should we call it instead "enumerator values can be larger than int" ?
> 
> Yes, I have rephrased that entry.
> 
> > > +   * - Unspecified escape sequence is encountered in a character constant
> > > or a string literal token
> > > +     - X86_64
> > > +     - \\m:
> > > +          non-documented GCC extension.
> > 
> > Are you saying that we are using \m and \m is not allowed by the C
> > standard?
> 
> The C standard does not specify that escape sequence, so what is
> done with it, in particular by the preprocessor, is not specified.
> 
> > > +   * - Non-standard type
> > 
> > Should we call it "128-bit Integers" ?
> 
> I have rephrased this as suggested by Jan.
> 
> Thanks for your review.  I will submit a revised patch
> on Monday.