* [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