From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by mx.groups.io with SMTP id smtpd.web10.4515.1585741637896467107 for ; Wed, 01 Apr 2020 04:47:18 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=CjIVRewL; spf=pass (domain: redhat.com, ip: 205.139.110.61, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1585741636; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=9Gvr0iyHUSSNfdccsNl/DtrYKfkWX16oNE5rm+dl0CM=; b=CjIVRewLNAgf986np4ayac5OPZWdkD6r6uiSUHjdd6If3INnSOwgplT9ogNnTHdQb9Yov0 OwM5cTabUNmJwSjXvOgmoz5ZHa2x+1rxLYtDZgdNanLcOx9CJniSroM4/Q1uv5SyCg+my/ ISqPFd0/POBOltnqyGRfas7IAhSiLuE= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-4-l-CLkppMNhiRa3QtF_PFfw-1; Wed, 01 Apr 2020 07:47:14 -0400 X-MC-Unique: l-CLkppMNhiRa3QtF_PFfw-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A657F107ACCC; Wed, 1 Apr 2020 11:47:13 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-114-94.ams2.redhat.com [10.36.114.94]) by smtp.corp.redhat.com (Postfix) with ESMTP id 755605C1B0; Wed, 1 Apr 2020 11:47:12 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH] OvmfPkg/PvScsiDxe: Fix VS2019 build error because of implicit cast To: Ard Biesheuvel Cc: Liran Alon , edk2-devel-groups-io , Sean Brogan References: <20200331110452.51992-1-liran.alon@oracle.com> <22866.1585670000047029434@groups.io> <4b90b41a-2cf1-8b6e-6619-1c652820265e@redhat.com> <5221e324-9f86-24b4-56a0-301a948151f8@oracle.com> <30a57406-0c09-043f-6b19-7e6e94afcb44@oracle.com> <56339bff-ae66-2902-eb7b-3426a6b2cb7a@redhat.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 1 Apr 2020 13:47:11 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit On 04/01/20 10:50, Ard Biesheuvel wrote: > On Wed, 1 Apr 2020 at 10:37, Laszlo Ersek 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