Hey Mirko

On 10.11.21 11:07, Mirko Brodesser wrote:

I don't know, how much static analysis currently helps to detect overflows of signed types, which would be one benefit of using signed instead of unsigned types. Does anyone know?

Static analysis is not very good in accurately detecting these issues, but we do have dynamic analysis though UndefinedBehaviorSanitizer [1] available to detect both types of overflows. The primary difference between signed and unsigned here is that signed overflow is actual undefined behavior and considered harmful in certain situations (the compiler can perform optimizations and remove code based on the assumption that signed types will not overflow).

One problem in our codebase is that we have a lot of old code that makes excessive use of signed overflow where the results don't matter, which makes deploying this kind of analysis challenging. I believe we are not running this anywhere right now, despite the fact that we have an extensive suppression list [2] and build system support for it [3]. I'll actually go and figure out if we are planning to deploy this at any point and get back to you when I know more about that.

For new code, if possible we should use `CheckedInt` which has a diagnostic assert that indicates unchecked overflows. If that is not possible for some reason and a variable is not supposed to contain negative values (and not supposed to overflow), then from a memory safety standpoint, it might be better to have it signed (if the variable flows into a size parameter, it will be too large rather than too small), but it really depends on the underlying code I guess. The UB occurring might have negative side effects too, but that at least makes the bug detectable, while we have very little chance of finding unintended unsigned overflows unless they crash in some way.

For the situation above we should also always investigate why `CheckedInt` wouldn't be applicable.


- Chris


[1] https://releases.llvm.org/13.0.0/tools/clang/docs/UndefinedBehaviorSanitizer.html

[2] https://searchfox.org/mozilla-central/rev/a48e21143960b383004afa9ff9411c5cf6d5a958/build/moz.configure/toolchain.configure#1832

[3] https://searchfox.org/mozilla-central/rev/a48e21143960b383004afa9ff9411c5cf6d5a958/build/sanitizers/ubsan_signed_overflow_blacklist.txt

--
You received this message because you are subscribed to the Google Groups 
"[email protected]" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To view this discussion on the web visit 
https://groups.google.com/a/mozilla.org/d/msgid/dev-platform/e22ec50d-e3ba-0b1c-95a6-14a03bbf6deb%40mozilla.com.

Reply via email to