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

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

[Marvin Häuser]

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

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

[Gao, Liming]

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

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

[Marvin Häuser]

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

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

[Yao, Jiewen]

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

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

[Marvin Häuser]

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

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

[Yao, Jiewen]

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

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

[Marvin Häuser]

Oh, if you are prefering a good implementation, I will be all for that. This was just a 'quick workaround', same as currently done for GCC.
I'm actually unsure whether a good implementation is possible with a flat memory model. It will likely be mere luck whether there is enough space free below the stack, except for maybe when it's located very high (preferably past the 32-bit space).
Has there been any prior discussion on this topic? Would be interested to follow up if there was.

Thanks,
Marvin

> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Wednesday, May 9, 2018 2:13 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
> 
> There are some open source implementation:
> MSVC: https://github.com/ispc/ispc/issues/542
> GCC:
> https://android.googlesource.com/toolchain/gcc/+/b094d6c4bf572654a031e
> cc4afe675154c886dc5/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

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

[Yao, Jiewen]

We discussed internally in Intel.

The quick workaround is: use /Gs65536. :-)

At the same time, our recommendation is to revisit the code which triggers this error. Why this function need such a big stack? And try to reduce the local stack usage.

What is why we still use /Gs32768 as default, instead of /Gs65536.

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 5:21 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> Oh, if you are prefering a good implementation, I will be all for that. This was just
> a 'quick workaround', same as currently done for GCC.
> I'm actually unsure whether a good implementation is possible with a flat
> memory model. It will likely be mere luck whether there is enough space free
> below the stack, except for maybe when it's located very high (preferably past
> the 32-bit space).
> Has there been any prior discussion on this topic? Would be interested to follow
> up if there was.
> 
> Thanks,
> Marvin
> 
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Wednesday, May 9, 2018 2:13 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
> >
> > There are some open source implementation:
> > MSVC: https://github.com/ispc/ispc/issues/542
> > GCC:
> > https://android.googlesource.com/toolchain/gcc/+/b094d6c4bf572654a031e
> > cc4afe675154c886dc5/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
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

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

[Tim Lewis]

I think that this is a fatal error in EDK2. It basically says, "we are out
of stack space." The alternative is: the system hangs in an unexpected way
since the stack overflows into other pages.

Tim

-----Original Message-----
From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Yao, Jiewen
Sent: Tuesday, May 8, 2018 5:25 PM
To: Marvin.Haeuser@outlook.com; edk2-devel@lists.01.org
Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

We discussed internally in Intel.

The quick workaround is: use /Gs65536. :-)

At the same time, our recommendation is to revisit the code which triggers
this error. Why this function need such a big stack? And try to reduce the
local stack usage.

What is why we still use /Gs32768 as default, instead of /Gs65536.

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 5:21 PM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>
> Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> Oh, if you are prefering a good implementation, I will be all for 
> that. This was just a 'quick workaround', same as currently done for GCC.
> I'm actually unsure whether a good implementation is possible with a 
> flat memory model. It will likely be mere luck whether there is enough 
> space free below the stack, except for maybe when it's located very 
> high (preferably past the 32-bit space).
> Has there been any prior discussion on this topic? Would be interested 
> to follow up if there was.
> 
> Thanks,
> Marvin
> 
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Wednesday, May 9, 2018 2:13 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
> >
> > There are some open source implementation:
> > MSVC: https://github.com/ispc/ispc/issues/542
> > GCC:
> > https://android.googlesource.com/toolchain/gcc/+/b094d6c4bf572654a03
> > 1e cc4afe675154c886dc5/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
> 
> _______________________________________________
> 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: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

[Marvin Häuser]

Hey Tim,

The compiler has no information regarding the size of the stack and hence cannot determine an overflow.
This is basically a warning that stack of a single function is big enough to reach past the Guard Page and ensures it will be accessed.

Yao,

I just realized there already is Stack protection code in edk2, I was only aware of Heap protection.
Wouldn't a proper implementation make sense now to maybe not expand the stack, but at least runtime-error?

Regards,
Marvin

> -----Original Message-----
> From: Tim Lewis <tim.lewis@insyde.com>
> Sent: Wednesday, May 9, 2018 3:28 AM
> To: 'Yao, Jiewen' <jiewen.yao@intel.com>; Marvin.Haeuser@outlook.com;
> edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> I think that this is a fatal error in EDK2. It basically says, "we are out of stack
> space." The alternative is: the system hangs in an unexpected way since the
> stack overflows into other pages.
> 
> Tim
> 
> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Yao,
> Jiewen
> Sent: Tuesday, May 8, 2018 5:25 PM
> To: Marvin.Haeuser@outlook.com; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> We discussed internally in Intel.
> 
> The quick workaround is: use /Gs65536. :-)
> 
> At the same time, our recommendation is to revisit the code which triggers
> this error. Why this function need such a big stack? And try to reduce the
> local stack usage.
> 
> What is why we still use /Gs32768 as default, instead of /Gs65536.
> 
> 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 5:21 PM
> > To: edk2-devel@lists.01.org
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> >
> > Oh, if you are prefering a good implementation, I will be all for
> > that. This was just a 'quick workaround', same as currently done for GCC.
> > I'm actually unsure whether a good implementation is possible with a
> > flat memory model. It will likely be mere luck whether there is enough
> > space free below the stack, except for maybe when it's located very
> > high (preferably past the 32-bit space).
> > Has there been any prior discussion on this topic? Would be interested
> > to follow up if there was.
> >
> > Thanks,
> > Marvin
> >
> > > -----Original Message-----
> > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > Sent: Wednesday, May 9, 2018 2:13 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
> > >
> > > There are some open source implementation:
> > > MSVC: https://github.com/ispc/ispc/issues/542
> > > GCC:
> > >
> https://android.googlesource.com/toolchain/gcc/+/b094d6c4bf572654a03
> > > 1e cc4afe675154c886dc5/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
> >
> > _______________________________________________
> > 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: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

[Tim Lewis]

Marvin -

My suggestion would be the same as yours: to generate a runtime error.

Tim

-----Original Message-----
From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Marvin
Häuser
Sent: Wednesday, May 9, 2018 1:45 AM
To: edk2-devel@lists.01.org
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Tim Lewis <tim.lewis@insyde.com>
Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c

Hey Tim,

The compiler has no information regarding the size of the stack and hence
cannot determine an overflow.
This is basically a warning that stack of a single function is big enough to
reach past the Guard Page and ensures it will be accessed.

Yao,

I just realized there already is Stack protection code in edk2, I was only
aware of Heap protection.
Wouldn't a proper implementation make sense now to maybe not expand the
stack, but at least runtime-error?

Regards,
Marvin

> -----Original Message-----
> From: Tim Lewis <tim.lewis@insyde.com>
> Sent: Wednesday, May 9, 2018 3:28 AM
> To: 'Yao, Jiewen' <jiewen.yao@intel.com>; Marvin.Haeuser@outlook.com; 
> edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> I think that this is a fatal error in EDK2. It basically says, "we are 
> out of stack space." The alternative is: the system hangs in an 
> unexpected way since the stack overflows into other pages.
> 
> Tim
> 
> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of Yao, 
> Jiewen
> Sent: Tuesday, May 8, 2018 5:25 PM
> To: Marvin.Haeuser@outlook.com; edk2-devel@lists.01.org
> Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
> 
> We discussed internally in Intel.
> 
> The quick workaround is: use /Gs65536. :-)
> 
> At the same time, our recommendation is to revisit the code which 
> triggers this error. Why this function need such a big stack? And try 
> to reduce the local stack usage.
> 
> What is why we still use /Gs32768 as default, instead of /Gs65536.
> 
> 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 5:21 PM
> > To: edk2-devel@lists.01.org
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>
> > Subject: Re: [edk2] [PATCH] MdePkg/BaseLib: Globally include 
> > ChkStk.c
> >
> > Oh, if you are prefering a good implementation, I will be all for 
> > that. This was just a 'quick workaround', same as currently done for
GCC.
> > I'm actually unsure whether a good implementation is possible with a 
> > flat memory model. It will likely be mere luck whether there is 
> > enough space free below the stack, except for maybe when it's 
> > located very high (preferably past the 32-bit space).
> > Has there been any prior discussion on this topic? Would be 
> > interested to follow up if there was.
> >
> > Thanks,
> > Marvin
> >
> > > -----Original Message-----
> > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > Sent: Wednesday, May 9, 2018 2:13 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
> > >
> > > There are some open source implementation:
> > > MSVC: https://github.com/ispc/ispc/issues/542
> > > GCC:
> > >
> https://android.googlesource.com/toolchain/gcc/+/b094d6c4bf572654a03
> > > 1e cc4afe675154c886dc5/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
> >
> > _______________________________________________
> > 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