Got it, we no need to add intel copyright, I will not check in this patch.
Thanks, Eric -----Original Message----- From: Olivier Martin [mailto:olivier.mar...@arm.com] Sent: Monday, September 01, 2014 5:24 PM To: Gao, Liming; Dong, Eric Cc: edk2-devel@lists.sourceforge.net Subject: RE: [edk2] [PATCH] MdePkg: Update code for BaseStackCheckLib to follow EDKII coding style That's correct. If the idea is to add the Intel copyright at the top of the file, you can still add the space before 'CpuDeadLoop()'... > -----Original Message----- > From: Gao, Liming [mailto:liming....@intel.com] > Sent: 28 August 2014 15:52 > To: Dong, Eric; Olivier Martin > Cc: edk2-devel@lists.sourceforge.net > Subject: RE: [edk2] [PATCH] MdePkg: Update code for BaseStackCheckLib > to follow EDKII coding style > > Eric: > I think this function is compiler Intrinsic functions. Its name > can't be changed. > > Thanks > Liming > -----Original Message----- > From: Dong, Eric > Sent: Thursday, August 28, 2014 9:02 PM > To: Gao, Liming; Olivier Martin (olivier.mar...@arm.com) > Cc: edk2-devel@lists.sourceforge.net > Subject: [edk2] [PATCH] MdePkg: Update code for BaseStackCheckLib to > follow EDKII coding style > > Hi Olivier & Liming, > > Please help to review this patch, thanks. > > > Refine MdePkg: U BaseStackCheckLib library code to follow edkii coding > style. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Eric Dong <eric.d...@intel.com> > > > Thanks, > Eric > -----Original Message----- > From: Olivier Martin [mailto:olivier.mar...@arm.com] > Sent: Thursday, August 21, 2014 5:27 PM > To: Gao, Liming; edk2-devel@lists.sourceforge.net; Kinney, Michael D > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > BaseStackCheckLib > > Sorry, I committed the v3 patch without taking attention of its > content. > The FixedPcdGet() works well in the NULL library because the library > is linked to a module. So the FixedPcdGet behaves correctly in this case. > But the library cannot be built by itself with FixedPcdGet(). > > I think it was better to keep the PCD. But you are the maintainer. > So, I have just pushed SVN rev15866 that makes the changes you > suggested (that removes the PCD). > > Thanks, > Olivier > > > -----Original Message----- > > From: Gao, Liming [mailto:liming....@intel.com] > > Sent: 21 August 2014 09:59 > > To: Olivier Martin; edk2-devel@lists.sourceforge.net; Kinney, > > Michael D > > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > BaseStackCheckLib > > > > Oliver: > > I see your commit patch still includes FixedPcdGet() usage in > > library. Does it pass build? If not, how about remove PCD first. > > > > And, another issue that new introduced PCD > > gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary token number is same to > > gEfiMdePkgTokenSpaceGuid.PcdValidateOrderedCollection. But, PCD > > token number should be unique. So, its token number should be > > changed to another value, such as 0x0000002b. > > > > Thanks > > Liming > > -----Original Message----- > > From: Olivier Martin [mailto:olivier.mar...@arm.com] > > Sent: Wednesday, August 20, 2014 11:29 PM > > To: Gao, Liming; edk2-devel@lists.sourceforge.net; Kinney, Michael D > > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > BaseStackCheckLib > > > > Are you happy to add your "Reviewed-By" for this patch? > > If yes do you want me to commit it? Or you are happy to do it? > > > > Thanks, > > Olivier > > > > > -----Original Message----- > > > From: Gao, Liming [mailto:liming....@intel.com] > > > Sent: 20 August 2014 16:23 > > > To: Olivier Martin; edk2-devel@lists.sourceforge.net; Kinney, > > > Michael D > > > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > > BaseStackCheckLib > > > > > > Oliver: > > > Sorry for late response. Library can't use FixedPcdGet(), > > > because > > it > > > may be linked to the different drivers those may configure this > > > PCD with the different value. That may be the reason you don't do > > > it before. > > > Because this value is the recommended constant, it should be > > > fine > > to > > > hard code it first without PCD. > > > > > > Thanks > > > Liming > > > -----Original Message----- > > > From: Olivier Martin [mailto:olivier.mar...@arm.com] > > > Sent: Wednesday, August 20, 2014 5:47 PM > > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; Gao, > Liming > > > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > > BaseStackCheckLib > > > > > > Any feedback? > > > Thanks, > > > Olivier > > > > > > > -----Original Message----- > > > > From: Olivier Martin [mailto:olivier.mar...@arm.com] > > > > Sent: 06 August 2014 15:49 > > > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; 'Gao, > > > Liming' > > > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > > > BaseStackCheckLib > > > > > > > > I addressed your comment 1. into my local patch. > > > > > > > > But for comment 2., I had initially added BaseStackLib to > > MdePkg.dsc. > > > > But it was not building as the FixedPcd was not defined: > > > > ------- > > > > > > > > > > /home/olivier/tianocore/MdePkg/Library/BaseStackCheckLib/BaseStackChec > > > > k > > > > Gcc.c > > > > :26:34: error: "_PCD_VALUE_PcdBaseStackCanary" undeclared here > > > > (not > > > in > > > > a > > > > function) > > > > VOID *__stack_chk_guard = (VOID*)FixedPcdGet64 > > > > (PcdBaseStackCanary); > > > > ------- > > > > > > > > _PCD_VALUE_* are only generated when a binary module is built. > And > > > > there is no Module in MdePkg so adding > > > > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > > > > [LibraryClasses] would not help neither. > > > > One solution would be to add > > > > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > > > > MdeModulePkg.dsc > > > > > > > > Which solution would you suggested for your comment 2.? > > > > > > > > Thanks, > > > > Olivier > > > > > > > > > -----Original Message----- > > > > > From: Gao, Liming [mailto:liming....@intel.com] > > > > > Sent: 06 August 2014 04:00 > > > > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D > > > > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > > > BaseStackCheckLib > > > > > > > > > > Olivier: > > > > > I have two minor comment. > > > > > 1. The declaration of __stack_chk_fail() misses the function > > > > comments. > > > > > 2. Add it into Mdepkg.dsc [Components.ARM, Components.AARCH64] > > > > section > > > > > to verify its build. > > > > > > > > > > Other part is good to me. Reviewed-by: Gao, Liming > > > > > <liming....@intel.com> > > > > > > > > > > Thanks > > > > > Liming > > > > > -----Original Message----- > > > > > From: Olivier Martin [mailto:olivier.mar...@arm.com] > > > > > Sent: Tuesday, August 05, 2014 5:48 PM > > > > > To: Kinney, Michael D > > > > > Cc: edk2-devel@lists.sourceforge.net > > > > > Subject: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > > > > BaseStackCheckLib > > > > > > > > > > This library only support GCC, RVCT and XCode for now. > > > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > > > Signed-off-by: Andrew Fish <af...@apple.com> > > > > > Signed-off-by: Olivier Martin <olivier.mar...@arm.com> > > > > > --- > > > > > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 61 > > > > > ++++++++++++++++++++++ > > > > > .../BaseStackCheckLib/BaseStackCheckLib.inf | 42 > > > > > +++++++++++++++ > > > > > MdePkg/MdePkg.dec | 4 ++ > > > > > 3 files changed, 107 insertions(+) create mode 100644 > > > > > MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > > > > > create mode 100644 > > > > > MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > > > > > > > > > diff --git > > > > > a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > > > > > b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > > > > > new file mode 100644 > > > > > index 0000000..4160aff > > > > > --- /dev/null > > > > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > > > > > @@ -0,0 +1,61 @@ > > > > > +/** @file > > > > > + Base Stack Check library for GCC/clang. > > > > > + > > > > > + Use -fstack-protector-all compiler flag to make the compiler > > > > > + insert the __stack_chk_guard "canary" value into the stack > and > > > > > + check the value prior to exiting the function. If the > "canary" > > > > > + is overwritten > > > > > + __stack_chk_fail() is called. This is GCC specific code. > > > > > + > > > > > + Copyright (c) 2012, Apple Inc. 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 <Base.h> > > > > > +#include <Library/BaseLib.h> > > > > > +#include <Library/DebugLib.h> #include <Library/PcdLib.h> > > > > > + > > > > > +VOID > > > > > +__stack_chk_fail ( > > > > > + VOID > > > > > + ); > > > > > + > > > > > + > > > > > +/// "canary" value that is inserted by the compiler into the > > > > > +stack > > > > > frame. > > > > > +VOID *__stack_chk_guard = (VOID*)FixedPcdGet64 > > > > > +(PcdBaseStackCanary); > > > > > + > > > > > +// If ASLR was enabled we could use //void > > > > > +(*__stack_chk_guard)(void) = __stack_chk_fail; > > > > > + > > > > > +/** > > > > > + Error path for compiler generated stack "canary" value check > > > code. > > > > If > > > > > +the stack canary has been overwritten this function gets > > > > > +called > > > on > > > > > +exit of the function. > > > > > +**/ > > > > > +VOID > > > > > +__stack_chk_fail ( > > > > > + VOID > > > > > + ) > > > > > +{ > > > > > + UINT8 DebugPropertyMask; > > > > > + > > > > > + DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow in > > > > > + function %a.\n", __builtin_return_address(0))); > > > > > + > > > > > + // > > > > > + // Generate a Breakpoint, DeadLoop, or NOP based on PCD > > > > > + settings > > > > > even > > > > > +if > > > > > + // BaseDebugLibNull is in use. > > > > > + // > > > > > + DebugPropertyMask = PcdGet8 (PcdDebugPropertyMask); > > > > > + if ((DebugPropertyMask & > > > > > +DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) > > > > > != 0) { > > > > > + CpuBreakpoint (); > > > > > + } else if ((DebugPropertyMask & > > > > > DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { > > > > > + CpuDeadLoop (); > > > > > + } > > > > > +} > > > > > diff --git > > > > > a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > > > > b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > > > > new file mode 100644 > > > > > index 0000000..3304284 > > > > > --- /dev/null > > > > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > > > > @@ -0,0 +1,42 @@ > > > > > +## @file > > > > > +# Stack Check Library > > > > > +# > > > > > +# Copyright (c) 2014, ARM Ltd. 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. > > > > > +# > > > > > +# > > > > > +## > > > > > + > > > > > +[Defines] > > > > > + INF_VERSION = 0x00010005 > > > > > + BASE_NAME = BaseStackCheckLib > > > > > + FILE_GUID = 5f6579f7-b648-4fdb-9f19- > > > > > 4c17e27e8eff > > > > > + MODULE_TYPE = BASE > > > > > + VERSION_STRING = 1.0 > > > > > + LIBRARY_CLASS = NULL > > > > > + > > > > > + > > > > > +# > > > > > +# VALID_ARCHITECTURES = ARM AARCH64 > > > > > +# > > > > > + > > > > > +[Sources] > > > > > + BaseStackCheckGcc.c | GCC > > > > > + BaseStackCheckGcc.c | RVCT > > > > > + > > > > > +[Packages] > > > > > + MdePkg/MdePkg.dec > > > > > + > > > > > +[LibraryClasses] > > > > > + BaseLib > > > > > + DebugLib > > > > > + > > > > > +[FixedPcd] > > > > > + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary > > > > > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > > > > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > > > > > 4daf3e6..fbb7d2b 100644 > > > > > --- a/MdePkg/MdePkg.dec > > > > > +++ b/MdePkg/MdePkg.dec > > > > > @@ -1544,6 +1544,10 @@ > > > > > # The required memory space is decided by the value of > > > > > PcdMaximumGuidedExtractHandler. > > > > > > > > > > > > > > > > > > > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x1000000 > > > > | > > > > > UINT64|0x30001015 > > > > > > > > > > + ## Canary value for the stack overflow protection. This PCD > > can > > > > > + be used by a firmware vendor # or for debugging purposes to > > > > > + change > > > > the > > > > > recommended value. > > > > > + > > > > > > gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary|0x0AFF|UINT64|0x0000002A > > > > > + > > > > > [PcdsFixedAtBuild.IPF] > > > > > ## The base address of IO port space for IA64 arch > > > > > > > > > > > > > > > > > > > > gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000|UI > > > > N > > > > > T64|0x0000000f > > > > > -- > > > > > 1.8.5 > > > > > > > > > > > > > > > -------------------------------------------------------------- > > > > > - > - > > > > > - > > - > > > > > - > > > - > > > > > - > > > > -- > > > > > ------- > > > > > Infragistics Professional > > > > > Build stunning WinForms apps today! > > > > > Reboot your WinForms applications with our WinForms controls. > > > > > Build a bridge from your legacy apps to the future. > > > > > > > > > > > > > > > http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg. > > > > c > > > > > lktrk > > > > > _______________________________________________ > > > > > edk2-devel mailing list > > > > > edk2-devel@lists.sourceforge.net > > > > > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > > > > > > > > -------------------------------------------------------------- > > > > > - > - > > > > > - > > - > > > > > - > > > - > > > > > - > > > > -- > > > > > ------- > > > > > Infragistics Professional > > > > > Build stunning WinForms apps today! > > > > > Reboot your WinForms applications with our WinForms controls. > > > > > Build a bridge from your legacy apps to the future. > > > > > > > > > > > > > > > http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg. > > > > c > > > > > lktrk > > > > > _______________________________________________ > > > > > edk2-devel mailing list > > > > > edk2-devel@lists.sourceforge.net > > > > > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > > > > > > > > > > > > > > > > > > > > > > ---------------------------------------------------------------- > > > > - > - > > > > - > > - > > > > - > > > - > > > > - > > > > ------- > > > > Infragistics Professional > > > > Build stunning WinForms apps today! > > > > Reboot your WinForms applications with our WinForms controls. > > > > Build a bridge from your legacy apps to the future. > > > > > > > > > > http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg. > > > > c > > > > lktrk > > > > _______________________________________________ > > > > edk2-devel mailing list > > > > edk2-devel@lists.sourceforge.net > > > > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > > > > > > > > > > > > > > > > > > > > > > > > ---------------------------------------------------------------------- > - > ------- > Slashdot TV. > Video for Nerds. Stuff that matters. > http://tv.slashdot.org/ > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel -- IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you. ARM Limited, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2557590 ARM Holdings plc, Registered office 110 Fulbourn Road, Cambridge CB1 9NJ, Registered in England & Wales, Company No: 2548782 ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
