[packchk] issue warning for explicit include paths in *.pdsc to config files #1644 (#1136) (#1958)

* [packchk] issue warning for explicit include paths in *.pdsc to config
files #1644

---------

Co-authored-by: Thorsten de Buhr <[email protected]>

Author: spcaipers-arm

.gitignore

@@ -20,4 +20,5 @@
 *.opendb
 .vscode
 /build*/
-**/cmake-*
+**/cmake-*
+/.cache

tools/packchk/include/ValidateSemantic.h

@@ -44,7 +44,8 @@ class ValidateSemantic : public Validate
   bool FindFileFromList(const std::string& systemHeader, const std::set<RteFile*>& targFiles);
   bool CheckDeviceDependencies(RteDeviceItem* device, RteProject* rteProject);
   bool HasExternalGenerator(RteComponentAggregate* aggregate);
-
+  bool FileIsHeader(const std::string& name);
+  
   const std::map<std::string, compiler_s>& GetCompilers() {
     return m_compilers;
   }

tools/packchk/src/PackChk_Msgs.cpp

@@ -172,7 +172,7 @@ const MsgTable PackChk::msgTable = {
   { "M354", { MsgLevel::LEVEL_WARNING3, CRLF_B, "Multiple '%FILECAT%' Files found for Component '%COMP%' for '[%VENDOR%] %MCU%' (Compiler: %COMPILER% [%OPTION%])" } },
   { "M355", { MsgLevel::LEVEL_WARNING3, CRLF_B, "No '%FILECAT%' Directory found for Component '%COMP%' (%COMPID%) for '[%VENDOR%] %MCU%' (Compiler: %COMPILER% [%OPTION%])" } },
   { "M356", { MsgLevel::LEVEL_ERROR,    CRLF_B, "Path '%PATH%' does not specify a file!" } },
-  { "M357", { MsgLevel::LEVEL_WARNING,  CRLF_B, "" } },
+  { "M357", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Config File '%NAME%', which gets copied to project, is also found in the include path to the original pack folder. Do not place config header files in the same directory as regular header files." } },
   { "M358", { MsgLevel::LEVEL_WARNING3, CRLF_B, "Header File '%HFILE%' for '%CFILE%', used by Component '%COMP%' (%COMPID%)\n"\
                                                 "  for Device '[%VENDOR%] %MCU%' (Compiler: %COMPILER% [%OPTION%]) not found on file system in any include paths: %PATH%" } },
   { "M359", { MsgLevel::LEVEL_WARNING,  CRLF_B, "Family has no Device(s) or Subfamilies: '%FAMILY%" } },

tools/packchk/src/ValidateSemantic.cpp

@@ -450,6 +450,12 @@ bool ValidateSemantic::FindFileFromList(const string& systemHeader, const set<Rt
   return false;
 }
 
+bool ValidateSemantic::FileIsHeader(const std::string& name)
+{
+  return RteFsUtils::FileCategoryFromExtension(name) == "header";
+}
+
+
 /**
  * @brief Check device dependencies. Tests if all dependencies are solved and a minimum
  *        on support files and configuration has been defined
@@ -526,6 +532,7 @@ bool ValidateSemantic::CheckDeviceDependencies(RteDeviceItem *device, RteProject
 
         for(auto aggregate : startupComponents) {
           ErrLog::Get()->SetFileName(aggregate->GetPackage()->GetPackageFileName());
+          string targetPath = RteUtils::ExtractFilePath(aggregate->GetPackage()->GetPackageFileName(), false);
 
           for(auto &[componentKey, componentMap] : aggregate->GetAllComponents()) {
             int foundSystemC = 0, foundStartup = 0;
@@ -569,6 +576,15 @@ bool ValidateSemantic::CheckDeviceDependencies(RteDeviceItem *device, RteProject
                   }
                   const string& attribute = file->GetAttribute("attr");
 
+                  if(attribute == "config" && FileIsHeader(fileName)) {
+                    const string& fullFileName = targetPath + "/" + file->GetName();
+                    const string hPath = RteUtils::ExtractFilePath(fullFileName, false);
+                    const auto incPathFound = incPaths.find(hPath);
+                    if(incPathFound != incPaths.end()) {
+                      LogMsg("M357", NAME(file->GetName()), file->GetLineNumber());
+                    }
+                  }
+
                   if(FindName(fileName, "system_", ".c")) {
                     foundSystemC++;
                     lineSystem = file->GetLineNumber();

tools/packchk/test/data/ConfigFileInIncludePath/Files/header1.h

@@ -0,0 +1 @@
+//header 1

tools/packchk/test/data/ConfigFileInIncludePath/TestVendor.ConfigFileInIncludePathPack.pdsc

@@ -0,0 +1,66 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<package schemaVersion="1.4" xmlns:xs="http://www.w3.org/2001/XMLSchema-instance" xs:noNamespaceSchemaLocation="PACK.xsd">
+  <vendor>TestVendor</vendor>
+  <url>http://www.testurl.com/pack/</url>
+  <name>ConfigFileInIncludePathPack</name>
+  <description>Include Path vs Config File. Files that have attr=config in the *.pdsc are copied to the project. An include path is automatically generated.
+It is a potential risk to add an additional include path to original location in the *.pdsc file. This file does not contain configuration settings. 
+It depends on the search order of the compiler to pickup the right header file (as effectively the same header file name is twice in the search path).</description>
+
+  <releases>
+    <release version="0.0.1" date="2021-09-06">>
+      Initial release of ConfigFileInIncludePath.
+    </release>
+  </releases>
+
+  <keywords>
+    <keyword>ConfigFileInIncludePath</keyword>
+  </keywords>
+
+  <conditions>
+    <condition id="TestDevices">
+      <description>Test devices</description>
+      <require Dvendor="ARM:82" Dname="Test*"/>
+    </condition>
+    <condition id="Test ARMCC">
+      <description>filter for STM32F1xx Value Line Low Density Device and the ARM compiler</description>
+      <require condition="TestDevices"/>
+      <require Tcompiler="ARMCC"/>
+    </condition>
+  </conditions>
+
+  <devices>
+    <family Dfamily="TestFamily" Dvendor="ARM:82">
+      <algorithm name="Flash/ARMCMx_512.FLM"  start="0x00000000" size="0x00040000" default="1"/>
+
+      <!-- ******************************  TestSubFamily  ****************************** -->
+      <subFamily DsubFamily="TestSubFamily">
+        <compile header="Files/TestDevicesFoo.h"  define="TESTDEVICES"/>
+
+        <description>
+The TestSubFamily is ...
+        </description>
+
+        <!-- ******************************  TestDevice1  ***************************** -->
+        <device Dname="TestDevice1">
+          <processor Dcore="Cortex-M4"  DcoreVersion="r0p1" Dfpu="SP_FPU" Dmpu="MPU" Dendian="Little-endian" Dclock="204000000"/>
+          <memory  id="IROM1"                  start="0x00000000" size="0x00040000" default="1" startup="1"/>
+          <memory  id="IRAM1"                  start="0x10000000" size="0x00010000" default="1" init   ="0"/>
+          <memory  id="IRAM2"                  start="0x20000000" size="0x00010000" default="0" init   ="0"/>
+        </device>
+      </subFamily>
+    </family>
+  </devices>
+
+  <components>
+    <component Cclass="Device" Cgroup="Startup" Cversion="1.0.0" condition="TestDevices">
+      <description>System Startup for STMicroelectronics STM32F1xx device series</description>
+      <files>
+        <file category="include" name="Files/"/>
+        <file category="source"  name="Files/header1.h" attr="config" version="1.0.0" condition="Test ARMCC"/>
+      </files>
+    </component>
+  </components>
+
+</package>

tools/packchk/test/integtests/src/PackChkIntegTests.cpp

@@ -753,6 +753,36 @@ TEST_F(PackChkIntegTests, CheckDuplicateFlashAlgo) {
   }
 }
 
+// Validate invalid file path (file is directory)
+TEST_F(PackChkIntegTests, CheckConfigFileInIncludePath) {
+  const char* argv[3];
+
+  const string& pdscFile = PackChkIntegTestEnv::localtestdata_dir +
+    "/ConfigFileInIncludePath/TestVendor.ConfigFileInIncludePathPack.pdsc";
+  ASSERT_TRUE(RteFsUtils::Exists(pdscFile));
+
+  argv[0] = (char*)"";
+  argv[1] = (char*)pdscFile.c_str();
+  argv[2] = (char *)"--disable-validation";
+
+  PackChk packChk;
+  EXPECT_EQ(0, packChk.Check(3, argv, nullptr));
+
+  auto errMsgs = ErrLog::Get()->GetLogMessages();
+  int M357_foundCnt = 0;
+  for (const string& msg : errMsgs) {
+    size_t s;
+    if ((s = msg.find("M357")) != string::npos) {
+      M357_foundCnt++;
+    }
+  }
+
+  if (!M357_foundCnt) {
+    FAIL() << "error: missing message M357";
+  }
+}
+
+
 // Test schema validation
 TEST_F(PackChkIntegTests, CheckSchemaValidation) {
   const char* argv[3];