From d651b6baf118c2ab10dc925b2847ec2a436b6fdd Mon Sep 17 00:00:00 2001
From: Michael Drake <mdrake.unique@gmail.com>
Date: Mon, 13 Feb 2023 19:54:21 +0000
Subject: [PATCH] dump: Fix concurrency problem with dump files (#4757)

* dump: Fix concurrency problem with dump files

This adds the process ID for the cppcheck process to the filenames
of the .dump and .ctu-info files that the process generates.

So
  lib/cppcheck.cpp.dump
becomes
  lib/cppcheck.cpp.<PID>.dump

For example:
  lib/cppcheck.cpp.2637871.dump

The reason for this change is that if there is a buildsystem which
supports concurrency, multiple instances of cppcheck may be run for
the same file.  For example, if the same file is compiled in multiple
build variants, or for multiple targets.

If running the MISRA plugin over such a project with concurrency
enabled in the buildsystem, the plugin ends up crashing as multiple
jobs attempt to create/trample/delete the same files while other
jobs are using them.

For more information see:
    https://sourceforge.net/p/cppcheck/discussion/general/thread/02c757b4af/

* dump: Include pid in filename if dump not explicit

Only change the dump and ctu-info filenames to include the PID if
they are being generated due to an addon.

This means that existing scripts that use `--dump` will still work
if they depend on the previous naming behaviour. The more robust
filenames containing the pid will be used when the dump files are
used as an internal implementation detail for passing data to addons.

However this means that anything that does explicitly use `--dump`
will be susceptible to concurrency problems.

* test: Update addon dump file test to account for pid

This test causes a dump file to be created by enabling the misra
addon. Since the dump files now include the cppcheck process pid
this test had to be updated to account for the change.
---
 lib/cppcheck.cpp            | 26 ++++++++++++++++++++++++--
 test/cli/test-helloworld.py |  8 +++++++-
 2 files changed, 31 insertions(+), 3 deletions(-)

diff --git a/lib/cppcheck.cpp b/lib/cppcheck.cpp
index 55749b9d8..dae0974f5 100644
--- a/lib/cppcheck.cpp
+++ b/lib/cppcheck.cpp
@@ -56,6 +56,12 @@
 #include <utility>
 #include <vector>
 
+#ifndef _WIN32
+#include <unistd.h>
+#else
+#include <process.h>
+#endif
+
 #define PICOJSON_USE_INT64
 #include <picojson.h>
 
@@ -229,13 +235,29 @@ static std::vector<std::string> split(const std::string &str, const std::string
     return ret;
 }
 
+static int getPid()
+{
+#ifndef _WIN32
+    return getpid();
+#else
+    return _getpid();
+#endif
+}
+
 static std::string getDumpFileName(const Settings& settings, const std::string& filename)
 {
     if (!settings.dumpFile.empty())
         return settings.dumpFile;
+
+    std::string extension;
+    if (settings.dump)
+        extension = ".dump";
+    else
+        extension = "." + std::to_string(getPid()) + ".dump";
+
     if (!settings.dump && !settings.buildDir.empty())
-        return AnalyzerInformation::getAnalyzerInfoFile(settings.buildDir, filename, emptyString) + ".dump";
-    return filename + ".dump";
+        return AnalyzerInformation::getAnalyzerInfoFile(settings.buildDir, filename, emptyString) + extension;
+    return filename + extension;
 }
 
 static std::string getCtuInfoFileName(const std::string &dumpFile)
diff --git a/test/cli/test-helloworld.py b/test/cli/test-helloworld.py
index bb9af0f3c..0090cfda3 100644
--- a/test/cli/test-helloworld.py
+++ b/test/cli/test-helloworld.py
@@ -5,6 +5,7 @@ import os
 import re
 import tempfile
 import pytest
+import glob
 
 from testutils import create_gui_project_file, cppcheck
 
@@ -187,7 +188,12 @@ def test_build_dir_dump_output():
 
         cppcheck(args.split())
         cppcheck(args.split())
-        with open(f'{tempdir}/main.a1.dump', 'rt') as f:
+
+        filename = f'{tempdir}/main.a1.*.dump'
+        filelist = glob.glob(filename)
+        assert(len(filelist) == 1)
+
+        with open(filelist[0], 'rt') as f:
             dump = f.read()
             assert '</dump>' in dump, 'invalid dump data: ...' + dump[-100:]