Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Fix VS2019 build error because of implicit cast

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

On 04/01/20 10:50, Ard Biesheuvel wrote:
> On Wed, 1 Apr 2020 at 10:37, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 04/01/20 00:17, Liran Alon wrote:
>>
>>> I would also mention that there are some bizzare code in EDK2 that
>>> defines it's own ASSERT() macro that just does CpuDeadLoop(). E.g.
>>> ArmVirtPkg/Library/ArmVirtDxeHobLib/HobLib.c
>>
>> This is a very special case.
>>
>> Please see the justification in commit ad90df8ac018
>> ("ArmPlatformPkg/ArmVirtualizationPkg: Add private HobLib implementation
>> for DXE phase", 2014-09-18).
>>
>> The stock HobLib instance depends on DebugLib, for using the normal
>> ASSERT() macro.
>>
>> Furthermore, the stock serial-based DebugLib instance depends on
>> SerialPortLib, for printing messages.
>>
>> That produces a HobLib -> DebugLib -> SerialPortLib dependency chain.
>>
>> But, in case of this particular platform, our SerialPortLib instance
>> depends on HobLib, for retrieving the particulars of the serial port.
>> This creates a dependency cycle:
>>
>>   HobLib -> DebugLib -> SerialPortLib -> HobLib
>>
>> which makes the platform un-buildable.
>>
>> We had to break the dependency cycle somewhere, and the best (or maybe
>> only -- I don't recall exactly anymore) link to break was the HobLib ->
>> DebugLib dependency. We introduced our own HobLib instance, which (IIRC)
>> was almost identical to the stock one, except that its (only) DebugLib
>> dependency, namely the ASSERT(), was reimplemented with a plain
>> CpuDeadLoop(). And so the dependency chain ended up as:
>>
>>   DebugLib -> SerialPortLib -> HobLib
>>
>> Not circular any more.
>>
>>> and OvmfPkg/Library/XenConsoleSerialPortLib/XenConsoleSerialPortLib.c)
>>
>> Very similar same case; here we even have a comment:
>>
>> //
>> // We can't use DebugLib due to a constructor dependency cycle between DebugLib
>> // and ourselves.
>> //
>>
>> The BaseDebugLibSerialPort instance depends on SerialPortLib, so a
>> SerialPortLib instance cannot "depend back" on DebugLib, in combination
>> with BaseDebugLibSerialPort.
>>
> 
> There's an additional problem in EDK2 that we never fixed, which is
> the fact that constructor dependencies are not transitive across
> library implementations that don't have a constructor themselves. For
> instance, in the following case
> 
> LibA (+)
>   depends on
> LibB (-)
>   depends on
> LibC (+)
> 
> where (+) means 'has constructor' and (-) means 'has no constructor',
> the EDK2 build tools may emit the LibA and LibC constructor
> invocations in any order. However, as soon as you try to fix this, we
> end up with circular dependencies all over the place, and none of the
> platforms can be built anymore.
> 

Yes, I've been aware of this; I've thought about it for a few minutes
just the other day. I was considering yet another "dynamic PCD setter"
NULL-class library instance, to be linked into various DXE modules.
Given the above problem, I figured if I ever started work on said lib
instance, I'd probably give it an empty CONSTRUCTOR -- not because that
function would have to do anything, but because the lib instance would
likely depend on other libraries, and *those* lib instances might have
important CONSTRUCTORs. And I wouldn't want this new lib instance to
break the dependency chain in the middle.

So in practice, all new lib instances seem to need CONSTRUCTORs now
(even if they are empty), just to avoid the problem you mention. And if
there is a build failure (circular dependency), try to deal with that on
a case by case basis.

(I admit that I didn't remember that we had tried fixing the problem in
BaseTools, and given up because of a multitude of sudden circular
dependencies! Thanks for the reminder!)

Thanks!
Laszlo