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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 16.06.2023 01:26, Stefano Stabellini wrote:
> On Thu, 15 Jun 2023, Roberto Bagnara wrote:
> I have a few comments below, mostly to clarify the description of some
> of the less documented GCC extensions, for the purpose of having all
> community members be able to understand what they can and cannot use.

What do you mean by "can and cannot use"? Is this document intended to
forbid the use of any extensions we may not currently use, or we use
but which aren't enumerated here?

One of the reasons that kept me from replying to this submission is
that the full purpose of this new doc isn't stated in the description.
Which in turn leaves open whether certain items actually need to be
here (see e.g. the libc related remark below). Another is that it's
hard to tell how to convince oneself of this being an exhaustive
enumeration. One extension we use extensively yet iirc is missing here
is omission of the middle operand of the ternary operator.

>> --- /dev/null
>> +++ b/docs/misra/C-language-toolchain.rst
>> @@ -0,0 +1,465 @@
>> +=============================================
>> +C Dialect and Translation Assumptions for Xen
>> +=============================================
>> +
>> +This document specifies the C language dialect used by Xen and
>> +the assumptions Xen makes on the translation toolchain.
>> +It covers, in particular:
>> +
>> +1. the used language extensions;
>> +2. the translation limits that the translation toolchains must be able
>> +   to accommodate;
>> +3. the implementation-defined behaviors upon which Xen may depend.
>> +
>> +All points are of course relevant for portability.  In addition,
>> +programming in C is impossible without a detailed knowledge of the
>> +implementation-defined behaviors.  For this reason, it is recommended
>> +that Xen developers have familiarity with this document and the
>> +documentation referenced therein.
>> +
>> +This document needs maintenance and adaptation in the following
>> +circumstances:
>> +
>> +- whenever the compiler is changed or updated;
>> +- whenever the use of a certain language extension is added or removed;
>> +- whenever code modifications cause exceeding the stated translation limits.
>> +
>> +
>> +Applicable C Language Standard
>> +______________________________
>> +
>> +Xen is written in C99 with extensions.  The relevant ISO standard is
>> +
>> +    *ISO/IEC 9899:1999/Cor 3:2007*: Programming Languages - C,
>> +    Technical Corrigendum 3.
>> +    ISO/IEC, Geneva, Switzerland, 2007.
>> +
>> +
>> +Reference Documentation
>> +_______________________
>> +
>> +The following documents are referred to in the sequel:
>> +
>> +GCC_MANUAL:
>> +  https://gcc.gnu.org/onlinedocs/gcc-12.1.0/gcc.pdf
>> +CPP_MANUAL:
>> +  https://gcc.gnu.org/onlinedocs/gcc-12.1.0/cpp.pdf

Why 12.1 when meanwhile there's 12.3 and 13.1?

>> +ARM64_ABI_MANUAL:
>> +  
>> https://github.com/ARM-software/abi-aa/blob/60a8eb8c55e999d74dac5e368fc9d7e36e38dda4/aapcs64/aapcs64.rst
>> +X86_64_ABI_MANUAL:
>> +  
>> https://gitlab.com/x86-psABIs/x86-64-ABI/-/jobs/artifacts/master/raw/x86-64-ABI/abi.pdf?job=build
>> +ARM64_LIBC_MANUAL:
>> +  https://www.gnu.org/software/libc/manual/pdf/libc.pdf
>> +X86_64_LIBC_MANUAL:
>> +  https://www.gnu.org/software/libc/manual/pdf/libc.pdf

How is libc relevant to the hypervisor?

>> +   * - Empty declaration
>> +     - ARM64, X86_64
>> +     - Non-documented GCC extension.
> 
> For the non-documented GCC extensions, would it be possible to add a
> very brief example or a couple of words in the "References" sections?
> Otherwise I think people might not understand what we are talking about.
> 
> For instance in this case I would say:
> 
> An empty declaration is a semicolon with nothing before it.
> Non-documented GCC extension.

Which then could be confused with empty statements. I think in a document
like this language needs to be very precise, to avoid ambiguities and
confusion as much as possible. (Iirc from going over this doc yesterday
this applies elsewhere as well.)

>> +   * - 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?

+1

>> +
>> +   * - 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?

This exists in the __ASSEMBLY__ part of a header, and I had previously
commented on Roberto's diagnosis (possibly derived from Eclair's) here.
As per that I don't think the item should be here, but I'm of course
open to be shown that my understanding of translation phases is wrong.

>> +   * - Non-standard type
> 
> Should we call it "128-bit Integers" ?

Or better more generally "Extended integer types" (or something along
these lines, i.e. as these are called in the spec)?

Jan