1. Patchew
  2. EDK2
  3. View series

[edk2] [Patch] BaseTools: add error check for Macro usage in the INF file

Yonghong Zhu posted 1 patch 7 years, 2 months ago
Failed in applying to current master (apply log)
BaseTools/Source/Python/Workspace/MetaFileParser.py    | 9 ++++++++-
BaseTools/Source/Python/Workspace/WorkspaceDatabase.py | 4 +++-
2 files changed, 11 insertions(+), 2 deletions(-)
[edk2] [Patch] BaseTools: add error check for Macro usage in the INF file
Posted by Yonghong Zhu 7 years, 2 months ago 
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
Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file
Posted by Gao, Liming 7 years, 2 months ago 
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file
Posted by Ard Biesheuvel 7 years, 2 months ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file
Posted by Zhu, Yonghong 7 years, 2 months ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file
Posted by Laszlo Ersek 7 years, 1 month ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file
Posted by Laszlo Ersek 7 years, 1 month ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file
Posted by Gao, Liming 7 years, 1 month ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch] BaseTools: add error check for Macro usage in the INF file
Posted by Laszlo Ersek 7 years, 1 month ago 
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
> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.