Skip to content

donate-cpu.py: improved error detection#1766

Merged
danmar merged 4 commits into
cppcheck-opensource:masterfrom
firewave:daca-error
Mar 29, 2019
Merged

donate-cpu.py: improved error detection#1766
danmar merged 4 commits into
cppcheck-opensource:masterfrom
firewave:daca-error

Conversation

@firewave
Copy link
Copy Markdown
Collaborator

@firewave firewave commented Mar 28, 2019 •
edited
Loading

Currently only signal 11 (aka SIGSEGV aka Segmentation fault) is being handled as a crash. This adds handling for all signals and exitcodes which are not 0 as well as ThreadExecutor errors.

I have seen reproducible ThreadExecutor errors with fork() failing for php5 and another package I cannot remember. I haven't look into these, but these are obviously causing incomplete results.

Other signal should also be handled like signal 11 and produce stack trace. The most obvious would be SIGABRT. As the actual signal will be reflected in the negative elapsedTime value we can add this handling step by step.

firewave added 2 commits March 28, 2019 15:54
@firewave
threadexecutor.cpp: streamlined error messages
7fc3599
@firewave
donate-cpu.py: detect additional signals and exitcode != 0 as crash a…
19fc704 
…s well and (ab)use elapsedTime to make the errorcode visible in the output / also detect ThreadExecutor issues
amai2012
amai2012 reviewed Mar 28, 2019
View reviewed changes
Comment thread tools/donate-cpu.py
if returncode != 0:
print('Error!')
return returncode, '', '', returncode, options
if stderr.find('Internal error: Child process crashed with signal ') > 0:
Copy link
Copy Markdown
Collaborator

@amai2012 amai2012 Mar 28, 2019

Choose a reason for hiding this comment

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

Those greps require an English language environment?

Copy link
Copy Markdown
Collaborator Author

@firewave firewave Mar 28, 2019

Choose a reason for hiding this comment

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

It#s the same string, that was already used in the crash detection and is written exactly like this by cppcheck, so there should be locale issues.

@versat
Copy link
Copy Markdown
Collaborator

versat commented Mar 28, 2019

I have seen reproducible ThreadExecutor errors with fork() failing for php5 and another package I cannot remember.

That is interesting. We had issues with fork() under Cygwin and disabled fork() for Cygwin. Do you remember where these issues occurred? Was it Cygwin or Linux?

I haven't looked in detail at the changes but from what you write that all makes sense to me.

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Mar 28, 2019

Was it Cygwin or Linux?

It was under WSL. But the package which I forgot was fine with head, but had the thread issue with 1.87. I ran lots of other didn't see this at all.

@firewave firewave marked this pull request as ready for review March 28, 2019 20:00
@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Mar 28, 2019

Please do not merge this yet. I need to investigate those thread errors a bit more as I know see more of them.

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Mar 28, 2019

That was stupid...

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Mar 28, 2019

I am done with this now.

@danmar danmar merged commit 8d7d93a into cppcheck-opensource:master Mar 29, 2019
@versat
Copy link
Copy Markdown
Collaborator

versat commented Mar 29, 2019

There are packages that do not contain useful files for Cppcheck and the "temp" directory is simply empty (at least under Cygwin).
This results now in the output "Error!" which is a bit irritating:

Download package ftp://ftp.se.debian.org/debian/pool/main/h/haskell-focuslist/haskell-focuslist_0.1.0.1.orig.tar.gz
--2019-03-29 09:40:58--  ftp://ftp.se.debian.org/debian/pool/main/h/haskell-focuslist/haskell-focuslist_0.1.0.1.orig.tar.gz
           => ‘/home/user/cppcheck-donate-cpu-workfolder/temp.tgz’
Resolving ftp.se.debian.org (ftp.se.debian.org)... 194.71.11.173, 194.71.11.165, 2001:6b0:19::173, ...
Connecting to ftp.se.debian.org (ftp.se.debian.org)|194.71.11.173|:21... connected.
Logging in as anonymous ... Logged in!
==> SYST ... done.    ==> PWD ... done.
==> TYPE I ... done.  ==> CWD (1) /debian/pool/main/h/haskell-focuslist ... done.
==> SIZE haskell-focuslist_0.1.0.1.orig.tar.gz ... 14457
==> PASV ... done.    ==> RETR haskell-focuslist_0.1.0.1.orig.tar.gz ... done.
Length: 14457 (14K) (unauthoritative)

haskell-focuslist_0.1.0.1.orig.tar.gz                           100%[====================================================================================================================================================>]  14.12K  --.-KB/s    in 0.002s

2019-03-29 09:40:59 (6.29 MB/s) - ‘/home/user/cppcheck-donate-cpu-workfolder/temp.tgz’ saved [14457]

Unpacking..
Analyze..
nice cppcheck/cppcheck -j1 --library=posix --library=gnu -D__GNUC__ --check-library --inconclusive --enable=style,information --platform=unix64 --template=daca2 -rp=temp temp
cppcheck finished with 1
Error!
Analyze..
nice 1.87/cppcheck -j1 --library=posix --library=gnu -D__GNUC__ --check-library --inconclusive --enable=style,information --platform=unix64 --template=daca2 -rp=temp temp
cppcheck finished with 1
Error!
No results

It is not really a bug or so, but i first thought something really went wrong while Cppcheck simply returns with 1 if there is nothing to analyze.
Maybe we should not start analysis if "temp" is empty and just skip that useless package (with a short hint that there are no files to check)?

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Mar 29, 2019

This results now in the output "Error!" which is a bit irritating

This was explicitly detected as a Crash! before as it was looking for cppcheck: error: could not find or open any of the paths given. in the output and -1 was returned. As the returncode is positive the code which calls this function will no longer detect this as a crash now. But as there were no results it was never uploaded. That needs to be addressed.

@firewave
Copy link
Copy Markdown
Collaborator Author

firewave commented Mar 29, 2019

I already did some changes for this, but need a few more examples to be sure of the changes as well as the stale report :-)

@firewave firewave deleted the daca-error branch March 29, 2019 11:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@amai2012 amai2012 amai2012 left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@firewave @versat @amai2012 @danmar