* [Patch] BaseTools: add error check for Macro usage in the INF file @ 2017-02-21 1:18 Yonghong Zhu 2017-02-21 10:03 ` Gao, Liming 2017-02-23 1:02 ` Laszlo Ersek 0 siblings, 2 replies; 9+ messages in thread From: Yonghong Zhu @ 2017-02-21 1:18 UTC (permalink / raw) To: edk2-devel; +Cc: Liming Gao Use of MACRO statements in the EDK II INF files is limited to local usage only; global or external macros are not permitted. This patch add the check for not defined macros. Cc: Liming Gao <liming.gao@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com> --- BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++++++++- BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py b/BaseTools/Source/Python/Workspace/MetaFileParser.py index 1a5fdf5..37a7f5d 100644 --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py @@ -1,9 +1,9 @@ ## @file # This file is used to parse meta files # -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> # (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<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 @@ -349,10 +349,17 @@ class MetaFileParser(object): EdkLogger.error('Parser', FORMAT_INVALID, "No value specified", ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in self._ValueList] Name, Value = self._ValueList[1], self._ValueList[2] + MacroUsed = GlobalData.gMacroRefPattern.findall(Value) + if len(MacroUsed) != 0: + for Macro in MacroUsed: + if Macro in GlobalData.gGlobalDefines: + EdkLogger.error("Parser", FORMAT_INVALID, "Global macro %s is not permitted." % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) + else: + EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) # Sometimes, we need to make differences between EDK and EDK2 modules if Name == 'INF_VERSION': if re.match(r'0[xX][\da-f-A-F]{5,8}', Value): self._Version = int(Value, 0) elif re.match(r'\d+\.\d+', Value): diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py index e7bc87d..0686721 100644 --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py @@ -1,9 +1,9 @@ ## @file # This file is used to create a database used by build tool # -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<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 @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject): self.__Macros = {} # EDK_GLOBAL defined macros can be applied to EDK module if self.AutoGenVersion < 0x00010005: self.__Macros.update(GlobalData.gEdkGlobal) self.__Macros.update(GlobalData.gGlobalDefines) + else: + self.__Macros.update(self.Defines) return self.__Macros ## Get architecture def _GetArch(self): return self._Arch -- 2.6.1.windows.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: add error check for Macro usage in the INF file 2017-02-21 1:18 [Patch] BaseTools: add error check for Macro usage in the INF file Yonghong Zhu @ 2017-02-21 10:03 ` Gao, Liming 2017-02-22 11:44 ` Ard Biesheuvel 2017-02-23 1:02 ` Laszlo Ersek 1 sibling, 1 reply; 9+ messages in thread From: Gao, Liming @ 2017-02-21 10:03 UTC (permalink / raw) To: Zhu, Yonghong, edk2-devel@lists.01.org Reviewed-by: Liming Gao <liming.gao@intel.com> -----Original Message----- From: Zhu, Yonghong Sent: Tuesday, February 21, 2017 9:18 AM To: edk2-devel@lists.01.org Cc: Gao, Liming <liming.gao@intel.com> Subject: [Patch] BaseTools: add error check for Macro usage in the INF file Use of MACRO statements in the EDK II INF files is limited to local usage only; global or external macros are not permitted. This patch add the check for not defined macros. Cc: Liming Gao <liming.gao@intel.com> Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com> --- BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++++++++- BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py b/BaseTools/Source/Python/Workspace/MetaFileParser.py index 1a5fdf5..37a7f5d 100644 --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py @@ -1,9 +1,9 @@ ## @file # This file is used to parse meta files # -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> # (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<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 @@ -349,10 +349,17 @@ class MetaFileParser(object): EdkLogger.error('Parser', FORMAT_INVALID, "No value specified", ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in self._ValueList] Name, Value = self._ValueList[1], self._ValueList[2] + MacroUsed = GlobalData.gMacroRefPattern.findall(Value) + if len(MacroUsed) != 0: + for Macro in MacroUsed: + if Macro in GlobalData.gGlobalDefines: + EdkLogger.error("Parser", FORMAT_INVALID, "Global macro %s is not permitted." % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) + else: + EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) # Sometimes, we need to make differences between EDK and EDK2 modules if Name == 'INF_VERSION': if re.match(r'0[xX][\da-f-A-F]{5,8}', Value): self._Version = int(Value, 0) elif re.match(r'\d+\.\d+', Value): diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py index e7bc87d..0686721 100644 --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py @@ -1,9 +1,9 @@ ## @file # This file is used to create a database used by build tool # -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<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 @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject): self.__Macros = {} # EDK_GLOBAL defined macros can be applied to EDK module if self.AutoGenVersion < 0x00010005: self.__Macros.update(GlobalData.gEdkGlobal) self.__Macros.update(GlobalData.gGlobalDefines) + else: + self.__Macros.update(self.Defines) return self.__Macros ## Get architecture def _GetArch(self): return self._Arch -- 2.6.1.windows.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: add error check for Macro usage in the INF file 2017-02-21 10:03 ` Gao, Liming @ 2017-02-22 11:44 ` Ard Biesheuvel 2017-02-22 13:38 ` Zhu, Yonghong 2017-02-22 16:04 ` Zhu, Yonghong 0 siblings, 2 replies; 9+ messages in thread From: Ard Biesheuvel @ 2017-02-22 11:44 UTC (permalink / raw) To: Gao, Liming, Laszlo Ersek; +Cc: Zhu, Yonghong, edk2-devel@lists.01.org On 21 February 2017 at 10:03, Gao, Liming <liming.gao@intel.com> wrote: > Reviewed-by: Liming Gao <liming.gao@intel.com> > This patch has broken ArmVirtQemu in the most mysterious way: it causes the constructor invocations in AutoGen.c to be emitted twice, e.g., ProcessLibraryConstructorList ( IN EFI_HANDLE ImageHandle, IN EFI_SYSTEM_TABLE *SystemTable ) { EFI_STATUS Status; Status = HobLibConstructor (ImageHandle, SystemTable); ASSERT_EFI_ERROR (Status); Status = HobLibConstructor (ImageHandle, SystemTable); ASSERT_EFI_ERROR (Status); Status = FdtPL011SerialPortLibInitialize (); ASSERT_EFI_ERROR (Status); Status = FdtPL011SerialPortLibInitialize (); ASSERT_EFI_ERROR (Status); Status = BaseDebugLibSerialPortConstructor (); ASSERT_EFI_ERROR (Status); Status = BaseDebugLibSerialPortConstructor (); ASSERT_EFI_ERROR (Status); which obviously breaks some assumptions, for instance, some HII registrations now hit an ASSERT() because of the duplicate GUID I have no idea how this happens, or what this patch does in the first place, so please investigate Thanks, Ard. > -----Original Message----- > From: Zhu, Yonghong > Sent: Tuesday, February 21, 2017 9:18 AM > To: edk2-devel@lists.01.org > Cc: Gao, Liming <liming.gao@intel.com> > Subject: [Patch] BaseTools: add error check for Macro usage in the INF file > > Use of MACRO statements in the EDK II INF files is limited to local > usage only; global or external macros are not permitted. This patch > add the check for not defined macros. > > Cc: Liming Gao <liming.gao@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com> > --- > BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++++++++- > BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py b/BaseTools/Source/Python/Workspace/MetaFileParser.py > index 1a5fdf5..37a7f5d 100644 > --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py > +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py > @@ -1,9 +1,9 @@ > ## @file > # This file is used to parse meta files > # > -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> > # (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<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 > @@ -349,10 +349,17 @@ class MetaFileParser(object): > EdkLogger.error('Parser', FORMAT_INVALID, "No value specified", > ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) > > self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in self._ValueList] > Name, Value = self._ValueList[1], self._ValueList[2] > + MacroUsed = GlobalData.gMacroRefPattern.findall(Value) > + if len(MacroUsed) != 0: > + for Macro in MacroUsed: > + if Macro in GlobalData.gGlobalDefines: > + EdkLogger.error("Parser", FORMAT_INVALID, "Global macro %s is not permitted." % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) > + else: > + EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) > # Sometimes, we need to make differences between EDK and EDK2 modules > if Name == 'INF_VERSION': > if re.match(r'0[xX][\da-f-A-F]{5,8}', Value): > self._Version = int(Value, 0) > elif re.match(r'\d+\.\d+', Value): > diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > index e7bc87d..0686721 100644 > --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > @@ -1,9 +1,9 @@ > ## @file > # This file is used to create a database used by build tool > # > -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<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 > @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject): > self.__Macros = {} > # EDK_GLOBAL defined macros can be applied to EDK module > if self.AutoGenVersion < 0x00010005: > self.__Macros.update(GlobalData.gEdkGlobal) > self.__Macros.update(GlobalData.gGlobalDefines) > + else: > + self.__Macros.update(self.Defines) > return self.__Macros > > ## Get architecture > def _GetArch(self): > return self._Arch > -- > 2.6.1.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: add error check for Macro usage in the INF file 2017-02-22 11:44 ` Ard Biesheuvel @ 2017-02-22 13:38 ` Zhu, Yonghong 2017-02-22 16:04 ` Zhu, Yonghong 1 sibling, 0 replies; 9+ messages in thread From: Zhu, Yonghong @ 2017-02-22 13:38 UTC (permalink / raw) To: Ard Biesheuvel, Gao, Liming, Laszlo Ersek Cc: edk2-devel@lists.01.org, Zhu, Yonghong Hi Ard, Thanks. I will try to find out the root cause and fix it ASAP. Best Regards, Zhu Yonghong -----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] Sent: Wednesday, February 22, 2017 7:44 PM To: Gao, Liming <liming.gao@intel.com>; Laszlo Ersek <lersek@redhat.com> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org Subject: Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file On 21 February 2017 at 10:03, Gao, Liming <liming.gao@intel.com> wrote: > Reviewed-by: Liming Gao <liming.gao@intel.com> > This patch has broken ArmVirtQemu in the most mysterious way: it causes the constructor invocations in AutoGen.c to be emitted twice, e.g., ProcessLibraryConstructorList ( IN EFI_HANDLE ImageHandle, IN EFI_SYSTEM_TABLE *SystemTable ) { EFI_STATUS Status; Status = HobLibConstructor (ImageHandle, SystemTable); ASSERT_EFI_ERROR (Status); Status = HobLibConstructor (ImageHandle, SystemTable); ASSERT_EFI_ERROR (Status); Status = FdtPL011SerialPortLibInitialize (); ASSERT_EFI_ERROR (Status); Status = FdtPL011SerialPortLibInitialize (); ASSERT_EFI_ERROR (Status); Status = BaseDebugLibSerialPortConstructor (); ASSERT_EFI_ERROR (Status); Status = BaseDebugLibSerialPortConstructor (); ASSERT_EFI_ERROR (Status); which obviously breaks some assumptions, for instance, some HII registrations now hit an ASSERT() because of the duplicate GUID I have no idea how this happens, or what this patch does in the first place, so please investigate Thanks, Ard. > -----Original Message----- > From: Zhu, Yonghong > Sent: Tuesday, February 21, 2017 9:18 AM > To: edk2-devel@lists.01.org > Cc: Gao, Liming <liming.gao@intel.com> > Subject: [Patch] BaseTools: add error check for Macro usage in the INF > file > > Use of MACRO statements in the EDK II INF files is limited to local > usage only; global or external macros are not permitted. This patch > add the check for not defined macros. > > Cc: Liming Gao <liming.gao@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com> > --- > BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++++++++- > BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py > b/BaseTools/Source/Python/Workspace/MetaFileParser.py > index 1a5fdf5..37a7f5d 100644 > --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py > +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py > @@ -1,9 +1,9 @@ > ## @file > # This file is used to parse meta files # -# Copyright (c) 2008 - > 2016, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2008 - 2017, Intel Corporation. All rights > +reserved.<BR> > # (C) Copyright 2015-2016 Hewlett Packard Enterprise Development > LP<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 > @@ -349,10 +349,17 @@ class MetaFileParser(object): > EdkLogger.error('Parser', FORMAT_INVALID, "No value specified", > ExtraData=self._CurrentLine, > File=self.MetaFile, Line=self._LineIndex + 1) > > self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in self._ValueList] > Name, Value = self._ValueList[1], self._ValueList[2] > + MacroUsed = GlobalData.gMacroRefPattern.findall(Value) > + if len(MacroUsed) != 0: > + for Macro in MacroUsed: > + if Macro in GlobalData.gGlobalDefines: > + EdkLogger.error("Parser", FORMAT_INVALID, "Global macro %s is not permitted." % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) > + else: > + EdkLogger.error("Parser", FORMAT_INVALID, "%s not > + defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, > + Line=self._LineIndex + 1) > # Sometimes, we need to make differences between EDK and EDK2 modules > if Name == 'INF_VERSION': > if re.match(r'0[xX][\da-f-A-F]{5,8}', Value): > self._Version = int(Value, 0) > elif re.match(r'\d+\.\d+', Value): > diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > index e7bc87d..0686721 100644 > --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > @@ -1,9 +1,9 @@ > ## @file > # This file is used to create a database used by build tool # -# > Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2008 - 2017, Intel Corporation. All rights > +reserved.<BR> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<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 > @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject): > self.__Macros = {} > # EDK_GLOBAL defined macros can be applied to EDK module > if self.AutoGenVersion < 0x00010005: > self.__Macros.update(GlobalData.gEdkGlobal) > self.__Macros.update(GlobalData.gGlobalDefines) > + else: > + self.__Macros.update(self.Defines) > return self.__Macros > > ## Get architecture > def _GetArch(self): > return self._Arch > -- > 2.6.1.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: add error check for Macro usage in the INF file 2017-02-22 11:44 ` Ard Biesheuvel 2017-02-22 13:38 ` Zhu, Yonghong @ 2017-02-22 16:04 ` Zhu, Yonghong 1 sibling, 0 replies; 9+ messages in thread From: Zhu, Yonghong @ 2017-02-22 16:04 UTC (permalink / raw) To: Ard Biesheuvel, Gao, Liming, Laszlo Ersek Cc: edk2-devel@lists.01.org, Zhu, Yonghong Hi Ard, I just sent out a patch to fix this issue,thanks. Best Regards, Zhu Yonghong -----Original Message----- From: Zhu, Yonghong Sent: Wednesday, February 22, 2017 9:39 PM To: 'Ard Biesheuvel' <ard.biesheuvel@linaro.org>; Gao, Liming <liming.gao@intel.com>; Laszlo Ersek <lersek@redhat.com> Cc: edk2-devel@lists.01.org; Zhu, Yonghong <yonghong.zhu@intel.com> Subject: RE: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file Hi Ard, Thanks. I will try to find out the root cause and fix it ASAP. Best Regards, Zhu Yonghong -----Original Message----- From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] Sent: Wednesday, February 22, 2017 7:44 PM To: Gao, Liming <liming.gao@intel.com>; Laszlo Ersek <lersek@redhat.com> Cc: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@lists.01.org Subject: Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file On 21 February 2017 at 10:03, Gao, Liming <liming.gao@intel.com> wrote: > Reviewed-by: Liming Gao <liming.gao@intel.com> > This patch has broken ArmVirtQemu in the most mysterious way: it causes the constructor invocations in AutoGen.c to be emitted twice, e.g., ProcessLibraryConstructorList ( IN EFI_HANDLE ImageHandle, IN EFI_SYSTEM_TABLE *SystemTable ) { EFI_STATUS Status; Status = HobLibConstructor (ImageHandle, SystemTable); ASSERT_EFI_ERROR (Status); Status = HobLibConstructor (ImageHandle, SystemTable); ASSERT_EFI_ERROR (Status); Status = FdtPL011SerialPortLibInitialize (); ASSERT_EFI_ERROR (Status); Status = FdtPL011SerialPortLibInitialize (); ASSERT_EFI_ERROR (Status); Status = BaseDebugLibSerialPortConstructor (); ASSERT_EFI_ERROR (Status); Status = BaseDebugLibSerialPortConstructor (); ASSERT_EFI_ERROR (Status); which obviously breaks some assumptions, for instance, some HII registrations now hit an ASSERT() because of the duplicate GUID I have no idea how this happens, or what this patch does in the first place, so please investigate Thanks, Ard. > -----Original Message----- > From: Zhu, Yonghong > Sent: Tuesday, February 21, 2017 9:18 AM > To: edk2-devel@lists.01.org > Cc: Gao, Liming <liming.gao@intel.com> > Subject: [Patch] BaseTools: add error check for Macro usage in the INF > file > > Use of MACRO statements in the EDK II INF files is limited to local > usage only; global or external macros are not permitted. This patch > add the check for not defined macros. > > Cc: Liming Gao <liming.gao@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com> > --- > BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++++++++- > BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py > b/BaseTools/Source/Python/Workspace/MetaFileParser.py > index 1a5fdf5..37a7f5d 100644 > --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py > +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py > @@ -1,9 +1,9 @@ > ## @file > # This file is used to parse meta files # -# Copyright (c) 2008 - > 2016, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2008 - 2017, Intel Corporation. All rights > +reserved.<BR> > # (C) Copyright 2015-2016 Hewlett Packard Enterprise Development > LP<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 > @@ -349,10 +349,17 @@ class MetaFileParser(object): > EdkLogger.error('Parser', FORMAT_INVALID, "No value specified", > ExtraData=self._CurrentLine, > File=self.MetaFile, Line=self._LineIndex + 1) > > self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in self._ValueList] > Name, Value = self._ValueList[1], self._ValueList[2] > + MacroUsed = GlobalData.gMacroRefPattern.findall(Value) > + if len(MacroUsed) != 0: > + for Macro in MacroUsed: > + if Macro in GlobalData.gGlobalDefines: > + EdkLogger.error("Parser", FORMAT_INVALID, "Global macro %s is not permitted." % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) > + else: > + EdkLogger.error("Parser", FORMAT_INVALID, "%s not > + defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, > + Line=self._LineIndex + 1) > # Sometimes, we need to make differences between EDK and EDK2 modules > if Name == 'INF_VERSION': > if re.match(r'0[xX][\da-f-A-F]{5,8}', Value): > self._Version = int(Value, 0) > elif re.match(r'\d+\.\d+', Value): > diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > index e7bc87d..0686721 100644 > --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > @@ -1,9 +1,9 @@ > ## @file > # This file is used to create a database used by build tool # -# > Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2008 - 2017, Intel Corporation. All rights > +reserved.<BR> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<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 > @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject): > self.__Macros = {} > # EDK_GLOBAL defined macros can be applied to EDK module > if self.AutoGenVersion < 0x00010005: > self.__Macros.update(GlobalData.gEdkGlobal) > self.__Macros.update(GlobalData.gGlobalDefines) > + else: > + self.__Macros.update(self.Defines) > return self.__Macros > > ## Get architecture > def _GetArch(self): > return self._Arch > -- > 2.6.1.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: add error check for Macro usage in the INF file 2017-02-21 1:18 [Patch] BaseTools: add error check for Macro usage in the INF file Yonghong Zhu 2017-02-21 10:03 ` Gao, Liming @ 2017-02-23 1:02 ` Laszlo Ersek 2017-02-23 1:12 ` Laszlo Ersek 1 sibling, 1 reply; 9+ messages in thread From: Laszlo Ersek @ 2017-02-23 1:02 UTC (permalink / raw) To: Yonghong Zhu, edk2-devel; +Cc: Liming Gao Hi, On 02/21/17 02:18, Yonghong Zhu wrote: > Use of MACRO statements in the EDK II INF files is limited to local > usage only; global or external macros are not permitted. This patch > add the check for not defined macros. > > Cc: Liming Gao <liming.gao@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com> > --- > BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++++++++- > BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++- > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py b/BaseTools/Source/Python/Workspace/MetaFileParser.py > index 1a5fdf5..37a7f5d 100644 > --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py > +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py > @@ -1,9 +1,9 @@ > ## @file > # This file is used to parse meta files > # > -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> > # (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<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 > @@ -349,10 +349,17 @@ class MetaFileParser(object): > EdkLogger.error('Parser', FORMAT_INVALID, "No value specified", > ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) > > self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in self._ValueList] > Name, Value = self._ValueList[1], self._ValueList[2] > + MacroUsed = GlobalData.gMacroRefPattern.findall(Value) > + if len(MacroUsed) != 0: > + for Macro in MacroUsed: > + if Macro in GlobalData.gGlobalDefines: > + EdkLogger.error("Parser", FORMAT_INVALID, "Global macro %s is not permitted." % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) > + else: > + EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) > # Sometimes, we need to make differences between EDK and EDK2 modules > if Name == 'INF_VERSION': > if re.match(r'0[xX][\da-f-A-F]{5,8}', Value): > self._Version = int(Value, 0) > elif re.match(r'\d+\.\d+', Value): > diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > index e7bc87d..0686721 100644 > --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py > @@ -1,9 +1,9 @@ > ## @file > # This file is used to create a database used by build tool > # > -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> > +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> > # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<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 > @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject): > self.__Macros = {} > # EDK_GLOBAL defined macros can be applied to EDK module > if self.AutoGenVersion < 0x00010005: > self.__Macros.update(GlobalData.gEdkGlobal) > self.__Macros.update(GlobalData.gGlobalDefines) > + else: > + self.__Macros.update(self.Defines) > return self.__Macros > > ## Get architecture > def _GetArch(self): > return self._Arch > I don't understand how, but this patch (commit dc4c770763d0) breaks OVMF for me. I bisected it, I didn't want to believe it, then I built the tree right before it, and that worked. I also built the tree at current master (526f160f311c, "ArmPkg/ArmMmuLib: AARCH64: enable stack alignment checking", 2017-02-22), with this commit reverted on top, and that also works. This is the error I see in the OVMF log: > Loading driver E660EA85-058E-4B55-A54B-F02F83A24707 > InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7E8752C0 > Loading driver at 0x0007E4B7000 EntryPoint=0x0007E4B727B DisplayEngine.efi > InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7E875518 > ProtectUefiImageCommon - 0x7E8752C0 > - 0x000000007E4B7000 - 0x000000000001BAE0 > ASSERT .../MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.c(928): mCDLStringPackHandle != ((void *) 0) I don't have the slightest clue what's going on. Apparently, this change causes BaseTools to mis-build OVMF. Thanks Laszlo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: add error check for Macro usage in the INF file 2017-02-23 1:02 ` Laszlo Ersek @ 2017-02-23 1:12 ` Laszlo Ersek 2017-02-23 1:14 ` Gao, Liming 0 siblings, 1 reply; 9+ messages in thread From: Laszlo Ersek @ 2017-02-23 1:12 UTC (permalink / raw) To: Yonghong Zhu, edk2-devel; +Cc: Liming Gao On 02/23/17 02:02, Laszlo Ersek wrote: > Hi, > > On 02/21/17 02:18, Yonghong Zhu wrote: >> Use of MACRO statements in the EDK II INF files is limited to local >> usage only; global or external macros are not permitted. This patch >> add the check for not defined macros. >> >> Cc: Liming Gao <liming.gao@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com> >> --- >> BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++++++++- >> BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++- >> 2 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py b/BaseTools/Source/Python/Workspace/MetaFileParser.py >> index 1a5fdf5..37a7f5d 100644 >> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py >> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py >> @@ -1,9 +1,9 @@ >> ## @file >> # This file is used to parse meta files >> # >> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> >> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> >> # (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<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 >> @@ -349,10 +349,17 @@ class MetaFileParser(object): >> EdkLogger.error('Parser', FORMAT_INVALID, "No value specified", >> ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) >> >> self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in self._ValueList] >> Name, Value = self._ValueList[1], self._ValueList[2] >> + MacroUsed = GlobalData.gMacroRefPattern.findall(Value) >> + if len(MacroUsed) != 0: >> + for Macro in MacroUsed: >> + if Macro in GlobalData.gGlobalDefines: >> + EdkLogger.error("Parser", FORMAT_INVALID, "Global macro %s is not permitted." % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) >> + else: >> + EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) >> # Sometimes, we need to make differences between EDK and EDK2 modules >> if Name == 'INF_VERSION': >> if re.match(r'0[xX][\da-f-A-F]{5,8}', Value): >> self._Version = int(Value, 0) >> elif re.match(r'\d+\.\d+', Value): >> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py >> index e7bc87d..0686721 100644 >> --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py >> +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py >> @@ -1,9 +1,9 @@ >> ## @file >> # This file is used to create a database used by build tool >> # >> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> >> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> >> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<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 >> @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject): >> self.__Macros = {} >> # EDK_GLOBAL defined macros can be applied to EDK module >> if self.AutoGenVersion < 0x00010005: >> self.__Macros.update(GlobalData.gEdkGlobal) >> self.__Macros.update(GlobalData.gGlobalDefines) >> + else: >> + self.__Macros.update(self.Defines) >> return self.__Macros >> >> ## Get architecture >> def _GetArch(self): >> return self._Arch >> > > I don't understand how, but this patch (commit dc4c770763d0) breaks OVMF for me. > > I bisected it, I didn't want to believe it, then I built the tree right before it, and that worked. > > I also built the tree at current master (526f160f311c, "ArmPkg/ArmMmuLib: AARCH64: enable stack alignment checking", 2017-02-22), with this commit reverted on top, and that also works. > > This is the error I see in the OVMF log: > >> Loading driver E660EA85-058E-4B55-A54B-F02F83A24707 >> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7E8752C0 >> Loading driver at 0x0007E4B7000 EntryPoint=0x0007E4B727B DisplayEngine.efi >> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7E875518 >> ProtectUefiImageCommon - 0x7E8752C0 >> - 0x000000007E4B7000 - 0x000000000001BAE0 >> ASSERT .../MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.c(928): mCDLStringPackHandle != ((void *) 0) > > I don't have the slightest clue what's going on. Apparently, this change causes BaseTools to mis-build OVMF. If it's any help, I think that the second hunk breaks library constructors somehow. I compared the build report files between "good" (--> without this patch) and "bad" (--> with this patch), and the differences I see follow this pattern, under every module that links against library instance with a constructor: >----------------------------------------------------------------------------------------------------------------------< Library ------------------------------------------------------------------------------------------------------------------------ MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf {MemoryAllocationLib} MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf {PeiServicesTablePointerLib} MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf {PcdLib} MdePkg/Library/BaseLib/BaseLib.inf {BaseLib} MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf {BaseMemoryLib} MdePkg/Library/PeiHobLib/PeiHobLib.inf {HobLib} MdePkg/Library/PeiServicesLib/PeiServicesLib.inf {PeiServicesLib} MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf {DebugPrintErrorLevelLib} MdePkg/Library/BasePrintLib/BasePrintLib.inf {PrintLib} MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf {IoLib} OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf -{DebugLib: C = PlatformDebugLibIoPortConstructor} +{DebugLib: C = PlatformDebugLibIoPortConstructor PlatformDebugLibIoPortConstructor} MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf {PeimEntryPoint} <----------------------------------------------------------------------------------------------------------------------> Note "DebugLib". In the "bad" case, the constructor name seems to be duplicated. This applies to all other constructor functions as well. Thanks Laszlo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: add error check for Macro usage in the INF file 2017-02-23 1:12 ` Laszlo Ersek @ 2017-02-23 1:14 ` Gao, Liming 2017-02-23 10:22 ` Laszlo Ersek 0 siblings, 1 reply; 9+ messages in thread From: Gao, Liming @ 2017-02-23 1:14 UTC (permalink / raw) To: Laszlo Ersek, Zhu, Yonghong, edk2-devel@ml01.01.org Laszlo: Yonghong has sent the another patch to its regression issue. Could you verify it? Thanks Liming -----Original Message----- From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Thursday, February 23, 2017 9:12 AM To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@ml01.01.org Cc: Gao, Liming <liming.gao@intel.com> Subject: Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file On 02/23/17 02:02, Laszlo Ersek wrote: > Hi, > > On 02/21/17 02:18, Yonghong Zhu wrote: >> Use of MACRO statements in the EDK II INF files is limited to local >> usage only; global or external macros are not permitted. This patch >> add the check for not defined macros. >> >> Cc: Liming Gao <liming.gao@intel.com> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com> >> --- >> BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++++++++- >> BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++- >> 2 files changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py b/BaseTools/Source/Python/Workspace/MetaFileParser.py >> index 1a5fdf5..37a7f5d 100644 >> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py >> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py >> @@ -1,9 +1,9 @@ >> ## @file >> # This file is used to parse meta files >> # >> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> >> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> >> # (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<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 >> @@ -349,10 +349,17 @@ class MetaFileParser(object): >> EdkLogger.error('Parser', FORMAT_INVALID, "No value specified", >> ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) >> >> self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in self._ValueList] >> Name, Value = self._ValueList[1], self._ValueList[2] >> + MacroUsed = GlobalData.gMacroRefPattern.findall(Value) >> + if len(MacroUsed) != 0: >> + for Macro in MacroUsed: >> + if Macro in GlobalData.gGlobalDefines: >> + EdkLogger.error("Parser", FORMAT_INVALID, "Global macro %s is not permitted." % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) >> + else: >> + EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) >> # Sometimes, we need to make differences between EDK and EDK2 modules >> if Name == 'INF_VERSION': >> if re.match(r'0[xX][\da-f-A-F]{5,8}', Value): >> self._Version = int(Value, 0) >> elif re.match(r'\d+\.\d+', Value): >> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py >> index e7bc87d..0686721 100644 >> --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py >> +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py >> @@ -1,9 +1,9 @@ >> ## @file >> # This file is used to create a database used by build tool >> # >> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> >> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> >> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<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 >> @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject): >> self.__Macros = {} >> # EDK_GLOBAL defined macros can be applied to EDK module >> if self.AutoGenVersion < 0x00010005: >> self.__Macros.update(GlobalData.gEdkGlobal) >> self.__Macros.update(GlobalData.gGlobalDefines) >> + else: >> + self.__Macros.update(self.Defines) >> return self.__Macros >> >> ## Get architecture >> def _GetArch(self): >> return self._Arch >> > > I don't understand how, but this patch (commit dc4c770763d0) breaks OVMF for me. > > I bisected it, I didn't want to believe it, then I built the tree right before it, and that worked. > > I also built the tree at current master (526f160f311c, "ArmPkg/ArmMmuLib: AARCH64: enable stack alignment checking", 2017-02-22), with this commit reverted on top, and that also works. > > This is the error I see in the OVMF log: > >> Loading driver E660EA85-058E-4B55-A54B-F02F83A24707 >> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7E8752C0 >> Loading driver at 0x0007E4B7000 EntryPoint=0x0007E4B727B DisplayEngine.efi >> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7E875518 >> ProtectUefiImageCommon - 0x7E8752C0 >> - 0x000000007E4B7000 - 0x000000000001BAE0 >> ASSERT .../MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.c(928): mCDLStringPackHandle != ((void *) 0) > > I don't have the slightest clue what's going on. Apparently, this change causes BaseTools to mis-build OVMF. If it's any help, I think that the second hunk breaks library constructors somehow. I compared the build report files between "good" (--> without this patch) and "bad" (--> with this patch), and the differences I see follow this pattern, under every module that links against library instance with a constructor: >----------------------------------------------------------------------------------------------------------------------< Library ------------------------------------------------------------------------------------------------------------------------ MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf {MemoryAllocationLib} MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf {PeiServicesTablePointerLib} MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf {PcdLib} MdePkg/Library/BaseLib/BaseLib.inf {BaseLib} MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf {BaseMemoryLib} MdePkg/Library/PeiHobLib/PeiHobLib.inf {HobLib} MdePkg/Library/PeiServicesLib/PeiServicesLib.inf {PeiServicesLib} MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf {DebugPrintErrorLevelLib} MdePkg/Library/BasePrintLib/BasePrintLib.inf {PrintLib} MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf {IoLib} OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf -{DebugLib: C = PlatformDebugLibIoPortConstructor} +{DebugLib: C = PlatformDebugLibIoPortConstructor PlatformDebugLibIoPortConstructor} MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf {PeimEntryPoint} <----------------------------------------------------------------------------------------------------------------------> Note "DebugLib". In the "bad" case, the constructor name seems to be duplicated. This applies to all other constructor functions as well. Thanks Laszlo ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Patch] BaseTools: add error check for Macro usage in the INF file 2017-02-23 1:14 ` Gao, Liming @ 2017-02-23 10:22 ` Laszlo Ersek 0 siblings, 0 replies; 9+ messages in thread From: Laszlo Ersek @ 2017-02-23 10:22 UTC (permalink / raw) To: Gao, Liming, Zhu, Yonghong, edk2-devel@ml01.01.org On 02/23/17 02:14, Gao, Liming wrote: > Laszlo: > Yonghong has sent the another patch to its regression issue. Could you verify it? Yes, thanks, I'll check that out soon. (Also, I'm sorry about reporting this after Ard's report; I was very busy last night and sort of looked at Thunderbird in write-only mode...) Thanks Laszlo > -----Original Message----- > From: Laszlo Ersek [mailto:lersek@redhat.com] > Sent: Thursday, February 23, 2017 9:12 AM > To: Zhu, Yonghong <yonghong.zhu@intel.com>; edk2-devel@ml01.01.org > Cc: Gao, Liming <liming.gao@intel.com> > Subject: Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file > > On 02/23/17 02:02, Laszlo Ersek wrote: >> Hi, >> >> On 02/21/17 02:18, Yonghong Zhu wrote: >>> Use of MACRO statements in the EDK II INF files is limited to local >>> usage only; global or external macros are not permitted. This patch >>> add the check for not defined macros. >>> >>> Cc: Liming Gao <liming.gao@intel.com> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Yonghong Zhu <yonghong.zhu@intel.com> >>> --- >>> BaseTools/Source/Python/Workspace/MetaFileParser.py | 9 ++++++++- >>> BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++- >>> 2 files changed, 11 insertions(+), 2 deletions(-) >>> >>> diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py b/BaseTools/Source/Python/Workspace/MetaFileParser.py >>> index 1a5fdf5..37a7f5d 100644 >>> --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py >>> +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py >>> @@ -1,9 +1,9 @@ >>> ## @file >>> # This file is used to parse meta files >>> # >>> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> >>> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> >>> # (C) Copyright 2015-2016 Hewlett Packard Enterprise Development LP<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 >>> @@ -349,10 +349,17 @@ class MetaFileParser(object): >>> EdkLogger.error('Parser', FORMAT_INVALID, "No value specified", >>> ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) >>> >>> self._ValueList = [ReplaceMacro(Value, self._Macros) for Value in self._ValueList] >>> Name, Value = self._ValueList[1], self._ValueList[2] >>> + MacroUsed = GlobalData.gMacroRefPattern.findall(Value) >>> + if len(MacroUsed) != 0: >>> + for Macro in MacroUsed: >>> + if Macro in GlobalData.gGlobalDefines: >>> + EdkLogger.error("Parser", FORMAT_INVALID, "Global macro %s is not permitted." % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) >>> + else: >>> + EdkLogger.error("Parser", FORMAT_INVALID, "%s not defined" % (Macro), ExtraData=self._CurrentLine, File=self.MetaFile, Line=self._LineIndex + 1) >>> # Sometimes, we need to make differences between EDK and EDK2 modules >>> if Name == 'INF_VERSION': >>> if re.match(r'0[xX][\da-f-A-F]{5,8}', Value): >>> self._Version = int(Value, 0) >>> elif re.match(r'\d+\.\d+', Value): >>> diff --git a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py >>> index e7bc87d..0686721 100644 >>> --- a/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py >>> +++ b/BaseTools/Source/Python/Workspace/WorkspaceDatabase.py >>> @@ -1,9 +1,9 @@ >>> ## @file >>> # This file is used to create a database used by build tool >>> # >>> -# Copyright (c) 2008 - 2016, Intel Corporation. All rights reserved.<BR> >>> +# Copyright (c) 2008 - 2017, Intel Corporation. All rights reserved.<BR> >>> # (C) Copyright 2016 Hewlett Packard Enterprise Development LP<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 >>> @@ -1828,10 +1828,12 @@ class InfBuildData(ModuleBuildClassObject): >>> self.__Macros = {} >>> # EDK_GLOBAL defined macros can be applied to EDK module >>> if self.AutoGenVersion < 0x00010005: >>> self.__Macros.update(GlobalData.gEdkGlobal) >>> self.__Macros.update(GlobalData.gGlobalDefines) >>> + else: >>> + self.__Macros.update(self.Defines) >>> return self.__Macros >>> >>> ## Get architecture >>> def _GetArch(self): >>> return self._Arch >>> >> >> I don't understand how, but this patch (commit dc4c770763d0) breaks OVMF for me. >> >> I bisected it, I didn't want to believe it, then I built the tree right before it, and that worked. >> >> I also built the tree at current master (526f160f311c, "ArmPkg/ArmMmuLib: AARCH64: enable stack alignment checking", 2017-02-22), with this commit reverted on top, and that also works. >> >> This is the error I see in the OVMF log: >> >>> Loading driver E660EA85-058E-4B55-A54B-F02F83A24707 >>> InstallProtocolInterface: 5B1B31A1-9562-11D2-8E3F-00A0C969723B 7E8752C0 >>> Loading driver at 0x0007E4B7000 EntryPoint=0x0007E4B727B DisplayEngine.efi >>> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF 7E875518 >>> ProtectUefiImageCommon - 0x7E8752C0 >>> - 0x000000007E4B7000 - 0x000000000001BAE0 >>> ASSERT .../MdeModulePkg/Library/CustomizedDisplayLib/CustomizedDisplayLib.c(928): mCDLStringPackHandle != ((void *) 0) >> >> I don't have the slightest clue what's going on. Apparently, this change causes BaseTools to mis-build OVMF. > > If it's any help, I think that the second hunk breaks library constructors somehow. > > I compared the build report files between "good" (--> without this patch) and "bad" (--> with this patch), and the differences I see follow this pattern, under every module that links against library instance with a constructor: > > >----------------------------------------------------------------------------------------------------------------------< > Library > ------------------------------------------------------------------------------------------------------------------------ > MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf > {MemoryAllocationLib} > MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf > {PeiServicesTablePointerLib} > MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf > {PcdLib} > MdePkg/Library/BaseLib/BaseLib.inf > {BaseLib} > MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRepStr.inf > {BaseMemoryLib} > MdePkg/Library/PeiHobLib/PeiHobLib.inf > {HobLib} > MdePkg/Library/PeiServicesLib/PeiServicesLib.inf > {PeiServicesLib} > MdePkg/Library/BaseDebugPrintErrorLevelLib/BaseDebugPrintErrorLevelLib.inf > {DebugPrintErrorLevelLib} > MdePkg/Library/BasePrintLib/BasePrintLib.inf > {PrintLib} > MdePkg/Library/BaseIoLibIntrinsic/BaseIoLibIntrinsic.inf > {IoLib} > OvmfPkg/Library/PlatformDebugLibIoPort/PlatformDebugLibIoPort.inf > -{DebugLib: C = PlatformDebugLibIoPortConstructor} > +{DebugLib: C = PlatformDebugLibIoPortConstructor PlatformDebugLibIoPortConstructor} > MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf > {PeimEntryPoint} > <----------------------------------------------------------------------------------------------------------------------> > > Note "DebugLib". In the "bad" case, the constructor name seems to be duplicated. > > This applies to all other constructor functions as well. > > Thanks > Laszlo > ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2017-02-23 10:22 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2017-02-21 1:18 [Patch] BaseTools: add error check for Macro usage in the INF file Yonghong Zhu 2017-02-21 10:03 ` Gao, Liming 2017-02-22 11:44 ` Ard Biesheuvel 2017-02-22 13:38 ` Zhu, Yonghong 2017-02-22 16:04 ` Zhu, Yonghong 2017-02-23 1:02 ` Laszlo Ersek 2017-02-23 1:12 ` Laszlo Ersek 2017-02-23 1:14 ` Gao, Liming 2017-02-23 10:22 ` Laszlo Ersek
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox