6 ms·

Bugs found in GCC with the help of PVS-Studio

186 points by AndreyKarpov 10y ago

ramshorns 10y ago
There's a mistake in the analysis of the ternary operator in the assert that doesn't check anything. Not only does the ternary ?: operator have lower precedence than the comparison <= operator, but it also has lower precedence than the logical && operator. So the expression inside the assert doesn't resolve to (die_offset > 0), it resolves to either 0xffff or 0xffffffff, both of which are true. The assertion really doesn't check anything at all.
0xmohit 10y ago
I'd have loved to see references to actual GCC bug reports. Did I miss those in the article? Thankfully, the world is changing. You have languages where variables are immutable by default or can be declared so that would prevent issues as http://www.viva64.com/en/d/0108/ http://www.viva64.com/en/d/0108/ Maybe gcc should do better for static analysis of a program. Expressions like `a && a` or `a == 1 || a != 2` should be caught easily.
- i_have_to_speak 10y ago
  If you're suggesting that discovery of a bug in a popular open source project holds the discoverer up to a moral responsibility of reporting it -- well then, maybe you are guilty too!
  - rcthompson 10y ago
    There's no obligation to report a bug you've found, of course. But if you've taken the time to write a blog post about it, one would hope you've also taken the time to directly notify the software authors as well.
    - i_have_to_speak 10y ago
      But if you hadn't taken as much time -- say you've only come across a well written blog post clearly describing the bug -- would you report it?
  - 0xmohit 10y ago
    Given (1) the blog post, (2) the author posted the blog link on HN, I assumed that the bugs would have reported upstream. If it were a blog describing a 0-day vulnerability in a given software, I wouldn't have assumed that it would have been reported.
- AndreyKarpov 10y ago
  https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77421 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77421
  - 0xmohit 10y ago
    Thanks. That provides pointers to the specific issues: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71345 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71345 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77430 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77430 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64767 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64767 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77444 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77444 -- And fixes have also started coming in: https://gcc.gnu.org/viewcvs?rev=239959&root=gcc&view=rev https://gcc.gnu.org/viewcvs?rev=239959&root=gcc&view=rev Kudos for investigating and reporting the issues.
- mtkd 10y ago
  a == 1 || a != 2 needs a warning but it's not necessarily a bug sometimes I may keep overlapping expressions in a statement to remind me of the triggers I'm looking for at this point - and would likely use some meaningful constant names for the 1 and 2 statements aren't just logic - they're often documentation too - especially on code you might not look at again for months I used to have many arguments with a professor whose code I was working on - he would factor out everything in an expression - removing all the parentheses and rely on operator precedence - it was often impenetrable (for me) - so I'd unpack it, wrap parentheses everywhere, change single letter vars to meaningful ones etc. - he'd be in my office an hour later ranting
  - 0xmohit 10y ago
    > a == 1 || a != 2 needs a warning Yes, it needs a warning because it's kind of obvious that there is something amiss about this expression. Only the author could tell what was specifically wrong.
  - raverbashing 10y ago
    Yes I would advocate for the maintenance of both expressions if the comparison is tricky If this is a hot path that must absolutely be optimized then a comment explaining the conditions is needed And of course, the != 2 question might change (either because of a bug or logic change) but the == 1 might still be valid, however, if you took the first one your change will break the code
paulddraper 10y ago
Andrey, why did you title this different than your blog post? I like that one. "Bugs found in GCC with the help of PVS-Studio"
- sctb 10y ago
  We've updated the submission title from “GCC bugs”.
savanpatel 10y ago
how about using PVS Studio over code of PVS studio. :D
garbage_stain 10y ago
PVS-Studio (or more specifically Andrey) used to do a really nice program called "CppHints", where they would email a little tip each day (and later each week I think). Anyway, I guess I just wanted to say, Andrey, I appreciated your work and enjoyed reading those tips! Thanks!
ScottBurson 10y ago
I gather there's no relationship to the Prototype Verification System from SRI [0]. [0] http://pvs.csl.sri.com/index.shtml http://pvs.csl.sri.com/index.shtml
- AndreyKarpov 10y ago
  Yes. No relationship. PVS: OOO "Program Verification Systems" (Co. Ltd.)
valleyer 10y ago
This is now my favorite blog. Andrey, any plans to support Objective-C? (You are flagging things I can't get clang to warn about.)
- AndreyKarpov 10y ago
  Not yet. We are now experimenting with C# support and with Linux support. What's next, we do not know yet. There are two perspective areas: Objective-C and Java.
  - duaneb 10y ago
    Java or JVM?
    - Asooka 10y ago
      Since PVS is a static source-level checker, I don't see how it would make sense to check JVM bytecode.
    - AndreyKarpov 10y ago
      Java
  - blub 10y ago
    Objective-C would be a bad idea now, it seems to be replaced by Swift at an impressive rate.

186 points by AndreyKarpov 10y ago

There's a mistake in the analysis of the ternary operator in the assert that doesn't check anything. Not only does the ternary ?: operator have lower precedence than the comparison <= operator, but it also has lower precedence than the logical && operator. So the expression inside the assert doesn't resolve to (die_offset > 0), it resolves to either 0xffff or 0xffffffff, both of which are true. The assertion really doesn't check anything at all.

0xmohit 10y ago

I'd have loved to see references to actual GCC bug reports. Did I miss those in the article? Thankfully, the world is changing. You have languages where variables are immutable by default or can be declared so that would prevent issues as http://www.viva64.com/en/d/0108/ http://www.viva64.com/en/d/0108/ Maybe gcc should do better for static analysis of a program. Expressions like `a && a` or `a == 1 || a != 2` should be caught easily.

i_have_to_speak 10y ago
If you're suggesting that discovery of a bug in a popular open source project holds the discoverer up to a moral responsibility of reporting it -- well then, maybe you are guilty too!
- rcthompson 10y ago
  There's no obligation to report a bug you've found, of course. But if you've taken the time to write a blog post about it, one would hope you've also taken the time to directly notify the software authors as well.
  - i_have_to_speak 10y ago
    But if you hadn't taken as much time -- say you've only come across a well written blog post clearly describing the bug -- would you report it?
- 0xmohit 10y ago
  Given (1) the blog post, (2) the author posted the blog link on HN, I assumed that the bugs would have reported upstream. If it were a blog describing a 0-day vulnerability in a given software, I wouldn't have assumed that it would have been reported.
AndreyKarpov 10y ago
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77421 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77421
- 0xmohit 10y ago
  Thanks. That provides pointers to the specific issues: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71345 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=71345 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77430 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77430 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64767 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64767 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77444 https://gcc.gnu.org/bugzilla/show_bug.cgi?id=77444 -- And fixes have also started coming in: https://gcc.gnu.org/viewcvs?rev=239959&root=gcc&view=rev https://gcc.gnu.org/viewcvs?rev=239959&root=gcc&view=rev Kudos for investigating and reporting the issues.
mtkd 10y ago
a == 1 || a != 2 needs a warning but it's not necessarily a bug sometimes I may keep overlapping expressions in a statement to remind me of the triggers I'm looking for at this point - and would likely use some meaningful constant names for the 1 and 2 statements aren't just logic - they're often documentation too - especially on code you might not look at again for months I used to have many arguments with a professor whose code I was working on - he would factor out everything in an expression - removing all the parentheses and rely on operator precedence - it was often impenetrable (for me) - so I'd unpack it, wrap parentheses everywhere, change single letter vars to meaningful ones etc. - he'd be in my office an hour later ranting
- 0xmohit 10y ago
  > a == 1 || a != 2 needs a warning Yes, it needs a warning because it's kind of obvious that there is something amiss about this expression. Only the author could tell what was specifically wrong.
- raverbashing 10y ago
  Yes I would advocate for the maintenance of both expressions if the comparison is tricky If this is a hot path that must absolutely be optimized then a comment explaining the conditions is needed And of course, the != 2 question might change (either because of a bug or logic change) but the == 1 might still be valid, however, if you took the first one your change will break the code

paulddraper 10y ago

Andrey, why did you title this different than your blog post? I like that one. "Bugs found in GCC with the help of PVS-Studio"

sctb 10y ago
We've updated the submission title from “GCC bugs”.

savanpatel 10y ago

how about using PVS Studio over code of PVS studio. :D

garbage_stain 10y ago

PVS-Studio (or more specifically Andrey) used to do a really nice program called "CppHints", where they would email a little tip each day (and later each week I think). Anyway, I guess I just wanted to say, Andrey, I appreciated your work and enjoyed reading those tips! Thanks!

ScottBurson 10y ago

I gather there's no relationship to the Prototype Verification System from SRI [0]. [0] http://pvs.csl.sri.com/index.shtml http://pvs.csl.sri.com/index.shtml

AndreyKarpov 10y ago
Yes. No relationship. PVS: OOO "Program Verification Systems" (Co. Ltd.)

valleyer 10y ago

This is now my favorite blog. Andrey, any plans to support Objective-C? (You are flagging things I can't get clang to warn about.)

AndreyKarpov 10y ago
Not yet. We are now experimenting with C# support and with Linux support. What's next, we do not know yet. There are two perspective areas: Objective-C and Java.
- duaneb 10y ago
  Java or JVM?
  - Asooka 10y ago
    Since PVS is a static source-level checker, I don't see how it would make sense to check JVM bytecode.
  - AndreyKarpov 10y ago
    Java
- blub 10y ago
  Objective-C would be a bad idea now, it seems to be replaced by Swift at an impressive rate.