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

["Zhu, Yonghong" <yonghong.zhu@intel.com>]

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