www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - Run Microsoft Analyzer over dmd source code

reply Walter Bright <newshound2 digitalmars.com> writes:
http://www.reddit.com/r/programming/comments/jar93/john_carmack_gives_his_thoughts_on_static/c2akmf8

If someone has this on their system, I think it'd be great if you could run the 
dmd source code through it and see if it finds any bugs.
Aug 06 2011
next sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
At about 1:10:00, it sounds like he's asking for SafeD!
Aug 06 2011
parent Walter Bright <newshound2 digitalmars.com> writes:
On 8/6/2011 1:08 PM, Adam D. Ruppe wrote:

 At about 1:10:00, it sounds like he's asking for SafeD!


It sure does.
Aug 06 2011
prev sibling next sibling parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sat, 06 Aug 2011 22:38:15 +0300, Walter Bright  
<newshound2 digitalmars.com> wrote:


 http://www.reddit.com/r/programming/comments/jar93/john_carmack_gives_his_thoughts_on_static/c2akmf8

 If someone has this on their system, I think it'd be great if you could  
 run the dmd source code through it and see if it finds any bugs.


Before that, someone first needs to get dmd to even compile with Visual  
C++. It's not a trivial task - there is no complex number support, lots of  
compiler-dependent defines are outdated, and there seem to be tons of  
regular compiler warnings (mostly signed/unsigned comparisons and long to  
int/etc. assignments).

-- 
Best regards,
  Vladimir                            mailto:vladimir thecybershadow.net
Aug 07 2011
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/7/2011 3:24 AM, Vladimir Panteleev wrote:

 Before that, someone first needs to get dmd to even compile with Visual C++.
 It's not a trivial task - there is no complex number support, lots of
 compiler-dependent defines are outdated, and there seem to be tons of regular
 compiler warnings (mostly signed/unsigned comparisons and long to int/etc.
 assignments).



It's less complex (!) if you are not trying to make a working dmd. It just
needs 
to compile.
Aug 07 2011
parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 07 Aug 2011 13:29:20 +0300, Walter Bright  
<newshound2 digitalmars.com> wrote:


 It's less complex (!) if you are not trying to make a working dmd. It  
 just needs to compile.


OK, that wasn't actually too bad.  
https://github.com/CyberShadow/dmd/tree/compile-on-vs10

2979 warnings with code analysis with the "Microsoft Minimum Recommended  
Rules" ruleset.
http://dump.thecybershadow.net/2e0571641194d945869a1b12b29aacdc/DMD.log

I'll see if I can get it in a more readable format (something like the  
HTML files clang's scan-build outputs).

-- 
Best regards,
  Vladimir                            mailto:vladimir thecybershadow.net
Aug 07 2011
next sibling parent "Marco Leise" <Marco.Leise gmx.de> writes:
Am 07.08.2011, 16:23 Uhr, schrieb Vladimir Panteleev  
<vladimir thecybershadow.net>:


 On Sun, 07 Aug 2011 13:29:20 +0300, Walter Bright  
 <newshound2 digitalmars.com> wrote:


 It's less complex (!) if you are not trying to make a working dmd. It  
 just needs to compile.


 OK, that wasn't actually too bad.  
 https://github.com/CyberShadow/dmd/tree/compile-on-vs10

 2979 warnings with code analysis with the "Microsoft Minimum Recommended  
 Rules" ruleset.
 http://dump.thecybershadow.net/2e0571641194d945869a1b12b29aacdc/DMD.log

 I'll see if I can get it in a more readable format (something like the  
 HTML files clang's scan-build outputs).


I start to think, the closer a code analysis tool is to '0 warnings' the  
better it is :p
Aug 07 2011
prev sibling next sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
Vladimir Panteleev:


 2979 warnings with code analysis with the "Microsoft Minimum Recommended  
 Rules" ruleset.


Thank you. Info about this minimum level:
http://msdn.microsoft.com/en-us/library/dd264893.aspx
Later a higher level will show other warnings.



 http://dump.thecybershadow.net/2e0571641194d945869a1b12b29aacdc/DMD.log


It contains FP warnings like the one I have asked for D too. DMD doesn't
perform certain unsafe FP operations because they and break IEEE conformance,
but casting a double to float is accepted and regarded "safe" (I am not sure of
this):
lexer.c(2500): warning C4244: 'initializing' : conversion from 'double' to
'float', possible loss of data


There are 21 warnings like:
C:\Projects\Extern\D\dmd\src\tk\vec.c(493): warning C4102: 'Lret' :
unreferenced label

Hours ago I have asked for this warning in D:
http://d.puremagic.com/issues/show_bug.cgi?id=6449


A bit of statistics. There are 114 warnings like:
backend\cgcs.c(656): warning C4018: '<' : signed/unsigned mismatch

And there are 134 warnings like (this is the most common):
c:\projects\extern\d\dmd\src\root\dchar.h(164): warning C6328: 'char' passed as
parameter '1' when 'unsigned char' is required in call to 'isalpha'

Bye,
bearophile
Aug 07 2011
next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/7/2011 10:11 AM, bearophile wrote:

 It contains FP warnings like the one I have asked for D too. DMD doesn't
 perform certain unsafe FP operations because they and break IEEE conformance,
 but casting a double to float is accepted and regarded "safe" (I am not sure
 of this): lexer.c(2500): warning C4244: 'initializing' : conversion from
 'double' to 'float', possible loss of data



You mean implicit casting of double to float. Yes, it is accepted without 
complaint by dmd. The problem with requiring a cast is a cast is a blunt
instrument:

    float f;
    double d;
    ...
    f = (float) d;

Now, suppose the maintenance guy decides to upgrade f to being a double to get 
more precision:

    double f;
    double d;
    ...
    f = (float) d;

and there's that cast to float he overlooked, sabotaging his upgrade. Even 
worse, suppose the type of f or d or both is changed to some user defined type, 
like BigFloat? That cast is just going to *cause* a bug, not fix it.

Requiring the programmer to load up on casts is not necessarily making the code 
less "bug-prone".
Aug 07 2011
parent reply bearophile <bearophileHUGS lycos.com> writes:
Walter:


 and there's that cast to float he overlooked, sabotaging his upgrade. Even 
 worse, suppose the type of f or d or both is changed to some user defined
type, 
 like BigFloat? That cast is just going to *cause* a bug, not fix it.
 
 Requiring the programmer to load up on casts is not necessarily making the
code 
 less "bug-prone".


Thank you for your good explanation.

I presume this doesn't improve the situation a lot:

float f;
double d;
...
f = cast(f.typeof)d;

Bye,
bearophile
Aug 07 2011
parent Walter Bright <newshound2 digitalmars.com> writes:
On 8/7/2011 10:43 AM, bearophile wrote:

 and there's that cast to float he overlooked, sabotaging his upgrade. Even
 worse, suppose the type of f or d or both is changed to some user defined type,
 like BigFloat? That cast is just going to *cause* a bug, not fix it.

 Requiring the programmer to load up on casts is not necessarily making the code
 less "bug-prone".


 Thank you for your good explanation.

 I presume this doesn't improve the situation a lot:

 float f;
 double d;
 ...
 f = cast(f.typeof)d;


That can improve things for the case where f is available, but that isn't
always 
the case (such as when passing d to a function that takes a float parameter).
Aug 07 2011
prev sibling parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 07 Aug 2011 20:11:49 +0300, bearophile <bearophileHUGS lycos.com>  
wrote:


 Later a higher level will show other warnings.


I have the following levels to choose from:

Microsoft All Rules
Microsoft Basic Correctness Rules
Microsoft Basic Design Guideline Rules
Microsoft Extended Correctness Rules
Microsoft Extended Design Guideline Rules
Microsoft Globalization Rules
Microsoft Minimum Recommended Rules
Microsoft Security Rules

I imagine that higher settings will have higher signal-to-noise ratios,  
but I can re-run it on another ruleset if anyone's interested.

-- 
Best regards,
  Vladimir                            mailto:vladimir thecybershadow.net
Aug 07 2011
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/7/2011 11:13 AM, Vladimir Panteleev wrote:

 On Sun, 07 Aug 2011 20:11:49 +0300, bearophile <bearophileHUGS lycos.com>
wrote:


 Later a higher level will show other warnings.


 I have the following levels to choose from:

 Microsoft All Rules
 Microsoft Basic Correctness Rules
 Microsoft Basic Design Guideline Rules
 Microsoft Extended Correctness Rules
 Microsoft Extended Design Guideline Rules
 Microsoft Globalization Rules
 Microsoft Minimum Recommended Rules
 Microsoft Security Rules

 I imagine that higher settings will have higher signal-to-noise ratios, but I
 can re-run it on another ruleset if anyone's interested.


Try the All Rules.
Aug 07 2011
parent "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 07 Aug 2011 21:17:30 +0300, Walter Bright  
<newshound2 digitalmars.com> wrote:


 Try the All Rules.


Ah, my mistake. The rulesets only apply to managed (.NET) code. It looks  
like C/C++ code analysis is just an on/off switch.

http://msdn.microsoft.com/en-us/library/d3bbz7tz(v=vs.80).aspx

-- 
Best regards,
  Vladimir                            mailto:vladimir thecybershadow.net
Aug 07 2011
prev sibling next sibling parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/7/2011 7:23 AM, Vladimir Panteleev wrote:

 2979 warnings with code analysis with the "Microsoft Minimum Recommended Rules"
 ruleset.
 http://dump.thecybershadow.net/2e0571641194d945869a1b12b29aacdc/DMD.log


Thanks, this is what I was looking for!



 I'll see if I can get it in a more readable format (something like the HTML
 files clang's scan-build outputs).


clang's format is more readable, but it's actually rather agonizing to go 
through because it takes upwards of a minute for each report to load. When
there 
are hundreds to look at, you're looking at hours of waiting.
Aug 07 2011
parent Robert Clipsham <robert octarineparrot.com> writes:
On 07/08/2011 18:34, Walter Bright wrote:

 On 8/7/2011 7:23 AM, Vladimir Panteleev wrote:

 2979 warnings with code analysis with the "Microsoft Minimum
 Recommended Rules"
 ruleset.
 http://dump.thecybershadow.net/2e0571641194d945869a1b12b29aacdc/DMD.log


 Thanks, this is what I was looking for!



 I'll see if I can get it in a more readable format (something like the
 HTML
 files clang's scan-build outputs).


 clang's format is more readable, but it's actually rather agonizing to
 go through because it takes upwards of a minute for each report to load.
 When there are hundreds to look at, you're looking at hours of waiting.


It's actually instant[1], the only reason it seemed so slow for you is 
because the html is on my (wireless) home server, and you were 
connecting to my web server which was proxying all the content with no 
caching (yay quick hacks) - so you had to put up with my slow upload 
speeds and slow wireless as well as the proxying.

[1] This is based on me accessing it locally or across my home network.

-- 
Robert
http://octarineparrot.com/
Aug 07 2011
prev sibling next sibling parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 07 Aug 2011 17:23:48 +0300, Vladimir Panteleev  
<vladimir thecybershadow.net> wrote:


 I'll see if I can get it in a more readable format (something like the  
 HTML files clang's scan-build outputs).


http://thecybershadow.net/d/vcanalysis/

Created with:
https://github.com/CyberShadow/ColorerVCLog

-- 
Best regards,
  Vladimir                            mailto:vladimir thecybershadow.net
Aug 07 2011
next sibling parent Walter Bright <newshound2 digitalmars.com> writes:
On 8/7/2011 11:06 AM, Vladimir Panteleev wrote:

 I'll see if I can get it in a more readable format (something like the HTML
 files clang's scan-build outputs).


 http://thecybershadow.net/d/vcanalysis/


Very nice!
Aug 07 2011
prev sibling next sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
Vladimir Panteleev:


 http://thecybershadow.net/d/vcanalysis/


I don't see the pages in inner directories like "root\root.c".

Bye,
bearophile
Aug 07 2011
parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 07 Aug 2011 21:28:18 +0300, bearophile <bearophileHUGS lycos.com>  
wrote:


 Vladimir Panteleev:


 http://thecybershadow.net/d/vcanalysis/


 I don't see the pages in inner directories like "root\root.c".


Could you please elaborate? The list of links is sorted alphabetically, so  
files under "root" are lower down the list.

-- 
Best regards,
  Vladimir                            mailto:vladimir thecybershadow.net
Aug 07 2011
parent reply bearophile <bearophileHUGS lycos.com> writes:
Vladimir Panteleev:


 Could you please elaborate? The list of links is sorted alphabetically, so  
 files under "root" are lower down the list.


What do you see if you click on a link like "root\root.c"?

Bye,
bearophile
Aug 07 2011
parent "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 07 Aug 2011 21:47:07 +0300, bearophile <bearophileHUGS lycos.com>  
wrote:


 Vladimir Panteleev:


 Could you please elaborate? The list of links is sorted alphabetically,  
 so
 files under "root" are lower down the list.


 What do you see if you click on a link like "root\root.c"?


Ah! I didn't reproduce the problem, but I know what it is - I didn't  
bother replacing Windows' backslashes with forward slashes. Will fix.

-- 
Best regards,
  Vladimir                            mailto:vladimir thecybershadow.net
Aug 07 2011
prev sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
Vladimir Panteleev:


 http://thecybershadow.net/d/vcanalysis/


As with (first report of) Clang I see that assert(p); p->foo... are marked as
"Dereferencing NULL pointer".


Do you know the purpose of this?
      os->name = strdup(name);
warning C4996: 'strdup': The POSIX name for this item is deprecated. Instead,
use the ISO C++ conformant name: _strdup. See online help for details.
c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\include\string.h(238) :
see declaration of 'strdup' 


And do you know what kind of troubles this warning helps to avoid?
c:\projects\extern\d\dmd\src\root\dchar.h(164): warning C6328: 'char' passed as
parameter '1' when 'unsigned char' is required in call to 'isalpha'

Bye,
bearophile
Aug 07 2011
next sibling parent reply KennyTM~ <kennytm gmail.com> writes:
On Aug 8, 11 02:45, bearophile wrote:

 Vladimir Panteleev:


 http://thecybershadow.net/d/vcanalysis/


 As with (first report of) Clang I see that assert(p); p->foo... are marked as
"Dereferencing NULL pointer".


 Do you know the purpose of this?
        os->name = strdup(name);
 warning C4996: 'strdup': The POSIX name for this item is deprecated. Instead,
use the ISO C++ conformant name: _strdup. See online help for details.
c:\Program Files (x86)\Microsoft Visual Studio 10.0\VC\include\string.h(238) :
see declaration of 'strdup'


http://stackoverflow.com/questions/14386/fopen-deprecated-warning


 And do you know what kind of troubles this warning helps to avoid?
 c:\projects\extern\d\dmd\src\root\dchar.h(164): warning C6328: 'char' passed
as parameter '1' when 'unsigned char' is required in call to 'isalpha'


You could search the error code in Google to get the info in MSDN.

http://msdn.microsoft.com/en-us/library/ms245348%28v=VS.100%29.aspx:

"For routines starting with is*, passing an argument of type char might 
yield unpredictable results. For example, an SBCS or MBCS single-byte 
character of type char with a value greater than 0x7F is negative. If a 
char is passed, the compiler might convert the value to a signed int or 
a signed long. This value could be sign-extended by the compiler, with 
unexpected results."


 Bye,
 bearophile
Aug 07 2011
parent KennyTM~ <kennytm gmail.com> writes:
On Aug 8, 11 03:08, KennyTM~ wrote:

 On Aug 8, 11 02:45, bearophile wrote:

 Vladimir Panteleev:


 http://thecybershadow.net/d/vcanalysis/


 As with (first report of) Clang I see that assert(p); p->foo... are
 marked as "Dereferencing NULL pointer".


 Do you know the purpose of this?
 os->name = strdup(name);
 warning C4996: 'strdup': The POSIX name for this item is deprecated.
 Instead, use the ISO C++ conformant name: _strdup. See online help for
 details. c:\Program Files (x86)\Microsoft Visual Studio
 10.0\VC\include\string.h(238) : see declaration of 'strdup'


 http://stackoverflow.com/questions/14386/fopen-deprecated-warning


Oops sorry that link is for those silly _s versions. _strdup is actually 
worse, it's only because Microsoft has chosen to add a '_' to all POSIX 
functions with no good reason (maybe just §17.6.4.3.2=[global.names]/1).

http://msdn.microsoft.com/en-us/library/y471khhc%28v=VS.100%29.aspx

I recommend ignoring all C4996 since the DMD source code shouldn't just 
accommodate for MSVC.


 And do you know what kind of troubles this warning helps to avoid?
 c:\projects\extern\d\dmd\src\root\dchar.h(164): warning C6328: 'char'
 passed as parameter '1' when 'unsigned char' is required in call to
 'isalpha'


 You could search the error code in Google to get the info in MSDN.

 http://msdn.microsoft.com/en-us/library/ms245348%28v=VS.100%29.aspx:

 "For routines starting with is*, passing an argument of type char might
 yield unpredictable results. For example, an SBCS or MBCS single-byte
 character of type char with a value greater than 0x7F is negative. If a
 char is passed, the compiler might convert the value to a signed int or
 a signed long. This value could be sign-extended by the compiler, with
 unexpected results."


 Bye,
 bearophile
Aug 07 2011
prev sibling parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 07 Aug 2011 21:45:45 +0300, bearophile <bearophileHUGS lycos.com>  
wrote:


 Vladimir Panteleev:


 http://thecybershadow.net/d/vcanalysis/


 As with (first report of) Clang I see that assert(p); p->foo... are  
 marked as "Dereferencing NULL pointer".


Ah, that would probably be in files that #include <assert.h> instead of  
"tassert.h". (Odd that Microsoft's code analyzer doesn't understand the  
standard assert facility.) I'll try fixing that.


 Do you know the purpose of this?
       os->name = strdup(name);
 warning C4996: 'strdup': The POSIX name for this item is deprecated.  
 Instead, use the ISO C++ conformant name: _strdup. See online help for  
 details. c:\Program Files (x86)\Microsoft Visual Studio  
 10.0\VC\include\string.h(238) : see declaration of 'strdup'


I don't think there's more to it than what the message says - a  
recommendation to use the ISO C++ conformant name instead of the  
deprecated POSIX name.

-- 
Best regards,
  Vladimir                            mailto:vladimir thecybershadow.net
Aug 07 2011
parent reply "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Sun, 07 Aug 2011 22:11:35 +0300, Vladimir Panteleev  
<vladimir thecybershadow.net> wrote:


 On Sun, 07 Aug 2011 21:45:45 +0300, bearophile  
 <bearophileHUGS lycos.com> wrote:


 Vladimir Panteleev:


 http://thecybershadow.net/d/vcanalysis/


 As with (first report of) Clang I see that assert(p); p->foo... are  
 marked as "Dereferencing NULL pointer".


 Ah, that would probably be in files that #include <assert.h> instead of  
 "tassert.h". (Odd that Microsoft's code analyzer doesn't understand the  
 standard assert facility.) I'll try fixing that.


Done:  
http://dump.thecybershadow.net/b1e4cb6ef0a8d3c5f54d5cb09ddd1a9e/DMD.log

HTML files updated in place.

-- 
Best regards,
  Vladimir                            mailto:vladimir thecybershadow.net
Aug 07 2011
next sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
Vladimir Panteleev:


 Done:  
 http://dump.thecybershadow.net/b1e4cb6ef0a8d3c5f54d5cb09ddd1a9e/DMD.log


There are only 10 NULL deference warnings left:

s2ir.c(1043): warning C6011: Dereferencing NULL pointer 'pu++'
s2ir.c(980): warning C6011: Dereferencing NULL pointer 'cases'
statement.c(3666): warning C6011: Dereferencing NULL pointer 'tbret'
template.c(5353): warning C6011: Dereferencing NULL pointer 'sds'
todt.c(536): warning C6011: Dereferencing NULL pointer 'type'
todt.c(746): warning C6011: Dereferencing NULL pointer 'var'
module.c(793): warning C6011: Dereferencing NULL pointer 'sc'
mtype.c(4887): warning C6011: Dereferencing NULL pointer 'fparam->type'
mtype.c(8130): warning C6011: Dereferencing NULL pointer 'arg->type'
root\root.c(1498): warning C6011: Dereferencing NULL pointer 'f->touchtime'

Bye,
bearophile
Aug 07 2011
parent Don <nospam nospam.com> writes:
bearophile wrote:

 Vladimir Panteleev:
 

 Done:  
 http://dump.thecybershadow.net/b1e4cb6ef0a8d3c5f54d5cb09ddd1a9e/DMD.log


 
 There are only 10 NULL deference warnings left:
 
 s2ir.c(1043): warning C6011: Dereferencing NULL pointer 'pu++'


false (only if malloc fails)

 s2ir.c(980): warning C6011: Dereferencing NULL pointer 'cases'


false

 statement.c(3666): warning C6011: Dereferencing NULL pointer 'tbret'


clang complained about this too, might be correct.

 template.c(5353): warning C6011: Dereferencing NULL pointer 'sds'


false

 todt.c(536): warning C6011: Dereferencing NULL pointer 'type'


false

 todt.c(746): warning C6011: Dereferencing NULL pointer 'var'


false
But there's a NULL dereference in todt(59) which it missed (bug 3863).

 module.c(793): warning C6011: Dereferencing NULL pointer 'sc'


HIT! That looks real.

 mtype.c(4887): warning C6011: Dereferencing NULL pointer 'fparam->type'


maybe

 mtype.c(8130): warning C6011: Dereferencing NULL pointer 'arg->type'


maybe.

 root\root.c(1498): warning C6011: Dereferencing NULL pointer 'f->touchtime'


maybe.
Aug 08 2011
prev sibling parent reply David Nadlinger <see klickverbot.at> writes:
On 8/7/11 9:47 PM, Vladimir Panteleev wrote:

 http://thecybershadow.net/d/vcanalysis/






 http://dump.thecybershadow.net/b1e4cb6ef0a8d3c5f54d5cb09ddd1a9e/DMD.log


Is there a way to disable exceptions with MSVC like -fno-exceptions for 
GCC to help get rid of the associated false positives?

David
Aug 12 2011
parent "Trass3r" <un known.com> writes:
 Is there a way to disable exceptions with MSVC like 
 -fno-exceptions for GCC to help get rid of the associated false 
 positives?


Sure, no /EHsc and /D_HAS_EXCEPTIONS=0 for its STL.
Jul 09 2014
prev sibling parent reply KennyTM~ <kennytm gmail.com> writes:
On Aug 7, 11 22:23, Vladimir Panteleev wrote:

 On Sun, 07 Aug 2011 13:29:20 +0300, Walter Bright
 <newshound2 digitalmars.com> wrote:


 It's less complex (!) if you are not trying to make a working dmd. It
 just needs to compile.


 OK, that wasn't actually too bad.
 https://github.com/CyberShadow/dmd/tree/compile-on-vs10

 2979 warnings with code analysis with the "Microsoft Minimum Recommended
 Rules" ruleset.
 http://dump.thecybershadow.net/2e0571641194d945869a1b12b29aacdc/DMD.log

 I'll see if I can get it in a more readable format (something like the
 HTML files clang's scan-build outputs).


Just at a glance, half of them are false positive, or is arguably safe:

1. 382 (13%) of them are C4996 (use those Microsoft _s functions)
2. 401 (13%) of them are C4068 (unknown pragma)
3. 505 (17%) of them are C6328 (passing 'signed char' to the ctype 
functions)
4. 67 (2%) of them are C6239 (true && something) or C6240 (something && 
true) - many of them are of them (!I16 && stuff), so that's legacy code 
for 16-bit platform??
5. 37 (1%) of them are C6255 (using alloca) or C6263 (using alloca in a 
loop).
6. 56 (2%) of them are C4305 or C4309 (double -> float)

And 37% of them can be caught trivially with some -Wall flag.

4. 262 (9%) of them are C4244 (stuff like int64 -> int32)
5. 415 (14%) of them are C4018 (signed/unsigned comparison)
6. 157 (5%) of them are C4101 (unused locals)
7. 50 (2%) of them are C4102 (unused labels)
8. 212 (7%) of them are C6246 or C6244 or C4258 (local variable name 
hiding outer scope)
9. 8 (0.3%) of them are C4390 ('if(stuff);')

The really interesting things:

8. 117 (4%) of them are C6211 (leak on exception) - but a bigger problem 
is DMD is using too much memory even without leaking.
9. 34 (1%) of them are C6001 (using uninitialized memory)
10. 125 (4%) of them are C6011 (NULL dereferencing)
11. 6 (0.2%) of them are C6386 and 17 (0.6%) of them are C6385 (buffer 
overrun)
Aug 07 2011
parent reply Don <nospam nospam.com> writes:
KennyTM~ wrote:

 On Aug 7, 11 22:23, Vladimir Panteleev wrote:

 On Sun, 07 Aug 2011 13:29:20 +0300, Walter Bright
 <newshound2 digitalmars.com> wrote:


 It's less complex (!) if you are not trying to make a working dmd. It
 just needs to compile.


 OK, that wasn't actually too bad.
 https://github.com/CyberShadow/dmd/tree/compile-on-vs10

 2979 warnings with code analysis with the "Microsoft Minimum Recommended
 Rules" ruleset.
 http://dump.thecybershadow.net/2e0571641194d945869a1b12b29aacdc/DMD.log

 I'll see if I can get it in a more readable format (something like the
 HTML files clang's scan-build outputs).


 
 Just at a glance, half of them are false positive, or is arguably safe:
 
 1. 382 (13%) of them are C4996 (use those Microsoft _s functions)
 2. 401 (13%) of them are C4068 (unknown pragma)
 3. 505 (17%) of them are C6328 (passing 'signed char' to the ctype 
 functions)
 4. 67 (2%) of them are C6239 (true && something) or C6240 (something && 
 true) - many of them are of them (!I16 && stuff), so that's legacy code 
 for 16-bit platform??
 5. 37 (1%) of them are C6255 (using alloca) or C6263 (using alloca in a 
 loop).
 6. 56 (2%) of them are C4305 or C4309 (double -> float)
 
 And 37% of them can be caught trivially with some -Wall flag.
 
 4. 262 (9%) of them are C4244 (stuff like int64 -> int32)
 5. 415 (14%) of them are C4018 (signed/unsigned comparison)


I noticed some pretty annoying behaviour:
When s is signed and u is unsigned:
   if (s >= 0 && s < u) ...
generates a 'signed/unsigned comparison' error.
So there's a lot of false positives in there.


 6. 157 (5%) of them are C4101 (unused locals)
 7. 50 (2%) of them are C4102 (unused labels)
 8. 212 (7%) of them are C6246 or C6244 or C4258 (local variable name 
 hiding outer scope)
 9. 8 (0.3%) of them are C4390 ('if(stuff);')
 
 The really interesting things:
 
 8. 117 (4%) of them are C6211 (leak on exception) - but a bigger problem 
 is DMD is using too much memory even without leaking.


I don't think anything in DMD ever throws an exception. Those are all 
false positives in practice (but would become real bugs if DMD ever 
starts using exceptions).


 9. 34 (1%) of them are C6001 (using uninitialized memory)


I think these are false positives too. The ones I saw were of the form:
When p is a pointer,
  assert(p);
  y = p->x; // error: p is uninitialized


 10. 125 (4%) of them are C6011 (NULL dereferencing)
 11. 6 (0.2%) of them are C6386 and 17 (0.6%) of them are C6385 (buffer 
 overrun)


This looks more promising than clang.
Aug 08 2011
parent reply bearophile <bearophileHUGS lycos.com> writes:
Don:


 I think these are false positives too. The ones I saw were of the form:
 When p is a pointer,
   assert(p);
   y = p->x; // error: p is uninitialized


This was true in the first "release" of those warnings files, but is it true
still? There are only 10 of those errors reported now.

Bye,
bearophile
Aug 08 2011
parent reply Don <nospam nospam.com> writes:
bearophile wrote:

 Don:
 

 I think these are false positives too. The ones I saw were of the form:
 When p is a pointer,
   assert(p);
   y = p->x; // error: p is uninitialized


 
 This was true in the first "release" of those warnings files, but is it true
still? There are only 10 of those errors reported now.
 
 Bye,
 bearophile


Yes. Still true.

4995:     FuncDeclaration *fd;
4996:     Expression *pthis = NULL;
4997:     if (deleg->op == TOKdelegate)
4998:     {
4999:         fd = ((DelegateExp *)deleg)->func;
5000:         pthis = ((DelegateExp *)deleg)->e1;
5001:     }
5002:     else if (deleg->op == TOKfunction)
5003:         fd = ((FuncExp*)deleg)->fd;
5004:
5005:     assert(fd && fd->fbody);

warning C6001: Using uninitialized memory 'fd': Lines: 4995, 4996, 4997, 
5002, 5005
Aug 08 2011
parent reply David Nadlinger <see klickverbot.at> writes:
On 8/9/11 5:46 AM, Don wrote:

 Don:


 I think these are false positives too. The ones I saw were of the form: […]




 4995: FuncDeclaration *fd;
 4996: Expression *pthis = NULL;
 4997: if (deleg->op == TOKdelegate)
 4998: {
 4999: fd = ((DelegateExp *)deleg)->func;
 5000: pthis = ((DelegateExp *)deleg)->e1;
 5001: }
 5002: else if (deleg->op == TOKfunction)
 5003: fd = ((FuncExp*)deleg)->fd;
 5004:
 5005: assert(fd && fd->fbody);

 warning C6001: Using uninitialized memory 'fd': Lines: 4995, 4996, 4997,
 5002, 5005


If deleg->op isn't guaranteed to be either TOKdelegate or TOKfunction, 
this is indeed a bug because fd contains garbage otherwise (you don't 
nullptr-initialize it).

David
Aug 08 2011
next sibling parent reply Don <nospam nospam.com> writes:
David Nadlinger wrote:

 On 8/9/11 5:46 AM, Don wrote:

 Don:


 I think these are false positives too. The ones I saw were of the 
 form: […]




 4995: FuncDeclaration *fd;
 4996: Expression *pthis = NULL;
 4997: if (deleg->op == TOKdelegate)
 4998: {
 4999: fd = ((DelegateExp *)deleg)->func;
 5000: pthis = ((DelegateExp *)deleg)->e1;
 5001: }
 5002: else if (deleg->op == TOKfunction)
 5003: fd = ((FuncExp*)deleg)->fd;
 5004:
 5005: assert(fd && fd->fbody);

 warning C6001: Using uninitialized memory 'fd': Lines: 4995, 4996, 4997,
 5002, 5005


 
 If deleg->op isn't guaranteed to be either TOKdelegate or TOKfunction, 
 this is indeed a bug because fd contains garbage otherwise (you don't 
 nullptr-initialize it).
 
 David


Aargh, you're right.
Aug 08 2011
parent reply Walter Bright <newshound2 digitalmars.com> writes:
On 8/8/2011 9:44 PM, Don wrote:

 Aargh, you're right.


I've been slowly going through the reports. While I've found only one real bug, 
and several "sort of" problems if you squint at them the right way, most are 
issues I put in the category of "not up to modern coding best practices". Those 
I'm working on fixing.
Aug 08 2011
parent reply Don <nospam nospam.com> writes:
Walter Bright wrote:

 On 8/8/2011 9:44 PM, Don wrote:

 Aargh, you're right.


 
 I've been slowly going through the reports. While I've found only one 
 real bug, and several "sort of" problems if you squint at them the right 
 way, most are issues I put in the category of "not up to modern coding 
 best practices". Those I'm working on fixing.


A big issue I've noticed relates to the type of array indices. If it's a 
32 bit compiler and a 32 bit target, I'd go for size_t.
But what about a 32-bit compiler with a 64 bit target? An index could be 
34 bits...
Aug 09 2011
parent Walter Bright <newshound2 digitalmars.com> writes:
On 8/9/2011 12:14 AM, Don wrote:

 Walter Bright wrote:

 On 8/8/2011 9:44 PM, Don wrote:

 Aargh, you're right.


 I've been slowly going through the reports. While I've found only one real
 bug, and several "sort of" problems if you squint at them the right way, most
 are issues I put in the category of "not up to modern coding best practices".
 Those I'm working on fixing.


 A big issue I've noticed relates to the type of array indices. If it's a 32 bit
 compiler and a 32 bit target, I'd go for size_t.
 But what about a 32-bit compiler with a 64 bit target? An index could be 34
bits...


Well, you cannot have a relocation offset larger than 32 bits on an x86-64.
Aug 09 2011
prev sibling parent reply bearophile <bearophileHUGS lycos.com> writes:
David Nadlinger:


 If deleg->op isn't guaranteed to be either TOKdelegate or TOKfunction, 
 this is indeed a bug because fd contains garbage otherwise (you don't 
 nullptr-initialize it).


The HTML Clang output was quite good at showing the path that leads to such
problems. Such HTML is a small thing, but it helps to understand the error
report (and to understand when Clang is wrong too). It's an idea worth stealing
:-)

Bye,
bearophile
Aug 09 2011
parent "Vladimir Panteleev" <vladimir thecybershadow.net> writes:
On Tue, 09 Aug 2011 16:31:18 +0300, bearophile <bearophileHUGS lycos.com>  
wrote:


 David Nadlinger:


 If deleg->op isn't guaranteed to be either TOKdelegate or TOKfunction,
 this is indeed a bug because fd contains garbage otherwise (you don't
 nullptr-initialize it).


 The HTML Clang output was quite good at showing the path that leads to  
 such problems. Such HTML is a small thing, but it helps to understand  
 the error report (and to understand when Clang is wrong too). It's an  
 idea worth stealing :-)


Microsoft's analyzer outputs a list of line numbers related some  
diagnostic messages. I suppose I could add some JavaScript to highlight  
said lines when moving the mouse over the error message.

-- 
Best regards,
  Vladimir                            mailto:vladimir thecybershadow.net
Aug 09 2011
prev sibling parent ponce <ponce nospam.net> writes:
Le 06/08/2011 21:38, Walter Bright a écrit :

 http://www.reddit.com/r/programming/comments/jar93/john_carmack_gives_his_thoughts_on_static/c2akmf8


 If someone has this on their system, I think it'd be great if you could
 run the dmd source code through it and see if it finds any bugs.


Other static analysis tools are great too.
- cppcheck is very easy to integrate, not that much false positives and 
probably extensible with custom rules.
- PVS studio integrates with Visual Studio, has a free demo with a handy 
ignoring mechanism.

AFAIK the Microsoft Analyzer has become less available and you can't 
have it unless owning VS2005, being a console developer or messing with 
the Windows DDK. I couldn't use it.
Aug 08 2011