Resolving Some CryptoPkg Build Issues

[edk2] [PATCH v2 0/4] Resolving Some CryptoPkg Build Issues

Posted by Long Qin 7 years, 7 months ago

From: Qin Long <qin.long@intel.com>

V2:
  Updated the patches as the comments from Laszlo (lersek@redhat.com).
  And filed two TianoCore BZ (#455, #456) to track the further follow-ups
  on openssl and EDKII-CryptoPkg:
    https://bugzilla.tianocore.org/show_bug.cgi?id=455
    https://bugzilla.tianocore.org/show_bug.cgi?id=456

This patch series introduced some hotfixes and workaround to resolve
the build issues under different toolchain, and from potential external
consumers, including:
  - build warning under GCC48 and VS2010 toolchain;
  - Potential unresolved external symbol link issue;
  - One bug fix of timer() wrapper in ConstantTimeClock.c;
  - One workaround to resolve macro re-definitions issue from some
    external BaseCryptLib consumer.

  (https://github.com/qloong/edk2/commits/dev-openssl-hotfix)

Qin Long (4):
  CryptoPkg/OpensslLib: Suppress extra build warnings in openssl source
  CryptoPkg: Fix possible unresolved external symbol issue.
  CryptoPkg/BaseCryptLib: Adding NULL checking in time() wrapper.
  CryptoPkg: One workaround to resolve potential build issue.

 CryptoPkg/Include/CrtLibSupport.h                  |   1 +
 CryptoPkg/Include/openssl/e_os2.h                  | 321 +++++++++++++++++++++
 .../BaseCryptLib/SysCall/ConstantTimeClock.c       |   6 +-
 CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c  |  10 +-
 CryptoPkg/Library/OpensslLib/OpensslLib.inf        |  15 +-
 CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
 6 files changed, 355 insertions(+), 13 deletions(-)
 create mode 100644 CryptoPkg/Include/openssl/e_os2.h

-- 
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 v2 0/4] Resolving Some CryptoPkg Build Issues

Posted by Laszlo Ersek 7 years, 7 months ago

On 04/01/17 07:38, Long Qin wrote:
> From: Qin Long <qin.long@intel.com>
> 
> V2:
>   Updated the patches as the comments from Laszlo (lersek@redhat.com).
>   And filed two TianoCore BZ (#455, #456) to track the further follow-ups
>   on openssl and EDKII-CryptoPkg:
>     https://bugzilla.tianocore.org/show_bug.cgi?id=455
>     https://bugzilla.tianocore.org/show_bug.cgi?id=456

Thanks for the update.

Before we proceed with this patch set, can you please report an upstream
OpenSSL issue at

  https://github.com/openssl/openssl/issues

for each one of the two problems worked around in this patch set? (*)

And, those upstream OpenSSL tickets should be referenced in the
corresponding TianoCore bug reports #455 and #456, by URL.

The goal is that, at any point in the future, we can quickly check the
status of the OpenSSL upstreaming, by looking at the upstream OpenSSL
bug reports.

(*) The OpenSSL bug tracker is on github now, according to

  https://www.openssl.org/community/

and

  https://www.openssl.org/blog/blog/2016/10/12/f2f-rt-github/

Thanks!
Laszlo

> 
> This patch series introduced some hotfixes and workaround to resolve
> the build issues under different toolchain, and from potential external
> consumers, including:
>   - build warning under GCC48 and VS2010 toolchain;
>   - Potential unresolved external symbol link issue;
>   - One bug fix of timer() wrapper in ConstantTimeClock.c;
>   - One workaround to resolve macro re-definitions issue from some
>     external BaseCryptLib consumer.
> 
>   (https://github.com/qloong/edk2/commits/dev-openssl-hotfix)
> 
> Qin Long (4):
>   CryptoPkg/OpensslLib: Suppress extra build warnings in openssl source
>   CryptoPkg: Fix possible unresolved external symbol issue.
>   CryptoPkg/BaseCryptLib: Adding NULL checking in time() wrapper.
>   CryptoPkg: One workaround to resolve potential build issue.
> 
>  CryptoPkg/Include/CrtLibSupport.h                  |   1 +
>  CryptoPkg/Include/openssl/e_os2.h                  | 321 +++++++++++++++++++++
>  .../BaseCryptLib/SysCall/ConstantTimeClock.c       |   6 +-
>  CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c  |  10 +-
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf        |  15 +-
>  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
>  6 files changed, 355 insertions(+), 13 deletions(-)
>  create mode 100644 CryptoPkg/Include/openssl/e_os2.h
> 

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

Re: [edk2] [PATCH v2 0/4] Resolving Some CryptoPkg Build Issues

Posted by Laszlo Ersek 7 years, 6 months ago

On 04/01/17 07:38, Long Qin wrote:
> From: Qin Long <qin.long@intel.com>
> 
> V2:
>   Updated the patches as the comments from Laszlo (lersek@redhat.com).
>   And filed two TianoCore BZ (#455, #456) to track the further follow-ups
>   on openssl and EDKII-CryptoPkg:
>     https://bugzilla.tianocore.org/show_bug.cgi?id=455
>     https://bugzilla.tianocore.org/show_bug.cgi?id=456
> 
> This patch series introduced some hotfixes and workaround to resolve
> the build issues under different toolchain, and from potential external
> consumers, including:
>   - build warning under GCC48 and VS2010 toolchain;
>   - Potential unresolved external symbol link issue;
>   - One bug fix of timer() wrapper in ConstantTimeClock.c;
>   - One workaround to resolve macro re-definitions issue from some
>     external BaseCryptLib consumer.
> 
>   (https://github.com/qloong/edk2/commits/dev-openssl-hotfix)
> 
> Qin Long (4):
>   CryptoPkg/OpensslLib: Suppress extra build warnings in openssl source
>   CryptoPkg: Fix possible unresolved external symbol issue.
>   CryptoPkg/BaseCryptLib: Adding NULL checking in time() wrapper.
>   CryptoPkg: One workaround to resolve potential build issue.
> 
>  CryptoPkg/Include/CrtLibSupport.h                  |   1 +
>  CryptoPkg/Include/openssl/e_os2.h                  | 321 +++++++++++++++++++++
>  .../BaseCryptLib/SysCall/ConstantTimeClock.c       |   6 +-
>  CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c  |  10 +-
>  CryptoPkg/Library/OpensslLib/OpensslLib.inf        |  15 +-
>  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
>  6 files changed, 355 insertions(+), 13 deletions(-)
>  create mode 100644 CryptoPkg/Include/openssl/e_os2.h
> 

I can see the upstream OpenSSL pull req / issue report references in
TianoCore BZs 455 and 456.

series
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 0/4] Resolving Some CryptoPkg Build Issues

Posted by Long, Qin 7 years, 6 months ago

Thanks, Laszlo. 

And the last "workaround" patch can be dropped, since we introduced the new [Includes.Common.Private] setting in Package DEC file (from my last patch). This will help to eliminate the potential macro re-definition risk. 
It's still valuable to refine openssl e_os2.h definition for consistence, which was submitted / approved by the PR (https://github.com/openssl/openssl/pull/3121)


Best Regards & Thanks,
LONG, Qin

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Thursday, April 6, 2017 4:55 PM
> To: Long, Qin <qin.long@intel.com>; edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Tian,
> Feng <feng.tian@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: Re: [edk2] [PATCH v2 0/4] Resolving Some CryptoPkg Build Issues
> 
> On 04/01/17 07:38, Long Qin wrote:
> > From: Qin Long <qin.long@intel.com>
> >
> > V2:
> >   Updated the patches as the comments from Laszlo (lersek@redhat.com).
> >   And filed two TianoCore BZ (#455, #456) to track the further follow-ups
> >   on openssl and EDKII-CryptoPkg:
> >     https://bugzilla.tianocore.org/show_bug.cgi?id=455
> >     https://bugzilla.tianocore.org/show_bug.cgi?id=456
> >
> > This patch series introduced some hotfixes and workaround to resolve
> > the build issues under different toolchain, and from potential
> > external consumers, including:
> >   - build warning under GCC48 and VS2010 toolchain;
> >   - Potential unresolved external symbol link issue;
> >   - One bug fix of timer() wrapper in ConstantTimeClock.c;
> >   - One workaround to resolve macro re-definitions issue from some
> >     external BaseCryptLib consumer.
> >
> >   (https://github.com/qloong/edk2/commits/dev-openssl-hotfix)
> >
> > Qin Long (4):
> >   CryptoPkg/OpensslLib: Suppress extra build warnings in openssl source
> >   CryptoPkg: Fix possible unresolved external symbol issue.
> >   CryptoPkg/BaseCryptLib: Adding NULL checking in time() wrapper.
> >   CryptoPkg: One workaround to resolve potential build issue.
> >
> >  CryptoPkg/Include/CrtLibSupport.h                  |   1 +
> >  CryptoPkg/Include/openssl/e_os2.h                  | 321
> +++++++++++++++++++++
> >  .../BaseCryptLib/SysCall/ConstantTimeClock.c       |   6 +-
> >  CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c  |  10 +-
> >  CryptoPkg/Library/OpensslLib/OpensslLib.inf        |  15 +-
> >  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
> >  6 files changed, 355 insertions(+), 13 deletions(-)  create mode
> > 100644 CryptoPkg/Include/openssl/e_os2.h
> >
> 
> I can see the upstream OpenSSL pull req / issue report references in
> TianoCore BZs 455 and 456.
> 
> series
> 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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v2 0/4] Resolving Some CryptoPkg Build Issues

Posted by Laszlo Ersek 7 years, 6 months ago

On 04/06/17 13:26, Long, Qin wrote:
> Thanks, Laszlo. 
> 
> And the last "workaround" patch can be dropped, since we introduced
> the new [Includes.Common.Private] setting in Package DEC file (from
> my last patch). This will help to eliminate the potential macro
> re-definition risk.

Is the [Includes.Common.Private] section documented somewhere? I checked
the DEC spec v1.25, and it's not described there. Should I file a
documentation BZ about this?

Thanks
Laszlo

> It's still valuable to refine openssl e_os2.h definition for
> consistence, which was submitted / approved by the PR
> (https://github.com/openssl/openssl/pull/3121)
> 
> 
> Best Regards & Thanks,
> LONG, Qin
> 
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
>> Laszlo Ersek
>> Sent: Thursday, April 6, 2017 4:55 PM
>> To: Long, Qin <qin.long@intel.com>; edk2-devel@lists.01.org
>> Cc: Ye, Ting <ting.ye@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Tian,
>> Feng <feng.tian@intel.com>; Dong, Eric <eric.dong@intel.com>
>> Subject: Re: [edk2] [PATCH v2 0/4] Resolving Some CryptoPkg Build Issues
>>
>> On 04/01/17 07:38, Long Qin wrote:
>>> From: Qin Long <qin.long@intel.com>
>>>
>>> V2:
>>>   Updated the patches as the comments from Laszlo (lersek@redhat.com).
>>>   And filed two TianoCore BZ (#455, #456) to track the further follow-ups
>>>   on openssl and EDKII-CryptoPkg:
>>>     https://bugzilla.tianocore.org/show_bug.cgi?id=455
>>>     https://bugzilla.tianocore.org/show_bug.cgi?id=456
>>>
>>> This patch series introduced some hotfixes and workaround to resolve
>>> the build issues under different toolchain, and from potential
>>> external consumers, including:
>>>   - build warning under GCC48 and VS2010 toolchain;
>>>   - Potential unresolved external symbol link issue;
>>>   - One bug fix of timer() wrapper in ConstantTimeClock.c;
>>>   - One workaround to resolve macro re-definitions issue from some
>>>     external BaseCryptLib consumer.
>>>
>>>   (https://github.com/qloong/edk2/commits/dev-openssl-hotfix)
>>>
>>> Qin Long (4):
>>>   CryptoPkg/OpensslLib: Suppress extra build warnings in openssl source
>>>   CryptoPkg: Fix possible unresolved external symbol issue.
>>>   CryptoPkg/BaseCryptLib: Adding NULL checking in time() wrapper.
>>>   CryptoPkg: One workaround to resolve potential build issue.
>>>
>>>  CryptoPkg/Include/CrtLibSupport.h                  |   1 +
>>>  CryptoPkg/Include/openssl/e_os2.h                  | 321
>> +++++++++++++++++++++
>>>  .../BaseCryptLib/SysCall/ConstantTimeClock.c       |   6 +-
>>>  CryptoPkg/Library/IntrinsicLib/MemoryIntrinsics.c  |  10 +-
>>>  CryptoPkg/Library/OpensslLib/OpensslLib.inf        |  15 +-
>>>  CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf  |  15 +-
>>>  6 files changed, 355 insertions(+), 13 deletions(-)  create mode
>>> 100644 CryptoPkg/Include/openssl/e_os2.h
>>>
>>
>> I can see the upstream OpenSSL pull req / issue report references in
>> TianoCore BZs 455 and 456.
>>
>> series
>> 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
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 

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