BaseTools/Ecc: Add check for traditional include guards

Adds a new ECC check, `IncludeFileCheckPragmaOnce` (error code 6006),
that flags header files using a traditional `#ifndef`/`#define`
include guard and recommends `#pragma once` instead.

A guard is detected when a '#ifndef NAME' is immediately followed by a
valueless '#define NAME' using the same macro name. Feature macros
such as '#define FOO 1' and files already using '#pragma once' are not
flagged.

The check reports against the parsed preprocessor directive rows in
the identifier tables rather than the File table. Those rows carry the
actual source line number, whereas File-level findings resolve to
"line 1" in the report. This gives an accurate line number, to the
EccCheck CI plugin, so it can reconcile findings with the changed line
ranges of a commit.

It uses the binary extension list and the exception list, consistent
with the other include file checks.

Signed-off-by: Michael Kubacki <michael.kubacki@microsoft.com>

4 files changed, 76 insertions, 0 deletions

diff --git a/BaseTools/Source/Python/Ecc/Check.py b/BaseTools/Source/Python/Ecc/Check.py
index e0db6cb142..8edca5d0d6 100644
--- a/BaseTools/Source/Python/Ecc/Check.py
+++ b/BaseTools/Source/Python/Ecc/Check.py
@@ -575,6 +575,75 @@ class Check(object):
     def IncludeFileCheck(self):

         self.IncludeFileCheckData()

         self.IncludeFileCheckSameName()

+        self.IncludeFileCheckPragmaOnce()

+

+    # Check whether include files use '#pragma once' instead of a traditional #ifndef/#define include guard

+    def IncludeFileCheckPragmaOnce(self):

+        if EccGlobalData.gConfig.IncludeFileCheckPragmaOnce == '1' or EccGlobalData.gConfig.IncludeFileCheckAll == '1' or EccGlobalData.gConfig.CheckAll == '1':

+            EdkLogger.quiet("Checking if header file uses '#pragma once' ...")

+

+            # Build the set of header files to check, indexed by File table ID.

+            HeaderFileSet = {}

+            SqlCommand = """select ID, FullPath, ExtName from File where ExtName in ('h')"""

+            for Record in EccGlobalData.gDb.TblFile.Exec(SqlCommand):

+                if Record[2].upper() not in EccGlobalData.gConfig.BinaryExtList:

+                    HeaderFileSet[Record[0]] = Record[1]

+            if not HeaderFileSet:

+                return

+

+            #

+            # The preprocessor directives of each source file are parsed into the

+            # identifier table(s) with their actual line numbers. Reporting against

+            # those rows (instead of the File table) gives an accurate line number,

+            # which is required by the incremental EccCheck plugin that filters

+            # findings by the changed line ranges of a commit.

+            #

+            for IdentifierTable in EccGlobalData.gIdentifierTableList:

+                SqlCommand = """select ID, Value, BelongsToFile from %s

+                                where Model in (%s, %s, %s)

+                                order by BelongsToFile, StartLine, ID""" \

+                                % (IdentifierTable,

+                                   MODEL_IDENTIFIER_MACRO_IFNDEF,

+                                   MODEL_IDENTIFIER_MACRO_DEFINE,

+                                   MODEL_IDENTIFIER_MACRO_PRAGMA)

+                RecordSet = EccGlobalData.gDb.TblFile.Exec(SqlCommand)

+

+                # Group the parsed directives by the file they belong to.

+                FileRecordDict = {}

+                for Record in RecordSet:

+                    if Record[2] not in HeaderFileSet:

+                        continue

+                    FileRecordDict.setdefault(Record[2], []).append(Record)

+

+                for FileId, Records in FileRecordDict.items():

+                    Path = mws.relpath(HeaderFileSet[FileId], EccGlobalData.gWorkspace)

+                    if EccGlobalData.gException.IsException(ERROR_INCLUDE_FILE_CHECK_PRAGMA_ONCE, Path):

+                        continue

+

+                    # Walk the directives in file order. '#pragma once' makes the file

+                    # compliant. Otherwise a '#ifndef NAME' directly followed by a

+                    # valueless '#define NAME' is treated as a traditional include guard.

+                    PragmaOnceFound = False

+                    GuardRecord = None

+                    PrevIfndef = None

+                    for Record in Records:

+                        Content = Record[1].strip().splitlines()[0].strip() if Record[1].strip() else ''

+                        if re.match(r'#\s*pragma\s+once\b', Content):

+                            PragmaOnceFound = True

+                            break

+                        MatchIfndef = re.match(r'#\s*ifndef\s+(\w+)', Content)

+                        if MatchIfndef:

+                            PrevIfndef = (MatchIfndef.group(1), Record)

+                            continue

+                        MatchDefine = re.match(r'#\s*define\s+(\w+)\s*(//.*|/\*.*)?$', Content)

+                        if MatchDefine and PrevIfndef and MatchDefine.group(1) == PrevIfndef[0]:

+                            GuardRecord = PrevIfndef[1]

+                            break

+                        PrevIfndef = None

+

+                    if not PragmaOnceFound and GuardRecord is not None:

+                        OtherMsg = "Include file [%s] uses a traditional #ifndef/#define include guard, please use '#pragma once' instead" % Path

+                        EccGlobalData.gDb.TblReport.Insert(ERROR_INCLUDE_FILE_CHECK_PRAGMA_ONCE, OtherMsg=OtherMsg, BelongsToTable=IdentifierTable, BelongsToItem=GuardRecord[0])

 

     # Check whether having include files with same name

     def IncludeFileCheckSameName(self):

diff --git a/BaseTools/Source/Python/Ecc/Configuration.py b/BaseTools/Source/Python/Ecc/Configuration.py
index 974d3c1b1b..b435e98475 100644
--- a/BaseTools/Source/Python/Ecc/Configuration.py
+++ b/BaseTools/Source/Python/Ecc/Configuration.py
@@ -71,6 +71,7 @@ _ConfigFileToInternalTranslation = {
     "HeaderCheckFunction":"HeaderCheckFunction",

     "IncludeFileCheckAll":"IncludeFileCheckAll",

     "IncludeFileCheckData":"IncludeFileCheckData",

+    "IncludeFileCheckPragmaOnce":"IncludeFileCheckPragmaOnce",

     "IncludeFileCheckSameName":"IncludeFileCheckSameName",

     "MetaDataFileCheckAll":"MetaDataFileCheckAll",

     "MetaDataFileCheckBinaryInfInFdf":"MetaDataFileCheckBinaryInfInFdf",

@@ -243,6 +244,8 @@ class Configuration(object):
         # Check whether include files contain only public or only private data

         # Check whether include files NOT contain code or define data variables

         self.IncludeFileCheckData = 1

+        # Check whether include files use '#pragma once' instead of a traditional #ifndef/#define include guard

+        self.IncludeFileCheckPragmaOnce = 1

 

         ## Declarations and Data Types Checking

         self.DeclarationDataTypeCheckAll = 0

diff --git a/BaseTools/Source/Python/Ecc/EccToolError.py b/BaseTools/Source/Python/Ecc/EccToolError.py
index 1b179fc9a2..6ddfb5e2f7 100644
--- a/BaseTools/Source/Python/Ecc/EccToolError.py
+++ b/BaseTools/Source/Python/Ecc/EccToolError.py
@@ -45,6 +45,7 @@ ERROR_C_FUNCTION_LAYOUT_CHECK_FUNCTION_PROTO_TYPE_3 = 5010
 ERROR_INCLUDE_FILE_CHECK_ALL = 6000

 ERROR_INCLUDE_FILE_CHECK_DATA = 6004

 ERROR_INCLUDE_FILE_CHECK_NAME = 6005

+ERROR_INCLUDE_FILE_CHECK_PRAGMA_ONCE = 6006

 

 ERROR_DECLARATION_DATA_TYPE_CHECK_ALL = 7000

 ERROR_DECLARATION_DATA_TYPE_CHECK_NO_USE_C_TYPE = 7001

@@ -139,6 +140,7 @@ gEccErrorMessage = {
     ERROR_INCLUDE_FILE_CHECK_ALL : "",

     ERROR_INCLUDE_FILE_CHECK_DATA : "Include files should contain only public or only private data and cannot contain code or define data variables",

     ERROR_INCLUDE_FILE_CHECK_NAME : "No permission for the include file with same names",

+    ERROR_INCLUDE_FILE_CHECK_PRAGMA_ONCE : "Include files should use '#pragma once' instead of a traditional #ifndef/#define include guard",

 

     ERROR_DECLARATION_DATA_TYPE_CHECK_ALL : "",

     ERROR_DECLARATION_DATA_TYPE_CHECK_NO_USE_C_TYPE : "There should be no use of int, unsigned, char, void, long in any .c, .h or .asl files",

diff --git a/BaseTools/Source/Python/Ecc/config.ini b/BaseTools/Source/Python/Ecc/config.ini
index 943a87af0a..1f260aa0b8 100644
--- a/BaseTools/Source/Python/Ecc/config.ini
+++ b/BaseTools/Source/Python/Ecc/config.ini
@@ -137,6 +137,8 @@ IncludeFileCheckSameName = 1
 # Check whether include files contain only public or only private data

 # Check whether include files NOT contain code or define data variables

 IncludeFileCheckData = 1

+# Check whether include files use '#pragma once' instead of a traditional #ifndef/#define include guard

+IncludeFileCheckPragmaOnce = 1

 

 #

 # Declarations and Data Types Checking