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

[Laszlo Ersek <lersek@redhat.com>]

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