Running python addons - fix

[John Coffey]

I've been having lots of problems trying to get the misra C warnings from the cppcheck gui. Actually this affects all the python addons. I found the source of the error (when running just misra checks) and am including it here in the hopes that someone can fix it. I'm a but rusty on how to create and submit requests but here is the fix for those interested.

Note that when I built the GUI I needed to modify the visual studio solution settings to add the following defines to the preprocessor flags section.

WITH_QCHART;USE_MATCHCOMPILER;HAVE_RULES;

Lastly I think there may be an issue when the path to my misra text file:

"c:\tools\cppcheck\misra\MISRA C_2012 - Appendix A.txt"

I think the problem rooted in the split function where the string is not properly escaped or something. It crashed cppcheck.

John

cppcheck.cpp line 221...

const std::string args = cmdFileName(addonInfo.scriptFile) + " --cli" + addonInfo.args + " " + cmdFileName(dumpFile);
std::string result;
// @JC executeCommand returns 'true' when successful
if (/*@JC !*/executeCommand(pythonExe, split(args), redirect, &result))
     throw InternalError(nullptr, "Failed to execute addon (command: '" + pythonExe + " " + args + "')");

// Validate output..
std::istringstream istr(result);
std::string line;

Edit: The above worked when I just used the misra test in the project addons coupled with a specific path to c:\tools\python\python3.exe. When I removed python from the settings (defaulting to python in the path) and when I turned the other settings on (Cert and 2038y) they stopped working. Looks like the misra.py script is returning the opposite for success compared to the other addIns (and therefore it throws the internal error which is silently ignored)

[John Coffey]

After debugging things a bit further - here is the fix I ended up with. The comments should be self explanatory - basically misra.py returns 'false' for success while the other addons return 'true' for a successful return from the script. Ideally the misra.py should be fixed - but I an not familiar with how python scripts work. Here is the updated block of code in cppcheck.cpp while I await a fix to the misra script.

John

const std::string args = cmdFileName(addonInfo.scriptFile) + " --cli" + addonInfo.args + " " + cmdFileName(dumpFile);
std::string result;

// @TODO Logic seems to be reversed for 'misra.py' - this is a hack workaround
// @TODO until someone who knows the 'misra.py' fixes the return code from
// @TODO the script.
if (addonInfo.scriptFile.find("misra.py") != std::string::npos) {
    if (executeCommand(pythonExe, split(args), redirect, &result)) {
        throw InternalError(nullptr, "Failed to execute addon (command: '" + pythonExe + " " + args + "')");
    }
} else { // this is the original way the code works
    if (!executeCommand(pythonExe, split(args), redirect, &result))
        throw InternalError(nullptr, "Failed to execute addon (command: '" + pythonExe + " " + args + "')");
}

// Validate output..
std::istringstream istr(result);
std::string line;

[Daniel Marjamäki]

Thanks! I agree misra.py should be changed then. As far as I see it's normal behavior to return 0 for success.

[John Coffey]

Daniel, for some unknown reason, the only way I can get misra and the other addins to work is to special case the misra return. BTW I can also confirm that having spaces in the misra text file path causes a crash. There need to be quotes placed around the parameter when it is taken out of the json settings and passed to cppcheck.exe process.

[Richard Smith]

POSIX exit codes are 0 for success and anything else for some sort of error. misra.py shoud not be change but rather all the other addons need to match misra.py

[Daniel Marjamäki]

Yes. Standard behavior is to return 0 for success.

[Daniel Marjamäki]

I am just a bit unsure when it is "success". when it's executed without exceptions? when no errors are found?

[Daniel Marjamäki]

I created ticket https://trac.cppcheck.net/ticket/9830 so this will not be forgotten.

[Richard Smith]

That's a good point.

In our usage we run the addon directly via CMake so if if it finds any violation then it returns non-success and the CMake rule fails thus triggering a CI failure. If success is defined as no-exeptions then there will have to be some other method of determining that violations exist. That will require much more complex CMake rules.
My vote would be for 0 to mean no exceptions and no violations were found and then differientate the exceptions from violations via differnt return codes. This allows CMake to continue working normally.

[Daniel Marjamäki]

For me it's in theory fine to return non-zero if violations are found by default. however I imagine that often addons have various violations that the user suppress/ignore. In misra there is some handling for that but the other addons just writes everything they find as far as I remember. If --addon is used then cppcheck itself will suppress the unwanted violations and it would be good if it returns with SUCCESS etc.

[Daniel Marjamäki]

I am not sure if a shell is required to handle return code 2,3,4,... so I don't know if it would be 100% safe to return such codes. Using the macros EXIT_SUCCESS / EXIT_FAILURE should be safe.

[Daniel Marjamäki]

how about:

if cppcheck executes an addon it uses argument --cli; if that argument is used then the addon will only fail if something abnormal happens (exceptions and stuff).

if --cli is not used then addons will fail if violations are found.

does this sound reasonable.

[Richard Smith]

Wrapping the return code with --cli is a good idea. That would preseve the existing behavior and then allow cppcheck to manage the return codes when run via --addon.

[Julien]

Hi, I tried to change misra.py main function return codes to try to fix this from your previous comments but with any return values I always get :
Bailing out from checking C:\Julien\myFile.c since there was an internal error: Failed to execute addon (command: 'python.exe "C:\Program Files\Cppcheck\addons\misra.py" --cli --rule-texts=C:/Julien/MISRA/misra_rules.txt C:\Julien\cppcheck-build-dir\myFile.a1.dump')
Using this same command from cmd line works well I can see MISRA check reports but the GUI is not reporting any MISRA errors.

I also tried to play with Python binary path on addons preferences :
"C:/Python27/python.exe" -> Bailing out error
default python from path (python 3.8) -> Bailing out error
* "C:/Python27/" -> No errors but also no MISRA reports

[Julien]

Update : After some investigation, I was able to identify the following cases

• From Cppcheck GUI : no MISRA errors

• From cmd line using project config : I get MISRA errors but does not find rules
  cppcheck --xml --addon=misra.json --project=test.cppcheck 2>MISRA_errors_GUI.xml

• From cmd line : I get MISRA errors with correct rules
  cppcheck --xml --addon=misra.json ..\*.c ..\*.h 2> MISRA_errors.xml

[Daniel Marjamäki]

I have updated the exit codes. However.. I did not see any obvious non-compliant plugins before, they seemed to return 0 on success before also.

[Julien]

Thank you for your updates, I will give it a try.
I just have to copy the cppcheck-gui.exe compiled by Qt in Cppcheck install directory ?

Please note that I still have to do
C:\Program Files\Cppcheck>cppcheckgui.exe --data-dir="C:\Program Files\Cppcheck"
after installing Cppcheck v2.1 to enable addons.
This was written as fixed here : https://sourceforge.net/p/cppcheck/discussion/general/thread/ccbe9e89/?page=1

[Daniel Marjamäki]

I tested it manually like this in Linux:

cppcheck --dump somefile.c
python3 addons/cert.py [--cli] somefile.c.dump ; echo $?

The echo $? prints the exit code, I guess that only works in Linux. maybe something like echo %ERRORLEVEL% can be used in DOS.

Using this same command from cmd line works well I can see MISRA check reports but the GUI is not reporting any MISRA errors.

For information, I doubt that problem is fixed by the updated exit code.

[Julien]

Ok, the python script is working for me using command line but the issue is with :
command line using .cppcheck project -> cppcheck detects MISRA errors but does not find misra rules
using GUI -> no MISRA checks are done
Looking at the GUI code I am not sure it currently implements MISRA checks. It seems there are only two addons supported now : CLANG_ANALYZER and CLANG_TIDY.

[Daniel Marjamäki]

Looking at the GUI code I am not sure it currently implements MISRA checks. It seems there are only two addons supported now : CLANG_ANALYZER and CLANG_TIDY.

I am not sure why it does not work for you . It works for me. Maybe there was a DATADIR problem also, I made another fix this evening to try to handle that better.
If you open the settings and look in the tab "Addons".. there you can configure the Python path and path to the misra rules file.. does it help if you configure those?

[Daniel Marjamäki]

If you are interested we have a nightly build for windows here: https://github.com/danmar/cppcheck/actions?query=workflow%3Arelease-windows

there will be a new build in a few hours.. I hope DATADIR will work better..

there are two types of artifacts you can download. The "installer" package contains a msi installation program. The "deploy" package contains the raw files.

[Daniel Marjamäki]

I can see that the cmd is not working well.. will look at that.

[Julien]

Sorry I did not seen your previous post I was stuck on code but I found some interesting things

[Julien]

Sorry for my previous post I was on the wrong addon functions. MISRA addon is implemented and functionnal on the GUI.

I found that the problem is from this line :
const std::string args = cmdFileName(addonInfo.scriptFile) + " --cli" + addonInfo.args + " " + cmdFileName(dumpFile);

On Windows Cppcheck is by default installed here : C:\Program Files\Cppcheck with a f..! space character (sorry I hate this folder because this is not the first time...) !

cmdFileName(addonInfo.scriptFile)
gives
"C:/Program Files/Cppcheck/addons/misra.py"and the addon is not executed.

Copying all python scripts directly on C: and replacing cmdFileName(addonInfo.scriptFile) by "C:/misra.py" fix the issue : I can now see MISRA errors on the GUI.

I reverted my modifications on cmdFileName(addonInfo.scriptFile) and tried to fix this better without moving files and found this :

On checkthread.cpp function executeCommand() I replaced this :

        args2 << QString::fromStdString(arg);

which gives on s2 : python.exe: can't open file '\"C:\\Program Files\\Cppcheck\\addons\\misra.py\"': [Errno 22] Invalid argument
by this :

if(QString::fromStdString(arg).contains("Program Files"))
           args2 << "C:\\Program Files\\Cppcheck\\addons\\misra.py";
else
        args2 << QString::fromStdString(arg);

And this fix the issue for now but there is probably a better fix.

[Daniel Marjamäki]

I am not sure how it should work in cmd. It seems unfortunate that you need to use --addon=misra.json. The quickest solution is to store python path and rule texts path (if configured) in the project file. I am not sure that they should be in the project file because those might be user dependent.