Re: [PATCH] OvmfPkg/QemuKernelLoaderFsDxe: drop tentative const object definition

["Laszlo Ersek" <lersek@redhat.com>]

On 03/06/20 17:40, Ard Biesheuvel wrote:
> On Fri, 6 Mar 2020 at 17:14, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 03/06/20 08:38, Ard Biesheuvel wrote:
>>> Bob reports that VS2017 chokes on a tentative definition of the const
>>> object 'mEfiFileProtocolTemplate', with the following error:
>>>
>>>   OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130):
>>>       error C2220: warning treated as error - no 'object' file generated
>>>   OvmfPkg\QemuKernelLoaderFsDxe\QemuKernelLoaderFsDxe.c(130):
>>>       warning C4132: 'mEfiFileProtocolTemplate': const object should be initialized
>>>
>>> Let's turn the only function that relies on this tentative definition
>>> into a forward declaration itself, and move its definition after the
>>> normal definition of the object. That allows us to drop the tentative
>>
>> (1) s/normal/external/
>>
>
> Are you sure? The const object has static linkage.

Yes, I'm sure.

- Linkage can be external, internal, or none.

- Storage duration can be static, automatic, or allocated.

- Type qualifiers are: const, restrict, and volatile.

Quoting ISO C99 "6.9 External definitions" (as background):

> Constraints
>
> 3 There shall be no more than one external definition for each
>   identifier declared with internal linkage in a translation unit.
>   Moreover, if an identifier declared with internal linkage is used in
>   an expression (other than as a part of the operand of a sizeof
>   operator whose result is an integer constant), there shall be
>   exactly one external definition for the identifier in the
>   translation unit.
>
> Semantics
>
> 4 As discussed in 5.1.1.1, the unit of program text after
>   preprocessing is a translation unit, which consists of a sequence of
>   external declarations. These are described as "external" because
>   they appear outside any function (and hence have file scope). As
>   discussed in 6.7, a declaration that also causes storage to be
>   reserved for an object or a function named by the identifier is a
>   definition.
>
> 5 An external definition is an external declaration that is also a
>   definition of a function (other than an inline definition) or an
>   object. If an identifier declared with external linkage is used in
>   an expression (other than as part of the operand of a sizeof
>   operator whose result is an integer constant), somewhere in the
>   entire program there shall be exactly one external definition for
>   the identifier; otherwise, there shall be no more than one. 140)
>
> Footnote 140: Thus, if an identifier declared with external linkage is
>               not used in an expression, there need be no external
>               definition for it.

Furthermore, from "6.9.2 External object definitions":

> Semantics
>
> 1 If the declaration of an identifier for an object has file scope and
>   an initializer, the declaration is an external definition for the
>   identifier.
>
> 2 A declaration of an identifier for an object that has file scope
>   without an initializer, and without a storage-class specifier or
>   with the storage-class specifier static, constitutes a tentative
>   definition. If a translation unit contains one or more tentative
>   definitions for an identifier, and the translation unit contains no
>   external definition for that identifier, then the behavior is
>   exactly as if the translation unit contains a file scope declaration
>   of that identifier, with the composite type as of the end of the
>   translation unit, with an initializer equal to 0.
>
> 3 If the declaration of an identifier for an object is a tentative
>   definition and has internal linkage, the declared type shall not be
>   an incomplete type.

In the original code, the first

  STATIC CONST EFI_FILE_PROTOCOL mEfiFileProtocolTemplate;

is a tentative definition, per 6.9.2p2, because:

- it has file scope,
- it has no initializer,
- it has storage-class specifier "static".

Note that type qualifiers (such as "const") are not relevant for this.
(Which is why the error message from VS2017 is a bug.)

Paragraph 6.9.2p3 also applies:

- the linkage is internal [*],

- the type is *not* incomplete, so we do satisfy paragraph 3.

(

[*] The linkage is internal per "6.2.2 Linkages of identifiers":

> 3 If the declaration of a file scope identifier for an object [...]
>   contains the storage-class specifier static, the identifier has
>   internal linkage. [...]

)

And, finally, to answer your question,

  STATIC CONST EFI_FILE_PROTOCOL mEfiFileProtocolTemplate = {...};

is an external declaration per 6.9.2p1:

- file scope: check,
- initializer: check,
- linkage: does not matter.

The confusion arises because "external" is used in two senses: once for
linkage, and another time for scope (file scope). But linkage and scope
are different concepts.


Unfortunately, these bits an pieces are quite scattered all over the
language standard. Which is why I had prepared the following handy table
several years -- more then a decade -- ago, as a reference for myself.

Note that the chapter and paragraph locations in the table are still
based on the C89 standard; I have never bothered to update them to C99.

Feel free to keep it for further reference, if you feel you can trust it
:)

The first two columns constitute the "input", i.e., they let you match a
declaration "pattern", and the scope (file or block) where the
declaration occurs.

Then the other three ("output") columns tell you what the linkage,
storage duration, and definition (if any) of the declaration are.

> Name space: ordinary identifiers
>
> Declaration             | Scope | Linkage                                                            | Storage duration       | Definition
> ------------------------+-------+--------------------------------------------------------------------+------------------------+--------------------------
>        int obj_1;       | file  | external ((6.1.2.2 p5)                                             | static (6.1.2.4 p2)    | tentative (6.7.2 p2)
> extern int obj_2;       | file  | same as any visible file scope declaration / external (6.1.2.2 p4) | static (6.1.2.4 p2)*   | none (6.7.2)
> static int obj_3;       | file  | internal (6.1.2.2 p3)                                              | static (6.1.2.4 p2)    | tentative (6.7.2 p2)
>        int obj_4 = 1;   | file  | external ((6.1.2.2 p5)                                             | static (6.1.2.4 p2)    | external (6.7.2 p1)
> extern int obj_5 = 1;   | file  | same as any visible file scope declaration / external (6.1.2.2 p4) | static (6.1.2.4 p2)*   | external (6.7.2 p1)
> static int obj_6 = 1;   | file  | internal (6.1.2.2 p3)                                              | static (6.1.2.4 p2)    | external (6.7.2 p1)
>        int fun_1(void); | file  | same as any visible file scope declaration / external (6.1.2.2 p5) | n/a (6.1.2.4)          | none (6.5 p4 fn56, 6.7.1)
> extern int fun_2(void); | file  | same as any visible file scope declaration / external (6.1.2.2 p4) | n/a (6.1.2.4)          | none (6.5 p4 fn56, 6.7.1)
> static int fun_3(void); | file  | internal (6.1.2.2 p3)                                              | n/a (6.1.2.4)          | none (6.5 p4 fn56, 6.7.1)
> ------------------------+-------+--------------------------------------------------------------------+------------------------+--------------------------
>        int obj_7;       | block | none (6.1.2.2 p6)                                                  | automatic (6.1.2.4 p3) | yes (6.1.2.4 p3, 6.5 p4)
> extern int obj_8;       | block | same as any visible file scope declaration / external (6.1.2.2 p4) | static (6.1.2.4 p2)*   | none (6.7.2)
> static int obj_9;       | block | none (6.1.2.2 p6)                                                  | static (6.1.2.4 p2)    | yes (6.1.2.4 p2, 6.5 p4)
>        int obj_10 = 1;  | block | none (6.1.2.2 p6)                                                  | automatic (6.1.2.4 p3) | yes (6.1.2.4 p3, 6.5 p4)
> extern int obj_11 = 1;  | block | INVALID (6.5.7 p4)**                                               | INVALID                | INVALID
> static int obj_12 = 1;  | block | none (6.1.2.2 p6)                                                  | static (6.1.2.4 p2)    | yes (6.1.2.4 p2, 6.5 p4)
>        int fun_4(void); | block | same as any visible file scope declaration / external (6.1.2.2 p5) | n/a (6.1.2.4)          | none (6.5 p4 fn56, 6.7.1)
> extern int fun_5(void); | block | same as any visible file scope declaration / external (6.1.2.2 p4) | n/a (6.1.2.4)          | none (6.5 p4 fn56, 6.7.1)
> static int fun_6(void); | block | INVALID (6.1.2.2 p3 fn13, 6.5.1 p4)                                | INVALID                | INVALID
>
> * any visible file scope declaration can only specify either external or internal linkage
>
> ** such a declaration would specify "same as any visible file scope declaration / external (6.1.2.2 p4)" linkage, that is, external or internal (see *)

Using this table,

  STATIC CONST EFI_FILE_PROTOCOL mEfiFileProtocolTemplate;

is matched by "obj_3" in the table:

- declaration:
  static int obj_3;

- scope:
  file

and

  STATIC CONST EFI_FILE_PROTOCOL mEfiFileProtocolTemplate = {...};

is matched by "obj_6" in the table:

- declaration:
  static int obj_6 = 1;

- scope:
  file

In both cases (obj_3, obj_6), we have internal linkage, and static
storage duration.

In the first case (obj_3), we have a tentative definition.

In the second case (obj_6), we have an external definition.

... Since we're talking cheat sheets, here's another one I like to refer
to, for operator binding and associativity (I typed this up in 2004):

> operator                                    associativity
> ---------------------------------------------------------
> ()  []  .  ->                               left to right
> !  ~  -  ++  --  &  *  (type)  sizeof       right to left
> *  /  %                                     left to right
> +  -                                        left to right
> <<  >>                                      left to right
> <  <=  >  >=                                left to right
> ==  !=                                      left to right
> &                                           left to right
> ^                                           left to right
> |                                           left to right
> &&                                          left to right
> ||                                          left to right
> ?:                                          right to left
> =  *=  /=  %=  +=  -=  <<= >>=  &=  ^=  |=  right to left
> ,                                           left to right

Operators near the top of the table bind more strongly.

Thanks,
Laszlo