Hi--
On Apr 23, 2014, at 3:06 AM, Erik Cederstrand <[email protected]> wrote:
> Den 23/04/2014 kl. 03.12 skrev Ronald F. Guilmette <[email protected]>:
[ ... ]
>> I do imagine that the truth or falsehood of your assertion may depend
>> quite substantally on what one does or does not consider a "false
>> positive" in this context.
>
> Have a look at the ~10.000 reports at
> http://scan.freebsd.your.org/freebsd-head/ (unavailable ATM). Silly things
> are reported like missing return at the end of main() or not free()ing memory
> two lines before program exit. There are nonsensical reports because the
> analyzer doesn't detect exit() in a usage() function because usage() is
> defined in a separate compilation unit, or this:
Sure, static analysis isn't perfect and runs into false positives, some of
which are truly harmless and some of which actually do indicate an area where
refactoring the code in light of the warning would be an improvement.
It's worth noting that even if you believe that (e.g.) the clang static
analyzer isn't properly doing liveness analysis and misjudging whether there's
a dead assignment (writing to a variable which is never read), the clang
compiler will be using the same analysis when doing dead-code elimination and
common-subexpression elimination and such while optimizing.
> int foo(int y, int z) {
> int x;
> if (y == z) {
> x = 0;
> } else {
> if (y != z) {
> x = 1;
> }
> }
> return x;
> }
>
> warning that x may be uninitialized. Fixing these require considerable effort
> e.g. improving IPA and adding alpha-remaning support to the analyzer's
> constraint manager, or would result in unnecessary code churn in FreeBSD just
> to work around the reports.
Ah, that's a classic example. If you declared y and z as const, then I'd agree
that the compiler should be free to make assumptions that one of the two if
statements must be true.
On the other hand, if you assume that the arguments are volatile and that maybe
another thread might update y or z on the stack between the time when the first
if test is evaluated and the second if, one realizes that the static analyzer
might actually have a point. (Or you're on an SMP system and don't get
sequential consistency / total-store ordering without memory barriers....)
Sure, your code might never intentionally try to mess with the stack, but
there's a long history of bugs like typing ~1030 characters at a password
prompt and blowing past a char passwd[1024] buffer that someone assumed would
be more than enough.
The most straightforward changes to this snippet would be either:
int foo(int y, int z) {
int x;
if (y == z) {
x = 0;
} else {
x = 1;
}
return x;
}
...or:
int foo(int y, int z) {
int x = 0;
if (y != z) {
x = 1;
}
return x;
}
Not only are both of these shorter and they pass clang's static analyzer
without a warning, I'd argue that the second version is noticeably cleaner.
Regards,
--
-Chuck
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-security
To unsubscribe, send any mail to "[email protected]"
