database: multiple bugfixes (#393)

This PR contains the following database changes:

### [bug] Account for library instances that do not exist in the DSC

The instanced_inf_table generator did not account for the fact that
library class instances can choose to only support phases.

1. Updates the DSC parser's ScopedLibraryDict to store a list of
instances that match the scope, instead of the first instance it finds.

2. The instanced_inf_table generator now checks the list of instances (
    in order of appearance in the DSC) to find the first instance that
    matches the scope, but also supports the specific phase.


### [bug] Account for library instances that support only a subset of phases.

Due to platforms performing complex includes in their DSCs, There is a
scenario in which a library instance references a library class that
does not exist. This is not an error as the library class instance may
not be consumed by a component in the platform, so the missing
dependency is acceptable.

This commit adds the ability to handle this scenario, and will not raise
an error for a missing library instance.

### [enhancement] Include library class and library instance in "LIBRARIES_USED" column of the instanced_inf table

The LIBRARIES_USED column in the instanced_inf table only contained
the the edk2 relative path to the library instance. This made it
impossible to determine (with 100% accuracy) which library class the
particular library instance was representing.

This commit changes the "LIBRARIES_USED" column to contain a list of
(lib_class, lib_instance) tuples rather than a list of lib_instances.
Please review commit messages in each for a more detailed description.

### [enhancement] Include "LIBRARY_CLASS" column in instanced_inf table

Previously, the library class name for a library INF was not included
in the table. This commit adds that information to the table as a row
to make it easier to find the library class name for a library INF.

### [bug] Better handling of RuleOverride in instanced_fv_table

When parsing an FDF file, an INF can be prepending with a RuleOverride
directive. The previous functionality to ignore this directive and
retrive the actual INF value could not handle any spaces in the
directive (such as RuleOverride = ACPITABLE). This commit expands the 
ability to handle spaces in the directive.

edk2toollib/database/queries/unused_component_query.py

@@ -91,4 +91,4 @@ def _recurse_inf(self, inf, table, library_list):
                 library_list.append(inf)
 
             for inf in result["LIBRARIES_USED"]:
-                self._recurse_inf(inf, table, library_list)
+                self._recurse_inf(inf[1], table, library_list)

edk2toollib/database/tables/instanced_fv_table.py

@@ -7,6 +7,7 @@
 # SPDX-License-Identifier: BSD-2-Clause-Patent
 ##
 """A module to generate a table containing fv information."""
+import re
 from pathlib import Path
 
 from tinyrecord import transaction
@@ -27,6 +28,9 @@ class InstancedFvTable(TableGenerator):
     |------------------------------------------------------|
     ```
     """  # noqa: E501
+
+    RULEOVERRIDE = re.compile(r'RuleOverride\s*=.+\s+(.+\.inf)', re.IGNORECASE)
+
     def __init__(self, *args, **kwargs):
         """Initialize the query with the specific settings."""
         self.env = kwargs.pop("env")
@@ -54,7 +58,7 @@ def parse(self, db: Edk2DB) -> None:
             inf_list = []  # Some INF's start with RuleOverride. We only need the INF
             for inf in fdfp.FVs[fv]["Infs"]:
                 if inf.lower().startswith("ruleoverride"):
-                    inf = inf.split(" ", 1)[-1]
+                    inf = InstancedFvTable.RULEOVERRIDE.findall(inf)[0]
                 if Path(inf).is_absolute():
                     inf = str(Path(self.pathobj.GetEdk2RelativePathFromAbsolutePath(inf)))
                 inf_list.append(Path(inf).as_posix())

edk2toollib/database/tables/instanced_inf_table.py

@@ -25,9 +25,9 @@ class InstancedInfTable(TableGenerator):
 
     ``` py
     table_name = "instanced_inf"
-    |----------------------------------------------------------------------------------------------------------------------------------|
-    | DSC | GUID | LIBRARY_CLASS | PATH | PHASES | SOURCES_USED | LIBRARIES_USED | PROTOCOLS_USED | GUIDS_USED | PPIS_USED | PCDS_USED |
-    |----------------------------------------------------------------------------------------------------------------------------------|
+    |----------------------------------------------------------------------------------------------------------------------------------------------|
+    | DSC | PATH | NAME | LIBRARY_CLASS | COMPONENT | MODULE_TYPE | ARCH | SOURCES_USED | LIBRARIES_USED | PROTOCOLS_USED | GUIDS_USED | PCDS_USED |
+    |----------------------------------------------------------------------------------------------------------------------------------------------|
     ```
     """  # noqa: E501
     SECTION_LIBRARY = "LibraryClasses"
@@ -43,6 +43,19 @@ def __init__(self, *args, **kwargs):
         self.arch = self.env["TARGET_ARCH"].split(" ")  # REQUIRED
         self.target = self.env["TARGET"]  # REQUIRED
 
+        # Prevent parsing the same INF multiple times
+        self._parsed_infs = {}
+
+    def inf(self, inf: str) -> InfP:
+        """Returns a parsed INF object."""
+        if inf in self._parsed_infs:
+            infp = self._parsed_infs[inf]
+        else:
+            infp = InfP().SetEdk2Path(self.pathobj)
+            infp.ParseFile(inf)
+            self._parsed_infs[inf] = infp
+        return infp
+
     def parse(self, db: Edk2DB) -> None:
         """Parse the workspace and update the database."""
         self.pathobj = db.pathobj
@@ -106,44 +119,60 @@ def _parse_inf_recursively(
 
         Will immediately return if the INF has already been visited.
         """
+        if inf is None:
+            return []
+
         logging.debug(f"  Parsing Library: [{inf}]")
         visited.append(inf)
-        library_instances = []
+        library_instance_list = []
+        library_class_list = []
+
 
         #
         # 0. Use the existing parser to parse the INF file. This parser parses an INF as an independent file
         #    and does not take into account the context of a DSC.
         #
-        infp = InfP().SetEdk2Path(self.pathobj)
-        infp.ParseFile(inf)
+        infp = self.inf(inf)
 
         #
         # 1. Convert all libraries to their actual instances for this component. This takes into account
         #    any overrides for this component
         #
         for lib in infp.get_libraries(self.arch):
-            lib = lib.split(" ")[0].lower()
-            library_instances.append(self._lib_to_instance(lib, scope, library_dict, override_dict))
+            lib = lib.split(" ")[0]
+            library_instance_list.append(self._lib_to_instance(lib.lower(), scope, library_dict, override_dict))
+            library_class_list.append(lib)
         # Append all NULL library instances
         for null_lib in override_dict["NULL"]:
-            library_instances.append(null_lib)
+            library_instance_list.append(null_lib)
+            library_class_list.append("NULL")
+
+        to_parse = list(filter(lambda lib: lib is not None, library_instance_list))
 
         # Time to visit in libraries that we have not visited yet.
         to_return = []
-        for library in filter(lambda lib: lib not in visited, library_instances):
+        for library in filter(lambda lib: lib not in visited, to_parse):
             to_return += self._parse_inf_recursively(library, component,
                                                      library_dict, override_dict, scope, visited)
 
+        # Transform path to edk2 relative form (POSIX)
+        def to_posix(path):
+            if path is None:
+                return None
+            return Path(path).as_posix()
+        library_instance_list = list(map(to_posix, library_instance_list))
+
         # Return Paths as posix paths, which is Edk2 standard.
         to_return.append({
             "DSC": Path(self.dsc).name,
             "PATH": Path(inf).as_posix(),
             "NAME": infp.Dict["BASE_NAME"],
+            "LIBRARY_CLASS": infp.LibraryClass,
             "COMPONENT": Path(component).as_posix(),
             "MODULE_TYPE": infp.Dict["MODULE_TYPE"],
             "ARCH": scope.split(".")[0].upper(),
             "SOURCES_USED": list(map(lambda p: Path(p).as_posix(), infp.Sources)),
-            "LIBRARIES_USED": list(map(lambda p: Path(p).as_posix(), library_instances)),
+            "LIBRARIES_USED": list(zip(library_class_list, library_instance_list)),
             "PROTOCOLS_USED": [],  # TODO
             "GUIDS_USED": [],  # TODO
             "PPIS_USED": [],  # TODO
@@ -158,39 +187,68 @@ def _lib_to_instance(self, library_class_name, scope, library_dict, override_dic
         """
         arch, module = tuple(scope.split("."))
 
+        # NOTE: it is recognized that the below code could be reduced to have less repetitiveness,
+        # but I personally believe that the below makes it more clear the order in which we search
+        # for matches, and that the order is quite important.
+
         # https://tianocore-docs.github.io/edk2-DscSpecification/release-1.28/2_dsc_overview/27_[libraryclasses]_section_processing.html#27-libraryclasses-section-processing
 
         # 1. If a Library class instance (INF) is specified in the Edk2 II [Components] section (an override),
-        #    then it will be used
+        #    and the library supports the module, then it will be used.
         if library_class_name in override_dict:
             return override_dict[library_class_name]
 
         # 2/3. If the Library Class instance (INF) is defined in the [LibraryClasses.$(ARCH).$(MODULE_TYPE)] section,
-        #    then it will be used.
+        #      and the library supports the module, then it will be used.
         lookup = f'{arch}.{module}.{library_class_name}'
         if lookup in library_dict:
-            return library_dict[lookup]
+            library_instance = self._reduce_lib_instances(module, library_dict[lookup])
+            if library_instance is not None:
+                return library_instance
 
         # 4. If the Library Class instance (INF) is defined in the [LibraryClasses.common.$(MODULE_TYPE)] section,
-        #   then it will be used.
+        #    and the library supports the module, then it will be used.
         lookup = f'common.{module}.{library_class_name}'
         if lookup in library_dict:
-            return library_dict[lookup]
+            library_instance = self._reduce_lib_instances(module, library_dict[lookup])
+            if library_instance is not None:
+                return library_instance
 
         # 5. If the Library Class instance (INF) is defined in the [LibraryClasses.$(ARCH)] section,
-        #    then it will be used.
+        #    and the library supports the module, then it will be used.
         lookup = f'{arch}.{library_class_name}'
         if lookup in library_dict:
-            return library_dict[lookup]
+            library_instance = self._reduce_lib_instances(module, library_dict[lookup])
+            if library_instance is not None:
+                return library_instance
 
         # 6. If the Library Class Instance (INF) is defined in the [LibraryClasses] section,
-        #    then it will be used.
+        #    and the library supports the module, then it will be used.
         lookup = f'common.{library_class_name}'
         if lookup in library_dict:
-            return library_dict[lookup]
+            library_instance = self._reduce_lib_instances(module, library_dict[lookup])
+            if library_instance is not None:
+                return library_instance
 
         logging.debug(f'scoped library contents: {library_dict}')
         logging.debug(f'override dictionary: {override_dict}')
         e = f'Cannot find library class [{library_class_name}] for scope [{scope}] when evaluating {self.dsc}'
-        logging.error(e)
-        raise RuntimeError(e)
+        logging.warn(e)
+        return None
+
+    def _reduce_lib_instances(self, module: str, library_instance_list: list[str]) -> str:
+        """For a DSC, multiple library instances for the same library class can exist.
+
+        This is either due to a mistake by the developer, or because the library class
+        instance only supports certain modules. That is to say a library class instance
+        defining `MyLib| PEIM` and one defining `MyLib| PEI_CORE` both being defined in
+        the same LibraryClasses section is acceptable.
+
+        Due to this, we need to filter to the first library class instance that supports
+        the module type.
+        """
+        for library_instance in library_instance_list:
+            infp = self.inf(library_instance)
+            if module.lower() in [phase.lower() for phase in infp.SupportedPhases]:
+                return library_instance
+        return None

edk2toollib/uefi/edk2/parsers/dsc_parser.py

@@ -24,8 +24,15 @@ class DscParser(HashFileParser):
         OtherMods (list): list of other Mods that are not IA32, X64 specific
         Libs (list): list of Libs (the line in the file)
         LibsEnhanced (list): Better parsed (in a dict) list of Libs
+        ScopedLibraryDict (dict): key (library class) value (list of instances)
         LibraryClassToInstanceDict (dict): Key (Library class) Value (Instance)
         Pcds (list): List of Pcds
+
+    !!! note
+        ScopedLibraryDict can have multiple library instances for the same scope because the INF
+        can also filter the module types it supports. For example, two library instances could be
+        in the scope of "common", but one filters to only PEI (MyLib| PEI_CORE) and the other
+        filters to PEIM (MyLib| PEIM).
     """
     SECTION_LIBRARY = "libraryclasses"
     SECTION_COMPONENT = "components"
@@ -359,7 +366,10 @@ def _parse_libraries(self):
             for scope in current_scope:
                 key = f"{scope.strip()}.{lib.strip()}".lower()
                 value = instance.strip()
-                self.ScopedLibraryDict[key] = value
+                if key in self.ScopedLibraryDict and value not in self.ScopedLibraryDict[key]:
+                    self.ScopedLibraryDict[key].insert(0, value)
+                else:
+                    self.ScopedLibraryDict[key] = [value]
 
         return
 

edk2toollib/uefi/edk2/parsers/inf_parser.py

@@ -12,7 +12,8 @@
 from edk2toollib.uefi.edk2.parsers.base_parser import HashFileParser
 
 AllPhases = ["SEC", "PEIM", "PEI_CORE", "DXE_DRIVER", "DXE_CORE", "DXE_RUNTIME_DRIVER", "UEFI_DRIVER",
-             "SMM_CORE", "DXE_SMM_DRIVER", "UEFI_APPLICATION"]
+             "SMM_CORE", "DXE_SMM_DRIVER", "UEFI_APPLICATION", "MM_STANDALONE", "MM_CORE_STANDALONE",
+             "HOST_APPLICATION",]
 
 
 class InfParser(HashFileParser):

tests.unit/database/common.py

@@ -156,12 +156,13 @@ def __init__(self, ws: Path):
     def create_library(self, name: str, lib_cls: str, **kwargs):
         """Creates a Library INF in the empty tree."""
         path = self.library_folder / f'{name}.inf'
-        kwargs["defines"] = {
+        default = {
             "FILE_GUID": str(uuid.uuid4()),
             "MODULE_TYPE": "BASE",
             "BASE_NAME": name,
             "LIBRARY_CLASS": lib_cls,
         }
+        kwargs["defines"] = {**default, **kwargs.get("defines", {})}
         create_inf_file(path, **kwargs)
         self.library_list.append(str(path))
         return str(path.relative_to(self.ws))

tests.unit/database/test_component_query.py

@@ -71,7 +71,7 @@ def test_simple_component(empty_tree: Tree):
     result = db.search(ComponentQuery(component = "TestDriver1"))
     assert len(result) == 1
 
-    assert sorted(result[0]['LIBRARIES_USED']) == sorted(['TestPkg/Library/TestLib2.inf', 'TestPkg/Library/TestLib3.inf'])
+    assert sorted(result[0]['LIBRARIES_USED']) == sorted([('TestCls','TestPkg/Library/TestLib2.inf'), ('NULL','TestPkg/Library/TestLib3.inf')])
 
     result = db.search(ComponentQuery(component = "NonExistantDriver"))
     assert len(result) == 0

tests.unit/database/test_instanced_fv_table.py

@@ -29,6 +29,7 @@ def test_valid_fdf(empty_tree: Tree):  # noqa: F811
     comp2 = empty_tree.create_component("TestDriver2", "DXE_DRIVER")
     comp3 = empty_tree.create_component("TestDriver3", "DXE_DRIVER")
     comp4 = str(Path('TestPkg','Extra Drivers','TestDriver4.inf'))
+    comp5 = empty_tree.create_component("TestDriver5", "DXE_DRIVER")
 
     dsc = empty_tree.create_dsc()
 
@@ -39,7 +40,8 @@ def test_valid_fdf(empty_tree: Tree):  # noqa: F811
             f"INF  {comp1}", # PP relative
             f'INF  {str(empty_tree.ws / comp2)}', # Absolute
             f'INF  RuleOverride=RESET_VECTOR {comp3}', # RuleOverride
-            f'INF  {comp4}' # Space in path
+            f'INF  {comp4}', # Space in path
+            f'INF  ruleoverride = RESET_VECTOR {comp5}', # RuleOverride lowercase & spaces
         ]
     )
 
@@ -62,5 +64,6 @@ def test_valid_fdf(empty_tree: Tree):  # noqa: F811
                 Path(comp1).as_posix(),
                 Path(comp2).as_posix(),
                 Path(comp3).as_posix(),
+                Path(comp5).as_posix(),
                 Path(comp4).as_posix(),
                 ])