1. Patchew
  2. EDK2
  3. [edk2] [Patch 0/4] *** Resolving CryptoPkg build issues ***
  4. View patch

[edk2] [Patch 1/4] CryptoPkg/OpensslLib: Suppress extra build warnings in openssl source

Qin Long posted 4 patches 7 years, 7 months ago
[edk2] [Patch 1/4] CryptoPkg/OpensslLib: Suppress extra build warnings in openssl source
Posted by Qin Long 7 years, 7 months ago 
This patch added some extra build options to suppress possible warnings
when building openssl source under GCC48 and VS2010. Including:

Adding "-Wno-error=maybe-uninitialized" to suppress the following GCC48
build warning:
  OpensslLib/openssl/ssl/statem/statem_clnt.c:2543:9: error: "len" may
     be used uninitialized in this function [-Werror=maybe-uninitialized]
       len += pskhdrlen;
           ^

And adding "/wd4306" to suppress the following VS2010 build warning:
  openssl\crypto\asn1\tasn_dec.c(795) : warning C4306: 'type cast' :
               conversion from 'int' to 'ASN1_VALUE *' of greater size

Cc: Ting Ye <ting.ye@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Qin Long <qin.long@intel.com>
---
 CryptoPkg/Library/OpensslLib/OpensslLib.inf       | 15 ++++++++++-----
 CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 15 ++++++++++-----
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
index 42f72f4f1f..cbabb34bdd 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
@@ -535,21 +535,26 @@
   #   C4244: conversion from type1 to type2, possible loss of data
   #   C4245: conversion from type1 to type2, signed/unsigned mismatch
   #   C4267: conversion from size_t to type, possible loss of data
+  #   C4306: 'identifier' : conversion from 'type1' to 'type2' of greater size
   #   C4389: 'operator' : signed/unsigned mismatch (xxxx)
   #   C4702: unreachable code
   #   C4706: assignment within conditional expression
   #
   MSFT:*_*_IA32_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
-  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
-  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
+  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389 /wd4702 /wd4706
+  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389 /wd4702 /wd4706
 
   INTEL:*_*_IA32_CC_FLAGS  = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
   INTEL:*_*_X64_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
   INTEL:*_*_IPF_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
 
-  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
-  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -DNO_MSABI_VA_FUNCS
-  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
+  #
+  # Suppress the following build warnings in openssl so we don't break the build with -Werror
+  #   -Werror=maybe-uninitialized: there exist some other paths for which the variable is not initialized.
+  #
+  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
+  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -DNO_MSABI_VA_FUNCS
+  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
   GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS)
   GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS)
 
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
index cbbb456d70..026b551bca 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
@@ -496,21 +496,26 @@
   #   C4244: conversion from type1 to type2, possible loss of data
   #   C4245: conversion from type1 to type2, signed/unsigned mismatch
   #   C4267: conversion from size_t to type, possible loss of data
+  #   C4306: 'identifier' : conversion from 'type1' to 'type2' of greater size
   #   C4389: 'operator' : signed/unsigned mismatch (xxxx)
   #   C4702: unreachable code
   #   C4706: assignment within conditional expression
   #
   MSFT:*_*_IA32_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
-  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
-  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
+  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389 /wd4702 /wd4706
+  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389 /wd4702 /wd4706
 
   INTEL:*_*_IA32_CC_FLAGS  = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
   INTEL:*_*_X64_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
   INTEL:*_*_IPF_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
 
-  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
-  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -DNO_MSABI_VA_FUNCS
-  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
+  #
+  # Suppress the following build warnings in openssl so we don't break the build with -Werror
+  #   -Werror=maybe-uninitialized: there exist some other paths for which the variable is not initialized.
+  #
+  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
+  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -DNO_MSABI_VA_FUNCS
+  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
   GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS)
   GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS)
 
-- 
2.12.2.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 1/4] CryptoPkg/OpensslLib: Suppress extra build warnings in openssl source
Posted by Laszlo Ersek 7 years, 7 months ago 
On 03/31/17 19:05, Qin Long wrote:
> This patch added some extra build options to suppress possible warnings
> when building openssl source under GCC48 and VS2010. Including:
> 
> Adding "-Wno-error=maybe-uninitialized" to suppress the following GCC48
> build warning:
>   OpensslLib/openssl/ssl/statem/statem_clnt.c:2543:9: error: "len" may
>      be used uninitialized in this function [-Werror=maybe-uninitialized]
>        len += pskhdrlen;
>            ^

This happens in the tls_construct_client_key_exchange() function. After
reviewing the function, the warning is indeed incorrect. I agree with
the fix (adding -Wno-error=maybe-uninitialized), but I propose the
following too:

- file an upstream OpenSSL bug report about this (the existent "len = 0"
assignment can be moved to the unconditional part of the code),
- file a TianoCore BZ as well, so that we don't forget removing
-Wno-error=maybe-uninitialized once the OpenSSL bug is fixed and we
adopt the next stable release with the OpenSSL change.

The point is, I think -Werror=maybe-uninitialized is a valuable warning,
and it might help us catch an actual bug in OpenSSL later on. So I don't
think it should be turned off permanently.

> And adding "/wd4306" to suppress the following VS2010 build warning:
>   openssl\crypto\asn1\tasn_dec.c(795) : warning C4306: 'type cast' :
>                conversion from 'int' to 'ASN1_VALUE *' of greater size

That again seems like a valid warning, the C standard doesn't guarantee
that (int) can be converted to a pointer type and vice versa. The code
should be using intptr_t or uintptr_t (in edk2 we use UINTN for that).

This is the code in question:

    case V_ASN1_NULL:
        if (len) {
            ASN1err(ASN1_F_ASN1_EX_C2I, ASN1_R_NULL_IS_WRONG_LENGTH);
            goto err;
        }
        *pval = (ASN1_VALUE *)1;
        break;

This is really awful. But, if it is necessary, it should be

  *pval = (ASN1_VALUE *)(uintptr_t)1;

Again, I think it's OK to suppress the warnings for now, but I think we
should have a longer term reminder to reenable them.

> 
> Cc: Ting Ye <ting.ye@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Qin Long <qin.long@intel.com>
> ---
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf       | 15 ++++++++++-----
>  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 15 ++++++++++-----
>  2 files changed, 20 insertions(+), 10 deletions(-)
> 
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> index 42f72f4f1f..cbabb34bdd 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> @@ -535,21 +535,26 @@
>    #   C4244: conversion from type1 to type2, possible loss of data
>    #   C4245: conversion from type1 to type2, signed/unsigned mismatch
>    #   C4267: conversion from size_t to type, possible loss of data
> +  #   C4306: 'identifier' : conversion from 'type1' to 'type2' of greater size
>    #   C4389: 'operator' : signed/unsigned mismatch (xxxx)
>    #   C4702: unreachable code
>    #   C4706: assignment within conditional expression
>    #
>    MSFT:*_*_IA32_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
> -  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
> -  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
> +  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389 /wd4702 /wd4706
> +  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389 /wd4702 /wd4706
>  
>    INTEL:*_*_IA32_CC_FLAGS  = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
>    INTEL:*_*_X64_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
>    INTEL:*_*_IPF_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
>  
> -  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
> -  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -DNO_MSABI_VA_FUNCS
> -  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
> +  #
> +  # Suppress the following build warnings in openssl so we don't break the build with -Werror
> +  #   -Werror=maybe-uninitialized: there exist some other paths for which the variable is not initialized.

If that was true, that would be an actual bug in the OpenSSL code base,
and suppressing the warning in such a case would also be a bug. We may
only suppress the warning because it is *invalid* -- there is no such
problematic code path, only the compiler (incorrectly) thinks there is.

So please update the comment. I would even go as far as including the
TianoCore BZ number in this comment, for the BZ which tracks the
longer-term re-enablement of this warning.

Looks good to me otherwise.

(Note: personally I can't reproduce the build issues that I reported, at
least with GCC48 that I use.)

Thanks!
Laszlo

> +  #
> +  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
> +  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -DNO_MSABI_VA_FUNCS
> +  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
>    GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS)
>    GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS)
>  
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> index cbbb456d70..026b551bca 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> @@ -496,21 +496,26 @@
>    #   C4244: conversion from type1 to type2, possible loss of data
>    #   C4245: conversion from type1 to type2, signed/unsigned mismatch
>    #   C4267: conversion from size_t to type, possible loss of data
> +  #   C4306: 'identifier' : conversion from 'type1' to 'type2' of greater size
>    #   C4389: 'operator' : signed/unsigned mismatch (xxxx)
>    #   C4702: unreachable code
>    #   C4706: assignment within conditional expression
>    #
>    MSFT:*_*_IA32_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
> -  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
> -  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702 /wd4706
> +  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389 /wd4702 /wd4706
> +  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389 /wd4702 /wd4706
>  
>    INTEL:*_*_IA32_CC_FLAGS  = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
>    INTEL:*_*_X64_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
>    INTEL:*_*_IPF_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) /w
>  
> -  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
> -  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -DNO_MSABI_VA_FUNCS
> -  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
> +  #
> +  # Suppress the following build warnings in openssl so we don't break the build with -Werror
> +  #   -Werror=maybe-uninitialized: there exist some other paths for which the variable is not initialized.
> +  #
> +  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
> +  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized -DNO_MSABI_VA_FUNCS
> +  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -Wno-error=maybe-uninitialized
>    GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS)
>    GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS)
>  
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 1/4] CryptoPkg/OpensslLib: Suppress extra build warnings in openssl source
Posted by Long, Qin 7 years, 7 months ago 
Exactly. Thanks, Laszlo.
Since we will keep the openssl source original after this upgrade, I have to leverage extra
build option to suppress these two warnings this time.
Yes, I prefer to keep /Werror as possible,  I will follow-up the next openssl PR and a TianoCore BZ. 


Best Regards & Thanks,
LONG, Qin

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Saturday, April 01, 2017 2:24 AM
> To: Long, Qin; edk2-devel@lists.01.org
> Cc: Ye, Ting; Wu, Hao A; Tian, Feng; Dong, Eric
> Subject: Re: [Patch 1/4] CryptoPkg/OpensslLib: Suppress extra build warnings
> in openssl source
> 
> On 03/31/17 19:05, Qin Long wrote:
> > This patch added some extra build options to suppress possible
> > warnings when building openssl source under GCC48 and VS2010. Including:
> >
> > Adding "-Wno-error=maybe-uninitialized" to suppress the following
> > GCC48 build warning:
> >   OpensslLib/openssl/ssl/statem/statem_clnt.c:2543:9: error: "len" may
> >      be used uninitialized in this function [-Werror=maybe-uninitialized]
> >        len += pskhdrlen;
> >            ^
> 
> This happens in the tls_construct_client_key_exchange() function. After
> reviewing the function, the warning is indeed incorrect. I agree with the fix
> (adding -Wno-error=maybe-uninitialized), but I propose the following too:
> 
> - file an upstream OpenSSL bug report about this (the existent "len = 0"
> assignment can be moved to the unconditional part of the code),
> - file a TianoCore BZ as well, so that we don't forget removing -Wno-
> error=maybe-uninitialized once the OpenSSL bug is fixed and we adopt the
> next stable release with the OpenSSL change.
> 
> The point is, I think -Werror=maybe-uninitialized is a valuable warning, and it
> might help us catch an actual bug in OpenSSL later on. So I don't think it
> should be turned off permanently.
> 
> > And adding "/wd4306" to suppress the following VS2010 build warning:
> >   openssl\crypto\asn1\tasn_dec.c(795) : warning C4306: 'type cast' :
> >                conversion from 'int' to 'ASN1_VALUE *' of greater size
> 
> That again seems like a valid warning, the C standard doesn't guarantee that
> (int) can be converted to a pointer type and vice versa. The code should be
> using intptr_t or uintptr_t (in edk2 we use UINTN for that).
> 
> This is the code in question:
> 
>     case V_ASN1_NULL:
>         if (len) {
>             ASN1err(ASN1_F_ASN1_EX_C2I, ASN1_R_NULL_IS_WRONG_LENGTH);
>             goto err;
>         }
>         *pval = (ASN1_VALUE *)1;
>         break;
> 
> This is really awful. But, if it is necessary, it should be
> 
>   *pval = (ASN1_VALUE *)(uintptr_t)1;
> 
> Again, I think it's OK to suppress the warnings for now, but I think we should
> have a longer term reminder to reenable them.
> 
> >
> > Cc: Ting Ye <ting.ye@intel.com>
> > Cc: Hao Wu <hao.a.wu@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Qin Long <qin.long@intel.com>
> > ---
> >  CryptoPkg/Library/OpensslLib/OpensslLib.inf       | 15 ++++++++++-----
> >  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 15
> > ++++++++++-----
> >  2 files changed, 20 insertions(+), 10 deletions(-)
> >
> > diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > index 42f72f4f1f..cbabb34bdd 100644
> > --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> > @@ -535,21 +535,26 @@
> >    #   C4244: conversion from type1 to type2, possible loss of data
> >    #   C4245: conversion from type1 to type2, signed/unsigned mismatch
> >    #   C4267: conversion from size_t to type, possible loss of data
> > +  #   C4306: 'identifier' : conversion from 'type1' to 'type2' of greater size
> >    #   C4389: 'operator' : signed/unsigned mismatch (xxxx)
> >    #   C4702: unreachable code
> >    #   C4706: assignment within conditional expression
> >    #
> >    MSFT:*_*_IA32_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER
> $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702
> /wd4706
> > -  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER
> $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702
> /wd4706
> > -  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER
> $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702
> /wd4706
> > +  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER
> $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389
> /wd4702 /wd4706
> > +  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER
> $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389
> /wd4702 /wd4706
> >
> >    INTEL:*_*_IA32_CC_FLAGS  = -U_WIN32 -U_WIN64 -U_MSC_VER -
> U__ICC $(OPENSSL_FLAGS) /w
> >    INTEL:*_*_X64_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -
> U__ICC $(OPENSSL_FLAGS) /w
> >    INTEL:*_*_IPF_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -
> U__ICC $(OPENSSL_FLAGS) /w
> >
> > -  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
> > -  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -
> DNO_MSABI_VA_FUNCS
> > -  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
> > +  #
> > +  # Suppress the following build warnings in openssl so we don't break the
> build with -Werror
> > +  #   -Werror=maybe-uninitialized: there exist some other paths for which
> the variable is not initialized.
> 
> If that was true, that would be an actual bug in the OpenSSL code base, and
> suppressing the warning in such a case would also be a bug. We may only
> suppress the warning because it is *invalid* -- there is no such problematic
> code path, only the compiler (incorrectly) thinks there is.
> 
> So please update the comment. I would even go as far as including the
> TianoCore BZ number in this comment, for the BZ which tracks the longer-
> term re-enablement of this warning.
> 
> Looks good to me otherwise.
> 
> (Note: personally I can't reproduce the build issues that I reported, at least
> with GCC48 that I use.)
> 
> Thanks!
> Laszlo
> 
> > +  #
> > +  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -
> Wno-error=maybe-uninitialized
> > +  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -
> Wno-error=maybe-uninitialized -DNO_MSABI_VA_FUNCS
> > +  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -
> Wno-error=maybe-uninitialized
> >    GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS)
> >    GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS)
> >
> > diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> > b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> > index cbbb456d70..026b551bca 100644
> > --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> > +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> > @@ -496,21 +496,26 @@
> >    #   C4244: conversion from type1 to type2, possible loss of data
> >    #   C4245: conversion from type1 to type2, signed/unsigned mismatch
> >    #   C4267: conversion from size_t to type, possible loss of data
> > +  #   C4306: 'identifier' : conversion from 'type1' to 'type2' of greater size
> >    #   C4389: 'operator' : signed/unsigned mismatch (xxxx)
> >    #   C4702: unreachable code
> >    #   C4706: assignment within conditional expression
> >    #
> >    MSFT:*_*_IA32_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER
> $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702
> /wd4706
> > -  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER
> $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702
> /wd4706
> > -  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER
> $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4389 /wd4702
> /wd4706
> > +  MSFT:*_*_X64_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER
> $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389
> /wd4702 /wd4706
> > +  MSFT:*_*_IPF_CC_FLAGS    = -U_WIN32 -U_WIN64 -U_MSC_VER
> $(OPENSSL_FLAGS) /wd4090 /wd4244 /wd4245 /wd4267 /wd4306 /wd4389
> /wd4702 /wd4706
> >
> >    INTEL:*_*_IA32_CC_FLAGS  = -U_WIN32 -U_WIN64 -U_MSC_VER -
> U__ICC $(OPENSSL_FLAGS) /w
> >    INTEL:*_*_X64_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -
> U__ICC $(OPENSSL_FLAGS) /w
> >    INTEL:*_*_IPF_CC_FLAGS   = -U_WIN32 -U_WIN64 -U_MSC_VER -
> U__ICC $(OPENSSL_FLAGS) /w
> >
> > -  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
> > -  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -
> DNO_MSABI_VA_FUNCS
> > -  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS)
> > +  #
> > +  # Suppress the following build warnings in openssl so we don't break the
> build with -Werror
> > +  #   -Werror=maybe-uninitialized: there exist some other paths for which
> the variable is not initialized.
> > +  #
> > +  GCC:*_*_IA32_CC_FLAGS    = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -
> Wno-error=maybe-uninitialized
> > +  GCC:*_*_X64_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -
> Wno-error=maybe-uninitialized -DNO_MSABI_VA_FUNCS
> > +  GCC:*_*_IPF_CC_FLAGS     = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) -
> Wno-error=maybe-uninitialized
> >    GCC:*_*_ARM_CC_FLAGS     = $(OPENSSL_FLAGS)
> >    GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS)
> >
> >

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

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.