Cppcheck: Reduction of False Positives with a MSVC Project File

classic Classic list List threaded Threaded
26 messages Options
Next » 12
slacka slacka
Reply | Threaded
Open this post in threaded view
|

Cppcheck: Reduction of False Positives with a MSVC Project File

By using our MSVC Project file to teach cppcheck about includes and preprocessor configurations, I was able to reduce the number of warnings from 9,141 to 25.  You can see the Report here:


While the results are thin, are there any valid issues here?

Keep in mind, that this is only testing the Windows code path. Also, the 'LibreOffice.sln' project is incomplete and cannot yet generate full builds.

This was an interesting experiment. But until we have a fully functional project file, we'll have to manually generate the configuration for cppcheck.

I've left my notes for anyone else interested on working on this over here:




_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
sberg sberg
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives with a MSVC Project File

On 07/09/18 06:31, Luke Benes wrote:
> By using our MSVC Project file to teach cppcheck about includes and
> preprocessor configurations, I was able to reduce the number of warnings
> from 9,141 to 25.  You can see the Report here:
>
> https://drive.google.com/file/d/1NCV4Zl8vcWl30f6cDfTbGwxvW7lhMxw-/view?usp=sharing
>
> While the results are thin, are there any valid issues here?

Commenting on the referenced cppcheck_reports2/index.html:

 > C:\Program Files (x86)\Windows
Kits\10\Include\10.0.17134.0\shared\WTypesbase.h
 > 357 syntaxError error syntax error
 > C:\Program Files (x86)\Windows Kits\10\Include\10.0.17134.0\ucrt\malloc.h
 > 150 variableScope 398 style The scope of the variable '_Marker' can
be reduced.

The above two are in system code; nothing we can do about.

 >
D:\cygwin\home\Hearthstone\lode\dev\core\dbaccess\win32\source\odbcconfig\odbcconfig.cxx
 > 119 resourceLeak 775 error Resource leak: hModule
 > 123 resourceLeak 775 error Resource leak: hModule

The above two should indeed be fixed

 >
D:\cygwin\home\Hearthstone\lode\dev\core\framework\inc\helper\shareablemutex.hxx
 > 73 noExplicitConstructor 398 style Class 'ShareGuard' has a
constructor with 1 argument that is not explicit.

For all the nine noExplicitConstructor occurrences:  For one, what
puzzles me is Cppcheck's fixation on "1 argument" ctors, as "explicit"
has long surpassed being only relevant for ctors that can be called with
one argument; if it warns about these, why doesn't it warn about ctors
taking more arguments, too?  For another, for each individual ctor, they
may be arguments for and against it being "explicit".  Would need deeper
inspection, but generally feels on the level of rather unhelpful noise.

 >
D:\cygwin\home\Hearthstone\lode\dev\core\framework\source\fwi\helper\shareablemutex.cxx
 > 33 copyCtorPointerCopying 398 style Value of pointer 'm_pMutexRef',
which points to allocated memory, is copied in copy constructor instead
of allocating new memory.

The above is a false positive.

 > D:\cygwin\home\Hearthstone\lode\dev\core\include\osl\endian.h
 > 77 preprocessorErrorDirective error #error undetermined endianness

The two "preprocessorErrorDirective" are apparently fallout from
Cppcheck (not being called in a correct way and thus?) not being able to
determine which #if branch to take.

 > D:\cygwin\home\Hearthstone\lode\dev\core\include\osl\mutex.hxx
 > 120 noExplicitConstructor 398 style Class 'Guard' has a constructor
with 1 argument that is not explicit.
 > 127 noExplicitConstructor 398 style Class 'Guard' has a constructor
with 1 argument that is not explicit.
 > 154 noExplicitConstructor 398 style Class 'ClearableGuard' has a
constructor with 1 argument that is not explicit.
 > 161 noExplicitConstructor 398 style Class 'ClearableGuard' has a
constructor with 1 argument that is not explicit.
 > 200 noExplicitConstructor 398 style Class 'ResettableGuard' has a
constructor with 1 argument that is not explicit.
 > 207 noExplicitConstructor 398 style Class 'ResettableGuard' has a
constructor with 1 argument that is not explicit.
 > D:\cygwin\home\Hearthstone\lode\dev\core\include\rtl\byteseq.h
 > 199 noExplicitConstructor 398 style Class 'ByteSequence' has a
constructor with 1 argument that is not explicit.
 > 210 noExplicitConstructor 398 style Class 'ByteSequence' has a
constructor with 1 argument that is not explicit.

(see above)

 > D:\cygwin\home\Hearthstone\lode\dev\core\include\sal\alloca.h
 > 60 preprocessorErrorDirective error #error "unknown platform: please
check for alloca"

(see above)

 > D:\cygwin\home\Hearthstone\lode\dev\core\sal\osl\w32\dllentry.cxx
 > 224 ignoredReturnValue 252 warning Return value of function
CreateThread() is not used.
 > 224 leakReturnValNotUsed 771 error Return value of allocation
function 'CreateThread' is not stored.

 From the comments around the above two, they appear to be intentional.

 > D:\cygwin\home\Hearthstone\lode\dev\core\sal\rtl\byteseq.cxx
 > 43 variableScope 398 style The scope of the variable 'nElements' can
be reduced.

The above could indeed be fixed.

 > 82 incorrectStringBooleanError 571 warning Conversion of string
literal "### null ptr!" to bool always evaluates to true.
 > D:\cygwin\home\Hearthstone\lode\dev\core\sot\source\sdstor\stgavl.cxx
 > 137 incorrectStringBooleanError 571 warning Conversion of string
literal "The pointer is not allowed to be NULL!" to bool always
evaluates to true.
 > 149 incorrectStringBooleanError 571 warning Conversion of string
literal "The pointer is not allowed to be NULL!" to bool always
evaluates to true.
 > 180 incorrectStringBooleanError 571 warning Conversion of string
literal "The pointer is not allowed to be NULL!" to bool always
evaluates to true.
 > 191 incorrectStringBooleanError 571 warning Conversion of string
literal "The pointer is not allowed to be NULL!" to bool always
evaluates to true.
 > 302 incorrectStringBooleanError 571 warning Conversion of string
literal "The pointers may not be NULL!" to bool always evaluates to true.

The above six are all occurrencs of the assert(X && "explanation")
idiom, and as such false positives.  (Odd that there are only warnings
for these six occurrences; I'm pretty sure there's more across the code
base.  Maybe that's because "the 'LibreOffice.sln' project is incomplete
and cannot yet generate full builds.")
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Kaganski Mike Kaganski Mike
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives with a MSVC Project File

Hi!

https://gerrit.libreoffice.org/60119
https://gerrit.libreoffice.org/60122
https://gerrit.libreoffice.org/60120
https://gerrit.libreoffice.org/60121
https://gerrit.libreoffice.org/60123

are meant to take care of those that aren't false positives...

--
Best regards,
Mike Kaganski
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
sberg sberg
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives with a MSVC Project File

In reply to this post by sberg
On 07/09/18 11:26, Stephan Bergmann wrote:

>  >
> D:\cygwin\home\Hearthstone\lode\dev\core\framework\inc\helper\shareablemutex.hxx
>
>  > 73    noExplicitConstructor    398    style    Class 'ShareGuard' has
> a constructor with 1 argument that is not explicit.
>
> For all the nine noExplicitConstructor occurrences:  For one, what
> puzzles me is Cppcheck's fixation on "1 argument" ctors, as "explicit"
> has long surpassed being only relevant for ctors that can be called with
> one argument; if it warns about these, why doesn't it warn about ctors
> taking more arguments, too?  For another, for each individual ctor, they
> may be arguments for and against it being "explicit".  Would need deeper
> inspection, but generally feels on the level of rather unhelpful noise.

Especially, ctors that are part of the stable URE interface should
likely not be changed to be explicit, to avoid incompatible changes.
See the discussion at <https://gerrit.libreoffice.org/#/c/60123/>
"Cppcheck: make 1-argument ctors explicit".
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
slacka slacka
Reply | Threaded
Open this post in threaded view
|

Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by slacka
By manually specifying  includes and preprocessor configurations, I was able to reduce the number of warning from ~9000 to 30.

https://drive.google.com/file/d/1Ai_Zcj18cdQxVIQESb4lJr5K9LnzjnpW/view?usp=sharing

You can view it by unzipping and opening 'index.html'.Did this uncover any valid issues?

The command line used to scan our code base was:

$ cppcheck -j4 -DLINUX -D__GNUC__ -DUNX -DNDEBUG -DCORE_LITTLE_ENDIAN -D__LITTLE_ENDIAN__ -D__x86_64__ -UMACOSX -UFREEBSD -U_WIN32 -i external/ -i workdir/ --includes-file=inc.txt --xml --suppressions-list=cppcheck_supp.txt --enable=all --max-configs=100  ./  2&gt; ./err.xml

inc.txt was generated with:
$ find . -type d \( -name "inc" -o -name "include" \) |sort &gt; inc.txt

If this is useful, I could modify the weekly Cppcheck Report script to include these improvements. Who could set me up with permissions?

Ideally, the next step would be to extract the "DEFS": and "INCLUDE": from gbuild-to-ide and pass that to cppcheck. But that's for another time.

-Luke
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Mike Kaganski-2 Mike Kaganski-2
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

Hi!

On 9/13/2018 8:59 AM, Luke Benes wrote:
> By manually specifying  includes and preprocessor configurations, I was able to reduce the number of warning from ~9000 to 30.

Great!

> Ideally, the next step would be to extract the "DEFS": and "INCLUDE": from gbuild-to-ide and pass that to cppcheck. But that's for another time.

 > dbaccess/source/shared/registrationhelper.cxx
 > 23    preprocessorErrorDirective        error    #error "don't build
this file directly! use dbu_reghelper.cxx instead!"

This one is because a .cxx file was processed directly. The
workdir/GbuildToJson/Library/dbulo.dll does contain
dbaccess/source/shared/dbu_reghelper in its CXXOBJECTS, but no entry for
dbaccess/source/shared/registrationhelper. So extracting this
information seems helpful, too, to reduce false positives (OTOH I wonder
what this inclusion technique is useful here for.)

--
Best regards,
Mike Kaganski
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
slacka slacka
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by slacka
As I mentioned before, by manually specifying  includes and preprocessor configurations, I was able to reduce the number of warning from ~9000 to 30.

https://drive.google.com/file/d/1Ai_Zcj18cdQxVIQESb4lJr5K9LnzjnpW/view?usp=sharing

You can view it by unzipping and opening 'index.html'.

Caolán or Stephan,
Could either one of you take a quick pass at this to see if my improvements are useful?

If this is useful, I could modify the weekly Cppcheck Report script to include these improvements. Who could set me up with permissions?

-Luke
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Maarten Hoes Maarten Hoes
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

Hi,


If, by specifying (additional ?) include files/directories and adding
defines, you managed to bring down the massive zillions of warnings back to
just 30, it seems to me that you managed to do what the script should have
been doing all along to begin with.
;-)

Just a small remark (and you probably intend to do so already anyway):
besides changing the way cppcheck itself is called, you also make use of a
'inc.txt' file (which I assume contains the names of directories that
contain include files). I would personally re-create that file dynamically
from within the script (instead of providing a static version) on each run,
so that if in the future additional similar directories are added (or the
directory names change) these will get picked up automatically by the
script, instead of someone having to manually re-recreate the 'inc.txt' file
by hand (which people might/will forget to do). Yes, this will mean that the
script will take slightly longer to complete it's run (my personal
measurements based on the example included below are an added 15 seconds or
so currently), but since it is run automatically without any live human
being waiting for the output as it is running (and it is only run once a
week anyway as I recall) this does not really matter.

How about something like the thing below ? It would find all header files in
the libreoffice source tree, regardless of what directory they are located
in, and then use 'dirname' to get just the directory names they are located
in, and 'sort -u' to mention each directory just once. Would that be
sufficient for your suggested modifications ?


- Maarten Hoes



#!/bin/sh

TMPFILE=/tmp/tmpfile.txt
INCLUDEFILE=/tmp/incfile.txt


rm -f "$TMPFILE"


find . -type f \( -name \*.hxx -o -name \*.h \) | \
while read FILE
do
        DIRNAME=$(dirname "$FILE")
        echo "$DIRNAME" >> "$TMPFILE"
done

sort -u "$TMPFILE" > "$INCLUDEFILE"

cppcheck --includes-file="$INCLUDEFILE"




--
Sent from: http://document-foundation-mail-archive.969070.n3.nabble.com/Dev-f1639786.html
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
slacka slacka
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by slacka
Maarten,
Thanks for your suggestion here and your earlier contributions to the Cppcheck Report. I agree that we should create the include file dynamically. However the approach used by your script seems like overkill. Cppcheck already finds that quoted includes like
#include "GraphicExportFilter.hxx"
.
Also when there seems to have been a coding style that all <> includes outside of /inc folders should be defined by their relative path. Cppcheck only complains about 4 missing includes that do not follow this pattern.(see my earlier email on oddball includes).

Unless, I'm missing something, I still prefer this approach:
$ find . -type d \( -name "inc" -o -name "include" \) |sort > inc.txt

inc.txt only has ~200 entries, where as  /tmp/tmpfile.txt has ~1,800 after sorting it.

-Luke

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Tamás Zolnai Tamás Zolnai
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

Hello Luke,

I'm not sure what your modification is doing. I just checked the report you attached and I compared it with the full report and I see your change filters out not only false positives. I used to use the cppcheck report to give my students a small task for their first patch. So it would be helpful to keep the full report (it can be kept next to the short one I guess), otherwise I would need to run cppcheck myself.

Thanks,
Tamás

Luke Benes <[hidden email]> ezt írta (időpont: 2018. szept. 30., V, 5:20):
Maarten,
Thanks for your suggestion here and your earlier contributions to the Cppcheck Report. I agree that we should create the include file dynamically. However the approach used by your script seems like overkill. Cppcheck already finds that quoted includes like
#include "GraphicExportFilter.hxx"
.
Also when there seems to have been a coding style that all <> includes outside of /inc folders should be defined by their relative path. Cppcheck only complains about 4 missing includes that do not follow this pattern.(see my earlier email on oddball includes).

Unless, I'm missing something, I still prefer this approach:
$ find . -type d \( -name "inc" -o -name "include" \) |sort > inc.txt

inc.txt only has ~200 entries, where as  /tmp/tmpfile.txt has ~1,800 after sorting it.

-Luke

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Maarten Hoes Maarten Hoes
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by slacka
Hi,


slacka wrote
> Ideally, the next step would be to extract the "DEFS": and "INCLUDE": from
> gbuild-to-ide and pass that to cppcheck. But that's for another time.


Well, the very name 'gbuild-to-ide' sounds intriguing, but I can't figure
out what it is supposed to do (and how could it help in the cppcheck-report
script context) or how to make it do it's thing (I *really* can't read
python code).

So, my futile attempt was :

./bin/gbuild-to-ide --ide vim --make make


Which resulted in this:

Traceback (most recent call last):
  File "./bin/gbuild-to-ide", line 1664, in <module>
    gbuildparser = GbuildParser(args.makecmd).parse()
  File "./bin/gbuild-to-ide", line 83, in __init__
    self.binpath = os.path.dirname(os.environ['GPERF']) # woha, this is
quite a hack
  File "/usr/lib64/python3.6/os.py", line 669, in __getitem__
    raise KeyError(key) from None
KeyError: 'GPERF'


Appearantly, 'gbuild-to-ide' assumes some environment variables are set
(GPERF, SRCDIR, BUILDDIR, INSTDIR, and WORKDIR), but I cannot determine what
would be sane/expected values for these variables. Okay,
SRCDIR/BUILDDIR/INSTDIR seem self-explanatory (but you know what happens
when we assUme), but what are WORKDIR and GPERF ?



- Maarten




--
Sent from: http://document-foundation-mail-archive.969070.n3.nabble.com/Dev-f1639786.html
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Maarten Hoes Maarten Hoes
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by Tamás Zolnai
Hi,

On Sun, Sep 30, 2018 at 11:29 AM Tamás Zolnai <[hidden email]> wrote:

I'm not sure what your modification is doing. I just checked the report you attached and I compared it with the full report and I see your change filters out not only false positives.


Am I interpreting correctly here that the proposed approach of doing this :

$ find . -type d \( -name "inc" -o -name "include" \) |sort > inc.txt
$ cppcheck -DLINUX -D__GNUC__ -DUNX -DNDEBUG -DCORE_LITTLE_ENDIAN -D__LITTLE_ENDIAN__ -D__x86_64__ -UMACOSX -UFREEBSD -U_WIN32 -i external/ -i workdir/ --includes-file=inc.txt --xml --suppressions-list=cppcheck_supp.txt --enable=all --max-configs=100  ./  2> ./err.xml


Versus what the cppcheck-report script currently is doing, results in actual real issues not being reported anymore ? If so, that's bad(TM), and needs further investigation at the very least before that suggested change gets implemented.


- Maarten.

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Maarten Hoes Maarten Hoes
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by slacka
Hi,

On Sun, Sep 30, 2018 at 5:20 AM Luke Benes <[hidden email]> wrote:
Maarten,
Thanks for your suggestion here and your earlier contributions to the Cppcheck Report. I agree that we should create the include file dynamically. However the approach used by your script seems like overkill. Cppcheck already finds that quoted includes like
#include "GraphicExportFilter.hxx"
.
Also when there seems to have been a coding style that all <> includes outside of /inc folders should be defined by their relative path. Cppcheck only complains about 4 missing includes that do not follow this pattern.(see my earlier email on oddball includes).

Unless, I'm missing something, I still prefer this approach:
$ find . -type d \( -name "inc" -o -name "include" \) |sort > inc.txt

inc.txt only has ~200 entries, where as  /tmp/tmpfile.txt has ~1,800 after sorting it.

-Luke



I was about to speak of the supposedly preferred approach that I took, for the reasons that my approach :


1.) Does not depend on the existence of and/or adherence to any customs/practices/coding styles, which can and do change over time, or are simply overlooked, causing errors that people will have to correct manually or even cause silent failure that no-one notices.
2.) Is fully dynamic, and does not have any paths 'hard-coded' into the script, which one would have to adjust if in the future the paths/names change. I already am worried about the fact that the approach uses hard-coded defines (-DFOO/-UBAR) on the commandline instead of determining them dynamically, but I currently see no way to do this differently, so I guess that has to stay for now. Also, I guess the chance that these defines change over time is far less likely than the chance that directory names and locations change.


But then I decided to put my money where my mouth is, and made the necessary changes to the cppcheck-report script, and guess what ? Apart from approach differences, your version completed in an acceptable execution time, whereas my approach took *ages* (after 40 minutes cppcheck was still only about 1% completed, after which I aborted the attempt).


As of now, I cannot tell what causes this massive difference in execution time, but the only visible difference in cppcheck output was this :


my version: Checking basctl/source/basicide/basides1.cxx: LINUX=1;__GNUC__=1;UNX=1;NDEBUG=1;CORE_LITTLE_ENDIAN=1;__LITTLE_ENDIAN__=1;__x86_64__=1;LINUX=1;__GNUC__=1;UNX=1;NDEBUG=1;CORE_LITTLE_ENDIAN=1;__LITTLE_ENDIAN__=1;__x86_64__=1...
your version: Checking basctl/source/basicide/basides1.cxx ...

Obvious things: 'my version' listed the -D in the output, your's did not. 'My version' lists the defines *twice* (which can't be good).

I don't know what's going on here.

- Maarten

 

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
sberg sberg
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by Maarten Hoes
On 30/09/2018 15:04, Maarten Hoes wrote:

> So, my futile attempt was :
>
> ./bin/gbuild-to-ide --ide vim --make make
>
>
> Which resulted in this:
>
> Traceback (most recent call last):
>    File "./bin/gbuild-to-ide", line 1664, in <module>
>      gbuildparser = GbuildParser(args.makecmd).parse()
>    File "./bin/gbuild-to-ide", line 83, in __init__
>      self.binpath = os.path.dirname(os.environ['GPERF']) # woha, this is
> quite a hack
>    File "/usr/lib64/python3.6/os.py", line 669, in __getitem__
>      raise KeyError(key) from None
> KeyError: 'GPERF'
>
>
> Appearantly, 'gbuild-to-ide' assumes some environment variables are set
> (GPERF, SRCDIR, BUILDDIR, INSTDIR, and WORKDIR), but I cannot determine what
> would be sane/expected values for these variables. Okay,
> SRCDIR/BUILDDIR/INSTDIR seem self-explanatory (but you know what happens
> when we assUme), but what are WORKDIR and GPERF ?

That smells like gbuild-to-ide needs to be run from within gbuild's
"config_host.mk polluted" environment, which can be done via the "cmd"
make target, i.e., something like

   make cmd cmd='bin/gbuild-to-ide --ide vim --make make'
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
sberg sberg
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by slacka
On 30/09/2018 05:20, Luke Benes wrote:
> Also when there seems to have been a coding style that all <> includes outside of /inc folders should be defined by their relative path. Cppcheck only complains about 4 missing includes that do not follow this pattern.(see my earlier email on oddball includes).

See the root README.md for "Rules for #include directives (C/C++)".
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Miklos Vajna-4 Miklos Vajna-4
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by sberg
Hi,

On Mon, Oct 01, 2018 at 08:52:19AM +0200, Stephan Bergmann <[hidden email]> wrote:
> That smells like gbuild-to-ide needs to be run from within gbuild's
> "config_host.mk polluted" environment, which can be done via the "cmd" make
> target, i.e., something like
>
>   make cmd cmd='bin/gbuild-to-ide --ide vim --make make'

This is exactly what

        make vim-ide-integration

does. :-) (Though this later first runs gbuildtojson.)

Regards,

Miklos

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice

signature.asc (201 bytes) Download Attachment
slacka slacka
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by Tamás Zolnai
The goal of my manual approach was to configure Cppcheck to minimize false
positives. In doing so, I was forced to only scan the Linux code base, as
only Linux has Linux system headers and does not have Window's or BSD's...
So I am not surprised that some valid issues were not reported.

There are many knobs I could tweak. For example, since my last post, I
discovered I could remove the "-DNDEBUG" to scan the debug code path. I
could also remove the "-j 4" option to allow Cppcheck to scan for unused
functions.  I don't know what is most useful, and what valid issues were not
being reported.  This is why I have asked the ML for feedback.

So if a dev wants give me some guidance, I could continue tweaking, or as
you suggested, we could run 2 reports.
1) a limited Linux only scan with few false positives (ala my manual
approach), and
2) a general scan with many false positives (the current Cppcheck Report).  

If you try to limit the false positives with include locations without also
limiting configuration, Cppcheck gets overloaded and generates tens of
thousands of "too many configuration" errors.



--
Sent from: http://document-foundation-mail-archive.969070.n3.nabble.com/Dev-f1639786.html
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
sberg sberg
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

On 02/10/2018 04:05, slacka wrote:
> There are many knobs I could tweak. For example, since my last post, I
> discovered I could remove the "-DNDEBUG" to scan the debug code path. I
> could also remove the "-j 4" option to allow Cppcheck to scan for unused
> functions.  I don't know what is most useful, and what valid issues were not
> being reported.  This is why I have asked the ML for feedback.

The knobs you can tweak are the configure options passed to LO's
autogen.sh.  For example, -DNDEBUG is controlled by
--en-/disable-assert-always-abort.  There is not per-se an "ideal" set
of configure options for static analysis purposes.  One reasonable
approach is of course to maximize code coverage, both at the large scale
(enabling as many optional modules as possible, from e.g. --enable-kde5
to --enable-ext-nlpsolver) and at small (e.g., enabling asserts with
--enable-assert-always-abort or debug-only code blocks with
--enable-debug or even --enable-dbgutil, both of which imply
--enable-assert-always-abort).
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Tamás Zolnai Tamás Zolnai
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by slacka
Hi Luke,

slacka <[hidden email]> ezt írta (időpont: 2018. okt. 2., K, 4:05):
The goal of my manual approach was to configure Cppcheck to minimize false
positives. In doing so, I was forced to only scan the Linux code base, as
only Linux has Linux system headers and does not have Window's or BSD's...
So I am not surprised that some valid issues were not reported.

There are many knobs I could tweak. For example, since my last post, I
discovered I could remove the "-DNDEBUG" to scan the debug code path. I
could also remove the "-j 4" option to allow Cppcheck to scan for unused
functions.  I don't know what is most useful, and what valid issues were not
being reported.  This is why I have asked the ML for feedback.

I used to find valid issues amongs the variableScope warnings for example. Check a frequently modified module (e.g. sw, sc, sd).
One example:
I just checked a few of these warnings now and they should be there in a Linux specific analysis too. The linked one seems not a platform specific or debug code. So it would be good to find out why your report does not contains this one.

In general I doubt that a static analyzer does not find any issue (at least some false positives) in sw module for example (if it was not cleaned up with this analyzer earlier). You report contains no issue in sw. It seems to me the scope of the analysis is greatly reduced by your change, that's why it does not find a lot of issues. I'm not sure how the false positives can be reduced by specifing the includes. Which false positives are coming from wrong includes?

Best Regards,
Tamás

_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
slacka slacka
Reply | Threaded
Open this post in threaded view
|

Re: Cppcheck: Reduction of False Positives: Manual Approach

In reply to this post by slacka
In my first attempt to improve the quality of the cppcheck reports,  Tamás Zolnai pointed out that including every possible header resulted in some valid warnings not being reported.

Instead, how about just including only our primary include folder of ./include with the '-Iinclude' parameter? This reduced the warnings from 4005 to 523.

You can see and compare the results by opening the 'index.html' file at
https://drive.google.com/open?id=1HaCVA_udd044uwRNn3RiyGJ01pQdYjwn

It seems many valid variableScope warnings are still being omitted. I'm still looking into that. Are there any other categories of valid errors that are missing? Specific examples would be helpful.

Overall, does this report have a higher signal-to-noise ratio than our current weekly report?


The full command to generate the report was:
../cppcheck/cppcheck -D__GNUC__ -DBOOST_ERROR_CODE_HEADER_ONLY -DBOOST_SYSTEM_NO_DEPRECATED -DCPPU_ENV=gcc3 -DLINUX -DNDEBUG -DOSL_DEBUG_LEVEL=0 -DUNIX -DUNX -DX86_64 -D_PTHREADS -D_REENTRANT -j4 -i external/ -i workdir/ -Iinclude --xml --suppressions-list=cppcheck_supp.txt --enable=all --max-configs=100  ./  2> ./err.xml
_______________________________________________
LibreOffice mailing list
[hidden email]
https://lists.freedesktop.org/mailman/listinfo/libreoffice
Next » 12