Skip to content

Fix #13440 (crash if there is exception in whole program analysis)#7113

Closed
danmar wants to merge 1 commit into
cppcheck-opensource:mainfrom
cppchecksolutions:fix-13440
Closed

Fix #13440 (crash if there is exception in whole program analysis)#7113
danmar wants to merge 1 commit into
cppcheck-opensource:mainfrom
cppchecksolutions:fix-13440

Conversation

@danmar
Copy link
Copy Markdown
Collaborator

@danmar danmar commented Dec 17, 2024

No description provided.

@danmar
Copy link
Copy Markdown
Collaborator Author

danmar commented Dec 17, 2024

@firewave any opinion about this? If we catch exception here then other addons can still be executed.

It feels like it could be a good idea to catch exceptions in CppCheck::analyseWholeProgram also but I can't reproduce any problems directly..

Comment thread lib/cppcheck.cpp
try {
results = executeAddon(addonInfo, mSettings.addonPython, fileList.empty() ? files[0] : fileList, mSettings.premiumArgs, mExecuteCommand);
} catch (const InternalError& e) {
const ErrorMessage errmsg = ErrorMessage::fromInternalError(e, nullptr, file0, "Bailing out from analysis: Addon '" + addonInfo.name + "' failed");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name we have in addonInfo.name looks like a placeholder

@firewave
Copy link
Copy Markdown
Collaborator

Judging from the failing tests it looks like this should be already handled and there are already tests for this. I will have a look later.

__test_checkclass_project(tmpdir, ['-j2', '--cppcheck-build-dir={}'.format(build_dir)])


def test_ctu_fail(tmpdir):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file only contain tests which relate to the whole-program folder so this should be added the other_test.py.

@firewave
Copy link
Copy Markdown
Collaborator

It feels like it could be a good idea to catch exceptions in CppCheck::analyseWholeProgram also but I can't reproduce any problems directly..

The whole exception handling in that area needs to be done a bit differently. But it is something which still needs some tests to be written. And I did not want to touch it until the common path works actually works as intended (see my current and upcoming analyzer information related PRs).

@danmar
Copy link
Copy Markdown
Collaborator Author

danmar commented Dec 17, 2024

@firewave nevermind I will have to take another look at this. please ignore it for now.

@firewave
Copy link
Copy Markdown
Collaborator

nevermind I will have to take another look at this. please ignore it for now.

I am looking at it right now because there are some gaps in the testing.

@danmar danmar marked this pull request as draft December 17, 2024 17:16
@danmar
Copy link
Copy Markdown
Collaborator Author

danmar commented Dec 17, 2024

I am looking at it right now because there are some gaps in the testing.

ok. thanks.

@firewave
Copy link
Copy Markdown
Collaborator

Ah - I missed the ctu option in the addon.

The change was fine and just needed some fine-tuning. I published #7114 as an alternative.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants