Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

["Yao, Jiewen" <jiewen.yao@intel.com>]

There are some open source implementation:
MSVC: https://github.com/ispc/ispc/issues/542
GCC: https://android.googlesource.com/toolchain/gcc/+/b094d6c4bf572654a031ecc4afe675154c886dc5/gcc-4.4.3/gcc/config/i386/chkstk.asm

The compiler generated code assumes the stack is enlarged after we can chkstk.

I agree empty function can make build pass.
But more important, we need make it work if there is a need to increase the stack.
The potential issue is that the later code (after chkstk) assumes the stack is increased, but actually it is not.

That is why I ask how this is validated.

Thank you
Yao Jiewen


> -----Original Message-----
> From: Marvin Häuser [mailto:Marvin.Haeuser@outlook.com]
> Sent: Tuesday, May 8, 2018 4:58 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> Hey Yao,
> 
> As far as I am aware, all __chkstk does is access the stack in 4 KB intervals from
> the current location to the maximum requested location to eventually hit the
> Windows Guard Page, which then triggers the stack increase.
> Because I do not know of any equivalent concept in edk2 and the intrinsic was
> already disabled for GCC, I supposed it was a good idea to do so globally. Are the
> potential issues I am not aware of?
> 
> Thanks,
> Marvin.
> 
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Wednesday, May 9, 2018 1:52 AM
> > To: Marvin.Haeuser@outlook.com; edk2-devel@lists.01.org
> > Cc: Gao, Liming <liming.gao@intel.com>
> > Subject: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> >
> > HI Marvin
> > Would you mind to share the information on how you test this update?
> >
> >
> > Per my experience, chkstk not only does the check but also does the real
> > work to allocate more stack.
> >
> > /Gs can be used to indicate the size (/Gs[num] control stack checking calls)
> >
> > We use /Gs32768 by default, see tools_def.txt.
> >
> > Usually, we just increase this value to override the default one, if we hit this
> > issue.
> >
> > Thank you
> > Yao Jiewen
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> > > Marvin H?user
> > > Sent: Tuesday, May 8, 2018 4:37 PM
> > > To: edk2-devel@lists.01.org
> > > Cc: Gao, Liming <liming.gao@intel.com>
> > > Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > >
> > > Hey Liming,
> > >
> > > According to the MSDN documentation, the call will be inserted for a
> > > stack usage past a defined threshold - 4 KB for IA32, 8 KB for X64 (source:
> > > https://msdn.microsoft.com/en-us/library/ms648426(v=vs.85).aspx).
> > >
> > > Thanks,
> > > Marvin
> > >
> > > > -----Ursprüngliche Nachricht-----
> > > > Von: Gao, Liming <liming.gao@intel.com>
> > > > Gesendet: Montag, 7. Mai 2018 14:16
> > > > An: Marvin.Haeuser@outlook.com; edk2-devel@lists.01.org
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>
> > > > Betreff: RE: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > > >
> > > > Marvin:
> > > >   In VS compiler, what case will trig this intrinsic function?
> > > >
> > > > Thanks
> > > > Liming
> > > > > -----Original Message-----
> > > > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On
> > > > > Behalf Of Marvin H?user
> > > > > Sent: Saturday, May 5, 2018 10:25 PM
> > > > > To: edk2-devel@lists.01.org
> > > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> > > > > <liming.gao@intel.com>
> > > > > Subject: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> > > > >
> > > > > Initially added for GCC build support, this patch includes the
> > > > > function for all compilers and all architectures. This is done as
> > > > > huge variables on the stack may cause the generation of calls to
> > > > > this intrinsic function for Microsoft compilers, even for the IA32
> > > > > architecture, too.
> > > > >
> > > > > Contributed-under: TianoCore Contribution Agreement 1.1
> > > > > Signed-off-by: Marvin Haeuser <Marvin.Haeuser@outlook.com>
> > > > > ---
> > > > >  MdePkg/Library/BaseLib/{ChkStkGcc.c => ChkStk.c} | 48
> > > > > ++++++++++------
> > > > ----
> > > > >  MdePkg/Library/BaseLib/BaseLib.inf               |  4 +-
> > > > >  2 files changed, 26 insertions(+), 26 deletions(-)
> > > > >
> > > > > diff --git a/MdePkg/Library/BaseLib/ChkStkGcc.c
> > > > > b/MdePkg/Library/BaseLib/ChkStk.c similarity index 74% rename from
> > > > > MdePkg/Library/BaseLib/ChkStkGcc.c
> > > > > rename to MdePkg/Library/BaseLib/ChkStk.c index
> > > > > ecba3853b159..59e6d73f9331 100644
> > > > > --- a/MdePkg/Library/BaseLib/ChkStkGcc.c
> > > > > +++ b/MdePkg/Library/BaseLib/ChkStk.c
> > > > > @@ -1,24 +1,24 @@
> > > > > -/** @file
> > > > > -  Provides hack function for passng GCC build.
> > > > > -
> > > > > -  Copyright (c) 2006 - 2008, Intel Corporation. All rights
> > > > > reserved.<BR>
> > > > > -  This program and the accompanying materials
> > > > > -  are licensed and made available under the terms and conditions
> > > > > of the BSD License
> > > > > -  which accompanies this distribution.  The full text of the
> > > > > license may be found at
> > > > > -  http://opensource.org/licenses/bsd-license.php.
> > > > > -
> > > > > -  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> > IS"
> > > > > BASIS,
> > > > > -  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER
> > > > EXPRESS OR IMPLIED.
> > > > > -
> > > > > -**/
> > > > > -
> > > > > -#include "BaseLibInternals.h"
> > > > > -
> > > > > -/**
> > > > > -  Hack function for passing GCC build.
> > > > > -**/
> > > > > -VOID
> > > > > -__chkstk()
> > > > > -{
> > > > > -}
> > > > > -
> > > > > +/** @file
> > > > > +  Provides hack function for passing build.
> > > > > +
> > > > > +  Copyright (c) 2006 - 2018, Intel Corporation. All rights
> > > > > + reserved.<BR>  This program and the accompanying materials  are
> > > > > + licensed and made available under the terms and conditions of
> > > > > + the BSD License  which accompanies this distribution.  The full
> > > > > + text of the license may be found at
> > > > > + http://opensource.org/licenses/bsd-
> > > > license.php.
> > > > > +
> > > > > +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS
> > IS"
> > > > > + BASIS,  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND,
> > > > EITHER EXPRESS OR IMPLIED.
> > > > > +
> > > > > +**/
> > > > > +
> > > > > +#include "BaseLibInternals.h"
> > > > > +
> > > > > +/**
> > > > > +  Hack function for passing build.
> > > > > +**/
> > > > > +VOID
> > > > > +__chkstk()
> > > > > +{
> > > > > +}
> > > > > +
> > > > > diff --git a/MdePkg/Library/BaseLib/BaseLib.inf
> > > > > b/MdePkg/Library/BaseLib/BaseLib.inf
> > > > > index 5fbbd02a94b6..d23a6db2581a 100644
> > > > > --- a/MdePkg/Library/BaseLib/BaseLib.inf
> > > > > +++ b/MdePkg/Library/BaseLib/BaseLib.inf
> > > > > @@ -1,7 +1,7 @@
> > > > >  ## @file
> > > > >  #  Base Library implementation.
> > > > >  #
> > > > > -#  Copyright (c) 2007 - 2016, Intel Corporation. All rights
> > > > > reserved.<BR>
> > > > > +#  Copyright (c) 2007 - 2018, Intel Corporation. All rights
> > > > > +reserved.<BR>
> > > > >  #  Portions copyright (c) 2008 - 2009, Apple Inc. All rights
> > > > > reserved.<BR>  #  Portions copyright (c) 2011 - 2013, ARM Ltd. All
> > > > > rights reserved.<BR>  # @@ -64,6 +64,7 @@ [Sources]
> > > > >    SafeString.c
> > > > >    String.c
> > > > >    FilePaths.c
> > > > > +  ChkStk.c
> > > > >    BaseLibInternals.h
> > > > >
> > > > >  [Sources.Ia32]
> > > > > @@ -781,7 +782,6 @@ [Sources.X64]
> > > > >    X64/DisableCache.S | GCC
> > > > >    X64/RdRand.nasm| GCC
> > > > >    X64/RdRand.S | GCC
> > > > > -  ChkStkGcc.c  | GCC
> > > > >
> > > > >  [Sources.IPF]
> > > > >    Ipf/AccessGp.s
> > > > > --
> > > > > 2.17.0.windows.1
> > > > >
> > > > > _______________________________________________
> > > > > 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