[edk2-devel] [PATCH v3 08/15] MdeModulePkg: Fix constructor invocation ordering

[Ard Biesheuvel]

On 6/26/20 3:22 PM, Laszlo Ersek wrote:
> On 06/25/20 15:51, Ard Biesheuvel wrote:
>> On 6/24/20 3:34 PM, Sami Mujawar wrote:
>>> The BaseSerialPortLib16550 library does not implement
>>> a constructor. This prevents the correct constructor
>>> invocation order for dependent libraries.
>>> e.g. A PlatformHookLib (for the Serial Port) may have
>>> a dependency on retrieving data from a Hob. A Hob
>>> library implementation may configure its initial state
>>> in the HobLib constructor. Since BaseSerialPortLib16550
>>> does not implement a constructor, the Basetools do not
>>> resolve the correct order for constructor invocation.
>>>
>>> To fix this, add an empty constructor to the serial port
>>> library BaseSerialPortLib16550.
>>>
>>> Signed-off-by: Sami Mujawar <sami.mujawar at arm.com>
>>
>> Acked-by: Ard Biesheuvel <ard.biesheuvel at arm.com>
>>
>> Note to maintainers:
>> this works around a long standing BaseTools bug where constructor
>> dependencies do not propagate through a library that lacks a constructor.
>>
>> For instance, in the following case
>>
>>    LibA depends on LibB depends on LibC
>>
>> the constructors are *only* guaranteed to be invoked in the correct
>> order (LibC, then LibB, then LibA) if LibB has a constructor in the
>> first place, otherwise, the remaining constructors for LibA and LibC
>> could be emitted in any order.
> 
> By now I've flipped my "mental default" to "use a constructor *unless*
> counter-indicated by something"...
> 
> If we get a cycle due to always using constructors, the tools at least
> complain (and we know something's fishy). With the opposite default, I
> simply cannot guarantee that my new library instance LibB will *never*
> break an eventual LibA -> LibB -> LibC constructor dependency chain.
> 
> In my new lib instance, I'm of course aware of the LibB -> LibC
> dependency, but I can't tell anything about a future LibA -> LibB
> dependency. :(
> 
> So I guess it's prudent to always add a constructor, even if it's empty.
> 

Indeed.

I guess the issue arises when LibA's constructor calls a function 
exposed by LibB. The fact that we care about constructor ordering at all 
implies that constructor implementations are permitted to depend on 
library routines, and so this should be perfectly fine. So it is really 
up to the BaseTools to ensure that calling LibB is safe at this point, 
which implies that the must constructors have been called for anything 
that LibB depends on.

So I don't see how this is not a bug - ordering is either enforced or it 
isn't, but the dependency graph should not omit libraries without 
constructors.




-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#61759): https://edk2.groups.io/g/devel/message/61759
Mute This Topic: https://groups.io/mt/75081484/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-