* Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
@ 2018-05-08 23:36 Marvin Häuser
2018-05-08 23:52 ` Yao, Jiewen
0 siblings, 1 reply; 11+ messages in thread
From: Marvin Häuser @ 2018-05-08 23:36 UTC (permalink / raw)
To: edk2-devel@lists.01.org; +Cc: Gao, Liming
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
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c 2018-05-08 23:36 [PATCH] MdePkg/BaseLib: Globally include ChkStk.c Marvin Häuser @ 2018-05-08 23:52 ` Yao, Jiewen 2018-05-08 23:57 ` Marvin Häuser 0 siblings, 1 reply; 11+ messages in thread From: Yao, Jiewen @ 2018-05-08 23:52 UTC (permalink / raw) To: Marvin.Haeuser@outlook.com, edk2-devel@lists.01.org; +Cc: Gao, Liming 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c 2018-05-08 23:52 ` Yao, Jiewen @ 2018-05-08 23:57 ` Marvin Häuser 2018-05-09 0:13 ` Yao, Jiewen 0 siblings, 1 reply; 11+ messages in thread From: Marvin Häuser @ 2018-05-08 23:57 UTC (permalink / raw) To: edk2-devel@lists.01.org; +Cc: Yao, Jiewen, liming.gao@intel.com 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c 2018-05-08 23:57 ` Marvin Häuser @ 2018-05-09 0:13 ` Yao, Jiewen 2018-05-09 0:21 ` Marvin Häuser 0 siblings, 1 reply; 11+ messages in thread From: Yao, Jiewen @ 2018-05-09 0:13 UTC (permalink / raw) To: Marvin.Haeuser@outlook.com, edk2-devel@lists.01.org; +Cc: Gao, Liming 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c 2018-05-09 0:13 ` Yao, Jiewen @ 2018-05-09 0:21 ` Marvin Häuser 2018-05-09 0:25 ` Yao, Jiewen 0 siblings, 1 reply; 11+ messages in thread From: Marvin Häuser @ 2018-05-09 0:21 UTC (permalink / raw) To: edk2-devel@lists.01.org; +Cc: Yao, Jiewen 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c 2018-05-09 0:21 ` Marvin Häuser @ 2018-05-09 0:25 ` Yao, Jiewen 2018-05-09 1:27 ` Tim Lewis 0 siblings, 1 reply; 11+ messages in thread From: Yao, Jiewen @ 2018-05-09 0:25 UTC (permalink / raw) To: Marvin.Haeuser@outlook.com, edk2-devel@lists.01.org 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c 2018-05-09 0:25 ` Yao, Jiewen @ 2018-05-09 1:27 ` Tim Lewis 2018-05-09 8:44 ` Marvin Häuser 0 siblings, 1 reply; 11+ messages in thread From: Tim Lewis @ 2018-05-09 1:27 UTC (permalink / raw) To: 'Yao, Jiewen', Marvin.Haeuser, edk2-devel 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c 2018-05-09 1:27 ` Tim Lewis @ 2018-05-09 8:44 ` Marvin Häuser 2018-05-09 14:11 ` Tim Lewis 0 siblings, 1 reply; 11+ messages in thread From: Marvin Häuser @ 2018-05-09 8:44 UTC (permalink / raw) To: edk2-devel@lists.01.org; +Cc: Tim Lewis, Yao, Jiewen 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c 2018-05-09 8:44 ` Marvin Häuser @ 2018-05-09 14:11 ` Tim Lewis 0 siblings, 0 replies; 11+ messages in thread From: Tim Lewis @ 2018-05-09 14:11 UTC (permalink / raw) To: 'Marvin Häuser', edk2-devel; +Cc: 'Yao, Jiewen' 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH] MdePkg/BaseLib: Globally include ChkStk.c
@ 2018-05-05 14:24 Marvin Häuser
2018-05-07 12:16 ` Gao, Liming
0 siblings, 1 reply; 11+ messages in thread
From: Marvin Häuser @ 2018-05-05 14:24 UTC (permalink / raw)
To: edk2-devel@lists.01.org; +Cc: michael.d.kinney@intel.com, liming.gao@intel.com
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
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH] MdePkg/BaseLib: Globally include ChkStk.c 2018-05-05 14:24 Marvin Häuser @ 2018-05-07 12:16 ` Gao, Liming 0 siblings, 0 replies; 11+ messages in thread From: Gao, Liming @ 2018-05-07 12:16 UTC (permalink / raw) To: Marvin.Haeuser@outlook.com, edk2-devel@lists.01.org; +Cc: Kinney, Michael D 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 ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-05-09 14:12 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-05-08 23:36 [PATCH] MdePkg/BaseLib: Globally include ChkStk.c Marvin Häuser 2018-05-08 23:52 ` Yao, Jiewen 2018-05-08 23:57 ` Marvin Häuser 2018-05-09 0:13 ` Yao, Jiewen 2018-05-09 0:21 ` Marvin Häuser 2018-05-09 0:25 ` Yao, Jiewen 2018-05-09 1:27 ` Tim Lewis 2018-05-09 8:44 ` Marvin Häuser 2018-05-09 14:11 ` Tim Lewis -- strict thread matches above, loose matches on Subject: below -- 2018-05-05 14:24 Marvin Häuser 2018-05-07 12:16 ` Gao, Liming
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox