[llvm-dev] [RFC] Coding Standards: "prefer `int` for regular arithmetic, use `unsigned` only for bitmask and when you intend to rely on wrapping behavior."

[Adrien Guinet via llvm-dev]

My 2 cents on this.

I watched Chandler's talk, and it looks like there are two statements that are a bit
contradictory one to another:

https://www.youtube.com/watch?v=yG1OZ69H_-o&t=2820s "it's fairly easy to get enough bits
w/o using the sign bits"
versus
https://www.youtube.com/watch?v=yG1OZ69H_-o&t=2846s "using size_t takes more space in data
structures"

Why not using size_t for indexes/length, while storing uint32_t integers and cast them
back to size_t when necessary (i.e. for space reasons)?

About checking for overflows, let's take the various examples here:
https://godbolt.org/z/TIGiVe, which are various ways to load a byte at index "n+10" where
n is user-controlled. In this case, we need to check that "n+10" does not overflow.
The first thing is that, IMHO, overflow checks are tricky to get right for both the signed
and unsigned case (and has led to numerous vulnerabilities). But that's a personal
opinion. If we talk about code size (and performance), the load3 function confirms what
Chandler showed in his talk (using uint32_t can be a performance issue on some 64-bit
systems, do to the enforced wrapping that has to be done on 32bits integers). The other
load* functions all emit the intended "mov eax, DWORD PTR [rdi+40+rsi*4]". Notable
differences arise in load2, which needs to sign extend esi to rsi. In load1, there is also
an extra instruction to set the value of (2**63-12).

One conclusion is that, at least on x86-64, using size_t for these kind of overflow checks
is the most efficient. But I might miss something / do something wrong in this analysis,
and I'd like another POV on this!

One argument for the usage of signed integers (actually integer types that don't allow
wrapping) is indeed the ability to automatically insert these overflow checks at compile time.


On 6/8/19 7:17 PM, Mehdi AMINI via llvm-dev wrote:
> Hi,
> 
> The LLVM coding style does not specify anything about the use of signed/unsigned integer,
> and the codebase is inconsistent (there is a majority of code that is using unsigned index
> in loops today though).
> 
> I'd like to suggest that we specify to prefer `int` when possible and use `unsigned` only
> for bitmask and when you intend to rely on wrapping behavior,
> see: https://reviews.llvm.org/D63049
> 
> A lot has been written about this, good references are [unsigned: A Guideline for Better
> Code] https://www.youtube.com/watch?v=wvtFGa6XJDU) and [Garbage In, Garbage Out: Arguing
> about Undefined Behavior...](https://www.youtube.com/watch?v=yG1OZ69H_-o), as well as this
> panel discussion:
> - https://www.youtube.com/watch?v=Puio5dly9N8#t=12m12s
> - https://www.youtube.com/watch?v=Puio5dly9N8#t=42m40s
> 
> Other coding guidelines already embrace this:
> 
> - http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-signed
> - http://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Res-unsigned
> - https://google.github.io/styleguide/cppguide.html#Integer_Types
> 
> It is rare that overflowing (and wrapping) an unsigned integer won't trigger a program bug
> when the overflow was not intentionally handled. Using signed arithmetic means that you
> can actually trap on over/underflow and catch these bugs (when using fuzzing for instance).
> 
> Chandler explained this nicely in his CPPCon 2016 talk "Garbage In, Garbage Out: Arguing
> about Undefined Behavior...". I encourage to watch the full talk but here is one relevant
> example: https://youtu.be/yG1OZ69H_-o?t=2006 , and here he mentions how Google
> experimented with this internally: https://youtu.be/yG1OZ69H_-o?t=2249
> 
> Unsigned integer also have a discontinuity right to the left of zero. Suppose A, B and C
> are small positive integers close to zero, say all less than a hundred or so. Then given:
> A + B > C
> and knowing elementary school algebra, one can rewrite that as:
> A > B - C
> But C might be greater than B, and the subtraction would produce some huge number. This
> happens even when working with seemingly harmless numbers like A=2, B=2, and C=3.
> 
> -- 
> Mehdi
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>