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