Date: Thu, 24 Oct 2013 14:46:06 -0700
On Thu, Oct 24, 2013 at 2:16 PM, John Regehr <regehr_at_[hidden]> wrote:
> one way to help resolve the issue would be to stress-test a few
> large applications compiled with integer undefined behavior checking and
> then take a close look at the resulting signed left-shift UBs.
>
FWIW, sorry I wasn't more explicit, I'm saying we have done this on at
least a few applications. Unfortunately, at the moment I just have my
memory. I can dig up data if it is necessary.
My memory is that LLVM was a mixture of two patterns followed by a long
tail of other stuff, and I have data that shows several large C++
applications at Google had similar distributions.
1) Code that *wanted* to be unsigned, but forgot that a literal '42' was
signed. We changed it to '42u' which was preferable on all fronts.
2) Bugs
3) Everything else
You can call #1 false positives, but the code reviewers for the patches to
projects based on this have consistently felt that this was a useful
fix/clarification of their intent in the code.
We did consider some examples of #3 to be unfortunate (requiring a cast,
etc), but they were quite rare relatively.
The other thing to consider is that the bugs we found in #2 were serious,
hard to find bugs that had gone unnoticed for a long time. Our developers
were very willing to pay the price of finding those bugs.
> one way to help resolve the issue would be to stress-test a few
> large applications compiled with integer undefined behavior checking and
> then take a close look at the resulting signed left-shift UBs.
>
FWIW, sorry I wasn't more explicit, I'm saying we have done this on at
least a few applications. Unfortunately, at the moment I just have my
memory. I can dig up data if it is necessary.
My memory is that LLVM was a mixture of two patterns followed by a long
tail of other stuff, and I have data that shows several large C++
applications at Google had similar distributions.
1) Code that *wanted* to be unsigned, but forgot that a literal '42' was
signed. We changed it to '42u' which was preferable on all fronts.
2) Bugs
3) Everything else
You can call #1 false positives, but the code reviewers for the patches to
projects based on this have consistently felt that this was a useful
fix/clarification of their intent in the code.
We did consider some examples of #3 to be unfortunate (requiring a cast,
etc), but they were quite rare relatively.
The other thing to consider is that the bugs we found in #2 were serious,
hard to find bugs that had gone unnoticed for a long time. Our developers
were very willing to pay the price of finding those bugs.
Received on 2013-10-24 23:46:08