[edk2] [patch v2] MdePkg/BaseSafeIntLib: Fix VS2015 IA32 NOOPT build failure

Dandan Bi posted 1 patch 6 years, 1 month ago
Failed in applying to current master (apply log)
MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf | 3 +++
MdePkg/Library/BaseSafeIntLib/SafeIntLib.c       | 9 +++++----
MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c     | 3 ++-
3 files changed, 10 insertions(+), 5 deletions(-)
[edk2] [patch v2] MdePkg/BaseSafeIntLib: Fix VS2015 IA32 NOOPT build failure
Posted by Dandan Bi 6 years, 1 month ago
v2: Add [LibraryClasses] section in INF file and refine coding style.

There are VS2015 NOOPT IA32 build failure like below in BaseSafeIntLib.
XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __allmul
XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __allshl
XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __aullshr

This patch replaces direct shift/multiplication of 64-bit integer
with related function call to fix these failure.

Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Dandan Bi <dandan.bi@intel.com>
---
 MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf | 3 +++
 MdePkg/Library/BaseSafeIntLib/SafeIntLib.c       | 9 +++++----
 MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c     | 3 ++-
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf b/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
index 20a83ed..8fbdafe 100644
--- a/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
+++ b/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
@@ -54,5 +54,8 @@
 [Sources.EBC]
   SafeIntLibEbc.c
 
 [Packages]
   MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  BaseLib
diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
index c5f13d7..e96327d 100644
--- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
+++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
@@ -26,10 +26,11 @@
 
 **/
 
 #include <Base.h>
 #include <Library/SafeIntLib.h>
+#include <Library/BaseLib.h>
 
 
 //
 // Magnitude of MIN_INT64 as expressed by a UINT64 number.
 //
@@ -3371,12 +3372,12 @@ SafeUint64Mult (
   // a * c must be 0 or there would be bits in the high 64-bits
   // a * d must be less than 2^32 or there would be bits in the high 64-bits
   // b * c must be less than 2^32 or there would be bits in the high 64-bits
   // then there must be no overflow of the resulting values summed up.
   //
-  DwordA = (UINT32)(Multiplicand >> 32);
-  DwordC = (UINT32)(Multiplier >> 32);
+  DwordA = (UINT32)RShiftU64 (Multiplicand, 32);
+  DwordC = (UINT32)RShiftU64 (Multiplier, 32);
 
   //
   // common case -- if high dwords are both zero, no chance for overflow
   //
   if ((DwordA == 0) && (DwordC == 0)) {
@@ -3407,11 +3408,11 @@ SafeUint64Mult (
         if ((ProductBC & 0xffffffff00000000) == 0) {
           //
           // now sum them all up checking for overflow.
           // shifting is safe because we already checked for overflow above
           //
-          if (!RETURN_ERROR (SafeUint64Add (ProductBC << 32, ProductAD << 32, &UnsignedResult))) {
+          if (!RETURN_ERROR (SafeUint64Add (LShiftU64 (ProductBC, 32), LShiftU64 (ProductAD, 32), &UnsignedResult))) {
             //
             // b * d
             //
             ProductBD = (((UINT64)DwordB) *(UINT64)DwordD);
 
@@ -4073,11 +4074,11 @@ SafeInt32Mult (
   IN  INT32  Multiplicand,
   IN  INT32  Multiplier,
   OUT INT32  *Result
   )
 {
-  return SafeInt64ToInt32 (((INT64)Multiplicand) *((INT64)Multiplier), Result);
+  return SafeInt64ToInt32 (MultS64x64 (Multiplicand, Multiplier), Result);
 }
 
 /**
   INT64 multiplication
 
diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c b/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
index 18bfb9e..ce66a92 100644
--- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
+++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
@@ -26,10 +26,11 @@
 
 **/
 
 #include <Base.h>
 #include <Library/SafeIntLib.h>
+#include <Library/BaseLib.h>
 
 /**
   INT32 -> UINTN conversion
 
   Converts the value specified by Operand to a value specified by Result type
@@ -547,8 +548,8 @@ SafeIntnMult (
   IN  INTN  Multiplicand,
   IN  INTN  Multiplier,
   OUT INTN  *Result
   )
 {
-  return SafeInt64ToIntn (((INT64)Multiplicand) *((INT64)Multiplier), Result);
+  return SafeInt64ToIntn (MultS64x64 (Multiplicand, Multiplier), Result);
 }
 
-- 
1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch v2] MdePkg/BaseSafeIntLib: Fix VS2015 IA32 NOOPT build failure
Posted by Laszlo Ersek 6 years, 1 month ago
On 02/27/18 02:47, Dandan Bi wrote:
> v2: Add [LibraryClasses] section in INF file and refine coding style.
> 
> There are VS2015 NOOPT IA32 build failure like below in BaseSafeIntLib.
> XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __allmul
> XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __allshl
> XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __aullshr
> 
> This patch replaces direct shift/multiplication of 64-bit integer
> with related function call to fix these failure.
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf | 3 +++
>  MdePkg/Library/BaseSafeIntLib/SafeIntLib.c       | 9 +++++----
>  MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c     | 3 ++-
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf b/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> index 20a83ed..8fbdafe 100644
> --- a/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> +++ b/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> @@ -54,5 +54,8 @@
>  [Sources.EBC]
>    SafeIntLibEbc.c
>  
>  [Packages]
>    MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
> index c5f13d7..e96327d 100644
> --- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
> +++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
> @@ -26,10 +26,11 @@
>  
>  **/
>  
>  #include <Base.h>
>  #include <Library/SafeIntLib.h>
> +#include <Library/BaseLib.h>
>  
>  
>  //
>  // Magnitude of MIN_INT64 as expressed by a UINT64 number.
>  //
> @@ -3371,12 +3372,12 @@ SafeUint64Mult (
>    // a * c must be 0 or there would be bits in the high 64-bits
>    // a * d must be less than 2^32 or there would be bits in the high 64-bits
>    // b * c must be less than 2^32 or there would be bits in the high 64-bits
>    // then there must be no overflow of the resulting values summed up.
>    //
> -  DwordA = (UINT32)(Multiplicand >> 32);
> -  DwordC = (UINT32)(Multiplier >> 32);
> +  DwordA = (UINT32)RShiftU64 (Multiplicand, 32);
> +  DwordC = (UINT32)RShiftU64 (Multiplier, 32);
>  
>    //
>    // common case -- if high dwords are both zero, no chance for overflow
>    //
>    if ((DwordA == 0) && (DwordC == 0)) {
> @@ -3407,11 +3408,11 @@ SafeUint64Mult (
>          if ((ProductBC & 0xffffffff00000000) == 0) {
>            //
>            // now sum them all up checking for overflow.
>            // shifting is safe because we already checked for overflow above
>            //
> -          if (!RETURN_ERROR (SafeUint64Add (ProductBC << 32, ProductAD << 32, &UnsignedResult))) {
> +          if (!RETURN_ERROR (SafeUint64Add (LShiftU64 (ProductBC, 32), LShiftU64 (ProductAD, 32), &UnsignedResult))) {
>              //
>              // b * d
>              //
>              ProductBD = (((UINT64)DwordB) *(UINT64)DwordD);
>  
> @@ -4073,11 +4074,11 @@ SafeInt32Mult (
>    IN  INT32  Multiplicand,
>    IN  INT32  Multiplier,
>    OUT INT32  *Result
>    )
>  {
> -  return SafeInt64ToInt32 (((INT64)Multiplicand) *((INT64)Multiplier), Result);
> +  return SafeInt64ToInt32 (MultS64x64 (Multiplicand, Multiplier), Result);
>  }
>  
>  /**
>    INT64 multiplication
>  
> diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c b/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
> index 18bfb9e..ce66a92 100644
> --- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
> +++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
> @@ -26,10 +26,11 @@
>  
>  **/
>  
>  #include <Base.h>
>  #include <Library/SafeIntLib.h>
> +#include <Library/BaseLib.h>
>  
>  /**
>    INT32 -> UINTN conversion
>  
>    Converts the value specified by Operand to a value specified by Result type
> @@ -547,8 +548,8 @@ SafeIntnMult (
>    IN  INTN  Multiplicand,
>    IN  INTN  Multiplier,
>    OUT INTN  *Result
>    )
>  {
> -  return SafeInt64ToIntn (((INT64)Multiplicand) *((INT64)Multiplier), Result);
> +  return SafeInt64ToIntn (MultS64x64 (Multiplicand, Multiplier), Result);
>  }
>  
> 

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [patch v2] MdePkg/BaseSafeIntLib: Fix VS2015 IA32 NOOPT build failure
Posted by Gao, Liming 6 years, 1 month ago
Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Bi, Dandan
> Sent: Tuesday, February 27, 2018 9:47 AM
> To: edk2-devel@lists.01.org
> Cc: Laszlo Ersek <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Subject: [patch v2] MdePkg/BaseSafeIntLib: Fix VS2015 IA32 NOOPT build failure
> 
> v2: Add [LibraryClasses] section in INF file and refine coding style.
> 
> There are VS2015 NOOPT IA32 build failure like below in BaseSafeIntLib.
> XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __allmul
> XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __allshl
> XXX.lib(XXX.obj): error LNK2001: unresolved external symbol __aullshr
> 
> This patch replaces direct shift/multiplication of 64-bit integer
> with related function call to fix these failure.
> 
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Dandan Bi <dandan.bi@intel.com>
> ---
>  MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf | 3 +++
>  MdePkg/Library/BaseSafeIntLib/SafeIntLib.c       | 9 +++++----
>  MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c     | 3 ++-
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf b/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> index 20a83ed..8fbdafe 100644
> --- a/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> +++ b/MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf
> @@ -54,5 +54,8 @@
>  [Sources.EBC]
>    SafeIntLibEbc.c
> 
>  [Packages]
>    MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  BaseLib
> diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
> index c5f13d7..e96327d 100644
> --- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
> +++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib.c
> @@ -26,10 +26,11 @@
> 
>  **/
> 
>  #include <Base.h>
>  #include <Library/SafeIntLib.h>
> +#include <Library/BaseLib.h>
> 
> 
>  //
>  // Magnitude of MIN_INT64 as expressed by a UINT64 number.
>  //
> @@ -3371,12 +3372,12 @@ SafeUint64Mult (
>    // a * c must be 0 or there would be bits in the high 64-bits
>    // a * d must be less than 2^32 or there would be bits in the high 64-bits
>    // b * c must be less than 2^32 or there would be bits in the high 64-bits
>    // then there must be no overflow of the resulting values summed up.
>    //
> -  DwordA = (UINT32)(Multiplicand >> 32);
> -  DwordC = (UINT32)(Multiplier >> 32);
> +  DwordA = (UINT32)RShiftU64 (Multiplicand, 32);
> +  DwordC = (UINT32)RShiftU64 (Multiplier, 32);
> 
>    //
>    // common case -- if high dwords are both zero, no chance for overflow
>    //
>    if ((DwordA == 0) && (DwordC == 0)) {
> @@ -3407,11 +3408,11 @@ SafeUint64Mult (
>          if ((ProductBC & 0xffffffff00000000) == 0) {
>            //
>            // now sum them all up checking for overflow.
>            // shifting is safe because we already checked for overflow above
>            //
> -          if (!RETURN_ERROR (SafeUint64Add (ProductBC << 32, ProductAD << 32, &UnsignedResult))) {
> +          if (!RETURN_ERROR (SafeUint64Add (LShiftU64 (ProductBC, 32), LShiftU64 (ProductAD, 32), &UnsignedResult))) {
>              //
>              // b * d
>              //
>              ProductBD = (((UINT64)DwordB) *(UINT64)DwordD);
> 
> @@ -4073,11 +4074,11 @@ SafeInt32Mult (
>    IN  INT32  Multiplicand,
>    IN  INT32  Multiplier,
>    OUT INT32  *Result
>    )
>  {
> -  return SafeInt64ToInt32 (((INT64)Multiplicand) *((INT64)Multiplier), Result);
> +  return SafeInt64ToInt32 (MultS64x64 (Multiplicand, Multiplier), Result);
>  }
> 
>  /**
>    INT64 multiplication
> 
> diff --git a/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c b/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
> index 18bfb9e..ce66a92 100644
> --- a/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
> +++ b/MdePkg/Library/BaseSafeIntLib/SafeIntLib32.c
> @@ -26,10 +26,11 @@
> 
>  **/
> 
>  #include <Base.h>
>  #include <Library/SafeIntLib.h>
> +#include <Library/BaseLib.h>
> 
>  /**
>    INT32 -> UINTN conversion
> 
>    Converts the value specified by Operand to a value specified by Result type
> @@ -547,8 +548,8 @@ SafeIntnMult (
>    IN  INTN  Multiplicand,
>    IN  INTN  Multiplier,
>    OUT INTN  *Result
>    )
>  {
> -  return SafeInt64ToIntn (((INT64)Multiplicand) *((INT64)Multiplier), Result);
> +  return SafeInt64ToIntn (MultS64x64 (Multiplicand, Multiplier), Result);
>  }
> 
> --
> 1.9.5.msysgit.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel