| Message ID | 1411075992-17220-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
Hi Ard, On 09/18/14 23:33, Ard Biesheuvel wrote: > Users of the LP64 data model should declare SIXTY_FOUR_BIT_LONG, > not SIXTY_FOUR_BIT when building OpenSSL. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Reviewed-By: Olivier Martin <olivier.martin@arm.com> > Reviewed-by: Andrew Fish <afish@apple.com> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > CryptoPkg/Library/OpensslLib/OpensslLib.inf | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf > index d380158a4339..f32afb9b65ff 100644 > --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf > +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf > @@ -655,10 +655,10 @@ > INTEL:*_*_X64_CC_FLAGS = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) /w -DSIXTY_FOUR_BIT > INTEL:*_*_IPF_CC_FLAGS = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) /w -DSIXTY_FOUR_BIT > GCC:*_*_IA32_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT > - GCC:*_*_X64_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT > - GCC:*_*_IPF_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT > + GCC:*_*_X64_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG > + GCC:*_*_IPF_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG > GCC:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT > - GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT > + GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG > > # suppress the following warnings in openssl so we don't break the build with warnings-as-errors: > # 1295: Deprecated declaration <entity> - give arg types > @@ -674,4 +674,4 @@ > # 1296: Extended constant initialiser used > RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) --library_interface=aeabi_clib99 --fpu=vfpv3 -DTHIRTY_TWO_BIT --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188 > XCODE:*_*_IA32_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT > - XCODE:*_*_X64_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT > + XCODE:*_*_X64_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG > what's the impact of this issue? Thanks Laszlo ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that Matters. http://pubads.g.doubleclick.net/gampad/clk?id=160591471&iu=/4140/ostg.clktrk
On 19 September 2014 03:38, Laszlo Ersek <lersek@redhat.com> wrote: > Hi Ard, > > On 09/18/14 23:33, Ard Biesheuvel wrote: >> Users of the LP64 data model should declare SIXTY_FOUR_BIT_LONG, >> not SIXTY_FOUR_BIT when building OpenSSL. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Reviewed-By: Olivier Martin <olivier.martin@arm.com> >> Reviewed-by: Andrew Fish <afish@apple.com> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> CryptoPkg/Library/OpensslLib/OpensslLib.inf | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf >> index d380158a4339..f32afb9b65ff 100644 >> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf >> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf >> @@ -655,10 +655,10 @@ >> INTEL:*_*_X64_CC_FLAGS = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) /w -DSIXTY_FOUR_BIT >> INTEL:*_*_IPF_CC_FLAGS = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) /w -DSIXTY_FOUR_BIT >> GCC:*_*_IA32_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT >> - GCC:*_*_X64_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT >> - GCC:*_*_IPF_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT >> + GCC:*_*_X64_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG >> + GCC:*_*_IPF_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG >> GCC:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT >> - GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT >> + GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG >> >> # suppress the following warnings in openssl so we don't break the build with warnings-as-errors: >> # 1295: Deprecated declaration <entity> - give arg types >> @@ -674,4 +674,4 @@ >> # 1296: Extended constant initialiser used >> RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) --library_interface=aeabi_clib99 --fpu=vfpv3 -DTHIRTY_TWO_BIT --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188 >> XCODE:*_*_IA32_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT >> - XCODE:*_*_X64_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT >> + XCODE:*_*_X64_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG >> > > what's the impact of this issue? > The impact of this issue is that, while the code appears to work correctly either way, you are essentially telling OpenSSL that sizeof(long) == 4, which at the very least means you are taking different code paths through OpenSSL than when you run the same exact version under the OS on the same hardware (note that OpenSSL uses a fair amount of #ifdef __GNUC__ and #ifdef <arch> conditionals as well). I don't think performance would be affected, i.e., the bignum code will probably use 8-byte ints either way, but it's better to be 100% correct with issues like this.
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf index d380158a4339..f32afb9b65ff 100644 --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf @@ -655,10 +655,10 @@ INTEL:*_*_X64_CC_FLAGS = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) /w -DSIXTY_FOUR_BIT INTEL:*_*_IPF_CC_FLAGS = -U_WIN32 -U_WIN64 -U_MSC_VER -U__ICC $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) /w -DSIXTY_FOUR_BIT GCC:*_*_IA32_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT - GCC:*_*_X64_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT - GCC:*_*_IPF_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT + GCC:*_*_X64_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG + GCC:*_*_IPF_CC_FLAGS = -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG GCC:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT - GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT + GCC:*_*_AARCH64_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG # suppress the following warnings in openssl so we don't break the build with warnings-as-errors: # 1295: Deprecated declaration <entity> - give arg types @@ -674,4 +674,4 @@ # 1296: Extended constant initialiser used RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) --library_interface=aeabi_clib99 --fpu=vfpv3 -DTHIRTY_TWO_BIT --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188 XCODE:*_*_IA32_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DTHIRTY_TWO_BIT - XCODE:*_*_X64_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT + XCODE:*_*_X64_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 $(OPENSSL_FLAGS) $(OPENSSL_EXFLAGS) -w -DSIXTY_FOUR_BIT_LONG