Improved addon execution errorhandling (#5451)

2023-09-20 10:40:57 +02:00 · 2023-09-20 10:40:57 +02:00 · a43b55a0ca
parent 58a7519cbb
commit a43b55a0ca
13 changed files with 427 additions and 100 deletions
--- a/addons/README.md
+++ b/addons/README.md
@ -10,6 +10,35 @@ Addons are scripts that analyses Cppcheck dump files to check compatibility with
  Checks Linux system for [year 2038 problem](https://en.wikipedia.org/wiki/Year_2038_problem) safety. This required [modified environment](https://github.com/3adev/y2038). See complete description [here](https://github.com/danmar/cppcheck/blob/main/addons/doc/y2038.txt).
 + [threadsafety.py](https://github.com/danmar/cppcheck/blob/main/addons/threadsafety.py) 
  Analyse Cppcheck dump files to locate threadsafety issues like static local objects used by multiple threads.
+ [naming.py](https://github.com/danmar/cppcheck/blob/main/addons/naming.py)
+  Enforces naming conventions across the code.
+ [namingng.py](https://github.com/danmar/cppcheck/blob/main/addons/namingng.py)
+  Enforces naming conventions across the code. Enhanced version with support for type prefixes in variable and function names.
+ [findcasts.py](https://github.com/danmar/cppcheck/blob/main/addons/findcasts.py)
+  Locates casts in the code.
+ [misc.py](https://github.com/danmar/cppcheck/blob/main/addons/misc.py)
+  Performs miscellaneous checks.
+
+### Other files
+
+- doc
+  Additional files for documentation generation.
+- tests
+  Contains various unit tests for the addons.
+- cppcheck.py
+  Internal helper used by Cppcheck binary to run the addons.
+- cppcheckdata.doxyfile
+  Configuration file for documentation generation.
+- cppcheckdata.py
+  Helper class for reading Cppcheck dump files within an addon.
+- misra_9.py
+  Implementation of the MISRA 9.x rules used by `misra` addon. 
+- naming.json
+  Example configuration for `namingng` addon. 
+- ROS_naming.json
+  Example configuration for the `namingng` addon enforcing the [ROS naming convention for C++ ](http://wiki.ros.org/CppStyleGuide#Files).
+- runaddon.py
+  Internal helper used by Cppcheck binary to run the addons.

 ## Usage

--- a/cli/cppcheckexecutor.cpp
+++ b/cli/cppcheckexecutor.cpp
@ -540,7 +540,7 @@ bool CppCheckExecutor::tryLoadLibrary(Library& destination, const std::string& b
 */
 // cppcheck-suppress passedByValue - used as callback so we need to preserve the signature
 // NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature
-bool CppCheckExecutor::executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output_)
+int CppCheckExecutor::executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output_)
 {
    output_.clear();

@ -567,9 +567,12 @@ bool CppCheckExecutor::executeCommand(std::string exe, std::vector<std::string>
 #else
    FILE *p = popen(cmd.c_str(), "r");
 #endif
+    //std::cout << "invoking command '" << cmd << "'" << std::endl;
    if (!p) {
-        // TODO: read errno
-        return false;
+        // TODO: how to provide to caller?
+        //const int err = errno;
+        //std::cout << "popen() errno " << std::to_string(err) << std::endl;
+        return -1;
    }
    char buffer[1024];
    while (fgets(buffer, sizeof(buffer), p) != nullptr)
@ -581,13 +584,19 @@ bool CppCheckExecutor::executeCommand(std::string exe, std::vector<std::string>
    const int res = pclose(p);
 #endif
    if (res == -1) { // error occured
-        // TODO: read errno
-        return false;
+        // TODO: how to provide to caller?
+        //const int err = errno;
+        //std::cout << "pclose() errno " << std::to_string(err) << std::endl;
+        return res;
    }
-    if (res != 0) { // process failed
-        // TODO: need to get error details
-        return false;
+#if !defined(WIN32) && !defined(__MINGW32__)
+    if (WIFEXITED(res)) {
+        return WEXITSTATUS(res);
    }
-    return true;
+    if (WIFSIGNALED(res)) {
+        return WTERMSIG(res);
+    }
+#endif
+    return res;
 }

--- a/cli/cppcheckexecutor.h
+++ b/cli/cppcheckexecutor.h
@ -99,9 +99,9 @@ public:
    static bool tryLoadLibrary(Library& destination, const std::string& basepath, const char* filename);

    /**
-     * Execute a shell command and read the output from it. Returns true if command terminated successfully.
+     * Execute a shell command and read the output from it. Returns exitcode of the executed command,.
     */
-    static bool executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output_);
+    static int executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output_);

 protected:

--- a/gui/checkthread.cpp
+++ b/gui/checkthread.cpp
@ -57,7 +57,7 @@
 #endif

 // NOLINTNEXTLINE(performance-unnecessary-value-param) - used as callback so we need to preserve the signature
-static bool executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output) // cppcheck-suppress passedByValue
+static int executeCommand(std::string exe, std::vector<std::string> args, std::string redirect, std::string &output) // cppcheck-suppress passedByValue
 {
    output.clear();

@ -80,7 +80,7 @@ static bool executeCommand(std::string exe, std::vector<std::string> args, std::
        std::ofstream fout(redirect.substr(3));
        fout << process.readAllStandardError().toStdString();
    }
-    return process.exitCode() == 0;
+    return process.exitCode();
 }


--- a/lib/cppcheck.cpp
+++ b/lib/cppcheck.cpp
@ -180,16 +180,16 @@ namespace {

        std::string getAddonInfo(const std::string &fileName, const std::string &exename) {
            if (fileName[0] == '{') {
-                std::istringstream in(fileName);
                picojson::value json;
-                in >> json;
+                const std::string err = picojson::parse(json, fileName);
+                (void)err; // TODO: report
                return parseAddonInfo(json, fileName, exename);
            }
            if (fileName.find('.') == std::string::npos)
                return getAddonInfo(fileName + ".py", exename);

            if (endsWith(fileName, ".py")) {
-                scriptFile = getFullPath(fileName, exename);
+                scriptFile = Path::fromNativeSeparators(getFullPath(fileName, exename));
                if (scriptFile.empty())
                    return "Did not find addon " + fileName;

@ -328,11 +328,35 @@ static void createDumpFile(const Settings& settings,
          << "/>" << '\n';
 }

-static std::string executeAddon(const AddonInfo &addonInfo,
-                                const std::string &defaultPythonExe,
-                                const std::string &file,
-                                const std::string &premiumArgs,
-                                const std::function<bool(std::string,std::vector<std::string>,std::string,std::string&)> &executeCommand)
+static std::string detectPython(const CppCheck::ExecuteCmdFn &executeCommand)
+{
+#ifdef _WIN32
+    const char *py_exes[] = { "python3.exe", "python.exe" };
+#else
+    const char *py_exes[] = { "python3", "python" };
+#endif
+    for (const char* py_exe : py_exes) {
+        std::string out;
+#ifdef _MSC_VER
+        // FIXME: hack to avoid debug assertion with _popen() in executeCommand() for non-existing commands
+        const std::string cmd = std::string(py_exe) + " --version >NUL";
+        if (system(cmd.c_str()) != 0) {
+            // TODO: get more detailed error?
+            break;
+        }
+#endif
+        if (executeCommand(py_exe, split("--version"), "2>&1", out) == EXIT_SUCCESS && startsWith(out, "Python ") && std::isdigit(out[7])) {
+            return py_exe;
+        }
+    }
+    return "";
+}
+
+static std::vector<picojson::value> executeAddon(const AddonInfo &addonInfo,
+                                                 const std::string &defaultPythonExe,
+                                                 const std::string &file,
+                                                 const std::string &premiumArgs,
+                                                 const CppCheck::ExecuteCmdFn &executeCommand)
 {
    const std::string redirect = "2>&1";

@ -345,28 +369,11 @@ static std::string executeAddon(const AddonInfo &addonInfo,
    else if (!defaultPythonExe.empty())
        pythonExe = cmdFileName(defaultPythonExe);
    else {
-#ifdef _WIN32
-        const char *py_exes[] = { "python3.exe", "python.exe" };
-#else
-        const char *py_exes[] = { "python3", "python" };
-#endif
-        for (const char* py_exe : py_exes) {
-            std::string out;
-#ifdef _MSC_VER
-            // FIXME: hack to avoid debug assertion with _popen() in executeCommand() for non-existing commands
-            const std::string cmd = std::string(py_exe) + " --version >NUL";
-            if (system(cmd.c_str()) != 0) {
-                // TODO: get more detailed error?
-                break;
-            }
-#endif
-            if (executeCommand(py_exe, split("--version"), redirect, out) && startsWith(out, "Python ") && std::isdigit(out[7])) {
-                pythonExe = py_exe;
-                break;
-            }
-        }
-        if (pythonExe.empty())
+        // store in static variable so we only look this up once
+        static const std::string detectedPythonExe = detectPython(executeCommand);
+        if (detectedPythonExe.empty())
            throw InternalError(nullptr, "Failed to auto detect python");
+        pythonExe = detectedPythonExe;
    }

    std::string args;
@ -380,27 +387,62 @@ static std::string executeAddon(const AddonInfo &addonInfo,
    args += fileArg;

    std::string result;
-    if (!executeCommand(pythonExe, split(args), redirect, result)) {
-        std::string message("Failed to execute addon '" + addonInfo.name + "' (command: '" + pythonExe + " " + args + "'). Exitcode is nonzero.");
+    if (const int exitcode = executeCommand(pythonExe, split(args), redirect, result)) {
+        std::string message("Failed to execute addon '" + addonInfo.name + "' - exitcode is " + std::to_string(exitcode));
+        std::string details = pythonExe + " " + args;
        if (result.size() > 2) {
-            message = message + "\n" + message + "\nOutput:\n" + result;
-            message.resize(message.find_last_not_of("\n\r"));
+            details += "\nOutput:\n";
+            details += result;
+            const auto pos = details.find_last_not_of("\n\r");
+            if (pos != std::string::npos)
+                details.resize(pos + 1);
        }
-        throw InternalError(nullptr, message);
+        throw InternalError(nullptr, message, details);
    }

+    std::vector<picojson::value> addonResult;
+
    // Validate output..
    std::istringstream istr(result);
    std::string line;
    while (std::getline(istr, line)) {
-        if (!startsWith(line,"Checking ") && !line.empty() && line[0] != '{') {
+        // TODO: also bail out?
+        if (line.empty()) {
+            //std::cout << "addon '" << addonInfo.name <<  "' result contains empty line" << std::endl;
+            continue;
+        }
+
+        // TODO: get rid of this
+        if (startsWith(line,"Checking ")) {
+            //std::cout << "addon '" << addonInfo.name <<  "' result contains 'Checking ' line" << std::endl;
+            continue;
+        }
+
+        if (line[0] != '{') {
+            //std::cout << "addon '" << addonInfo.name <<  "' result is not a JSON" << std::endl;
+
            result.erase(result.find_last_not_of('\n') + 1, std::string::npos); // Remove trailing newlines
            throw InternalError(nullptr, "Failed to execute '" + pythonExe + " " + args + "'. " + result);
        }
+
+        //std::cout << "addon '" << addonInfo.name <<  "' result is " << line << std::endl;
+
+        // TODO: make these failures?
+        picojson::value res;
+        const std::string err = picojson::parse(res, line);
+        if (!err.empty()) {
+            //std::cout << "addon '" << addonInfo.name <<  "' result is not a valid JSON (" << err << ")" << std::endl;
+            continue;
+        }
+        if (!res.is<picojson::object>()) {
+            //std::cout << "addon '" << addonInfo.name <<  "' result is not a JSON object" << std::endl;
+            continue;
+        }
+        addonResult.emplace_back(std::move(res));
    }

    // Valid results
-    return result;
+    return addonResult;
 }

 static std::string getDefinesFlags(const std::string &semicolonSeparatedString)
@ -413,7 +455,7 @@ static std::string getDefinesFlags(const std::string &semicolonSeparatedString)

 CppCheck::CppCheck(ErrorLogger &errorLogger,
                   bool useGlobalSuppressions,
-                   std::function<bool(std::string,std::vector<std::string>,std::string,std::string&)> executeCommand)
+                   ExecuteCmdFn executeCommand)
    : mErrorLogger(errorLogger)
    , mUseGlobalSuppressions(useGlobalSuppressions)
    , mExecuteCommand(std::move(executeCommand))
@ -532,7 +574,7 @@ unsigned int CppCheck::check(const std::string &path)
        }

        std::string output2;
-        if (!mExecuteCommand(exe,split(args2),redirect2,output2) || output2.find("TranslationUnitDecl") == std::string::npos) {
+        if (mExecuteCommand(exe,split(args2),redirect2,output2) != EXIT_SUCCESS || output2.find("TranslationUnitDecl") == std::string::npos) {
            std::cerr << "Failed to execute '" << exe << " " << args2 << " " << redirect2 << "'" << std::endl;
            return 0;
        }
@ -596,7 +638,8 @@ unsigned int CppCheck::check(const std::string &path)
            executeAddons(dumpFile);

        } catch (const InternalError &e) {
-            internalError(path, "Processing Clang AST dump failed: " + e.errorMessage);
+            const ErrorMessage errmsg = ErrorMessage::fromInternalError(e, nullptr, path, "Bailing out from analysis: Processing Clang AST dump failed");
+            reportErr(errmsg);
        } catch (const TerminateException &) {
            // Analysis is terminated
            return mExitCode;
@ -1022,7 +1065,8 @@ unsigned int CppCheck::checkFile(const std::string& filename, const std::string
    } catch (const std::bad_alloc &) {
        internalError(filename, "Checking file failed: out of memory");
    } catch (const InternalError &e) {
-        internalError(filename, "Checking file failed: " + e.errorMessage);
+        const ErrorMessage errmsg = ErrorMessage::fromInternalError(e, nullptr, filename, "Bailing out from analysis: Checking file failed");
+        reportErr(errmsg);
    }

    if (!mSettings.buildDir.empty()) {
@ -1448,23 +1492,14 @@ void CppCheck::executeAddons(const std::vector<std::string>& files)
        if (addonInfo.name != "misra" && !addonInfo.ctu && endsWith(files.back(), ".ctu-info"))
            continue;

-        const std::string results =
+        const std::vector<picojson::value> results =
            executeAddon(addonInfo, mSettings.addonPython, fileList.empty() ? files[0] : fileList, mSettings.premiumArgs, mExecuteCommand);
-        std::istringstream istr(results);
-        std::string line;

        const bool misraC2023 = mSettings.premiumArgs.find("--misra-c-2023") != std::string::npos;

-        while (std::getline(istr, line)) {
-            if (!startsWith(line,"{"))
-                continue;
-
-            picojson::value res;
-            std::istringstream istr2(line);
-            istr2 >> res;
-            if (!res.is<picojson::object>())
-                continue;
-
+        for (const picojson::value& res : results) {
+            // TODO: get rid of copy?
+            // this is a copy so we can access missing fields and get a default value
            picojson::object obj = res.get<picojson::object>();

            ErrorMessage errmsg;
@ -1519,7 +1554,8 @@ void CppCheck::executeAddonsWholeProgram(const std::map<std::string, std::size_t
    try {
        executeAddons(ctuInfoFiles);
    } catch (const InternalError& e) {
-        internalError("", "Whole program analysis failed: " + e.errorMessage);
+        const ErrorMessage errmsg = ErrorMessage::fromInternalError(e, nullptr, "", "Bailing out from analysis: Whole program analysis failed");
+        reportErr(errmsg);
    }

    if (mSettings.buildDir.empty()) {
@ -1687,8 +1723,8 @@ void CppCheck::analyseClangTidy(const ImportProject::FileSettings &fileSettings)

    const std::string args = "-quiet -checks=*,-clang-analyzer-*,-llvm* \"" + fileSettings.filename + "\" -- " + allIncludes + allDefines;
    std::string output;
-    if (!mExecuteCommand(exe, split(args), emptyString, output)) {
-        std::cerr << "Failed to execute '" << exe << "'" << std::endl;
+    if (const int exitcode = mExecuteCommand(exe, split(args), emptyString, output)) {
+        std::cerr << "Failed to execute '" << exe << "' (exitcode: " << std::to_string(exitcode) << ")" << std::endl;
        return;
    }

--- a/lib/cppcheck.h
+++ b/lib/cppcheck.h
@ -50,12 +50,14 @@ class Tokenizer;
 */
 class CPPCHECKLIB CppCheck : ErrorLogger {
 public:
+    using ExecuteCmdFn = std::function<int (std::string,std::vector<std::string>,std::string,std::string&)>;
+
    /**
     * @brief Constructor.
     */
    CppCheck(ErrorLogger &errorLogger,
             bool useGlobalSuppressions,
-             std::function<bool(std::string,std::vector<std::string>,std::string,std::string&)> executeCommand);
+             ExecuteCmdFn executeCommand);

    /**
     * @brief Destructor.
@ -230,7 +232,7 @@ private:
    AnalyzerInformation mAnalyzerInformation;

    /** Callback for executing a shell command (exe, args, output) */
-    std::function<bool(std::string,std::vector<std::string>,std::string,std::string&)> mExecuteCommand;
+    ExecuteCmdFn mExecuteCommand;

    std::ofstream mPlistFile;
 };
--- a/lib/errorlogger.cpp
+++ b/lib/errorlogger.cpp
@ -233,7 +233,7 @@ static void serializeString(std::string &oss, const std::string & str)
    oss += str;
 }

-ErrorMessage ErrorMessage::fromInternalError(const InternalError &internalError, const TokenList *tokenList, const std::string &filename)
+ErrorMessage ErrorMessage::fromInternalError(const InternalError &internalError, const TokenList *tokenList, const std::string &filename, const std::string& msg)
 {
    if (internalError.token)
        assert(tokenList != nullptr); // we need to make sure we can look up the provided token
@ -253,9 +253,12 @@ ErrorMessage ErrorMessage::fromInternalError(const InternalError &internalError,
    ErrorMessage errmsg(std::move(locationList),
                        tokenList ? tokenList->getSourceFilePath() : filename,
                        Severity::error,
-                        internalError.errorMessage,
+                        (msg.empty() ? "" : (msg + ": ")) + internalError.errorMessage,
                        internalError.id,
                        Certainty::normal);
+    // TODO: find a better way
+    if (!internalError.details.empty())
+        errmsg.mVerboseMessage = errmsg.mVerboseMessage + ": " + internalError.details;
    return errmsg;
 }

--- a/lib/errorlogger.h
+++ b/lib/errorlogger.h
@ -198,7 +198,7 @@ public:
        return mSymbolNames;
    }

-    static ErrorMessage fromInternalError(const InternalError &internalError, const TokenList *tokenList, const std::string &filename);
+    static ErrorMessage fromInternalError(const InternalError &internalError, const TokenList *tokenList, const std::string &filename, const std::string& msg = emptyString);

 private:
    static std::string fixInvalidChars(const std::string& raw);
--- a/lib/errortypes.cpp
+++ b/lib/errortypes.cpp
@ -18,31 +18,32 @@

 #include "errortypes.h"

-InternalError::InternalError(const Token *tok, std::string errorMsg, Type type) :
-    token(tok), errorMessage(std::move(errorMsg)), type(type)
+static std::string typeToString(InternalError::Type type)
 {
    switch (type) {
-    case AST:
-        id = "internalAstError";
-        break;
-    case SYNTAX:
-        id = "syntaxError";
-        break;
-    case UNKNOWN_MACRO:
-        id = "unknownMacro";
-        break;
-    case INTERNAL:
-        id = "cppcheckError";
-        break;
-    case LIMIT:
-        id = "cppcheckLimit";
-        break;
-    case INSTANTIATION:
-        id = "instantiationError";
-        break;
+    case InternalError::Type::AST:
+        return "internalAstError";
+    case InternalError::Type::SYNTAX:
+        return "syntaxError";
+    case InternalError::Type::UNKNOWN_MACRO:
+        return "unknownMacro";
+    case InternalError::Type::INTERNAL:
+        return "internalError";
+    case InternalError::Type::LIMIT:
+        return "cppcheckLimit";
+    case InternalError::Type::INSTANTIATION:
+        return "instantiationError";
    }
 }

+InternalError::InternalError(const Token *tok, std::string errorMsg, Type type) :
+    InternalError(tok, std::move(errorMsg), "", type)
+{}
+
+InternalError::InternalError(const Token *tok, std::string errorMsg, std::string details, Type type) :
+    token(tok), errorMessage(std::move(errorMsg)), details(std::move(details)), type(type), id(typeToString(type))
+{}
+
 std::string Severity::toString(Severity::SeverityType severity)
 {
    switch (severity) {
--- a/lib/errortypes.h
+++ b/lib/errortypes.h
@ -33,11 +33,15 @@
 class Token;

 /** @brief Simple container to be thrown when internal error is detected. */
-struct InternalError {
+struct CPPCHECKLIB InternalError {
    enum Type {AST, SYNTAX, UNKNOWN_MACRO, INTERNAL, LIMIT, INSTANTIATION};
+
    InternalError(const Token *tok, std::string errorMsg, Type type = INTERNAL);
+    InternalError(const Token *tok, std::string errorMsg, std::string details, Type type = INTERNAL);
+
    const Token *token;
    std::string errorMessage;
+    std::string details;
    Type type;
    std::string id;
 };
--- a/test/cli/test-other.py
+++ b/test/cli/test-other.py
@ -207,10 +207,8 @@ def test_execute_addon_failure_2(tmpdir):
    args = ['--addon=naming', '--addon-python=python5.x', test_file]

    _, _, stderr = cppcheck(args)
-    # /tmp/pytest-of-sshuser/pytest-215/test_execute_addon_failure_20/test.cpp:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'naming' (command: 'python5.x /mnt/s/GitHub/cppcheck-fw/addons/runaddon.py /mnt/s/GitHub/cppcheck-fw/addons/naming.py --cli /tmp/pytest-of-sshuser/pytest-215/test_execute_addon_failure_20/test.cpp.7306.dump'). Exitcode is nonzero. [internalError]\n\n^\n
-    # "C:\\Users\\Quotenjugendlicher\\AppData\\Local\\Temp\\pytest-of-Quotenjugendlicher\\pytest-15\\test_execute_addon_failure_20\\test.cpp:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon (command: 'python5.x S:\\GitHub\\cppcheck-fw\\bin\\debug\\addons\\runaddon.py S:\\GitHub\\cppcheck-fw\\bin\\debug\\addons\\naming.py --cli C:\\Users\\Quotenjugendlicher\\AppData\\Local\\Temp\\pytest-of-Quotenjugendlicher\\pytest-15\\test_execute_addon_failure_20\\test.cpp.9892.dump'). Exitcode is nonzero. [internalError]\n\n^\n
-    assert stderr.startswith('{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon \'naming\' (command: \'python5.x '.format(test_file))
-    assert stderr.endswith(' [internalError]\n\n^\n')
+    ec = 1 if os.name == 'nt' else 127
+    assert stderr == "{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'naming' - exitcode is {} [internalError]\n\n^\n".format(test_file, ec)


 # TODO: find a test case which always fails
@ -232,3 +230,217 @@ void f() {

    _, _, stderr = cppcheck(args)
    assert stderr == '{}:0:0: error: Bailing from out analysis: Checking file failed: converting \'1f\' to integer failed - not an integer [internalError]\n\n^\n'.format(test_file)
+
+
+def test_addon_misra(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.cpp')
+    with open(test_file, 'wt') as f:
+        f.write("""
+typedef int MISRA_5_6_VIOLATION;
+        """)
+
+    args = ['--addon=misra', '--enable=all', test_file]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file)
+    ]
+    assert stderr == '{}:2:1: style: misra violation (use --rule-texts=<file> to get proper output) [misra-c2012-2.3]\ntypedef int MISRA_5_6_VIOLATION;\n^\n'.format(test_file)
+
+
+def test_addon_y2038(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.cpp')
+    # TODO: trigger warning
+    with open(test_file, 'wt') as f:
+        f.write("""
+typedef int MISRA_5_6_VIOLATION;
+        """)
+
+    args = ['--addon=y2038', '--enable=all', test_file]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file)
+    ]
+    assert stderr == ''
+
+
+def test_addon_threadsafety(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.cpp')
+    # TODO: trigger warning
+    with open(test_file, 'wt') as f:
+        f.write("""
+typedef int MISRA_5_6_VIOLATION;
+        """)
+
+    args = ['--addon=y2038', '--enable=all', test_file]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file)
+    ]
+    assert stderr == ''
+
+
+def test_addon_naming(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.cpp')
+    # TODO: trigger warning
+    with open(test_file, 'wt') as f:
+        f.write("""
+typedef int MISRA_5_6_VIOLATION;
+        """)
+
+    args = ['--addon=y2038', '--enable=all', test_file]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file)
+    ]
+    assert stderr == ''
+
+
+def test_addon_namingng(tmpdir):
+    test_file = os.path.join(tmpdir, 'test.cpp')
+    # TODO: trigger warning
+    with open(test_file, 'wt') as f:
+        f.write("""
+typedef int MISRA_5_6_VIOLATION;
+        """)
+
+    args = ['--addon=y2038', '--enable=all', test_file]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file)
+    ]
+    assert stderr == ''
+
+
+def test_invalid_addon_json(tmpdir):
+    addon_file = os.path.join(tmpdir, 'addon1.json')
+    with open(addon_file, 'wt') as f:
+        f.write("""
+                """)
+
+    test_file = os.path.join(tmpdir, 'file.cpp')
+    with open(test_file, 'wt') as f:
+        f.write("""
+typedef int MISRA_5_6_VIOLATION;
+                """)
+
+    args = ['--addon={}'.format(addon_file), '--enable=all', test_file]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0  # TODO: needs to be 1
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file),
+        'Loading {} failed. syntax error at line 2 near: '.format(addon_file),
+        'Loading {} failed. syntax error at line 2 near: '.format(addon_file)
+    ]
+    assert stderr == ''
+
+
+def test_invalid_addon_py(tmpdir):
+    addon_file = os.path.join(tmpdir, 'addon1.py')
+    with open(addon_file, 'wt') as f:
+        f.write("""
+raise Exception()
+                """)
+
+    test_file = os.path.join(tmpdir, 'file.cpp')
+    with open(test_file, 'wt') as f:
+        f.write("""
+typedef int MISRA_5_6_VIOLATION;
+                """)
+
+    args = ['--addon={}'.format(addon_file), '--enable=all', test_file]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0  # TODO: needs to be 1
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file)
+    ]
+    assert stderr == "{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 1 [internalError]\n\n^\n".format(test_file)
+
+
+def test_invalid_addon_py_verbose(tmpdir):
+    addon_file = os.path.join(tmpdir, 'addon1.py')
+    with open(addon_file, 'wt') as f:
+        f.write("""
+raise Exception()
+                """)
+
+    test_file = os.path.join(tmpdir, 'file.cpp')
+    with open(test_file, 'wt') as f:
+        f.write("""
+typedef int MISRA_5_6_VIOLATION;
+                """)
+
+    args = ['--addon={}'.format(addon_file), '--enable=all', '--verbose', test_file]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0  # TODO: needs to be 1
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file),
+        'Defines:',
+        'Undefines:',
+        'Includes:',
+        'Platform:native'
+    ]
+    """
+/tmp/pytest-of-user/pytest-11/test_invalid_addon_py_20/file.cpp:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 1: python3 /home/user/CLionProjects/cppcheck/addons/runaddon.py /tmp/pytest-of-user/pytest-11/test_invalid_addon_py_20/addon1.py --cli /tmp/pytest-of-user/pytest-11/test_invalid_addon_py_20/file.cpp.24762.dump
+Output:
+Traceback (most recent call last):
+  File "/home/user/CLionProjects/cppcheck/addons/runaddon.py", line 8, in <module>
+    runpy.run_path(addon, run_name='__main__')
+  File "<frozen runpy>", line 291, in run_path
+  File "<frozen runpy>", line 98, in _run_module_code
+  File "<frozen runpy>", line 88, in _run_code
+  File "/tmp/pytest-of-user/pytest-11/test_invalid_addon_py_20/addon1.py", line 2, in <module>
+    raise Exception()
+Exceptio [internalError]
+    """
+    # /tmp/pytest-of-user/pytest-10/test_invalid_addon_py_20/file.cpp:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 256.: python3 /home/user/CLionProjects/cppcheck/addons/runaddon.py /tmp/pytest-of-user/pytest-10/test_invalid_addon_py_20/addon1.py --cli /tmp/pytest-of-user/pytest-10/test_invalid_addon_py_20/file.cpp.24637.dump
+    assert stderr.startswith("{}:0:0: error: Bailing out from analysis: Checking file failed: Failed to execute addon 'addon1' - exitcode is 1: ".format(test_file))
+    assert stderr.count('Output:\nTraceback')
+    assert stderr.endswith('raise Exception()\nException [internalError]\n\n^\n')
+
+
+def test_addon_result(tmpdir):
+    addon_file = os.path.join(tmpdir, 'addon1.py')
+    with open(addon_file, 'wt') as f:
+        f.write("""
+print("Checking ...")
+print("")
+print('{"file": "test.cpp", "linenr": 1, "column": 1, "severity": "style", "message": "msg", "addon": "addon1", "errorId": "id", "extra": ""}')
+print('{"loc": [{"file": "test.cpp", "linenr": 1, "column": 1, "info": ""}], "severity": "style", "message": "msg", "addon": "addon1", "errorId": "id", "extra": ""}')
+                """)
+
+    test_file = os.path.join(tmpdir, 'file.cpp')
+    with open(test_file, 'wt') as f:
+        f.write("""
+typedef int MISRA_5_6_VIOLATION;
+                """)
+
+    args = ['--addon={}'.format(addon_file), '--enable=all', test_file]
+
+    exitcode, stdout, stderr = cppcheck(args)
+    assert exitcode == 0  # TODO: needs to be 1
+    lines = stdout.splitlines()
+    assert lines == [
+        'Checking {} ...'.format(test_file)
+    ]
+    assert stderr == 'test.cpp:1:1: style: msg [addon1-id]\n\n^\n'
--- a/test/testerrorlogger.cpp
+++ b/test/testerrorlogger.cpp
@ -44,6 +44,7 @@ private:
        TEST_CASE(ErrorMessageConstructLocations);
        TEST_CASE(ErrorMessageVerbose);
        TEST_CASE(ErrorMessageVerboseLocations);
+        TEST_CASE(ErrorMessageFromInternalError);
        TEST_CASE(CustomFormat);
        TEST_CASE(CustomFormat2);
        TEST_CASE(CustomFormatLocations);
@ -153,6 +154,36 @@ private:
        ASSERT_EQUALS("[foo.cpp:5] -> [bar.cpp:8]: (error) Verbose error", msg.toString(true));
    }

+    void ErrorMessageFromInternalError() const {
+        // TODO: test with token
+        {
+            InternalError internalError(nullptr, "message", InternalError::INTERNAL);
+            const auto msg = ErrorMessage::fromInternalError(internalError, nullptr, "file.c");
+            ASSERT_EQUALS(1, msg.callStack.size());
+            const auto &loc = *msg.callStack.cbegin();
+            ASSERT_EQUALS(0, loc.fileIndex);
+            ASSERT_EQUALS(0, loc.line);
+            ASSERT_EQUALS(0, loc.column);
+            ASSERT_EQUALS("message", msg.shortMessage());
+            ASSERT_EQUALS("message", msg.verboseMessage());
+            ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(false));
+            ASSERT_EQUALS("[file.c:0]: (error) message", msg.toString(true));
+        }
+        {
+            InternalError internalError(nullptr, "message", "details", InternalError::INTERNAL);
+            const auto msg = ErrorMessage::fromInternalError(internalError, nullptr, "file.cpp", "msg");
+            ASSERT_EQUALS(1, msg.callStack.size());
+            const auto &loc = *msg.callStack.cbegin();
+            ASSERT_EQUALS(0, loc.fileIndex);
+            ASSERT_EQUALS(0, loc.line);
+            ASSERT_EQUALS(0, loc.column);
+            ASSERT_EQUALS("msg: message", msg.shortMessage());
+            ASSERT_EQUALS("msg: message: details", msg.verboseMessage());
+            ASSERT_EQUALS("[file.cpp:0]: (error) msg: message", msg.toString(false));
+            ASSERT_EQUALS("[file.cpp:0]: (error) msg: message: details", msg.toString(true));
+        }
+    }
+
    void CustomFormat() const {
        std::list<ErrorMessage::FileLocation> locs(1, fooCpp5);
        ErrorMessage msg(locs, emptyString, Severity::error, "Programming error.\nVerbose error", "errorId", Certainty::normal);
--- a/test/testsingleexecutor.cpp
+++ b/test/testsingleexecutor.cpp
@ -113,7 +113,7 @@ private:
            executeCommandCalled = true;
            exe = std::move(e);
            args = std::move(a);
-            return true;
+            return EXIT_SUCCESS;
        });
        cppcheck.settings() = settings;