[edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

Guomin Jiang posted 1 patch 4 years, 8 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/edk2 tags/patchew/20200330085247.2415-1-guomin.jiang@intel.com
CryptoPkg/CryptoPkg.dsc                       |  2 ++
.../Library/BaseCryptLib/BaseCryptLib.inf     |  1 +
.../Library/BaseCryptLib/PeiCryptLib.inf      |  1 +
.../Library/BaseCryptLib/RuntimeCryptLib.inf  |  1 +
.../Library/BaseCryptLib/SmmCryptLib.inf      |  1 +
CryptoPkg/Library/FltUsedLib/FltUsedLib.c     | 14 ++++++++
CryptoPkg/Library/FltUsedLib/FltUsedLib.inf   | 33 +++++++++++++++++++
CryptoPkg/Library/TlsLib/TlsLib.inf           |  1 +
8 files changed, 54 insertions(+)
create mode 100644 CryptoPkg/Library/FltUsedLib/FltUsedLib.c
create mode 100644 CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
[edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Guomin Jiang 4 years, 8 months ago
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2596

OpenSSL requires _fltused to be defined as a constant anywhere floating
point is used.
This is to satisfy the linker, however, it is possible to compile a
module with multiple definitions of fltused. This causes the
MSVC compiler to fail the build.
To solve this problem, the FltUsedLib was created that is one spot
that the global static can exist.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
---
 CryptoPkg/CryptoPkg.dsc                       |  2 ++
 .../Library/BaseCryptLib/BaseCryptLib.inf     |  1 +
 .../Library/BaseCryptLib/PeiCryptLib.inf      |  1 +
 .../Library/BaseCryptLib/RuntimeCryptLib.inf  |  1 +
 .../Library/BaseCryptLib/SmmCryptLib.inf      |  1 +
 CryptoPkg/Library/FltUsedLib/FltUsedLib.c     | 14 ++++++++
 CryptoPkg/Library/FltUsedLib/FltUsedLib.inf   | 33 +++++++++++++++++++
 CryptoPkg/Library/TlsLib/TlsLib.inf           |  1 +
 8 files changed, 54 insertions(+)
 create mode 100644 CryptoPkg/Library/FltUsedLib/FltUsedLib.c
 create mode 100644 CryptoPkg/Library/FltUsedLib/FltUsedLib.inf

diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
index 4cb37b1349..e9854087b5 100644
--- a/CryptoPkg/CryptoPkg.dsc
+++ b/CryptoPkg/CryptoPkg.dsc
@@ -97,6 +97,7 @@
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf                                          #???
   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+  FltUsedLib|CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
 
 [LibraryClasses.ARM]
@@ -232,6 +233,7 @@
   CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
   CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf
   CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
+  CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
   CryptoPkg/Library/TlsLib/TlsLib.inf
   CryptoPkg/Library/TlsLibNull/TlsLibNull.inf
   CryptoPkg/Library/OpensslLib/OpensslLib.inf
diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
index 1bbe4f435a..c82e703aa0 100644
--- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -84,6 +84,7 @@
   DebugLib
   OpensslLib
   IntrinsicLib
+  FltUsedLib
   PrintLib
 
 #
diff --git a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
index c836c257f8..342aad008c 100644
--- a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
@@ -78,6 +78,7 @@
   DebugLib
   OpensslLib
   IntrinsicLib
+  FltUsedLib
 
 #
 # Remove these [BuildOptions] after this library is cleaned up
diff --git a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
index bff308a4f5..dcf209d7f5 100644
--- a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
@@ -89,6 +89,7 @@
   DebugLib
   OpensslLib
   IntrinsicLib
+  FltUsedLib
   PrintLib
 
 #
diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
index cc0b65fd25..7fdaaa48a6 100644
--- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
@@ -88,6 +88,7 @@
   MemoryAllocationLib
   OpensslLib
   IntrinsicLib
+  FltUsedLib
   PrintLib
 
 #
diff --git a/CryptoPkg/Library/FltUsedLib/FltUsedLib.c b/CryptoPkg/Library/FltUsedLib/FltUsedLib.c
new file mode 100644
index 0000000000..8f2487ea13
--- /dev/null
+++ b/CryptoPkg/Library/FltUsedLib/FltUsedLib.c
@@ -0,0 +1,14 @@
+/** @file
+  Need include this when use floats.
+
+  Copyright (C) Microsoft Corporation. All rights reserved.<BR>
+  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+//
+// You need to include this to let the compiler know we are going to use
+// floating point
+//
+int _fltused = 0x9875;
diff --git a/CryptoPkg/Library/FltUsedLib/FltUsedLib.inf b/CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
new file mode 100644
index 0000000000..8d67eadfa5
--- /dev/null
+++ b/CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
@@ -0,0 +1,33 @@
+## @file
+# It is depent on when using floats
+#
+# Copyright (C) Microsoft Corporation. All rights reserved.<BR>
+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = FltUsedLib
+  FILE_GUID                      = C004F180-9FE2-4D2B-8318-BADC2A231774
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = FltUsedLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 AARCH64
+#
+
+[Sources]
+  FltUsedLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+
+[BuildOptions]
+  # Disable GL due to linker error LNK1237
+  # https://docs.microsoft.com/en-us/cpp/error-messages/tool-errors/linker-tools-error-lnk1237?view=vs-2017
+  MSFT:*_*_*_CC_FLAGS = /GL-
diff --git a/CryptoPkg/Library/TlsLib/TlsLib.inf b/CryptoPkg/Library/TlsLib/TlsLib.inf
index 2f3ce695c3..febbdf5149 100644
--- a/CryptoPkg/Library/TlsLib/TlsLib.inf
+++ b/CryptoPkg/Library/TlsLib/TlsLib.inf
@@ -37,6 +37,7 @@
   BaseMemoryLib
   DebugLib
   IntrinsicLib
+  FltUsedLib
   MemoryAllocationLib
   OpensslLib
   SafeIntLib
-- 
2.25.1.windows.1


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

View/Reply Online (#56613): https://edk2.groups.io/g/devel/message/56613
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Ard Biesheuvel 4 years, 8 months ago
On Mon, 30 Mar 2020 at 10:52, Guomin Jiang <guomin.jiang@intel.com> wrote:
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2596
>
> OpenSSL requires _fltused to be defined as a constant anywhere floating
> point is used.
> This is to satisfy the linker, however, it is possible to compile a
> module with multiple definitions of fltused. This causes the
> MSVC compiler to fail the build.
> To solve this problem, the FltUsedLib was created that is one spot
> that the global static can exist.
>
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>

Doesn't this affect *every* platform? Isn't there a better way to do
this? E.g., using weak linkage?

> ---
>  CryptoPkg/CryptoPkg.dsc                       |  2 ++
>  .../Library/BaseCryptLib/BaseCryptLib.inf     |  1 +
>  .../Library/BaseCryptLib/PeiCryptLib.inf      |  1 +
>  .../Library/BaseCryptLib/RuntimeCryptLib.inf  |  1 +
>  .../Library/BaseCryptLib/SmmCryptLib.inf      |  1 +
>  CryptoPkg/Library/FltUsedLib/FltUsedLib.c     | 14 ++++++++
>  CryptoPkg/Library/FltUsedLib/FltUsedLib.inf   | 33 +++++++++++++++++++
>  CryptoPkg/Library/TlsLib/TlsLib.inf           |  1 +
>  8 files changed, 54 insertions(+)
>  create mode 100644 CryptoPkg/Library/FltUsedLib/FltUsedLib.c
>  create mode 100644 CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
>
> diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
> index 4cb37b1349..e9854087b5 100644
> --- a/CryptoPkg/CryptoPkg.dsc
> +++ b/CryptoPkg/CryptoPkg.dsc
> @@ -97,6 +97,7 @@
>    IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf                                          #???
>    OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf
>    IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> +  FltUsedLib|CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
>    SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
>
>  [LibraryClasses.ARM]
> @@ -232,6 +233,7 @@
>    CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
>    CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf
>    CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf
> +  CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
>    CryptoPkg/Library/TlsLib/TlsLib.inf
>    CryptoPkg/Library/TlsLibNull/TlsLibNull.inf
>    CryptoPkg/Library/OpensslLib/OpensslLib.inf
> diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> index 1bbe4f435a..c82e703aa0 100644
> --- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> +++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> @@ -84,6 +84,7 @@
>    DebugLib
>    OpensslLib
>    IntrinsicLib
> +  FltUsedLib
>    PrintLib
>
>  #
> diff --git a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> index c836c257f8..342aad008c 100644
> --- a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> +++ b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> @@ -78,6 +78,7 @@
>    DebugLib
>    OpensslLib
>    IntrinsicLib
> +  FltUsedLib
>
>  #
>  # Remove these [BuildOptions] after this library is cleaned up
> diff --git a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> index bff308a4f5..dcf209d7f5 100644
> --- a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> +++ b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> @@ -89,6 +89,7 @@
>    DebugLib
>    OpensslLib
>    IntrinsicLib
> +  FltUsedLib
>    PrintLib
>
>  #
> diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> index cc0b65fd25..7fdaaa48a6 100644
> --- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> +++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
> @@ -88,6 +88,7 @@
>    MemoryAllocationLib
>    OpensslLib
>    IntrinsicLib
> +  FltUsedLib
>    PrintLib
>
>  #
> diff --git a/CryptoPkg/Library/FltUsedLib/FltUsedLib.c b/CryptoPkg/Library/FltUsedLib/FltUsedLib.c
> new file mode 100644
> index 0000000000..8f2487ea13
> --- /dev/null
> +++ b/CryptoPkg/Library/FltUsedLib/FltUsedLib.c
> @@ -0,0 +1,14 @@
> +/** @file
> +  Need include this when use floats.
> +
> +  Copyright (C) Microsoft Corporation. All rights reserved.<BR>
> +  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +//
> +// You need to include this to let the compiler know we are going to use
> +// floating point
> +//
> +int _fltused = 0x9875;
> diff --git a/CryptoPkg/Library/FltUsedLib/FltUsedLib.inf b/CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
> new file mode 100644
> index 0000000000..8d67eadfa5
> --- /dev/null
> +++ b/CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
> @@ -0,0 +1,33 @@
> +## @file
> +# It is depent on when using floats
> +#
> +# Copyright (C) Microsoft Corporation. All rights reserved.<BR>
> +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = FltUsedLib
> +  FILE_GUID                      = C004F180-9FE2-4D2B-8318-BADC2A231774
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = FltUsedLib
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 AARCH64
> +#
> +
> +[Sources]
> +  FltUsedLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +
> +[BuildOptions]
> +  # Disable GL due to linker error LNK1237
> +  # https://docs.microsoft.com/en-us/cpp/error-messages/tool-errors/linker-tools-error-lnk1237?view=vs-2017
> +  MSFT:*_*_*_CC_FLAGS = /GL-
> diff --git a/CryptoPkg/Library/TlsLib/TlsLib.inf b/CryptoPkg/Library/TlsLib/TlsLib.inf
> index 2f3ce695c3..febbdf5149 100644
> --- a/CryptoPkg/Library/TlsLib/TlsLib.inf
> +++ b/CryptoPkg/Library/TlsLib/TlsLib.inf
> @@ -37,6 +37,7 @@
>    BaseMemoryLib
>    DebugLib
>    IntrinsicLib
> +  FltUsedLib
>    MemoryAllocationLib
>    OpensslLib
>    SafeIntLib
> --
> 2.25.1.windows.1
>
>
> ------------
> Groups.io Links: You receive all messages sent to this group.
>
> View/Reply Online (#56613): https://edk2.groups.io/g/devel/message/56613
> Mute This Topic: https://groups.io/mt/72648022/1761188
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub  [ard.biesheuvel@linaro.org]
> ------------
>

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

View/Reply Online (#56615): https://edk2.groups.io/g/devel/message/56615
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Laszlo Ersek 4 years, 8 months ago
On 03/30/20 11:02, Ard Biesheuvel wrote:
> On Mon, 30 Mar 2020 at 10:52, Guomin Jiang <guomin.jiang@intel.com>
> wrote:
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2596
>>
>> OpenSSL requires _fltused to be defined as a constant anywhere
>> floating point is used.
>> This is to satisfy the linker, however, it is possible to compile a
>> module with multiple definitions of fltused. This causes the
>> MSVC compiler to fail the build.
>> To solve this problem, the FltUsedLib was created that is one spot
>> that the global static can exist.
>>
>> Cc: Jian J Wang <jian.j.wang@intel.com>
>> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
>> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
>
> Doesn't this affect *every* platform? Isn't there a better way to do
> this? E.g., using weak linkage?

We already have manually added files under
"CryptoPkg/Library/OpensslLib":

- ossl_store.c
- rand_pool.c
- rand_pool_noise.c
- rand_pool_noise_tsc.c

These files are then referenced in both OpensslLib.inf and
OpensslLibCrypto.inf, outside of the "Autogenerated files list".

In particular "ossl_store.c" looks like a good example -- it does
nothing, just provides a needed symbol.

The comment at <https://bugzilla.tianocore.org/show_bug.cgi?id=2596#c0>
states that _fltused is an OpenSSL requirement -- so why not move
_fltused into the edk2 openssl library instances, and even then, only
when building with MSVC?

BTW I don't think I understand the actual problem, from the bug report.
Matthew wrote, "it is possible to compile a module with multiple
definitions of fltused" -- I don't see how (and no example is provided),
assuming the module in question already uses IntrinsicLib.

In <https://bugzilla.tianocore.org/show_bug.cgi?id=2596#c5>, Sean
writes, "the reason we moved to a library to define this symbol was to
deal with two libraries within the same module.  If both libs defined it
then there were problems". -- And I don't understand why *either* of
those libraries defined _fltused at all; I think they should have only
dependend on InstrinsicLib, which already ensures there's exactly one
external definition of _fltused.

I just applied the following patch locally:

> diff --git a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> index 94fe341bec9d..6ae4c4c82ecf 100644
> --- a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> +++ b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> @@ -21,7 +21,9 @@ typedef UINTN  size_t;
>
>  /* OpenSSL will use floating point support, and C compiler produces the _fltused
>     symbol by default. Simply define this symbol here to satisfy the linker. */
> +#if 0
>  int  GLOBAL_USED _fltused = 1;
> +#endif
>
>  /* Sets buffers to a specified character */
>  void * memset (void *dest, int ch, size_t count)

and witnessed no build failures in my environment.

This external definition of "_fltused" comes from historical commit
97f98500c1d4 ("Add CryptoPkg (from UDK2010.UP3)", 2010-11-01), and has
been updated (tweaked) once since, in commit 933681b20844 ("CryptoPkg
IntrinsicLib: Make _fltused always be used", 2019-10-24), for
TianoCore#1603.

To me, even the initial addition (from 2010) seems incorrect.

Summary:

- I don't understand the problem. Please state it clearly, including
build platform, target (firmware) platform, toolchain, modules,
libraries, and so on.

- Assuming the _fltused external definition is needed in fact, I think
it should be moved into a C source file referenced by the OpenSSL INF
files. This will solve problems where some module depends on the
OpensslLib class, but not the IntrinsicLib class.

- And, this reference in the OpensslLib INF files should be toolchain
specific.

Adding a new lib *class* dependency to the CryptLib instances will break
*every* platform out there, which is especially incomprehensible given
that some platforms don't need _fltused *at all*. Please let's not make
this 9+ years old hack worse.

Thanks
Laszlo


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

View/Reply Online (#56679): https://edk2.groups.io/g/devel/message/56679
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Guomin Jiang 4 years, 8 months ago
Hi Laszlo,

Thanks for you spending time review the changes.

And I just want to present how to reproduce the build error.

When build OvmfPkgX64, you can encounter this issue with your local change. The error as below:
TcgPei.lib(AutoGen.obj) : error LNK2001: unresolved external symbol _fltused
c:\sourcecodes\tiano\Build\OvmfX64\DEBUG_VS2019\X64\SecurityPkg\Tcg\TcgPei\TcgPei\DEBUG\TcgPei.dll : fatal error LNK1120: 1 unresolved externals

The build command: build -p OvmfPkg\OvmfPkgX64.dsc -b DEBUG -t VS2019 -a X64 -D TPM_ENABLE
The code changes for reproducing this symptom:
-  int  GLOBAL_USED _fltused = 1;
+ //int  GLOBAL_USED _fltused = 1;
The machine: WIN10
The branch: edk2 master

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, March 31, 2020 3:05 AM
> To: devel@edk2.groups.io; ard.biesheuvel@linaro.org; Jiang, Guomin
> <guomin.jiang@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Sean Brogan
> <sean.brogan@microsoft.com>; macarl@microsoft.com
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for
> float.
> 
> On 03/30/20 11:02, Ard Biesheuvel wrote:
> > On Mon, 30 Mar 2020 at 10:52, Guomin Jiang <guomin.jiang@intel.com>
> > wrote:
> >>
> >> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2596
> >>
> >> OpenSSL requires _fltused to be defined as a constant anywhere
> >> floating point is used.
> >> This is to satisfy the linker, however, it is possible to compile a
> >> module with multiple definitions of fltused. This causes the MSVC
> >> compiler to fail the build.
> >> To solve this problem, the FltUsedLib was created that is one spot
> >> that the global static can exist.
> >>
> >> Cc: Jian J Wang <jian.j.wang@intel.com>
> >> Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> >> Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
> >
> > Doesn't this affect *every* platform? Isn't there a better way to do
> > this? E.g., using weak linkage?
> 
> We already have manually added files under
> "CryptoPkg/Library/OpensslLib":
> 
> - ossl_store.c
> - rand_pool.c
> - rand_pool_noise.c
> - rand_pool_noise_tsc.c
> 
> These files are then referenced in both OpensslLib.inf and
> OpensslLibCrypto.inf, outside of the "Autogenerated files list".
> 
> In particular "ossl_store.c" looks like a good example -- it does nothing, just
> provides a needed symbol.
> 
> The comment at <https://bugzilla.tianocore.org/show_bug.cgi?id=2596#c0>
> states that _fltused is an OpenSSL requirement -- so why not move _fltused
> into the edk2 openssl library instances, and even then, only when building
> with MSVC?
> 
> BTW I don't think I understand the actual problem, from the bug report.
> Matthew wrote, "it is possible to compile a module with multiple definitions
> of fltused" -- I don't see how (and no example is provided), assuming the
> module in question already uses IntrinsicLib.
> 
> In <https://bugzilla.tianocore.org/show_bug.cgi?id=2596#c5>, Sean writes,
> "the reason we moved to a library to define this symbol was to deal with two
> libraries within the same module.  If both libs defined it then there were
> problems". -- And I don't understand why *either* of those libraries defined
> _fltused at all; I think they should have only dependend on InstrinsicLib,
> which already ensures there's exactly one external definition of _fltused.
> 
> I just applied the following patch locally:
> 
> > diff --git a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> > b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> > index 94fe341bec9d..6ae4c4c82ecf 100644
> > --- a/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> > +++ b/CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c
> > @@ -21,7 +21,9 @@ typedef UINTN  size_t;
> >
> >  /* OpenSSL will use floating point support, and C compiler produces the
> _fltused
> >     symbol by default. Simply define this symbol here to satisfy the
> > linker. */
> > +#if 0
> >  int  GLOBAL_USED _fltused = 1;
> > +#endif
> >
> >  /* Sets buffers to a specified character */  void * memset (void
> > *dest, int ch, size_t count)
> 
> and witnessed no build failures in my environment.
> 
> This external definition of "_fltused" comes from historical commit
> 97f98500c1d4 ("Add CryptoPkg (from UDK2010.UP3)", 2010-11-01), and has
> been updated (tweaked) once since, in commit 933681b20844 ("CryptoPkg
> IntrinsicLib: Make _fltused always be used", 2019-10-24), for
> TianoCore#1603.
> 
> To me, even the initial addition (from 2010) seems incorrect.
> 
> Summary:
> 
> - I don't understand the problem. Please state it clearly, including build
> platform, target (firmware) platform, toolchain, modules, libraries, and so on.
> 
> - Assuming the _fltused external definition is needed in fact, I think it should
> be moved into a C source file referenced by the OpenSSL INF files. This will
> solve problems where some module depends on the OpensslLib class, but
> not the IntrinsicLib class.
> 
> - And, this reference in the OpensslLib INF files should be toolchain specific.
> 
> Adding a new lib *class* dependency to the CryptLib instances will break
> *every* platform out there, which is especially incomprehensible given that
> some platforms don't need _fltused *at all*. Please let's not make this 9+
> years old hack worse.
> 
> Thanks
> Laszlo


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

View/Reply Online (#56704): https://edk2.groups.io/g/devel/message/56704
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Ard Biesheuvel 4 years, 8 months ago
On Tue, 31 Mar 2020 at 03:40, Jiang, Guomin <guomin.jiang@intel.com> wrote:
>
> Hi Laszlo,
>
> Thanks for you spending time review the changes.
>
> And I just want to present how to reproduce the build error.
>
> When build OvmfPkgX64, you can encounter this issue with your local change. The error as below:
> TcgPei.lib(AutoGen.obj) : error LNK2001: unresolved external symbol _fltused
> c:\sourcecodes\tiano\Build\OvmfX64\DEBUG_VS2019\X64\SecurityPkg\Tcg\TcgPei\TcgPei\DEBUG\TcgPei.dll : fatal error LNK1120: 1 unresolved externals
>
> The build command: build -p OvmfPkg\OvmfPkgX64.dsc -b DEBUG -t VS2019 -a X64 -D TPM_ENABLE
> The code changes for reproducing this symptom:
> -  int  GLOBAL_USED _fltused = 1;
> + //int  GLOBAL_USED _fltused = 1;
> The machine: WIN10
> The branch: edk2 master
>

Doesn't the build error go away as well if you simply add FloatUsed.c
to all BaseCryptLib flavours if Visual Studio is being used?

Something like

diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
index 1bbe4f435aec..f40bf18e7f5d 100644
--- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -27,6 +27,7 @@ [Defines]
 #

 [Sources]
+  FloatUsed.c         | MSFT
   InternalCryptLib.h
   Hash/CryptMd4.c
   Hash/CryptMd5.c

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

View/Reply Online (#56725): https://edk2.groups.io/g/devel/message/56725
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Laszlo Ersek 4 years, 8 months ago
On 03/31/20 09:13, Ard Biesheuvel wrote:
> On Tue, 31 Mar 2020 at 03:40, Jiang, Guomin <guomin.jiang@intel.com> wrote:
>>
>> Hi Laszlo,
>>
>> Thanks for you spending time review the changes.
>>
>> And I just want to present how to reproduce the build error.
>>
>> When build OvmfPkgX64, you can encounter this issue with your local change. The error as below:
>> TcgPei.lib(AutoGen.obj) : error LNK2001: unresolved external symbol _fltused
>> c:\sourcecodes\tiano\Build\OvmfX64\DEBUG_VS2019\X64\SecurityPkg\Tcg\TcgPei\TcgPei\DEBUG\TcgPei.dll : fatal error LNK1120: 1 unresolved externals
>>
>> The build command: build -p OvmfPkg\OvmfPkgX64.dsc -b DEBUG -t VS2019 -a X64 -D TPM_ENABLE
>> The code changes for reproducing this symptom:
>> -  int  GLOBAL_USED _fltused = 1;
>> + //int  GLOBAL_USED _fltused = 1;
>> The machine: WIN10
>> The branch: edk2 master
>>
> 
> Doesn't the build error go away as well if you simply add FloatUsed.c
> to all BaseCryptLib flavours if Visual Studio is being used?
> 
> Something like
> 
> diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> index 1bbe4f435aec..f40bf18e7f5d 100644
> --- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> +++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> @@ -27,6 +27,7 @@ [Defines]
>  #
> 
>  [Sources]
> +  FloatUsed.c         | MSFT
>    InternalCryptLib.h
>    Hash/CryptMd4.c
>    Hash/CryptMd5.c
> 

This is *exactly* what I've had in mind!

(With the additional (optional) tweak that IMO "FloatUsed.c" belongs
with the openssl INF files, as those are where the floating point usage
originates from. The cryptlib instances themselves have no floating
point code, AIUI.)

Thanks
Laszlo


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

View/Reply Online (#56761): https://edk2.groups.io/g/devel/message/56761
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Matthew Carlson via Groups.Io 4 years, 8 months ago
So it's not required by OpenSSL, it's required by the compiler whenever floating point is used, which can be in multiple places. For example, this is used in mu_plus (the Microsoft UEFI value add to EDK2) by the OnScreenKeyboard driver as well as the UiToolKit driver. If you happen to use BaseCryptLib or the intrinsic in a driver that happens to need crypto as well, it can break due to multiple.

I do agree, this only applies to MSVC and requiring every platform to add a line in their DSC would be a situation I would prefer to avoid if possible. Is there a way to specify a library dependency only if a toolchain is being used?
--
- Matthew Carlson

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

View/Reply Online (#56689): https://edk2.groups.io/g/devel/message/56689
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Ard Biesheuvel 4 years, 8 months ago
On Mon, 30 Mar 2020 at 23:29, Matthew Carlson via Groups.Io
<macarl=microsoft.com@groups.io> wrote:
>
> So it's not required by OpenSSL, it's required by the compiler whenever floating point is used, which can be in multiple places. For example, this is used in mu_plus (the Microsoft UEFI value add to EDK2) by the OnScreenKeyboard driver as well as the UiToolKit driver. If you happen to use BaseCryptLib or the intrinsic in a driver that happens to need crypto as well, it can break due to multiple.
>
> I do agree, this only applies to MSVC and requiring every platform to add a line in their DSC would be a situation I would prefer to avoid if possible. Is there a way to specify a library dependency only if a toolchain is being used?

Yes, so this either belongs in one of the IntrinsicsLibs we have, or
in the various Entrypoint libraries we have for PEIMs, DXEs, etc.

However, given that we are talking about static libraries here, adding
a source file that *only* defines __fltused (and nothing else) to any
library should also work, as the resulting object file will only be
incorporated by the linker if it is needed to satisfy a symbol
dependency, and so it can never cause a conflict if it is the only
symbol in the object.

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

View/Reply Online (#56691): https://edk2.groups.io/g/devel/message/56691
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Laszlo Ersek 4 years, 8 months ago
On 03/30/20 23:41, Ard Biesheuvel wrote:
> On Mon, 30 Mar 2020 at 23:29, Matthew Carlson via Groups.Io
> <macarl=microsoft.com@groups.io> wrote:
>>
>> So it's not required by OpenSSL, it's required by the compiler whenever floating point is used, which can be in multiple places. For example, this is used in mu_plus (the Microsoft UEFI value add to EDK2) by the OnScreenKeyboard driver as well as the UiToolKit driver.

OK, so this is the part that was not obvious to me. This is basically
code that exists on top of edk2 (consumes edk2) *AND* uses floating
point *internally*.

> If you happen to use BaseCryptLib or the intrinsic in a driver that happens to need crypto as well, it can break due to multiple.
>>
>> I do agree, this only applies to MSVC and requiring every platform to add a line in their DSC would be a situation I would prefer to avoid if possible. Is there a way to specify a library dependency only if a toolchain is being used?
> 
> Yes, so this either belongs in one of the IntrinsicsLibs we have, or
> in the various Entrypoint libraries we have for PEIMs, DXEs, etc.
> 
> However, given that we are talking about static libraries here, adding
> a source file that *only* defines __fltused (and nothing else) to any
> library should also work, as the resulting object file will only be
> incorporated by the linker if it is needed to satisfy a symbol
> dependency, and so it can never cause a conflict if it is the only
> symbol in the object.

IMO: if edk2 intends to support out-of-tree modules that themselves
utilize floating point, *with or without* a dependency on OpensslLib,
then we should add Ard's suggestion:

 [Sources]
+  FloatUsed.c         | MSFT

to some of the "most core" edk2 library instances, i.e. those that *all*
modules of the given module type inevitably depend on. The entry point
libs look like a great idea to me.

That should link the _fltused external definition exactly once into all
modules built with MSVC, regardless of whether each such module uses
floating point or not.

Thanks
Laszlo


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

View/Reply Online (#56763): https://edk2.groups.io/g/devel/message/56763
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Michael D Kinney 4 years, 8 months ago
ARM and AARCH64 have a compiler intrinsic lib that is linked against all modules.

[LibraryClasses.ARM, LibraryClasses.AARCH64]
  #
  # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
  # This library provides the instrinsic functions generated by a given compiler.
  # [LibraryClasses.ARM] and NULL mean link this library into all ARM images.
  #
  NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf

Can we use this same technique for IA32/X64 VS builds?

Mike


> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Laszlo Ersek
> Sent: Tuesday, March 31, 2020 5:42 AM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>; edk2-
> devel-groups-io <devel@edk2.groups.io>;
> macarl@microsoft.com
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib:
> Add FltUsedLib for float.
> 
> On 03/30/20 23:41, Ard Biesheuvel wrote:
> > On Mon, 30 Mar 2020 at 23:29, Matthew Carlson via
> Groups.Io
> > <macarl=microsoft.com@groups.io> wrote:
> >>
> >> So it's not required by OpenSSL, it's required by the
> compiler whenever floating point is used, which can be in
> multiple places. For example, this is used in mu_plus
> (the Microsoft UEFI value add to EDK2) by the
> OnScreenKeyboard driver as well as the UiToolKit driver.
> 
> OK, so this is the part that was not obvious to me. This
> is basically
> code that exists on top of edk2 (consumes edk2) *AND*
> uses floating
> point *internally*.
> 
> > If you happen to use BaseCryptLib or the intrinsic in a
> driver that happens to need crypto as well, it can break
> due to multiple.
> >>
> >> I do agree, this only applies to MSVC and requiring
> every platform to add a line in their DSC would be a
> situation I would prefer to avoid if possible. Is there a
> way to specify a library dependency only if a toolchain
> is being used?
> >
> > Yes, so this either belongs in one of the
> IntrinsicsLibs we have, or
> > in the various Entrypoint libraries we have for PEIMs,
> DXEs, etc.
> >
> > However, given that we are talking about static
> libraries here, adding
> > a source file that *only* defines __fltused (and
> nothing else) to any
> > library should also work, as the resulting object file
> will only be
> > incorporated by the linker if it is needed to satisfy a
> symbol
> > dependency, and so it can never cause a conflict if it
> is the only
> > symbol in the object.
> 
> IMO: if edk2 intends to support out-of-tree modules that
> themselves
> utilize floating point, *with or without* a dependency on
> OpensslLib,
> then we should add Ard's suggestion:
> 
>  [Sources]
> +  FloatUsed.c         | MSFT
> 
> to some of the "most core" edk2 library instances, i.e.
> those that *all*
> modules of the given module type inevitably depend on.
> The entry point
> libs look like a great idea to me.
> 
> That should link the _fltused external definition exactly
> once into all
> modules built with MSVC, regardless of whether each such
> module uses
> floating point or not.
> 
> Thanks
> Laszlo
> 
> 
> 


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

View/Reply Online (#56768): https://edk2.groups.io/g/devel/message/56768
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Laszlo Ersek 4 years, 8 months ago
On 03/31/20 16:36, Kinney, Michael D wrote:
> ARM and AARCH64 have a compiler intrinsic lib that is linked against all modules.
> 
> [LibraryClasses.ARM, LibraryClasses.AARCH64]
>   #
>   # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
>   # This library provides the instrinsic functions generated by a given compiler.
>   # [LibraryClasses.ARM] and NULL mean link this library into all ARM images.
>   #
>   NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
> 
> Can we use this same technique for IA32/X64

It's not a problem for in-tree platforms (I do hope the edk2 patch
series will include the patch for OvmfPkg), but it will require all
out-of-tree platforms to be updated.

> VS builds?

Yes; I think the library instance should consist of one totally empty C
file, and an |MSFT specific C file providing the external definition of
_fltused. When building for GCC, the lib instance should compile to an
empty object (archive) file.

In my opinion, of course.

Thanks
Laszlo


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

View/Reply Online (#56785): https://edk2.groups.io/g/devel/message/56785
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Sean via Groups.Io 4 years, 8 months ago
Does anyone know off hand if defining this and enabling floating point has any negative side effects if you don't need it?  Size? Optimization? Other?   That is my only concern for enabling in all modules which is why the initial proposal was for a new library.

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

View/Reply Online (#56797): https://edk2.groups.io/g/devel/message/56797
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Michael D Kinney 4 years, 8 months ago
Hi Sean,

This lib defines a global variable that is referenced when a compiler detects use of float/double types.  If the global is not referenced, then it should be optimized away, so the size impact should be zero.  That can be verified as part of the review of this feature.

Mike



From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Sean via Groups.Io
Sent: Tuesday, March 31, 2020 3:58 PM
To: Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io
Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

Does anyone know off hand if defining this and enabling floating point has any negative side effects if you don't need it?  Size? Optimization? Other?   That is my only concern for enabling in all modules which is why the initial proposal was for a new library.


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

View/Reply Online (#56799): https://edk2.groups.io/g/devel/message/56799
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Ard Biesheuvel 4 years, 7 months ago
On Tue, 31 Mar 2020 at 16:36, Kinney, Michael D
<michael.d.kinney@intel.com> wrote:
>
> ARM and AARCH64 have a compiler intrinsic lib that is linked against all modules.
>
> [LibraryClasses.ARM, LibraryClasses.AARCH64]
>   #
>   # It is not possible to prevent ARM compiler calls to generic intrinsic functions.
>   # This library provides the instrinsic functions generated by a given compiler.
>   # [LibraryClasses.ARM] and NULL mean link this library into all ARM images.
>   #
>   NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf
>
> Can we use this same technique for IA32/X64 VS builds?
>

In my opinion, having these intrinsics libraries added via NULL
library class resolution was a mistake to begin with.

Every component that we build incorporates some kind of startup code
library that defines the _ModuleEntryPoint symbol, and it would be
much better to make those libraries include an IntrinsicsLibrary
dependency that can be satisfied by arch specific versions that also
encapsulate the toolchain dependencies (such as this _fltused symbol,
or memcpy/memset on ARM/GCC).

On another note, it is still deeply disappointing that we need to jump
through all of these hoops because the *only* single use of floating
point in OpenSSL is the entropy estimate of an RNG, which is in the
range of 0..1023 to begin with [IIRC). Remember that we also have a
softfloat submodule for 32-bit ARM, for the same stupid reason. If we
could stop pulling in that part of the code (or replace it with an
UINT16 when building for the UEFI target), the whole floating point
issue would mostly go away AFAICT.

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

View/Reply Online (#56817): https://edk2.groups.io/g/devel/message/56817
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Michael D Kinney 4 years, 7 months ago
Hi Ard,

I think adding a dependency in the module entry point
libs is also good way to guarantee an intrinsic lib
is available.  I agree that it can provide module type
and arch specific mappings.  The NULL lib instance can
do the same if the NULL instance is listed in the 
module/arch specific library classes sections. a few
example section names.

[LibraryClasses.ARM.PEIM, LibraryClasses.AARCH64.PEIM]

[LibraryClasses.IA32.UEFI_DRIVER]

[LibraryClasses.X64.DXE_DRIVER]

So either way, the DSC files need to provide the
library mapping.  The only difference between these
2 approaches is that adding a dependency to the
module entry point libs uses a formally defined
library class name for the intrinsic functions vs.
the un-named NULL library instance.  A formally
defined library class name is better supported for
things like unit tests from the UnitTestFrameworkPkg.

The fltused issue would not go away of we removed the 
float usage for OpenSSL.  One or more other libs or the
module could use float/double types and this issue
would pop up again.  I do agree it would be cleaner
if we could use OpenSSL without floating point.

I think adding an intrinsic lib for IA32/X64 for VS20xx
generation of intrinsic functions would address fltused
and other common C implementation styles that generate
intrinsic functions (e.g. 64-bit int math on 32-bit,
structure variable assignments, and loops that fill a buffer
with a const value) by VS20xx compilers.  An intrinsic
lib for GCC would also help with these same common C
implementation styles that generate intrinsic functions.

Mike

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On
> Behalf Of Ard Biesheuvel
> Sent: Tuesday, March 31, 2020 11:43 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: devel@edk2.groups.io; lersek@redhat.com;
> macarl@microsoft.com
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib:
> Add FltUsedLib for float.
> 
> On Tue, 31 Mar 2020 at 16:36, Kinney, Michael D
> <michael.d.kinney@intel.com> wrote:
> >
> > ARM and AARCH64 have a compiler intrinsic lib that is
> linked against all modules.
> >
> > [LibraryClasses.ARM, LibraryClasses.AARCH64]
> >   #
> >   # It is not possible to prevent ARM compiler calls to
> generic intrinsic functions.
> >   # This library provides the instrinsic functions
> generated by a given compiler.
> >   # [LibraryClasses.ARM] and NULL mean link this
> library into all ARM images.
> >   #
> >
> NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrins
> icsLib.inf
> >
> > Can we use this same technique for IA32/X64 VS builds?
> >
> 
> In my opinion, having these intrinsics libraries added
> via NULL
> library class resolution was a mistake to begin with.
> 
> Every component that we build incorporates some kind of
> startup code
> library that defines the _ModuleEntryPoint symbol, and it
> would be
> much better to make those libraries include an
> IntrinsicsLibrary
> dependency that can be satisfied by arch specific
> versions that also
> encapsulate the toolchain dependencies (such as this
> _fltused symbol,
> or memcpy/memset on ARM/GCC).
> 
> On another note, it is still deeply disappointing that we
> need to jump
> through all of these hoops because the *only* single use
> of floating
> point in OpenSSL is the entropy estimate of an RNG, which
> is in the
> range of 0..1023 to begin with [IIRC). Remember that we
> also have a
> softfloat submodule for 32-bit ARM, for the same stupid
> reason. If we
> could stop pulling in that part of the code (or replace
> it with an
> UINT16 when building for the UEFI target), the whole
> floating point
> issue would mostly go away AFAICT.
> 
> 


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

View/Reply Online (#56860): https://edk2.groups.io/g/devel/message/56860
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Ni, Ray 4 years, 7 months ago
UEFI spec allows using float operation so asking module developers avoid using
float may not make sense. Even UefiCpuPkg\Library\BaseUefiCpuLib\ provides
routine to initialize float support for X86.

Given ARM already uses NULL library class mechanism to supply the compiler stub,
I prefer X86 aligns to the ARM style.

The only unsure thing is the size impact when a module is not using float. We expect
there is no size impact when a module is not using float.

Thanks,
Ray

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Michael D
> Kinney
> Sent: Thursday, April 2, 2020 12:38 AM
> To: devel@edk2.groups.io; ard.biesheuvel@linaro.org; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: lersek@redhat.com; macarl@microsoft.com
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for
> float.
> 
> Hi Ard,
> 
> I think adding a dependency in the module entry point
> libs is also good way to guarantee an intrinsic lib
> is available.  I agree that it can provide module type
> and arch specific mappings.  The NULL lib instance can
> do the same if the NULL instance is listed in the
> module/arch specific library classes sections. a few
> example section names.
> 
> [LibraryClasses.ARM.PEIM, LibraryClasses.AARCH64.PEIM]
> 
> [LibraryClasses.IA32.UEFI_DRIVER]
> 
> [LibraryClasses.X64.DXE_DRIVER]
> 
> So either way, the DSC files need to provide the
> library mapping.  The only difference between these
> 2 approaches is that adding a dependency to the
> module entry point libs uses a formally defined
> library class name for the intrinsic functions vs.
> the un-named NULL library instance.  A formally
> defined library class name is better supported for
> things like unit tests from the UnitTestFrameworkPkg.
> 
> The fltused issue would not go away of we removed the
> float usage for OpenSSL.  One or more other libs or the
> module could use float/double types and this issue
> would pop up again.  I do agree it would be cleaner
> if we could use OpenSSL without floating point.
> 
> I think adding an intrinsic lib for IA32/X64 for VS20xx
> generation of intrinsic functions would address fltused
> and other common C implementation styles that generate
> intrinsic functions (e.g. 64-bit int math on 32-bit,
> structure variable assignments, and loops that fill a buffer
> with a const value) by VS20xx compilers.  An intrinsic
> lib for GCC would also help with these same common C
> implementation styles that generate intrinsic functions.
> 
> Mike
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On
> > Behalf Of Ard Biesheuvel
> > Sent: Tuesday, March 31, 2020 11:43 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: devel@edk2.groups.io; lersek@redhat.com;
> > macarl@microsoft.com
> > Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib:
> > Add FltUsedLib for float.
> >
> > On Tue, 31 Mar 2020 at 16:36, Kinney, Michael D
> > <michael.d.kinney@intel.com> wrote:
> > >
> > > ARM and AARCH64 have a compiler intrinsic lib that is
> > linked against all modules.
> > >
> > > [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > >   #
> > >   # It is not possible to prevent ARM compiler calls to
> > generic intrinsic functions.
> > >   # This library provides the instrinsic functions
> > generated by a given compiler.
> > >   # [LibraryClasses.ARM] and NULL mean link this
> > library into all ARM images.
> > >   #
> > >
> > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrins
> > icsLib.inf
> > >
> > > Can we use this same technique for IA32/X64 VS builds?
> > >
> >
> > In my opinion, having these intrinsics libraries added
> > via NULL
> > library class resolution was a mistake to begin with.
> >
> > Every component that we build incorporates some kind of
> > startup code
> > library that defines the _ModuleEntryPoint symbol, and it
> > would be
> > much better to make those libraries include an
> > IntrinsicsLibrary
> > dependency that can be satisfied by arch specific
> > versions that also
> > encapsulate the toolchain dependencies (such as this
> > _fltused symbol,
> > or memcpy/memset on ARM/GCC).
> >
> > On another note, it is still deeply disappointing that we
> > need to jump
> > through all of these hoops because the *only* single use
> > of floating
> > point in OpenSSL is the entropy estimate of an RNG, which
> > is in the
> > range of 0..1023 to begin with [IIRC). Remember that we
> > also have a
> > softfloat submodule for 32-bit ARM, for the same stupid
> > reason. If we
> > could stop pulling in that part of the code (or replace
> > it with an
> > UINT16 when building for the UEFI target), the whole
> > floating point
> > issue would mostly go away AFAICT.
> >
> >
> 
> 
> 


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

View/Reply Online (#57297): https://edk2.groups.io/g/devel/message/57297
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Guomin Jiang 4 years, 7 months ago
Summarize current status:

Problem Statement:
Openssl require _fltused to be defined as a constant anywhere floating point is used.
It may use float out of edk2 tree and need _fltused, for example, Microsoft’s OnScreenKeyboard and UiToolKit.

Current Proposal as below:

Proposal 1: Add FltUsed.c into exist library
Detail: Add FltUsed.c into EntryPointLib(PEIM, DXE).
Recommend: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Approve: Laszlo Ersek lersek@redhat.com
Netual: Michael D Kinney <michael.d.kinney@intel.com>
Benefit: Doesn’t need modify every .dsc description file.
Defection: I test that it will fail because of /GL option, the error show fatal error LNK1237: during code generation, compiler introduced reference to symbol '_fltused' defined in module 'UefiApplicationEntryPoint.lib(FltUsed.obj)' compiled with /GL

Proposal 2: Define NULL library
Recommend: Michael D Kinney michael.d.kinney@intel.com
Oppose: Sean sean.brogan=microsoft.com@groups.io , Ard Biesheuvel <ard.biesheuvel@linaro.org>
Benefit: I test it and prove that it is executable.
Defection: It is required that modify every description file.
Defection: It need be very careful that it should only apply some module type(PEIM, DXE_DRIVER, UEFI_APPLICATION) rather than all.
Defection: Build break up.
Action Required: Need evaluate the affection on size.
Consideration: Add PCD to control the feature

Proposal 3: Define FltUsedLib
Detail: Define FltUsedLib and add dependence
Oppose: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Benefit: Doesn’t need care that which module will use it, we will explicitly point it in component file.
Defection: More complex, It is required that modify every description file and modify component meanwhile.
Defection: Build breakup

Thanks
guomin

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ni, Ray
> Sent: Tuesday, April 14, 2020 1:03 PM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>;
> ard.biesheuvel@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: lersek@redhat.com; macarl@microsoft.com
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for
> float.
> 
> UEFI spec allows using float operation so asking module developers avoid
> using float may not make sense. Even UefiCpuPkg\Library\BaseUefiCpuLib\
> provides routine to initialize float support for X86.
> 
> Given ARM already uses NULL library class mechanism to supply the compiler
> stub, I prefer X86 aligns to the ARM style.
> 
> The only unsure thing is the size impact when a module is not using float. We
> expect there is no size impact when a module is not using float.
> 
> Thanks,
> Ray
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Michael
> > D Kinney
> > Sent: Thursday, April 2, 2020 12:38 AM
> > To: devel@edk2.groups.io; ard.biesheuvel@linaro.org; Kinney, Michael D
> > <michael.d.kinney@intel.com>
> > Cc: lersek@redhat.com; macarl@microsoft.com
> > Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib
> > for float.
> >
> > Hi Ard,
> >
> > I think adding a dependency in the module entry point libs is also
> > good way to guarantee an intrinsic lib is available.  I agree that it
> > can provide module type and arch specific mappings.  The NULL lib
> > instance can do the same if the NULL instance is listed in the
> > module/arch specific library classes sections. a few example section
> > names.
> >
> > [LibraryClasses.ARM.PEIM, LibraryClasses.AARCH64.PEIM]
> >
> > [LibraryClasses.IA32.UEFI_DRIVER]
> >
> > [LibraryClasses.X64.DXE_DRIVER]
> >
> > So either way, the DSC files need to provide the library mapping.  The
> > only difference between these
> > 2 approaches is that adding a dependency to the module entry point
> > libs uses a formally defined library class name for the intrinsic
> > functions vs.
> > the un-named NULL library instance.  A formally defined library class
> > name is better supported for things like unit tests from the
> > UnitTestFrameworkPkg.
> >
> > The fltused issue would not go away of we removed the float usage for
> > OpenSSL.  One or more other libs or the module could use float/double
> > types and this issue would pop up again.  I do agree it would be
> > cleaner if we could use OpenSSL without floating point.
> >
> > I think adding an intrinsic lib for IA32/X64 for VS20xx generation of
> > intrinsic functions would address fltused and other common C
> > implementation styles that generate intrinsic functions (e.g. 64-bit
> > int math on 32-bit, structure variable assignments, and loops that
> > fill a buffer with a const value) by VS20xx compilers.  An intrinsic
> > lib for GCC would also help with these same common C implementation
> > styles that generate intrinsic functions.
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> > > Biesheuvel
> > > Sent: Tuesday, March 31, 2020 11:43 PM
> > > To: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Cc: devel@edk2.groups.io; lersek@redhat.com; macarl@microsoft.com
> > > Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib:
> > > Add FltUsedLib for float.
> > >
> > > On Tue, 31 Mar 2020 at 16:36, Kinney, Michael D
> > > <michael.d.kinney@intel.com> wrote:
> > > >
> > > > ARM and AARCH64 have a compiler intrinsic lib that is
> > > linked against all modules.
> > > >
> > > > [LibraryClasses.ARM, LibraryClasses.AARCH64]
> > > >   #
> > > >   # It is not possible to prevent ARM compiler calls to
> > > generic intrinsic functions.
> > > >   # This library provides the instrinsic functions
> > > generated by a given compiler.
> > > >   # [LibraryClasses.ARM] and NULL mean link this
> > > library into all ARM images.
> > > >   #
> > > >
> > > NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrins
> > > icsLib.inf
> > > >
> > > > Can we use this same technique for IA32/X64 VS builds?
> > > >
> > >
> > > In my opinion, having these intrinsics libraries added via NULL
> > > library class resolution was a mistake to begin with.
> > >
> > > Every component that we build incorporates some kind of startup code
> > > library that defines the _ModuleEntryPoint symbol, and it would be
> > > much better to make those libraries include an IntrinsicsLibrary
> > > dependency that can be satisfied by arch specific versions that also
> > > encapsulate the toolchain dependencies (such as this _fltused
> > > symbol, or memcpy/memset on ARM/GCC).
> > >
> > > On another note, it is still deeply disappointing that we need to
> > > jump through all of these hoops because the *only* single use of
> > > floating point in OpenSSL is the entropy estimate of an RNG, which
> > > is in the range of 0..1023 to begin with [IIRC). Remember that we
> > > also have a softfloat submodule for 32-bit ARM, for the same stupid
> > > reason. If we could stop pulling in that part of the code (or
> > > replace it with an
> > > UINT16 when building for the UEFI target), the whole floating point
> > > issue would mostly go away AFAICT.
> > >
> > >
> >
> >
> >
> 
> 
> 


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

View/Reply Online (#57300): https://edk2.groups.io/g/devel/message/57300
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Ard Biesheuvel 4 years, 7 months ago
On Tue, 14 Apr 2020 at 09:01, Jiang, Guomin <guomin.jiang@intel.com> wrote:
>
> Summarize current status:
>
> Problem Statement:
> Openssl require _fltused to be defined as a constant anywhere floating point is used.
> It may use float out of edk2 tree and need _fltused, for example, Microsoft’s OnScreenKeyboard and UiToolKit.
>
> Current Proposal as below:
>
> Proposal 1: Add FltUsed.c into exist library
> Detail: Add FltUsed.c into EntryPointLib(PEIM, DXE).
> Recommend: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Approve: Laszlo Ersek lersek@redhat.com
> Netual: Michael D Kinney <michael.d.kinney@intel.com>
> Benefit: Doesn’t need modify every .dsc description file.
> Defection: I test that it will fail because of /GL option, the error show fatal error LNK1237: during code generation, compiler introduced reference to symbol '_fltused' defined in module 'UefiApplicationEntryPoint.lib(FltUsed.obj)' compiled with /GL
>

Can you elaborate on this issue? What does /GL do, and why is it
throwing this error?

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

View/Reply Online (#57501): https://edk2.groups.io/g/devel/message/57501
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Guomin Jiang 4 years, 7 months ago
Hi Ard,

it explain the reason at https://docs.microsoft.com/en-us/cpp/error-messages/tool-errors/linker-tools-error-lnk1237?view=vs-2019.
It can use ```MSFT:*_*_*_DLINK_FLAGS = /include:_fltused``` to resolve the error, but it is complex.

Best Regards
Guomin

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard
> Biesheuvel
> Sent: Friday, April 17, 2020 4:15 PM
> To: Jiang, Guomin <guomin.jiang@intel.com>; Ard Biesheuvel
> <ard.biesheuvel@arm.com>
> Cc: devel@edk2.groups.io; Ni, Ray <ray.ni@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; lersek@redhat.com; macarl@microsoft.com
> Subject: Re: [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for
> float.
> 
> On Tue, 14 Apr 2020 at 09:01, Jiang, Guomin <guomin.jiang@intel.com>
> wrote:
> >
> > Summarize current status:
> >
> > Problem Statement:
> > Openssl require _fltused to be defined as a constant anywhere floating
> point is used.
> > It may use float out of edk2 tree and need _fltused, for example,
> Microsoft’s OnScreenKeyboard and UiToolKit.
> >
> > Current Proposal as below:
> >
> > Proposal 1: Add FltUsed.c into exist library
> > Detail: Add FltUsed.c into EntryPointLib(PEIM, DXE).
> > Recommend: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Approve: Laszlo Ersek lersek@redhat.com
> > Netual: Michael D Kinney <michael.d.kinney@intel.com>
> > Benefit: Doesn’t need modify every .dsc description file.
> > Defection: I test that it will fail because of /GL option, the error
> > show fatal error LNK1237: during code generation, compiler introduced
> > reference to symbol '_fltused' defined in module
> > 'UefiApplicationEntryPoint.lib(FltUsed.obj)' compiled with /GL
> >
> 
> Can you elaborate on this issue? What does /GL do, and why is it throwing
> this error?
> 
> 


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

View/Reply Online (#57894): https://edk2.groups.io/g/devel/message/57894
Mute This Topic: https://groups.io/mt/72648022/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [EXTERNAL] [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.
Posted by Bret Barkelew via Groups.Io 4 years, 8 months ago
Reviewed-by: Bret Barkelew <bret.barkelew@microsoft.com>

- Bret

________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of Guomin Jiang via Groups.Io <guomin.jiang=intel.com@groups.io>
Sent: Monday, March 30, 2020 1:52:47 AM
To: devel@edk2.groups.io <devel@edk2.groups.io>
Cc: Jian J Wang <jian.j.wang@intel.com>; Xiaoyu Lu <xiaoyux.lu@intel.com>
Subject: [EXTERNAL] [edk2-devel] [PATCH] CryptoPkg/FltUsedLib: Add FltUsedLib for float.

REF: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2596&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7Cdcda1866343d4d6dab3908d7d487ba8e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637211551721384527&amp;sdata=LCgdHl1VW1DIGzMCei5PDRP7UQtD%2FERzkumrbgpYuJs%3D&amp;reserved=0

OpenSSL requires _fltused to be defined as a constant anywhere floating
point is used.
This is to satisfy the linker, however, it is possible to compile a
module with multiple definitions of fltused. This causes the
MSVC compiler to fail the build.
To solve this problem, the FltUsedLib was created that is one spot
that the global static can exist.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
Signed-off-by: Guomin Jiang <guomin.jiang@intel.com>
---
 CryptoPkg/CryptoPkg.dsc                       |  2 ++
 .../Library/BaseCryptLib/BaseCryptLib.inf     |  1 +
 .../Library/BaseCryptLib/PeiCryptLib.inf      |  1 +
 .../Library/BaseCryptLib/RuntimeCryptLib.inf  |  1 +
 .../Library/BaseCryptLib/SmmCryptLib.inf      |  1 +
 CryptoPkg/Library/FltUsedLib/FltUsedLib.c     | 14 ++++++++
 CryptoPkg/Library/FltUsedLib/FltUsedLib.inf   | 33 +++++++++++++++++++
 CryptoPkg/Library/TlsLib/TlsLib.inf           |  1 +
 8 files changed, 54 insertions(+)
 create mode 100644 CryptoPkg/Library/FltUsedLib/FltUsedLib.c
 create mode 100644 CryptoPkg/Library/FltUsedLib/FltUsedLib.inf

diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc
index 4cb37b1349..e9854087b5 100644
--- a/CryptoPkg/CryptoPkg.dsc
+++ b/CryptoPkg/CryptoPkg.dsc
@@ -97,6 +97,7 @@
   IoLib|MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf                                          #???

   OpensslLib|CryptoPkg/Library/OpensslLib/OpensslLib.inf

   IntrinsicLib|CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf

+  FltUsedLib|CryptoPkg/Library/FltUsedLib/FltUsedLib.inf

   SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf



 [LibraryClasses.ARM]

@@ -232,6 +233,7 @@
   CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf

   CryptoPkg/Library/BaseCryptLibNull/BaseCryptLibNull.inf

   CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf

+  CryptoPkg/Library/FltUsedLib/FltUsedLib.inf

   CryptoPkg/Library/TlsLib/TlsLib.inf

   CryptoPkg/Library/TlsLibNull/TlsLibNull.inf

   CryptoPkg/Library/OpensslLib/OpensslLib.inf

diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
index 1bbe4f435a..c82e703aa0 100644
--- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
@@ -84,6 +84,7 @@
   DebugLib

   OpensslLib

   IntrinsicLib

+  FltUsedLib

   PrintLib



 #

diff --git a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
index c836c257f8..342aad008c 100644
--- a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
@@ -78,6 +78,7 @@
   DebugLib

   OpensslLib

   IntrinsicLib

+  FltUsedLib



 #

 # Remove these [BuildOptions] after this library is cleaned up

diff --git a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
index bff308a4f5..dcf209d7f5 100644
--- a/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
@@ -89,6 +89,7 @@
   DebugLib

   OpensslLib

   IntrinsicLib

+  FltUsedLib

   PrintLib



 #

diff --git a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
index cc0b65fd25..7fdaaa48a6 100644
--- a/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
+++ b/CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf
@@ -88,6 +88,7 @@
   MemoryAllocationLib

   OpensslLib

   IntrinsicLib

+  FltUsedLib

   PrintLib



 #

diff --git a/CryptoPkg/Library/FltUsedLib/FltUsedLib.c b/CryptoPkg/Library/FltUsedLib/FltUsedLib.c
new file mode 100644
index 0000000000..8f2487ea13
--- /dev/null
+++ b/CryptoPkg/Library/FltUsedLib/FltUsedLib.c
@@ -0,0 +1,14 @@
+/** @file

+  Need include this when use floats.

+

+  Copyright (C) Microsoft Corporation. All rights reserved.<BR>

+  Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>

+  SPDX-License-Identifier: BSD-2-Clause-Patent

+

+**/

+

+//

+// You need to include this to let the compiler know we are going to use

+// floating point

+//

+int _fltused = 0x9875;

diff --git a/CryptoPkg/Library/FltUsedLib/FltUsedLib.inf b/CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
new file mode 100644
index 0000000000..8d67eadfa5
--- /dev/null
+++ b/CryptoPkg/Library/FltUsedLib/FltUsedLib.inf
@@ -0,0 +1,33 @@
+## @file

+# It is depent on when using floats

+#

+# Copyright (C) Microsoft Corporation. All rights reserved.<BR>

+# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>

+# SPDX-License-Identifier: BSD-2-Clause-Patent

+#

+##

+

+[Defines]

+  INF_VERSION                    = 0x00010005

+  BASE_NAME                      = FltUsedLib

+  FILE_GUID                      = C004F180-9FE2-4D2B-8318-BADC2A231774

+  MODULE_TYPE                    = BASE

+  VERSION_STRING                 = 1.0

+  LIBRARY_CLASS                  = FltUsedLib

+

+#

+# The following information is for reference only and not required by the build tools.

+#

+#  VALID_ARCHITECTURES           = IA32 X64 AARCH64

+#

+

+[Sources]

+  FltUsedLib.c

+

+[Packages]

+  MdePkg/MdePkg.dec

+

+[BuildOptions]

+  # Disable GL due to linker error LNK1237

+  # https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fdocs.microsoft.com%2Fen-us%2Fcpp%2Ferror-messages%2Ftool-errors%2Flinker-tools-error-lnk1237%3Fview%3Dvs-2017&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7Cdcda1866343d4d6dab3908d7d487ba8e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637211551721384527&amp;sdata=vULhLfRubt3e7xbVLa%2BHC0E34JTcgOSbdYndn%2FxNEps%3D&amp;reserved=0

+  MSFT:*_*_*_CC_FLAGS = /GL-

diff --git a/CryptoPkg/Library/TlsLib/TlsLib.inf b/CryptoPkg/Library/TlsLib/TlsLib.inf
index 2f3ce695c3..febbdf5149 100644
--- a/CryptoPkg/Library/TlsLib/TlsLib.inf
+++ b/CryptoPkg/Library/TlsLib/TlsLib.inf
@@ -37,6 +37,7 @@
   BaseMemoryLib

   DebugLib

   IntrinsicLib

+  FltUsedLib

   MemoryAllocationLib

   OpensslLib

   SafeIntLib

--
2.25.1.windows.1


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

View/Reply Online (#56613): https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Fmessage%2F56613&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7Cdcda1866343d4d6dab3908d7d487ba8e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637211551721384527&amp;sdata=DHVFRPBfEkbBLIgrIdwQxnGH0XjtN1oxsZYVyLDIqf4%3D&amp;reserved=0
Mute This Topic: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgroups.io%2Fmt%2F72648022%2F1822150&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7Cdcda1866343d4d6dab3908d7d487ba8e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637211551721384527&amp;sdata=KyjsUA%2BJ4U6YdFn22%2BHhr%2F0JF5AojkVTU52vmE1zh2Q%3D&amp;reserved=0
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2.groups.io%2Fg%2Fdevel%2Funsub&amp;data=02%7C01%7CBret.Barkelew%40microsoft.com%7Cdcda1866343d4d6dab3908d7d487ba8e%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637211551721384527&amp;sdata=04dDAgBNEz8Ra5JuAfOVK3dvos1vhfQDma5Y2Qv4eao%3D&amp;reserved=0  [brbarkel@microsoft.com]
-=-=-=-=-=-=


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

View/Reply Online (#56670): https://edk2.groups.io/g/devel/message/56670
Mute This Topic: https://groups.io/mt/72657253/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-