From c5aa7e7d94c0e6b3c0202e15dbf7a5c92dd6a01d Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Tue, 2 Jul 2024 10:27:02 -0700 Subject: [PATCH] BaseTools/Build: Output warning message for library class mismatch Performs a check that will verify that the library instance implements the library specified in the dsc by ensuring a LIBRARY_CLASS definition exists in the INF [Defines] section and the value matches the library it says it is implementing. As an example, from a platform dsc file: BaseBmpSupportLib|MdeModulePkg/Library/BaseBmpSupportLib/BaseBmpSupportLib.inf BaseBmpSupportLib is supposed to be of library class BmpSupportLib, but the dsc defines it incorrectly, the warning message will be displayed during build. Signed-off-by: Aaron Pop Co-authored-by: Poncho Figueroa --- .../Source/Python/AutoGen/AutoGenWorker.py | 1 + BaseTools/Source/Python/Common/GlobalData.py | 1 + .../Source/Python/Workspace/DscBuildData.py | 60 +++++++++++++++++-- .../Source/Python/Workspace/MetaFileParser.py | 20 +++++++ .../Source/Python/Workspace/MetaFileTable.py | 39 ++++++++++++ 5 files changed, 117 insertions(+), 4 deletions(-) diff --git a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py index 0ba2339bed..8adbab8120 100755 --- a/BaseTools/Source/Python/AutoGen/AutoGenWorker.py +++ b/BaseTools/Source/Python/AutoGen/AutoGenWorker.py @@ -218,6 +218,7 @@ class AutoGenWorkerInProcess(mp.Process): GlobalData.gEnableGenfdsMultiThread = self.data_pipe.Get("EnableGenfdsMultiThread") GlobalData.gPlatformFinalPcds = self.data_pipe.Get("gPlatformFinalPcds") GlobalData.file_lock = self.file_lock + GlobalData.gLogLibraryMismatch = False CommandTarget = self.data_pipe.Get("CommandTarget") pcd_from_build_option = [] for pcd_tuple in self.data_pipe.Get("BuildOptPcd"): diff --git a/BaseTools/Source/Python/Common/GlobalData.py b/BaseTools/Source/Python/Common/GlobalData.py index dd5316d283..3efcbaabd0 100755 --- a/BaseTools/Source/Python/Common/GlobalData.py +++ b/BaseTools/Source/Python/Common/GlobalData.py @@ -124,3 +124,4 @@ gSikpAutoGenCache = set() file_lock = None gStackCookieValues32 = [] gStackCookieValues64 = [] +gLogLibraryMismatch = True diff --git a/BaseTools/Source/Python/Workspace/DscBuildData.py b/BaseTools/Source/Python/Workspace/DscBuildData.py index bb9ba046ec..5e44911c63 100644 --- a/BaseTools/Source/Python/Workspace/DscBuildData.py +++ b/BaseTools/Source/Python/Workspace/DscBuildData.py @@ -43,6 +43,7 @@ import os import shutil import sys +LoggedLibraryWarnings = set() def _IsFieldValueAnArray (Value): Value = Value.strip() if Value.startswith(TAB_GUID) and Value.endswith(')'): @@ -767,9 +768,8 @@ class DscBuildData(PlatformBuildClassObject): # get module private library instance RecordList = self._RawData[MODEL_EFI_LIBRARY_CLASS, self._Arch, None, ModuleId] for Record in RecordList: - LibraryClass = Record[0] - LibraryPath = PathClass(NormPath(Record[1], Macros), GlobalData.gWorkspace, Arch=self._Arch) - LineNo = Record[-1] + LibraryClass, LibraryInstance, Dummy, Dummy, Dummy, Dummy, RecordId, LineNo = Record + LibraryPath = PathClass(NormPath(LibraryInstance, Macros), GlobalData.gWorkspace, Arch=self._Arch) # check the file validation ErrorCode, ErrorInfo = LibraryPath.Validate('.inf') @@ -777,6 +777,16 @@ class DscBuildData(PlatformBuildClassObject): EdkLogger.error('build', ErrorCode, File=self.MetaFile, Line=LineNo, ExtraData=ErrorInfo) + # Validate that the Library instance implements the specified Library Class + if not self._ValidateLibraryClass(LibraryClass, LibraryPath, self._Arch): + # LineNo counts against the file the entry was written in, which + # is not this DSC when the entry came from an !include. + OriginFile = self._RawData.GetOriginFile(RecordId) + if self._ShouldLogLibrary(OriginFile, LineNo): + EdkLogger.warn("build", + f"{str(LibraryPath)} does not support LIBRARY_CLASS {LibraryClass}", + File=OriginFile, Line=LineNo) + if LibraryClass == '' or LibraryClass == 'NULL': self._NullLibraryNumber += 1 LibraryClass = 'NULL%d' % self._NullLibraryNumber @@ -867,19 +877,30 @@ class DscBuildData(PlatformBuildClassObject): RecordList = self._RawData[MODEL_EFI_LIBRARY_CLASS, self._Arch, None, -1] Macros = self._Macros for Record in RecordList: - LibraryClass, LibraryInstance, Dummy, Arch, ModuleType, Dummy, Dummy, LineNo = Record + LibraryClass, LibraryInstance, Dummy, Arch, ModuleType, Dummy, RecordId, LineNo = Record if LibraryClass == '' or LibraryClass == 'NULL': self._NullLibraryNumber += 1 LibraryClass = 'NULL%d' % self._NullLibraryNumber EdkLogger.verbose("Found forced library for arch=%s\n\t%s [%s]" % (Arch, LibraryInstance, LibraryClass)) LibraryClassSet.add(LibraryClass) LibraryInstance = PathClass(NormPath(LibraryInstance, Macros), GlobalData.gWorkspace, Arch=self._Arch) + # check the file validation ErrorCode, ErrorInfo = LibraryInstance.Validate('.inf') if ErrorCode != 0: EdkLogger.error('build', ErrorCode, File=self.MetaFile, Line=LineNo, ExtraData=ErrorInfo) + # Validate that the Library instance implements the specified Library Class + if not self._ValidateLibraryClass(LibraryClass, LibraryInstance, Arch): + # LineNo counts against the file the entry was written in, which + # is not this DSC when the entry came from an !include. + OriginFile = self._RawData.GetOriginFile(RecordId) + if self._ShouldLogLibrary(OriginFile, LineNo): + EdkLogger.warn("build", + f"{str(LibraryInstance)} does not support LIBRARY_CLASS {LibraryClass}", + File=OriginFile, Line=LineNo) + if ModuleType != TAB_COMMON and ModuleType not in SUP_MODULE_LIST: EdkLogger.error('build', OPTION_UNKNOWN, "Unknown module type [%s]" % ModuleType, File=self.MetaFile, ExtraData=LibraryInstance, Line=LineNo) @@ -1139,6 +1160,37 @@ class DscBuildData(PlatformBuildClassObject): for item in delete_assign: GlobalData.BuildOptionPcd.remove(item) + def _ValidateLibraryClass(self, LibraryClass: str, LibraryInstance: PathClass, Arch: str) -> bool: + # + # Forced library instances have no class to match against. They are spelled + # 'NULL' (or left empty) in the DSC and renamed to 'NULL' while parsing, so + # both spellings can reach here depending on the caller. + # + if LibraryClass in ('', 'NULL'): + return True + if LibraryClass.startswith('NULL') and LibraryClass[4:].isdigit(): + return True + + ParsedLibraryInfo = self._Bdb[LibraryInstance, Arch, self._Target, self._Toolchain] + + for LibraryClassObject in ParsedLibraryInfo.LibraryClass: + if LibraryClassObject.LibraryClass == LibraryClass: + return True + return False + + def _ShouldLogLibrary(self, OriginFile, LineNo) -> bool: + if not GlobalData.gLogLibraryMismatch: + return False + + # Key on the file as well as the line, otherwise entries that share a line + # number across different DSC files silently suppress each other. + Key = (str(OriginFile), LineNo) + if Key in LoggedLibraryWarnings: + return False + + LoggedLibraryWarnings.add(Key) + return True + @staticmethod def HandleFlexiblePcd(TokenSpaceGuidCName, TokenCName, PcdValue, PcdDatumType, GuidDict, FieldName=''): if FieldName: diff --git a/BaseTools/Source/Python/Workspace/MetaFileParser.py b/BaseTools/Source/Python/Workspace/MetaFileParser.py index 1880148a6d..862f4fc436 100644 --- a/BaseTools/Source/Python/Workspace/MetaFileParser.py +++ b/BaseTools/Source/Python/Workspace/MetaFileParser.py @@ -1728,6 +1728,26 @@ class DscParser(MetaFileParser): self._ValueList = [ReplaceMacro(Value, self._Macros, RaiseError=False) for Value in self._ValueList] + ## Find the file a record's line number refers to + # + # !include'd records are spliced into the including file's record list, so + # a record's line number is not necessarily an offset into self.MetaFile. + # Callers reporting a line number to the user should report this path with + # it rather than assuming the top-level DSC. + # + # @param RecordId: ID of the record to locate + # + # @retval: Path of the file the record was parsed from + # + def GetOriginFile(self, RecordId): + for Table in (self._Table, self._RawTable): + if Table is None: + continue + OriginFile = Table.GetOriginFile(RecordId) + if OriginFile is not None: + return OriginFile + return self.MetaFile + def DisableOverrideComponent(self,module_id): for ori_id in self._IdMapping: if self._IdMapping[ori_id] == module_id: diff --git a/BaseTools/Source/Python/Workspace/MetaFileTable.py b/BaseTools/Source/Python/Workspace/MetaFileTable.py index 7ff5f2011d..3b9060b465 100644 --- a/BaseTools/Source/Python/Workspace/MetaFileTable.py +++ b/BaseTools/Source/Python/Workspace/MetaFileTable.py @@ -23,6 +23,11 @@ class MetaFileTable(): _ID_STEP_ = 1 _ID_MAX_ = 99999999 + # Column offsets into the rows this class appends to DB.TblFile. Keep in + # sync with the list built in __init__. + _FILE_PATH_ = 3 + _FILE_FROM_ITEM_ = 6 + ## Constructor def __init__(self, DB, MetaFile, FileType, Temporary, FromItem=None): self.MetaFile = MetaFile @@ -30,6 +35,7 @@ class MetaFileTable(): self.DB = DB self.CurrentContent = [] + # Columns here are addressed by the _FILE_*_ offsets above. DB.TblFile.append([MetaFile.Name, MetaFile.Ext, MetaFile.Dir, @@ -297,6 +303,10 @@ class PlatformTable(MetaFileTable): # used as table end flag, in case the changes to database is not committed to db file _DUMMY_ = [-1, -1, '====', '====', '====', '====', '====','====', -1, -1, -1, -1, -1, -1, -1] + # Column offsets into the rows built by Insert(), matching _COLUMN_ above. + _ID_ = 0 + _FROM_ITEM_ = 9 + ## Constructor def __init__(self, Cursor, MetaFile, Temporary, FromItem=0): MetaFileTable.__init__(self, Cursor, MetaFile, MODEL_FILE_DSC, Temporary, FromItem) @@ -386,6 +396,35 @@ class PlatformTable(MetaFileTable): if item[0] == comp_id or item[8] == comp_id: item[-1] = -1 + ## Find the file a record's line number refers to + # + # Records brought in by !include are spliced into the including file's + # record list, so this table's MetaFile is not necessarily the file the + # record's StartLine counts against. Such a record keeps the ID of the + # !include statement that pulled it in as its FromItem, and the table + # built for the included file stored that same ID, so FromItem maps back + # to the included file's path. + # + # @param RecordId: ID of the record to locate + # + # @retval: Path of the file the record was parsed from, or None if + # this table holds no such record + # + def GetOriginFile(self, RecordId): + for Record in self.CurrentContent: + if Record[self._ID_] != RecordId: + continue + FromItem = Record[self._FROM_ITEM_] + # A record parsed straight out of this file has no originating + # !include statement to resolve. + if not FromItem or FromItem < 0: + return self.MetaFile + for File in self.DB.TblFile: + if File[self._FILE_FROM_ITEM_] == FromItem: + return File[self._FILE_PATH_] + return self.MetaFile + return None + ## Factory class to produce different storage for different type of meta-file class MetaFileStorage(object): _FILE_TABLE_ = {