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@arm.com>
---
MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c | 17 +++++++++++++++++
MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf | 3 +++
2 files changed, 20 insertions(+)
diff --git a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
index 9cb50dd80d5634ab2aa6d68bf5ca7fb891463eef..0fd1382ee83c9de09d8250830bd9569056fcee2f 100644
--- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
+++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
@@ -4,6 +4,7 @@
(C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
Copyright (c) 2018, AMD Incorporated. All rights reserved.<BR>
+ Copyright (c) 2020, ARM Limited. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -1102,3 +1103,19 @@ SerialPortSetAttributes (
return RETURN_SUCCESS;
}
+/** Base Serial Port 16550 Library Constructor
+
+ @retval RETURN_SUCCESS Success.
+*/
+EFI_STATUS
+EFIAPI
+BaseSerialPortLib16550 (
+ VOID
+ )
+{
+ // Nothing to do here. This constructor is added to
+ // enable the chain of constructor invocation for
+ // dependent libraries.
+ return RETURN_SUCCESS;
+}
+
diff --git a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
index 8b4ae3f1d4ee1e2e9a8b81eab4c900541ce8cfb6..92b7a8b7896a305d2ce22589f8a9593618d37bb7 100644
--- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
+++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
@@ -2,6 +2,8 @@
# SerialPortLib instance for 16550 UART.
#
# Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2020, ARM Limited. All rights reserved.
+#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
##
@@ -14,6 +16,7 @@ [Defines]
MODULE_TYPE = BASE
VERSION_STRING = 1.1
LIBRARY_CLASS = SerialPortLib
+ CONSTRUCTOR = BaseSerialPortLib16550
[Packages]
MdePkg/MdePkg.dec
--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61670): https://edk2.groups.io/g/devel/message/61670
Mute This Topic: https://groups.io/mt/75081484/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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@arm.com>
Acked-by: Ard Biesheuvel <ard.biesheuvel@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.
> ---
> MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c | 17 +++++++++++++++++
> MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf | 3 +++
> 2 files changed, 20 insertions(+)
>
> diff --git a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> index 9cb50dd80d5634ab2aa6d68bf5ca7fb891463eef..0fd1382ee83c9de09d8250830bd9569056fcee2f 100644
> --- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> +++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.c
> @@ -4,6 +4,7 @@
> (C) Copyright 2014 Hewlett-Packard Development Company, L.P.<BR>
> Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> Copyright (c) 2018, AMD Incorporated. All rights reserved.<BR>
> + Copyright (c) 2020, ARM Limited. All rights reserved.
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -1102,3 +1103,19 @@ SerialPortSetAttributes (
> return RETURN_SUCCESS;
> }
>
> +/** Base Serial Port 16550 Library Constructor
> +
> + @retval RETURN_SUCCESS Success.
> +*/
> +EFI_STATUS
> +EFIAPI
> +BaseSerialPortLib16550 (
> + VOID
> + )
> +{
> + // Nothing to do here. This constructor is added to
> + // enable the chain of constructor invocation for
> + // dependent libraries.
> + return RETURN_SUCCESS;
> +}
> +
> diff --git a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> index 8b4ae3f1d4ee1e2e9a8b81eab4c900541ce8cfb6..92b7a8b7896a305d2ce22589f8a9593618d37bb7 100644
> --- a/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> +++ b/MdeModulePkg/Library/BaseSerialPortLib16550/BaseSerialPortLib16550.inf
> @@ -2,6 +2,8 @@
> # SerialPortLib instance for 16550 UART.
> #
> # Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
> +# Copyright (c) 2020, ARM Limited. All rights reserved.
> +#
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> ##
> @@ -14,6 +16,7 @@ [Defines]
> MODULE_TYPE = BASE
> VERSION_STRING = 1.1
> LIBRARY_CLASS = SerialPortLib
> + CONSTRUCTOR = BaseSerialPortLib16550
>
> [Packages]
> MdePkg/MdePkg.dec
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#61719): https://edk2.groups.io/g/devel/message/61719
Mute This Topic: https://groups.io/mt/75081484/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
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@arm.com> > > Acked-by: Ard Biesheuvel <ard.biesheuvel@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. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#61743): https://edk2.groups.io/g/devel/message/61743 Mute This Topic: https://groups.io/mt/75081484/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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@arm.com> >> >> Acked-by: Ard Biesheuvel <ard.biesheuvel@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/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2026 Red Hat, Inc.